Compiler warning in apps that don't use androidx.annotation:annotation #424
Milestone
Comments
jpd236
added a commit
to jpd236/volley
that referenced
this issue
Aug 10, 2021
We were depending directly on the .aar, which loses the dependency information from the build configuration itself. This caused android.annotations to (correctly) show up as a dependency. Since it is only needed for annotations (like `@Nullable`) that don't need to be retained at runtime, we can mark this as a compileOnly dependency, matching its former "provided" state, to prevent Volley users from needing it. However, this introduces a compiler warning in the tests, since @RestrictTo has RetentionPolicy.CLASS and is not present at compile time; since we apply -Werror to all compile tasks, this becomes a compiler error. For now, to keep the build as close as possible to the prior state, add the annotations as an implementation dependency to the tests; a proper fix is tracked in google#424. The only resulting change to the POM files is that volley-cronet declares a dependency on volley. Note that volley-cronet doesn't declare a dependency on Cronet because there are two plausible choices for how to use Cronet - the version in GmsCore, or the standalone version. There's no great way to codify this dependency choice, so leave it unstated. Fixes google#402
jpd236
added a commit
that referenced
this issue
Aug 10, 2021
We were depending directly on the .aar, which loses the dependency information from the build configuration itself. This caused android.annotations to (correctly) show up as a dependency. Since it is only needed for annotations (like `@Nullable`) that don't need to be retained at runtime, we can mark this as a compileOnly dependency, matching its former "provided" state, to prevent Volley users from needing it. However, this introduces a compiler warning in the tests, since @RestrictTo has RetentionPolicy.CLASS and is not present at compile time; since we apply -Werror to all compile tasks, this becomes a compiler error. For now, to keep the build as close as possible to the prior state, add the annotations as an implementation dependency to the tests; a proper fix is tracked in #424. The only resulting change to the POM files is that volley-cronet declares a dependency on volley. Note that volley-cronet doesn't declare a dependency on Cronet because there are two plausible choices for how to use Cronet - the version in GmsCore, or the standalone version. There's no great way to codify this dependency choice, so leave it unstated. Fixes #402
|
For 1.2.2, I think we should go ahead and add the dependency. It's the correct thing to do as the dependency is needed at compile-time, including for applications that depend on Volley. Given that Volley has a hard dependency on Android (e.g. can't be used directly from the JVM), and that Android's build system depends on the Google repository in large part, it shouldn't be a problem for any application use cases that I can think of. And there is precedent for popular Android-specific Maven Central artifacts declaring compile-time dependencies on androidx.annotation:annotation, e.g.: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
(Forking from #402)
When an app depends on Volley (1.2.0+, possibly older as well), and doesn't also depend on androidx.annotation:annotation, the resulting build will have compiler warnings:
These are non-fatal by default, but become fatal if
-Werroris enabled.This doesn't seem to have bothered anyone through now, but it'd be nice to fix. The problem is that neither option seems great.
@RestrictToneeds to be retained at class-time, not source-time, so it's technically not safe to depend on as compileOnly. However, androidx.annotation:annotation isn't available on Maven Central, so we'd be requiring a dependency on something that can't actually be obtained there, though in practice this may be perfectly reasonable since you can't build a Google app without artifacts from the Google repository anyway. Removing the annotation wouldn't be ideal since it'd provide no protection (however light) against applications depending on methods meant for internal use in Volley.The text was updated successfully, but these errors were encountered: