bumptech / glide

An image loading and caching library for Android focused on smooth scrolling
https://bumptech.github.io/glide/
Other
34.64k stars 6.12k forks source link

Conflicting dependencies in annotation processors #2059

Closed MihailovJava closed 7 years ago

MihailovJava commented 7 years ago

Glide Version: 4.0.0-RC1

Integration libraries: OkHttp3

Issue details / Repro steps / Use case background: I'm trying migrate to v4 and got stuck with auto-generated GlideApp.

Glide load line / GlideModule (if any) / list Adapter code (if any):

GlideApp.with...

I'v got a Glide Module class:

@GlideModule
public final class MyGlideModule extends AppGlideModule {

  @Inject 
  @Named("GLIDE_CLIENT")
  protected OkHttpClient client;

    public MyGlideModule() {
        AccountComponent().inject(this);
    }

    @Override
    public void applyOptions(Context context, GlideBuilder builder) {
        builder.setMemoryCache(new LruResourceCache(10 * 1024 * 1024));
    }

    @Override
    public void registerComponents(Context context, Registry glide) {
        glide.replace(GlideUrl.class, InputStream.class, new OkHttpUrlLoader.Factory(client));
        glide.register(SVG.class, PictureDrawable.class, new ImageLoadingUtils.SvgDrawableTranscoder())
                .append(InputStream.class, SVG.class, new ImageLoadingUtils.SvgDecoder());
    }

    @Override
    public boolean isManifestParsingEnabled() {
        return false;
    }
}

In my build.gradle file i have those dependencies

    compile "com.github.bumptech.glide:glide:4.0.0-RC1"
    compile 'com.android.support:support-v4:25.3.1'
    annotationProcessor 'com.github.bumptech.glide:compiler:4.0.0-RC1'
    compile 'com.caverock:androidsvg:1.2.1'

Stack trace / LogCat:

Error:(83, 4) error: cannot find symbol variable GlideApp

Also i cloned SVG's samples and there autogeneration works fine. Can Dagger2 conflict with Glide? Or any suggetions, how can i fix it?

MihailovJava commented 7 years ago

I figured out, I just updated dagger 2 dependency version.

TWiStErRob commented 7 years ago

@MihailovJava Awesome find @ https://github.com/bumptech/glide/issues/1984#issuecomment-310214767 So did you get the same exception along with the unresolved symbol?

@sjudd is this preventable? e.g. assert poet version at runtime(=apt time)

MihailovJava commented 7 years ago

@TWiStErRob , Yes, I'd got this exception when I was trying add -apt Glide's dependency. I guess that my old version dagger 2 depends on old javapoet. I removed apt and update all auto-gen libs, and it works fine now.

sjudd commented 7 years ago

I don't have any good ideas on how to fix this, definitely open to suggestions. It's certainly awkward to try to keep multiple libraries using exactly the same set of dependencies.

TWiStErRob commented 7 years ago

@sjudd repackage/jarjar, like Espresso does Guava? (reasoning: the apt dep doesn't have to be small because it's not packaged within the APK)

sjudd commented 7 years ago

That seems heavy weight. Or is there an automated way to repackage? I didn't know espresso did this.

TWiStErRob commented 7 years ago

@sjudd See #2363 and #2364, the weight is not that bad, considering it allows for Glide to be combined with any version of any APT.

TWiStErRob commented 7 years ago

@sjudd Hmm, actually, I just noticed that the whole problem stems from autoservice pulling in Guava, is it really worth it to have this dependency? Note that replacing the import, @AutoService and compile 'auto-service' (3 lines + 2MB of dependencies) with a file named src/main/resources/META-INF/services/javax.annotation.processing.Processor containing the fully qualified class name (1 line) would solve the issue as well.

... even if the above change is done, repackaging JavaPoet is still a good idea.

sjudd commented 7 years ago

Yeah it's not a big deal to to remove auto service, but if we're going to repackage we might as well repackage everything?

TWiStErRob commented 7 years ago

It's ok repackage auto, but there may be an issue, not sure if it works. There's a service file in META-INF that cannot be repackaged in fatjar, because Glide has the same file (see exclusions in Gradle in he PR).

The weirdest thing is that you need a compile time dependency on the full lib (incl. Guava) just so you can add a single annotation. In auto there's not seem to be a contract and implementation separation like with annotation and compiler modules we have.

sjudd commented 7 years ago

@MihailovJava What version of Dagger were you using that was conflicting? I tried creating a sample app that depends on Glide and dagger 2.0, and it seems to compile and run without any issues.

sjudd commented 7 years ago

Alright I was able to reproduce the jsr305 conflict from this comment: https://github.com/bumptech/glide/issues/2064#issuecomment-324300806.

I'd love to find a way to reproduce the guava or okhttp cases too to make sure they're fixed by the PR

sjudd commented 7 years ago

I'd guess this will fix these issues. I believe anyone experiencing this issue can test by depending on the 4.2.0-SNAPSHOT version, at least for now. It will go away the next time a change is submitted to master unless we merge the pull request.

MihailovJava commented 7 years ago

@sjudd Sorry for late response, actually i don't remember. I deleted those branch and can't help with testing