google / volley

https://google.github.io/volley
Apache License 2.0
3.38k stars 754 forks source link

R8 warning: Missing class: org.chromium.net.UrlRequest$Callback #412

Closed htteng1976 closed 3 years ago

htteng1976 commented 3 years ago

Hi, I'm getting this issue when integrating volley 1.2.0 into my project.

Add library dependency:

dependencies { ... implementation 'com.android.volley:volley:1.2.0' }

Android Studio 4.2 buid warning:

Task :library:minifyXXXReleaseWithR8 AGPBI: {"kind":"warning","text":"Missing class: org.chromium.net.UrlRequest$Callback","sources":[{}],"tool":"R8"}

Missing class: org.chromium.net.UrlRequest$Callback

Should I have to add google play service cronet manually?

dependencies { ... implementation 'com.android.volley:volley:1.2.0' implementation 'com.google.android.gms:play-services-cronet:17.0.0' }

jpd236 commented 3 years ago

Thanks for the report!

Is this just a build warning? If you're not explicitly opting into using the Cronet stack, you should be able to safely ignore this. The classes that are missing won't be used by other HTTP stacks, so it's fine that they're missing. If you are opting to use the Cronet stack, then you'd need to add a Cronet dependency (from Play Services or directly).

If this warning is a major issue, we could consider splitting out a separate :volley-cronet target from Volley itself to contain anything that depends on Cronet. But to me, this feels like a fair bit more build complexity to deal with just for a warning in the optimizer. 1.2.0 has been out for over two months and this is the first report/complaint we've gotten about this.

Will leave this open to get more input, but definitely let us know if this is actually breaking builds or if things aren't working when you omit the cronet dependency. You could also add a -dontwarn org.chromium.** to your Proguard config to silence the warning, I think (though you would have to watch out for legitimate issues elsewhere).

htteng1976 commented 3 years ago

Thanks for the quick reply.

It's just a build warning and didn't cause any build break. I will try your recommendataion to silence this warning. Very appreciated.

jpd236 commented 3 years ago

Seems that there are other complaints about this from an SDK which depends on Volley, and an SDK can't reasonably disable warnings for such a broad package. So we'll have to look into separating out the -cronet artifact into its own library so apps/libraries depending on Volley alone don't run into issues.

jpd236 commented 3 years ago

This should be resolved in the next release. Would be great if you or someone who was seeing this issue could verify that it is fixed by configuring prerelease versions of Volley and seeing if 1.2.1-SNAPSHOT fixes the problem.

znakeeye commented 3 years ago

Not sure this extra complexity was worth it. The warning appeared in r8 version 3.0 and was fixed in 3.0.41. Google issue exists since May 5.

Thus, any consumer of volley 1.2.0 could simply opt in for the latest r8:

dependencies {
    classpath 'com.android.tools:r8:3.0.41'
}

But perhaps this multi-module stuff comes with other advantages?

jpd236 commented 3 years ago

It doesn't seem great to leave it up to the build tool to decide whether or not the reference to a missing class is considered safe or not. Even if r8 behaves differently, Proguard does warn about it in certain situations, and there's no way to turn off that warning without doing so globally for the entire app's use of Cronet, which is unsafe. Other optimizers may also behave differently.

I do think the multi-module setup has other advantages; for example, it unblocks https://github.com/google/volley/issues/279, in that we could now create a volley-ktx module that makes things nicer for Kotlin users without having to depend on Kotlin in the core library itself.