guava-27.0-jre.jar contains failureaccess class files

[michaelhixson]

I tried upgrading to guava 27.0 in my project and noticed a new warning at build time:

[WARNING] guava-27.0-jre.jar, failureaccess-1.0.jar define 2 overlapping classes:
[WARNING]   - com.google.common.util.concurrent.internal.InternalFutureFailureAccess
[WARNING]   - com.google.common.util.concurrent.internal.InternalFutures
[WARNING] maven-shade-plugin has detected that some class files are
[WARNING] present in two or more JARs. When this happens, only one
[WARNING] single version of the class is copied to the uber jar.
[WARNING] Usually this is not harmful and you can skip these warnings,
[WARNING] otherwise try to manually exclude artifacts based on
[WARNING] mvn dependency:tree -Ddetail=true and the above output.
[WARNING] See http://maven.apache.org/plugins/maven-shade-plugin/

Indeed, I can see those .class files in the guava-27.0-jre.jar I downloaded from Maven. Are those class files supposed to be there? I ask because it seems like they are supposed to be in the failureaccess jar only, but they were included in the guava jar by mistake.

[martinm1000]

I think this is also a problem with JPMS modules, where the same package is in guava.jar and failureaccess.
Are we (non android projects) supposed to exclude failureaccess ? ( I've read https://groups.google.com/forum/#!topic/guava-announce/2nR5GdDdBYo but it isn't clear for me)

the unnamed module reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess

[michaelhixson]

I can reproduce the issue (a guava jar containing the failureaccess classes) locally by building the release artifact:

mvn clean package --projects guava -DskipTests

That tells me this issue is caused by a problem with a pom.xml probably.

I think the problem is here in the maven-bundle-plugin configuration:

guava/guava/pom.xml

Line 81 in 292118d

<Export-Package>!com.google.common.base.internal,com.google.common.*</Export-Package>

I don't know what maven-bundle-plugin does, but if I change that line to this...

<Export-Package>!com.google.common.base.internal,!com.google.common.util.concurrent.internal,com.google.common.*</Export-Package>

...then the built guava jar doesn't contain the failureaccess classes anymore.

[talios]

I notice that failureaccess/pom.xml doesn't include any OSGi bundle plugin configuration either, so once this is fixed and those classes are removed, Guava 27 probably won't work under OSGi without adding OSGi metadata to failureaccess and exporting such "internal" packages.

[rgoldberg]

I get 17 errors after upgrading Guava from Guava 26.0-jre to 27.0-jre in a Gradle 4.10.2 build (that uses my Jigsaw plugin to enable JPMS modules) that previously worked fine on 26.0-jre (maybe I could exclude com.google.guava:failureaccess from being transitively included, but the normal use case for guava shouldn't require anything other than a standard dependency, so I imagine that the whole artifact dependency hierarchy should be redone):

error: the unnamed module reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module kotlin.stdlib reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module gradle.api reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module groovy.all reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module gradle.installation.beacon reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module kotlin.stdlib.jdk8 reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module failureaccess reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module listenablefuture reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module checker.qual reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module error.prone.annotations reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module j2objc.annotations reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module animal.sniffer.annotations reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module kotlin.stdlib.jdk7 reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module kotlin.stdlib.common reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module annotations reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module com.google.common reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess
error: module mymodule reads package com.google.common.util.concurrent.internal from both com.google.common and failureaccess

[cgdecker]

The failureaccess classes are definitely not intended to be included in the main guava jar.

I've got a commit (a980611) that I'm hoping should fix this. I've confirmed at least that the failureaccess classes should no longer be included in the guava jar. I don't know much about OSGi though, so could someone check that the changes look right?

[stari4ek]

same for com.google.guava:guava:27.0-android

Program type already present: com.google.common.util.concurrent.internal.InternalFutures

[michaelhixson]

@cgdecker I have never used OSGI, so this may be a dumb question: When one builds the failureaccess jar, is its META-INF/MANIFEST.MF expected to contain an Export-Package line and a bunch of Bundle--prefixed lines?

I ask because when I pull your changes and then build failureaccess with mvn clean install, its MANIFEST.MF does not contain those lines. It's strange... I see another MANIFEST.MF that does contain those lines outside of the jar -- in my target/classes/META-INF directory -- but the one inside the jar is more basic. Do you see the same thing? Am I running the wrong command to build failureaccess?

Edit: Ohh, I think you need to add <packaging>bundle</packaging> in the failureaccess pom. That fixes the wrong-looking manifest in the jar for me locally.

[talios]

As @michaelhixson mentions - it looks like you're missing the <packaging>bundle</packaging> from failureaccess, switching on the bundle lifecycle which tells the Jar plugin to include the newly generated manifest.

[cgdecker]

@michaelhixson @talios Thanks, fixed that in 218bfd4.

[markkolich]

+1 here from the duplicate-finder-maven-plugin:

[INFO] --- duplicate-finder-maven-plugin:1.3.0:check (duplicate-check) @ common-test ---
[INFO] Checking compile classpath
[INFO] Checking runtime classpath
[INFO] Checking test classpath
[WARNING] Found duplicate (but equal) classes in [com.google.guava:failureaccess:1.0, com.google.guava:guava:27.0-jre]:
[WARNING]   com.google.common.util.concurrent.internal.InternalFutureFailureAccess
[WARNING]   com.google.common.util.concurrent.internal.InternalFutures

@cgdecker in the meantime, is it "safe" to add an <exclusion> to keep com.google.guava:failureaccess from being pulled in transitively via 27.0-jre?

<dependency>
    <groupId>com.google.guava</groupId>
    <artifactId>guava</artifactId>
    <version>27.0-jre</version>
    <exclusions>
        <!-- ugly, but safe? -->
        <exclusion>
            <groupId>com.google.guava</groupId>
            <artifactId>failureaccess</artifactId>
        </exclusion>
    </exclusions>
</dependency>

Lastly, is it worth introducing the org.basepom.maven:duplicate-finder-maven-plugin into the build to scan for duplicate classes in Guava prior to release? I'm happy to submit a PR if the maintainers and community-at-large feel that would be a worthwhile enhancement.

[natros]

you can do that with maven-enforcer-plugin using the rule banDuplicateClasses:
https://www.mojohaus.org/extra-enforcer-rules/banDuplicateClasses.html

[jodastephen]

@cgdecker Removing these classes from the main jar is not the right solution though. We care about the number of jar file dependencies, so splitting Guava into multiple pieces is very painful. And pushing a v9999 hack on the world really is a gross failure on the part of Guava maintainers.

Given that you have separate releases for -jre and -android, surely the -jre release should just be a single jar file, with the -android release split into pieces. This would avoid all the problems you are seeing.

[iman2420]

@stari4ek try to use this implementation:

`  implementation(group: 'com.google.guava', name: 'guava', version: '27.0-jre') {
    exclude group: 'com.google.guava' , module: 'failureaccess'
}`

[cgdecker]

@jodastephen Why do you care about the number of jar files? Just wondering, that's somewhat surprising to me.

In any case, this change isn't really about JRE vs. Android, though it was motivated by Android in some ways. The goal of the change was to allow a project to use ListenableFuture in its API without pulling in the rest of Guava (the Android Support libraries wanted to be able to do this without that dependency because of how widely used they are on Android) by depending on the listenablefuture artifact and implementing ListenableFuture internally to the project. The failureaccess artifact is to allow those internal implementations of ListenableFuture to provide a hook that Guava's utilities can use to do some things more efficiently.

Since such a project needs to pull in the failureaccess classes to use and we need to ensure that both Guava and the project are using consistent definitions of those, we use a separate artifact that both depend on... pretty standard I feel like. Unfortunately I can't speak in great detail as to why we did it exactly like this because I was largely not involved in this, and the person who was (@cpovirk) is on leave until January.

For now, I'm just trying to fix this to not duplicate the classes between the two jars (which was unintended). We can see about trying to do something else from there.

[jodastephen]

@cgdecker The approach you have taken to splitting out a submodule is indeed very standard, and well suited to corporate codebases and larger projects. But for a foundational library like Guava it is far from ideal. Put simply, it is not the case that all users ignore the number of jar file dependencies - for some users, details of, and number of, those dependencies really matters,

There are going to be plenty of projects that have a dependency on Guava and nothing else. Those projects will now be seeing a tripling of the number of dependencies. This affects me (more or less) with Joda-Beans and Joda-Convert.

In addition, many projects (like OpenGamma Strata) have to list all their dependencies in user documentation, and the version numbers thereof. The change requires both new dependencies to be added to the docs, and some kind of additional note added to try and explain the horrible v9999 hack.

In essence this gives the appearance of Google making a change for its convenience (Android) rather than providing stability and security for non-Google users, which is what some of us have worried about all along with Guava. Particularly as now the floodgates of dependencies are open I fear we willl see Guava split into many more jar files.

[jbduncan]

In addition, many projects (like OpenGamma Strata) have to list all their dependencies in user documentation, and the version numbers thereof. The change requires both new dependencies to be added to the docs, and some kind of additional note added to try and explain the horrible v9999 hack.

Hmm, that doesn't sound good. :(

In essence this gives the appearance of Google making a change for its convenience (Android) rather than providing stability and security for non-Google users, which is what some of us have worried about all along with Guava. Particularly as now the floodgates of dependencies are open I fear we willl see Guava split into many more jar files.

I can definitely see why Stephen sees it this way - the "failureaccess" artifact-based workaround seemed and still seems very strange to me, and if I weren't already on good terms with @cpovirk and everyone else on the Guava team, I think I might've taken it as an antagonistic move (although I'm sure that wasn't intended at all!)

When @cpovirk is back from leave, I'd love to hear his reasoning for the introduction of this artifact, and to discuss things further (whether publicly or privately) to see if it would be possible to achieve what the Android Support library authors would like whilst satisfying everyone else. :)

[talios]

When @cpovirk is back from leave, I'd love to hear his reasoning for the introduction of this artifact, and to discuss things further (whether publicly or privately) to see if it would be possible to achieve what the Android Support library authors would like whilst satisfying everyone else. :)

Guava was always traditionally very restrained in making changes because of the impact those changes had on the larger Google codebase ( I'm reminded of Kevin Bourrillion's post https://plus.google.com/113026104107031516488/posts/ZRdtjTL1MpM from back in 2011 ), I'm not sure if the current process is as strict on changes for Guava, and I guess the extraction of failureaccess wasn't made lightly - just not as cleanly/elegantly as could have been.

From my perspective as an OSGi user, and one who strictly excludes all transitive dependencies from our build poms, the extra artifact, and the issue of failureaccess being released as not being OSGi aware, and thus breaking guava - whilst not a major issue, does highlight something/somewhere was missed during the release cycle.

[talios]

Has there been any follow up on this at all?

[jbduncan]

Nope, I'm still waiting for @cpovirk to come back from leave. :(

[cgdecker]

I've released Guava 27.0.1, which no longer contains the failureaccess class files. It depends on failureaccess 1.0.1, which has the OSGi metadata.

I'm going to close this issue since it was about the duplicate classes; feel free to open a separate issues for any specific issues with the current dependency setup.

[jbduncan]

Okay @cgdecker!

I've gone and moved the discussion about the listenablefuture and failureaccess artifacts to #3320; I hope that's okay?

[vanminhphan2]

@stari4ek try to use this implementation:

`  implementation(group: 'com.google.guava', name: 'guava', version: '27.0-jre') {
    exclude group: 'com.google.guava' , module: 'failureaccess'
}`

thank so much, it's work for me! ^^