all: update animalsniffer to Java 7 and add Android 14 #4727
Conversation
| dependencies { | ||
| compile project(':grpc-context'), | ||
| libraries.gson, | ||
| libraries.guava, | ||
| libraries.errorprone, | ||
| libraries.jsr305 | ||
| libraries.jsr305, | ||
| libraries.animalsniffer_annotations |
There was a problem hiding this comment.
compileOnly. The annotation is Retention(Class)
There was a problem hiding this comment.
compileOnly triggers a Proguard warning (=> fails the Android build):
Warning: io.grpc.internal.JndiResourceResolverFactory$JndiResourceResolver: can't find referenced class org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement
I think JndiResourceResolverFactory should actually be stripped out by proguard, so adding a proguard -dontwarn is probably all that's necessary - but this would then be required in all downstream apps' proguard configs as well. Will look for a better solution here...
There was a problem hiding this comment.
I see a few options here: (fwiw, guava had some similar pain tracked in google/guava#2721)
- Keep the dependency as compileOnly, which will require Android apps to add a new -dontwarn to their proguard configs
- Bite the cost of making this a compile dependency in open-source. The annotation package is very small (no impact on Android app size/dex count).
- Use our own custom annotation (supported by animalsniffer) to avoid the dependency
- Drop the annotations altogether and configure animalsniffer to ignore JndiResourceResolverFactory's uses of unsupported APIs
I lean towards option 4. @ejona86 what do you think?
There was a problem hiding this comment.
(2) sounds appropriate. Let's just add it as a normal dep. Unfortunate, but whatever.
services/build.gradle
Outdated
| @@ -21,7 +21,8 @@ dependencies { | |||
| compileOnly libraries.javax_annotation | |||
| testCompile project(':grpc-testing'), | |||
| libraries.netty_epoll // for DomainSocketAddress | |||
| signature "org.codehaus.mojo.signature:java16:1.1@signature" | |||
| signature "org.codehaus.mojo.signature:java17:1.0@signature" | |||
| signature "net.sf.androidscents.signature:android-api-level-14:4.0_r4@signature" | |||
There was a problem hiding this comment.
No android here. This depends on protobuf full.
|
@ejona86 Added the dep to the bazel build rules as well (I don't think bazel supports a compileOnly equivalent, based on https://docs.bazel.build/versions/master/be/workspace.html#maven_jar). I will wait for re-approval before merging. |
|
@ericgribkoff, Bazel can do compileOnly via neverlink=1. #4702 has an example. Unfortunately, the ProGuard integration complains if you include the same dependency via both neverlink=0 and neverlink=1. |
No description provided.