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

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

Closed
htteng1976 opened this issue May 27, 2021 · 6 comments
Closed

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

htteng1976 opened this issue May 27, 2021 · 6 comments
Milestone

Comments

@htteng1976
Copy link

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
Copy link
Collaborator

jpd236 commented May 27, 2021

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).

@jpd236 jpd236 added this to the 1.2.1 milestone May 27, 2021
@htteng1976
Copy link
Author

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
Copy link
Collaborator

jpd236 commented Jun 24, 2021

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 added a commit that referenced this issue Jul 7, 2021
This splits the library into three Gradle modules:

- core - volley core + toolbox
- cronet - CronetHttpStack
- testing - common testing code

The primary purpose is to split out CronetHttpStack into its own Maven
artifact, com.android.volley:volley-cronet, to prevent applications
which aren't using it from having to either suppress Proguard/D8
warnings or pull in the unnecessary dependency.

The testing module is needed to share TestRequest between unit tests
in both core and cronet. It is not published to Maven.

This PR also removes support for the build.gradle/rules.gradle split
which allowed Gradle projects to depend on Volley by source and
specify their own SDK settings. This is no longer worth supporting,
and any client that wishes to depend on Volley via Gradle should do
so via published Maven artifacts instead.

See #412
@jpd236
Copy link
Collaborator

jpd236 commented Jul 7, 2021

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.

@jpd236 jpd236 closed this as completed Jul 7, 2021
@znakeeye
Copy link

znakeeye commented Aug 25, 2021

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
Copy link
Collaborator

jpd236 commented Aug 25, 2021

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 #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.

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

No branches or pull requests

3 participants