Confusing artifact name for caffeine guava adapter: guava-<version>.jar

[davido]

While trying to integrate Caffeine in Gerrit Code Review, we discovered that Caffeine Guava adapter library has the final artifact guava and can be easily confused with the Google's library.

Before integration with Caffeine, the gerrit.war has two artifacts with guava in its name:

  $ unzip -t bazel-bin/gerrit.war
    testing: WEB-INF/lib/guava-retrying-2.0.0.jar   OK
    testing: WEB-INF/lib/guava-28.1-jre.jar   OK

After the integration we have 3:

  $ unzip -t bazel-bin/gerrit.war | grep guava
    testing: WEB-INF/lib/guava-retrying-2.0.0.jar   OK
    testing: WEB-INF/lib/guava-2.8.0.jar   OK
    testing: WEB-INF/lib/guava-28.1-jre.jar   OK

It would be better to rename the guava artifact in Caffeine project to caffeinated-guava or caffeine-guava-adapter or some such to avoid potential collision.

Say at some point, both projects have reached version 42. In this case we would have a collision, and would require some hacks during the build process, like renaming the artifact names fetched from Maven Central or similar.

[ben-manes]

Thanks! I hadn't considered the artifacts being cobbled together in a single directory for the distribution. At build time they are separated by Maven/Gradle/etc into coordinate named directories. The archive name going forward will follow your suggestion.

[ben-manes]

This will have to wait until 3.0 since it changes the Maven coordinate. Ideally the lib would qualify the name of all jars to avoid conflicts, but I cannot fix it here without a significant impact.

[ben-manes]

The simplest solution would be to move off of the Guava adapters. That change shouldn't be too hard and drops the dependency, thereby avoiding this confusion.

[davido]

The simplest solution would be to move off of the Guava adapters.

We can't do it yet, because one single cache implementation in Gerrit is not recursion free. This is the tracking issue: [1].

For that reason, we switched PatchListCache to using Guava cache backend.

That why we still need Guava adapters for not intrusive changes to support per cache switch.

Any ETA for the fix here? Because we have to do some build acrobatic with artifact fetch and rename, we are hitting some Bazel specific issues: [3].

With the dedicated GAV: com.github.ben-manes.caffeine:caffeine-guava-adapter:v we could just use:

CAFFEINE_VERS = "3.0.0"

maven_jar(
    name = "caffeine",
    artifact = "com.github.ben-manes.caffeine:caffeine-guava-adapter:" + CAFFEINE_VERS,
    sha1 = <SHA1>,
)

[1] https://bugs.chromium.org/p/gerrit/issues/detail?id=11900
[2] https://gerrit.googlesource.com/gerrit.git/+/1d8f5ca43512ea7bc293520bfdbca23f1ad663fd
[3] bazelbuild/bazel#10612

[ben-manes]

When I looked into this previously, Maven's perspective on this is that the consuming build should rename it if there is a desire to qualify the file name. For example the WAR plugin offers outputFileNameMapping to resolve this. The Gradle team appears to make the same argument based on following the Maven team's decisions on this topic, e.g. for its WAR plugin. So while inconvenient and an annoyance of how Maven decided to name jars in the repository format, it appears to be left to the build tool to rectify if need be.

It would be a breaking change to blindly rename the artifact. If that was done or both coordinates were produced, it would violate the Jigsaw module rules where both jars could be on the classpath (as different coordinates) and own the same package. The only safe solution is if we instead provided the classes in a new package under a new coordinate.

Renaming when creating the WAR appears to be the consensus of build tools and seems like the best of the bad options at our disposal.

[davido]

Ping. Any progress here or ETA?

[ben-manes]

I have no plans to break compatibility. As described above, this is not a safe change. It is up to the build system to decide how it packages jars and both Maven and Gradle suggest using a custom mapping rule. It was an unfortunate oversight in the initial naming, but the responsibility of the build tool which has the coordinates to work from.

[ben-manes]

Closing due to being backwards incompatible, that build systems consider it the consumer's responsibility (and make it easy to resolve), and that the impact poses no harm.