Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Glide v4s gradle dependencies are incorrect #2547

Closed
sjudd opened this issue Nov 1, 2017 · 9 comments
Closed

Glide v4s gradle dependencies are incorrect #2547

sjudd opened this issue Nov 1, 2017 · 9 comments
Labels
Milestone

Comments

@sjudd
Copy link
Collaborator

sjudd commented Nov 1, 2017

We depend on support-fragment in library using provided. Unfortunately it looks like this is actually a hard requirement. I created a simple supportless sample app and it will fail to compile unless I also add a dependency on support-fragment to the sample app because of:

> Task :samples:simple:compileDebugJavaWithJavac
/Users/judds/opensource/glide/samples/simple/src/main/java/com/bumptech/glide/samples/simple/MainActivity.java:17: error: cannot access Fragment
    Glide.with(this)
         ^
  class file for android.support.v4.app.Fragment not found
1 error

I'm not sure there's going to be a reasonable way around that other than just adding the dependency.

@sjudd sjudd added the bug label Nov 1, 2017
@sjudd sjudd added this to the 4.4 milestone Nov 1, 2017
@sjudd
Copy link
Collaborator Author

sjudd commented Nov 1, 2017

Sample branch is here: https://github.com/sjudd/glide/tree/issue_2547

@TWiStErRob
Copy link
Collaborator

TWiStErRob commented Nov 1, 2017

We've been through this once: #137 + https://github.com/TWiStErRob/repros/tree/main/glide/support-Fragment_optional-dependency

It's only a hard req if you call Glide.with ;) in the generated GlideApp you could skip copying the method if the support library is not on the classpath, preventing this problem for APT users at least.

@TWiStErRob
Copy link
Collaborator

Mind you, the app will compile and work if you add an empty class named android.support.v4.app.Fragment to the project instead of the full support library.

@sjudd
Copy link
Collaborator Author

sjudd commented Nov 1, 2017

Ah yeah, for whatever reason I thought we'd actually made that change.

I think this fix here is simple then, we should just make it a compile dependency. It doesn't seem to complain about jar/aar dependencies either.

We haven't gotten a lot (any?) requests to avoid depending on the support library?

@TWiStErRob
Copy link
Collaborator

So it is required but we shouldn't decide which version of the support library the user is pulling in. If we add compile a lot of people will need to add exclude to prevent updating their support. The current behaviour, while weirder, prevents that pain for most people while affecting a really small percentage of others; and even if it fails the message is really clear.

@TWiStErRob
Copy link
Collaborator

Actually, given the trick of additing the class itself (not tested, but 90% sure), we could create an glide:integration-nocompat library that enables usage without support lib. And it will fail the build if the support lib actually exists.

@sjudd
Copy link
Collaborator Author

sjudd commented Nov 8, 2017

My biggest concern actually is users who aren't using the support library themselves.

Right now if they add a dependency on Glide, their build will fail because Gradle doesn't recognize that Glide has that dependency.

I guess it's sort of a guessing game. Do we want to break people who have/want to use an older version of the support library (that has an incompatible API with the version that Glide depends on) or do we want to break people who don't use the support library anywhere else?

@sjudd sjudd removed this from the 4.4 milestone Nov 8, 2017
@sjudd
Copy link
Collaborator Author

sjudd commented Nov 12, 2017

Thought about this a bit more, I think we need to include v4 as a direct dependency for now. If users want to get it to compile themselves and avoid using the support library, they can exclude the transitive dependency with gradle and add the empty class file.

@sjudd sjudd added this to the 4.4 milestone Nov 12, 2017
sjudd added a commit to sjudd/glide that referenced this issue Nov 12, 2017
sjudd added a commit to sjudd/glide that referenced this issue Nov 12, 2017
sjudd added a commit to sjudd/glide that referenced this issue Nov 12, 2017
sjudd added a commit to sjudd/glide that referenced this issue Nov 12, 2017
sjudd added a commit to sjudd/glide that referenced this issue Nov 12, 2017
@sjudd sjudd closed this as completed in c1c9be2 Nov 13, 2017
@Marcos170393
Copy link

Hi! This works for me:
in build.gradle paste this

dependencies {
  implementation 'com.github.bumptech.glide:glide:4.12.0'
  // Glide v4 uses this new annotation processor -- see https://bumptech.github.io/glide/doc/generatedapi.html
  annotationProcessor 'com.github.bumptech.glide:compiler:4.12.0'
}

then Sync project with gradle file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants