Gradle Module Metadata makes Guava's -android version dangerous for libraries

[ejona86]

Guava Version

Noticed on 33.2.1-android because of an old PR, but currently using 33.3.1-android

Description

TL;DR: Gradle uses the Module Metadata to use -jre version, but Maven and AGP will use -android meaning we compile and test with a superset of what our users will use.

In gRPC Java we mostly use Guava's -android versions because we support older Android versions. We also use the Gradle build system. However, we discovered that when compiling the -jre version is actually what's being used. gradle dependencies shows -android, but the jar file passed to javac is -jre.

This was quite the riddle until I remembered Guava uses Gradle Module Metadata, and yep, the -android version pulls in the -jre version. See also #7154

gRPC Java is a library, and used by Maven and Android users. While Gradle used the -jre version to compile, the published artifacts still depend on the -android version. So Maven and Android users will use -android. This means it is possible for -jre-only symbols to be used accidentally and then cause runtime failures when unavailable.

Most cases when -jre symbols are used will cause animalnsiffer failures, because it uses APIs not available on the target platform. But that is not universally the case. For example, the way we noticed this was Predicate.test(). The -jre version provides the test() method, because it extends java.util.function.Predicate. But the -android version does not even though it could without requiring Java 8 APIs.

Example

In a build.gradle:

dependencies {
  implementation 'com.google.guava:guava:33.2.1-android'
}


In a Foo.java:


final class Foo {
  public boolean foo(com.google.common.base.Predicate<Object> p) {
    return p.test(null);
  }
}

Expected Behavior

Javac could not find symbol Predicate.test().

Actual Behavior

./gradlew compileJava succeeds.

Packages

No response

Platforms

No response

Checklist

• I agree to follow the code of conduct.

• I can reproduce the bug with the latest version of Guava available.

[cpovirk]

Ouch :( Thanks for the report, and sorry.

It's convenient that we just exposed a bunch more APIs in the Android flavor in Guava 33.4.0 and that, as you note, most of what's been "missing" in Android is APIs that would trigger Animal Sniffer / NewApi errors, anyway. But as you also note, there's at least one high-profile exception, Predicate.test.

We've made some attempts to make our Predicate extend the JDK's Predicate, including adding test, internally. (We couldn't do it externally, of course, unless we were to start requiring library desugaring.) We ran into problems with some unusual build setups (b/336331059, b/336342607, cl/634384683), so we backtracked.

However, I don't think we'd considered what you've pointed out, which is that we could add test without extending the JDK Predicate type. In addition to helping with this issue that you've just filed, that might even serve as a stepping stone that could make me more confident in extending the JDK Predicate type internally in the future. I'm going to at least experiment with that. I'll report back.

[cpovirk]

• Another thing that we could look into is whether we could publish Animal Sniffer signatures for the Guava APIs, which presumably projects like gRPC could use to verify their compatibility with guava-android. But that's far from ideal, especially since users would need to opt in to enforcement.
• There may of course be Gradle magic that would enable you to express that gRPC is an "Android library" (though of course it is actually cross-platform, and I didn't know how well the model supports that) or at least that it requires the Android flavor of Guava. Given experiences like Forcing guava JRE versions in Android projects with new metadata is too tedious #6801, though, I'm not sure how smoothly that would go.

[cpovirk]

Even adding Predicate.test (without introducing the supertype) causes trouble (cl/709852915) :( [edit: Oh, huh, I had previously tried that back in cl/441739362.]

As before, it's possible that everything would work fine if I were to also add it externally, but I'm not sure, so I would be nervous about trying it.

I'm not sure if it's worth falling back to the Animal Sniffer route or to playing with Gradle. Do you have any additional ideas?

[ejona86]

gRPC has a parallel Bazel build for some components. That build doesn't notice the Gradle Module Metadata so can detect API cases. That's how we found -android didn't support test(). It wouldn't find ABI incompatibilities, but that seems unlikely to vary much in -jre vs -android.

I'm most disappointed by how hard it was/is to figure out what was going on. dependencyInsight can show you the results, but you already have to know exactly what you're looking for.

I tried the various attributes and select() goo. I got some of it working, but it was fickle and half the time when it breaks it quietly swaps back to the jre verion, so I don't have any real confidence in it. It also gave confusing errors about com.google.collections:google-collections apparently because of Guava itself; we don't have that in our dependency tree.

(I didn't get our re-implementation of requireUpperBoundDeps working, used to verify Maven wouldn't make poor decisions.)

I played with it a lot yesterday writing this reply, and I think the only reasonable play is to disable Module Metadata. It is simply too complicated with poor control and debugging tools. Even if I learned it enough to control it, there's no way my wider team does. Dependencies are hard enough without having to read every dependency's module metadata to understand what may impact my project. Without a high-level view like gradle dependencies, it is too dangerous. I'm about to send out a PR disabling it for our entire build:

    mavenCentral() {
        metadataSources {
            mavenPom()
            ignoreGradleMetadataRedirection()
        }
    }

We'll see how long it takes for this to bite us. I'm suspecting Kotlin will be the problem.

FWIW, the failures caused by adding Predicate.test() I looked at seem will be just as much a problem in the future as today; they don't seem JDK 8 API-related and there seems to be two root causes.

If Predicate.test() is the only thing in this special category of complication, I could live with that aspect. It's the surprises that worry me.

[cpovirk]

Thanks, that's probably the best course of action. (And we'll likely follow suit with any KMP libraries that we publish, since presumably they'll end up using Gradle, too.)

I assume that completely removing the Gradle Module Metadata at this point would be worse than leaving it in place, but maybe the problems will continue to trickle in until I have to reevaluate.

---

It's probably still possible to produce an error that wouldn't be caught by Animal Sniffer, but I'm hoping it would have to be something like "Call one Guava method that returns a Duration and pass it to a Guava method that accepts a Duration without otherwise touching it." I just skimmed the JRE-vs.Android diffs again, and nothing else is jumping out at me, at least.

[ejona86]

Yeah, I have no clue whether it is better to keep or remove the Gradle Module Metadata.

Thanks for double-checking the diffs. I knew you had them between versions, but I didn't realize you have them between -jre and -android (and yes, they are pretty easy to find in the release notes). That makes sense and is very helpful. I looked over them as well and agree; the only other missing methods that technically could be implemented are useless mutation methods for Immutable*.

I see what you mean about the Animal Sniffer hole of not calling any methods on a Java 8 class, but just using Guava APIs to interact with it. That could crop up in other cases, but Guava is much easier to do it with. I actually didn't realize y'all excluded Duration overloads in -android. gRPC's most recent release actually added them multiple places. I had thought missing methods was more limited to things like Function, which wouldn't be efficient to provide as you'd have to allocate an adapter. Good to realize the hole is there.

[ejona86]

I'm comfortable calling this resolved. I wanted it documented in case others were bit and thought you would want to be aware. I also feel comfortable that there aren't many ways for a gradle-using library to be surprised.

I'll let you decide when to close it, in case you want it open to mull over.