Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

guava:32.1.1-jre conflicts with google-cloud-datastore:2.16.0 #6642

Closed
brettkail-wk opened this issue Jul 20, 2023 · 16 comments
Closed

guava:32.1.1-jre conflicts with google-cloud-datastore:2.16.0 #6642

brettkail-wk opened this issue Jul 20, 2023 · 16 comments
Assignees
Labels
P1 package=general type=defect Bug, not working as expected

Comments

@brettkail-wk
Copy link

Using gradlew 7.4, this build.gradle file:

apply plugin: 'java'

repositories {
    mavenCentral()
}

dependencies {
    implementation 'com.google.cloud:google-cloud-datastore:2.16.0'
    implementation 'com.google.guava:guava:32.1.1-jre'
}

print configurations.runtimeClasspath.files

...fails like this:

$ gradlew dependencies

FAILURE: Build failed with an exception.

* Where:
Build file '/tmp/gradle-guava-error/build.gradle' line: 13

* What went wrong:
A problem occurred evaluating root project 'test'.
> Could not resolve all files for configuration ':runtimeClasspath'.
   > Could not resolve com.google.guava:guava:32.1.1-jre.
     Required by:
         project :
      > Module 'com.google.guava:guava' has been rejected:
           Cannot select module with conflict on capability 'com.google.guava:listenablefuture:1.0' also provided by [com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava(runtime)]
   > Could not resolve com.google.guava:guava:31.0.0-jre.
     Required by:
         project :
      > Module 'com.google.guava:guava' has been rejected:
           Cannot select module with conflict on capability 'com.google.guava:listenablefuture:1.0' also provided by [com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava(runtime)]
   > Could not resolve com.google.guava:guava:32.0.1-jre.
     Required by:
         project : > com.google.cloud:google-cloud-datastore:2.16.0
      > Module 'com.google.guava:guava' has been rejected:
           Cannot select module with conflict on capability 'com.google.guava:listenablefuture:1.0' also provided by [com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava(runtime)]
   > Could not resolve com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava.
     Required by:
         project : > com.google.cloud:google-cloud-datastore:2.16.0
         project : > com.google.cloud:google-cloud-datastore:2.16.0 > com.google.api.grpc:grpc-google-cloud-datastore-admin-v1:2.16.0
      > Module 'com.google.guava:listenablefuture' has been rejected:
           Cannot select module with conflict on capability 'com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava' also provided by [com.google.guava:guava:32.1.1-jre(jreRuntimeElements)]

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 314ms

It succeeds with 32.0.1-jre or if excluding the transitive listenablefuture dependency from datastore:

    implementation dependencies.create('com.google.cloud:google-cloud-datastore:2.16.0') {
        exclude group: 'com.google.guava', module: 'listenablefuture'
    }
@netdpb
Copy link
Member

netdpb commented Jul 21, 2023

Do you have any "capability" configuration in your build script? That looks like what's emitting those errors.

Alternatively, are you using the Google Cloud Libraries Bill of Materials (BOM)? That should help you use compatible versions of libraries.

@netdpb netdpb added type=defect Bug, not working as expected package=general labels Jul 21, 2023
@suztomo
Copy link
Member

suztomo commented Jul 21, 2023

@brettkail-wk The BOM should help you https://cloud.google.com/java/docs/bom#gradle (the same document as David shared above)

Declaring Guava version explicitly may help too. I remember @lqiu96 did that in one of our repositories.

@lqiu96
Copy link

lqiu96 commented Jul 21, 2023

I think I saw a similar error message here: googleapis/sdk-platform-java#1832 (comment)

I ended up specifying the guava version, which I believe I see you also do above. Can you see if the comments above help by using the latest libraries-bom (v26.19.0)? The v26.19.0 that one is the one we updated to use guava v32.1.1-jre and I believe that version of the bom uses datastore v2.16.1 (https://github.com/googleapis/java-cloud-bom/blob/25ca49ed39e0337f16ab310c20f4eafb53a5db27/google-cloud-bom/pom.xml#L196)

@cpovirk
Copy link
Member

cpovirk commented Jul 24, 2023

Does the suggestion at https://github.com/google/guava/releases/tag/v32.1.0#user-content-overlap work? I don't know enough about Gradle to say how it compares to the exclusion solution above.

@joergi
Copy link

joergi commented Jul 24, 2023

I used the workaround from this bug

 implementation("com.google.cloud:google-cloud-datastore:2.16.1") {
        modules {
            module("com.google.guava:listenablefuture") {
                replacedBy("com.google.guava:guava", "listenablefuture is part of guava")
            }
        }
    }

that worked for me.

@cpovirk
Copy link
Member

cpovirk commented Jul 26, 2023

Thanks. I'm in over my head a bit here. https://github.com/google/guava/releases/tag/v32.1.0#user-content-overlap would still be the first thing I would try. In case that fails, I've added a link to what I'm hoping is a more general version of the replacedBy approach, which is what I'd try next. Failing that, there's yet another replacedBy approach and an api approach in googleapis/sdk-platform-java#1832 (comment), which I've also linked to. It's quite possible that different solutions work in different cases.

@cpovirk
Copy link
Member

cpovirk commented Jul 28, 2023

I am going to look at backing out the handling of listenablefuture from the Gradle metadata: The reports here, including from Google Cloud users, suggest that the handling is going to create problems for increasingly many users.

The good news is that the listenablefuture handling isn't doing as much for users as the other parts of the metadata [edit: previously]: It's not preventing actual runtime problems (as the google-collections handling and especially jre/android handling is), and it's not trimming dependencies (as we've started doing with our annotation dependencies). It was nice that it was less of an ugly hack than our "9999.0" solution (which caused some kind of problem for Android Studio, which they graciously introduced a workaround for), but after the Android Studio fix, I'm not sure it was causing concrete harm.

(@jjohannes FYI)

copybara-service bot pushed a commit that referenced this issue Jul 28, 2023
…y `listenablefuture-9999....jar` instead of making Gradle report a conflict that users need to [resolve](https://github.com/google/guava/releases/tag/v32.1.0#user-content-overlap).

See discussion in #6642: The Gradle module metadata does multiple things, most of which bring clear benefits but one of which (the `listenablefuture` part) may bring more costs than benefits at this point. (Accordingly, this CL rolls back one tiny fragment of the larger cl/544108700.)

Fixes #6642

RELNOTES=Removed the section of our Gradle metadata that caused Gradle to report conflicts with `listenablefuture`.
PiperOrigin-RevId: 551959012
@jjohannes
Copy link
Contributor

The "right" way to solve the conflict would be to add this resolution strategy as suggested in the release notes:

configurations.all {
  resolutionStrategy.capabilitiesResolution.withCapability("com.google.guava:listenablefuture") {
    select("com.google.guava:guava:0")
  }
}

What I am surprised about is that you get a 9999.0-empty-to-avoid-conflict-with-guava version from another place than Guava. Why would any other library have a dependency to that? For me it looks like the metadata of that library – in this case com.google.api.grpc:grpc-google-cloud-datastore-admin-v1 – is wrong and something should be fixed there. Looking at that particular metadata. It seems to be – excuse my wording – bogus. It re-lists all transitive dependencies of Guava. The problem for me is clearly there and not in Guava.

It would be very sad to add the dependency to an empty Jar back to Guava. From my perspective, this is bad practice as everyone using Guava ends up with an empty Jar polluting there runtime classpath – shipping as part of their application in many cases - until they explicitly remove it. I consider removing these unneeded runtime dependencies one of the main advantages with the Gradle metadata. If we make these compromises, we could IMO almost roll back the whole thing. I wonder if it is still worth the extra maintenance effort. Which I can understand if that is what you like to do. The dependencies setup in the Java ecosystem is broken in so many places. Many folks publishing to Maven Central don't seem to understand the consequences of the dependency information they publish.

But of course you get all the reports here, because users do a Guava update and then something does not seem to work anymore that worked before.

For me issues like this show that what we did is right. Issues with metadata in other libraries are now revealed instead of silently hidden. But of course I also understand that many users won't care about that.

@suztomo
Copy link
Member

suztomo commented Jul 29, 2023

It re-lists all transitive dependencies of Guava.

These Google Java libraries use flatten Maven plugin when publishing pom.xml. https://www.mojohaus.org/flatten-maven-plugin/ This increases the likelihood of library users getting the same transitive dependency versions as the library was built with.

Do you know Why redeclaring the transitive dependency, the same version as Guava's, causes Gradle's capability problem (meaning that Gradle considers the redeclaration of dependency, the same version, harmful)?

@jjohannes
Copy link
Contributor

I am not very familiar with the flatten-maven-plugin, but I think the flattening does not mean that it re-lists all transitive dependencies. At least that does not sound like a useful default behaviour to me. I think it is about flattening the parent POM hierarchy.

Can you point me at the source repo for 'grpc-google-cloud-datastore-admin-v1'? I would be interested in what is going on, but wasn't able to find the sources.

The problem right now ist that the dependency was removed from Guava completely for Gradle users. Because we don't need it anymore to avoid potential problems. Instead we use Gradle's capability concept (see also https://blog.gradle.org/guava).

'grpc-google-cloud-datastore-admin-v1' now adds this dependency back that we do not want to have anymore. And the new capability-based solution then reports the conflict.

@cpovirk
Copy link
Member

cpovirk commented Jul 29, 2023

Thanks to you both for the information.

My vague memory (as someone who occasionally poked my head in on conversations years ago) is that Google Cloud (like, e.g., Spring) wants to use the same version of all dependencies across all the artifacts that it releases. They do that with a BOM. But a BOM uses dependencyManagement, and dependencyManagement affects what versions of dependencies project Foo gets during a build of Foo but not what versions of dependencies a user of Foo gets during the user's build (which is what actually matters to users). So the fix is to flatten(?) the transitive dependencies into Foo. (We're of course talking Maven here.)

Perhaps each Google Cloud project could exclude the listenablefuture dependency and then set some omitExclusions property that I just looked at?

@suztomo
Copy link
Member

suztomo commented Jul 30, 2023

The source of grpc-google-cloud-datastore-admin-v1 is https://github.com/googleapis/java-datastore/blob/main/grpc-google-cloud-datastore-admin-v1/pom.xml. Its parent is google-cloud-shared-config https://github.com/googleapis/java-shared-config/blob/main/pom.xml#L238, which uses flatten-maven-plugin with <flattenDependencyMode>all</flattenDependencyMode>:

        <plugin>
          <groupId>org.codehaus.mojo</groupId>
          <artifactId>flatten-maven-plugin</artifactId>
          <version>1.3.0</version>
...
          <configuration>
            <flattenMode>oss</flattenMode>
            <flattenDependencyMode>all</flattenDependencyMode>
            <pomElements>
              <build>remove</build>
            </pomElements>
          </configuration>

the dependency was removed from Guava completely for Gradle users

That's new information to me. Thank you. I was only looking at its pom.xml where it shows:

image

cpovirk has shared https://github.com/google/guava/blob/347ef4ec219fed3b2801c094d800af1dd5be1a76/guava/module.json to me. (A module.json file is new to me.) I guess it's dependencies section, which does not have the listenablefuture is the part you're referencing:

image

Perhaps each Google Cloud project could exclude the listenablefuture dependency and then set some omitExclusions property that I just looked at?

For future releases, it might work. But I have concern about the restriction that Gradle users cannot use the latest Guava and Google Cloud libraries prior to August 2023 together.

@cpovirk
Copy link
Member

cpovirk commented Jul 30, 2023

One other thing I might try to look into: Would it be better if Guava claimed to serve as an implementation of version 9999... of listenablefuture instead of version 1?

@jjohannes
Copy link
Contributor

Thank you for the explanations.

I don't understand though why <flattenDependencyMode>all</flattenDependencyMode> is used in the first place. It causes libraries to declare dependencies they don't have. Which can cause all kind of unexpected behaviour. Especially if you expect folks to update some of the used libraries to a different version. Like you expect things to keep working with a newer Guava version which this issue is about.

You can't expect a tool like Gradle or Maven to keep doing good automatic conflict resolution if you feed it wrong metadata. This is a general problem with the flattenDependencyMode setup in google-cloud-shared-config. This issue is just one of the things that can happen (and now happened). If Guava would make other changes to its dependencies, you can run into different trouble - with Gradle or Maven alike.

The fix for me is clearly to not do <flattenDependencyMode>all</flattenDependencyMode>. You can also use the omitExclusions as @cpovirk suggested. But then you should also exclude the other dependencies you don't have (probably the annotation libraries Guava brings in). It would be much clearer if you only declare the dependencies you need.

As for the current situation: There is nothing broken from my perspective. Users can address the capability conflicts as described further up. I think changing something in the Guava metadata in the next release would be the wrong thing to do. If you do that as a reaction to the discussion here, you admit that you cannot change anything in the dependencies ever, because upstream consumers decided to blindly copy your dependencies at a point in the past and now you are "locked in" because of them.

This should be fixed in google-cloud-shared-config.

copybara-service bot pushed a commit that referenced this issue Jul 31, 2023
…y `listenablefuture-9999....jar` instead of making Gradle report a conflict that users need to [resolve](https://github.com/google/guava/releases/tag/v32.1.0#user-content-overlap).

See discussion in #6642: The Gradle module metadata does multiple things, most of which bring clear benefits but one of which (the `listenablefuture` part) may bring more costs than benefits at this point. (Accordingly, this CL rolls back one tiny fragment of the larger cl/544108700.)

Fixes #6642

RELNOTES=Removed the section of our Gradle metadata that caused Gradle to report conflicts with `listenablefuture`.
PiperOrigin-RevId: 551959012
@cpovirk
Copy link
Member

cpovirk commented Jul 31, 2023

(I had that commit set up to auto-merge when I sent it for review on Friday; I didn't think about the fact that it might end up silently auto-closing this issue in the middle of the discussion.)

I think there are two things that flattenDependencyMode is addressing, one of which is of course "lol maven" and the other of which is about the meaning of "having a dependency."

The Maven problem comes up if google-cloud-foo depends on google-cloud-bar, which in turn uses an API added in (say) guava-32.1.0. A Maven user of google-cloud-foo might get guava-32.1.0. But if the user also depends on ancientlib, which depends on guava-10.0, then Maven picks guava-10.0 because of the "closest wins" policy. (I am not telling you anything you don't know here, nor anything that Guava users don't know from bitter experience....) By using flattenDependencyMode, google-cloud-foo can improve the chances that its users get guava-32.1.0. (And, if google-cloud-foo is good about keeping its dependencies up to date in general, then it's going to help its users get the closest behavior they can to Gradle-style "newest dependency wins.")

The other part of the story is the meaning of "having a dependency": If some Guava developer introduces a bug into Files.createTempDir, and if your app uses JFrog, then you need your app to run against a fixed version of Guava even if your own bytecode never refers to Guava, since JFrog uses Files.createTempDir internally. In this sense, any transitive dependency of your library might still be a dependency whose version you care about. So, when Cloud is testing a large number of libraries together, they want to help users select the same versions of all those libraries in case some behavioral difference between versions of one of them leads to a problem.

The downside that I would not have predicted is what we're seeing here. (I do recall some thought to a related downside: If a user wants to exclude (say) Guava's checker-qual dependency, then the user can't easily exclude it "surgically" by saying "ignore Guava's dependency on checker-qual." Instead, the user has to either exclude it universally ("all usages of checker-qual") or else write an additional exclusion for each cloud library that had the checker-qual dependency flattened in from Guava. I'm not sure how much of a practical problem that is, since the user is presumably excluding checker-qual with the goal of keeping it off the classpath altogether. But I mention it as another sign that flattenDependencyMode is "hiding" information about which dependencies are direct in a way that may cause problems... or solve problems, as noted above :))

you admit that you cannot change anything in the dependencies ever, because upstream consumers decided to blindly copy your dependencies at a point in the past and now you are "locked in" because of them.

The good news is here is that we already despair of changing anything :-P I exaggerate, but there are reasons that we almost never remove even @Beta APIs anymore, almost never change our objects' serialized forms, and spend a bunch of time fixing an obsolete method's behavior under Windows, despite our efforts to steer users away from depending on those things. It's a curse I will accept in exchange for getting to work on a highly used library, and I can't really "blame" users when I can't claim that I read the README of every library that I transitively depend on, either.

To close, I do still want to emphasize that I expect the handling of the jre/android variants to be a big win for users, and so the metadata as a whole will be, too. (Plus, as you say, it will shake out some existing strange project setups, hopefully most of which are more easily changed than this one.) We still hear about variant issues regularly (e.g., #6567, google/compile-testing#363—both from Gradle users who should be in good shape once they pick up a recent Guava version).

@jjohannes
Copy link
Contributor

Thanks for explaining the relationship between Mavens "closes win" strategy and the flattenDependencyMode thing @cpovirk. Now I understand why things are how they are (although I am not very happy with the solution).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment