Write release notes for Gradle Module Metadata change

cpovirk commented 1 year ago

@jjohannes, thanks for offering to have a look and fill in what I'm missing. Here's what I've come up with. You can post suggestions for edits, or you can copy the whole thing and post a comment with your full, revised version.

Gradle Module Metadata

The Gradle team has contributed a metadata file for Guava. If you use Gradle 6 or higher, you will see better handling of two kinds of dependency conflicts:

Selecting the appropriate flavor

When Gradle automatically selects the newest version of Guava in your dependency graph, it will now also select the appropriate flavor (-android or -jre) based on whether you project targets Android or not. For example, if you depend on 32.1.0-android and 30.0-jre, Gradle will select 32.1.0-jre. This is the version most likely to be compatible with all your dependencies.

If you need to override Gradle's choice, you can TODO: something like the following? I thought I'd convinced myself that I understood which part(s) of it were necessary, but now I'm confused again, sorry: https://github.com/google/guava/blob/ce78fc67cb8b72621e2d1154e8b8b343be97abd0/integration-tests/gradle/build.gradle.kts#L145-L167

Reporting dependencies that overlap with Guava

If your dependency graph contains the very old google-collections or the hacky listenablefuture, Gradle will now report that those libraries contain duplicates of Guava classes. When this happens, you'll need to tell Gradle to select Guava:

configurations.all {
    resolutionStrategy.capabilitiesResolution.withCapability("com.google.collections:google-collections") {
        select("com.google.guava:guava:0")
    }
    // and/or
    resolutionStrategy.capabilitiesResolution.withCapability("com.google.guava:listenablefuture") {
        select("com.google.guava:guava:0")
    }
}

(I could also mention that we'll now omit j2objc-annotations from the runtime classpath, but I think we may be better off focusing on the bigger changes, since there's plenty to discuss. Let me know if you disagree.)

jjohannes commented 1 year ago

Here is a better version of Listing 1 (the code you took from the test contains reflection to not have old Gradle versions break). I think for now you can assume that most folks use Gradle 7 or 8 where all of this syntax is supported:

dependencies.constraints {
  implementation("com.google.guava:guava") {
    attributes {
      attribute(TargetJvmEnvironment.TARGET_JVM_ENVIRONMENT_ATTRIBUTE, 
              objects.named(TargetJvmEnvironment, TargetJvmEnvironment.ANDROID))
    }
  }
}

// If the above leads to a conflict error, because there are additional transitive dependencies to Guava:
configurations.all {
  resolutionStrategy.capabilitiesResolution.withCapability("com.google.guava:guava") {
    select(candidates.find { it.variantName.contains("android") })
  }
}

2nd Listing and text all sounds good to me!

Regarding the runtime classpath: Maybe add just one general sentence on the topic?

Dependencies of Guava that are not needed at runtime are omitted from the runtime classpath (see #2824).

About the above topic: would it be helpful if I create a draft PR that also removes the other annotation libraries from the runtime classpath to further discuss that topic? In my opinion, it would be the better "default". Or is there already an issue on the topic that is still open (as #2824 is closed)?

There is a little thing I found while testing this gain. I don't like how the variants are "named" now. I think this is the result of the back and forth with some things in the original PR over the years. Usually the "names" are not that important (the attributes are the central thing) but there are places where they shine through if you need to fine tune things in Gradle.

Right now, in the metadata, the JRE variants are named like this: "name": "standard-jvmApiElements", I would prefer to use "jreApiElements" here instead to be in line with what it is called in Guava elsewhere (also the - in the name that otherwise uses camelCase is bugging me)

I created a small follow up PR to do that change if you agree: #6605

cpovirk commented 1 year ago

Thanks for the simplified text.

I can include the text about runtime dependencies.

I'll get the PR with the rename merged.

I'll try to think more about the other annotation dependencies. I've spent more time on Guava lately than I'd expected, and while I'm very happy with the results, I also want to postpone further work when I can get away with it, especially when there's a chance that any of the recent changes could end up requiring yet more work :) It's up to you whether you want to put together a PR now.

jjohannes commented 1 year ago

Thanks @cpovirk for your investment in this. I am fine with all of that and very happy that this got integrated eventually.

I created #6606 about the runtime classpath thing. It's also for myself to remember where this topic stands right now in case this should pop up in the future again. Don't feel obliged to spend any time on that now. Just leaving it there for the community to share feedback is totally fine for me.

Also: If after the next release, any issues pop up because of this, feel free to tag me in GitHub comments. I am happy to answer questions or help with problems.

google / guava