-
Notifications
You must be signed in to change notification settings - Fork 10.9k
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
Publish Gradle Module Metadata with Variants #3683
Conversation
Thanks very much for this. |
5894118
to
6343a50
Compare
We have published a blog post on the topic of how libraries can profit from Gradle Module Metadata and used Guava as an example: https://blog.gradle.org/guava It basically covers what I have written in the PR description in better words. :) While working on the post, I also wrote a Gradle plugin that adds the additional metadata to the existing Guava versions. So whoever is interested in trying this out today can use the plugin in a Gradle build: https://plugins.gradle.org/plugin/de.jjohannes.missing-metadata-guava ( @cgdecker As I got back to this topic, I thought about how we could add test coverage to this PR. I added an "integration test" that resolves the locally installed Guava SNAPSHOT version with Gradle and tests all different variations of classpathes. It tests against Gradle 5 (using POM metadata) and Gradle 6 (using GMM). This way, we make sure that both metadata files are in sync and that we only have the expected differences in the resolution result. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for this, and sorry that we have let it sit so long. I have a couple small questions, but we should probably err on the side of merging this soon, and we can always make changes later.
} | ||
} | ||
val guavaVersion = if (name.startsWith("jre")) { | ||
"HEAD-jre-SNAPSHOT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like we may need to update this manually (or tweak set_version
to update it with sed
or something) to test against a version that we're about to release? (And similarly at the top of the file.)
That should be fine. I just want to check my understanding (and invite you to let me know if there are other things we should do that I've overlooked).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I missed this.
I fixed it by extracting the version directly from the pom.xml
file.
pom.xml
Outdated
<variant.checkerframework>"checker-qual"</variant.checkerframework> | ||
<otherVariant.version>HEAD-android-SNAPSHOT</otherVariant.version> | ||
<otherVariant.jvmVersion>6</otherVariant.jvmVersion> | ||
<otherVariant.checkerframework>"checker-compat-qual", "version": { "requires": "2.5.5" }</otherVariant.checkerframework> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly: Do we need to specify the version number here? I see that it's omitted for checker-qual above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why the version is required here is that the version of checker-compat-qual
is not managed in the <dependencyManagement>
block below. It's only defined directly in android/pom.xml.
I think the better solution is to add it to the block as well. I made that change: 70eaaf7.
Let me know if that is fine.
Note: The version is then defined in two places, here and in android/pom.xml
. I am not sure why there are versions in android/pom.xml
at all. The android pom.xml
also uses this pom as parent. Maybe all the versions can be removed from android/pom.xml
?
7ee9ed6
to
70eaaf7
Compare
Thanks for taking the time to get back to this @cpovirk. I rebased this PR to latest From my perspective this can be merged as it is. The result of Gradle resolving a dependency to Guava is now pinned down in the integration test. That also tests the behaviour when resolving without Gradle Metadata. Which is a good addition to Guava's test coverage in general to make sure that changes to dependencies in pom files do not have unexpected side effects. What remains to be done is to (automatically) execute the integration test by calling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you again, this time for picking this up so quickly after our silence.
(Rewinding: I am going to ponder your earlier question about whether to move our annotation dependencies to compile-only scope. You can leave things as you have them, and I will tweak it if we want to be more conservative.)
What remains to be done is to (automatically) execute the integration test by calling
util/gradle_integration_tests.sh
. I am not sure where to put this best. (Should it become part of the travis configuration?) This can be done as a follow up.
I would assume Travis as well. A follow-up sounds perfectly fine.
android/pom.xml
Outdated
@@ -19,6 +19,13 @@ | |||
<maven-javadoc-plugin.version>3.1.0</maven-javadoc-plugin.version> | |||
<maven-source-plugin.version>3.2.0</maven-source-plugin.version> | |||
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | |||
<module.status>integration</module.status> | |||
<module.parentVersion>HEAD-jre-SNAPSHOT</module.parentVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your most recent round of comments, you said:
I am not sure why there are versions in
android/pom.xml
at all. The androidpom.xml
also uses this pom as parent. Maybe all the versions can be removed fromandroid/pom.xml
?
I don't think that's the case: I see no <parent>
element in android/pom.xml
.
There is certainly a decent argument that we should have a common parent for both flavors; that's just not how we have it set up at present.
Regardless, I like the idea of having the version of checker-compat-qual
defined in both flavors' pom.xml
files, since that lets the Gradle files refer to it consistently.
But: Given that the Android flavor has its own parent POM, should the line here refer to HEAD-android-SNAPSHOT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's the case: I see no element in android/pom.xml.
Uh, my bad. Yes the POM does not have the parent. And while it could be possible it does make sense that there is no common parent as each flavor is a separate maven build.
I don't know why I did this, but this parentVersion
property is actually not needed at all and the version of the parent simply has to match the version of the variant. Then the checker-compat-qual
version is also there.
So forget most of what I said earlier.
I fixed this. Thanks for clarifying and asking the right questions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks, I was still not seeing the implications of this :) All the more reason for the Gradle metadata to simplify things for users!
8313df1
to
1ca9f03
Compare
I went ahead and added the integration test as a step to travis to make sure it is working before we merge. Looks good as far a I can tell. At least it did run on this PR and passed. Please feel free to merge. And do further changes later if required (I am happy to help, just ping me). As a note regarding
You are most likely aware: this is this long-standing feature request in Maven that was never picked up. |
Sorry, I have one more question, which I should have asked long ago. You said in your original message that Gradle will automatically choose
But... do Android users do this? It sounds like Would we need to do something different in order to detect Android compiles (presumably by detecting something set by the Android Gradle Plugin)? [edit: I'm not clear on whether the following links are asking about something similar: https://discuss.gradle.org/t/how-to-detect-if-a-plugin-was-not-applied/28995, https://stackoverflow.com/q/46204642/28465] |
(Maybe Android Gradle plugin should declare an attribute? I could try talking to that team if you think it would be helpful. Historically, we haven't been in touch with them a lot, but I'd like to think they'd be interested in this.) |
3672c66
to
985d143
Compare
To be conservative, this commit does not removes most annotation artifacts from Gradle's runtime classpath, only j2objc-annotations. The other artifacts contain at least some annotations with RUNTIME visibility (IIRC). (Even this change could theoretically affect users who assume that they can read CLASS-retention annotations (of which j2objc-annotations has some) from bytecode and find them in the runtime classpath. But that seems unlikely, especially for j2objc annotations.) We may consider being more aggressive in the future. For now, this particular commit addresses #2824 for Gradle users just a tiny bit, and it helps with the problem that prompted #6567. Fixes #3683 RELNOTES=Added [Gradle Module Metadata](https://docs.gradle.org/current/userguide/publishing_gradle_module_metadata.html). If you use Gradle 6 or higher, Gradle can automatically intelligently resolve conflicts between `guava-android` and `guava-jre`, among [other benefits](#3683). PiperOrigin-RevId: 337348979
Small follow up to #3683 See: #6604 (comment) Fixes #6605 RELNOTES=n/a PiperOrigin-RevId: 544337005
Small follow up to #3683 See: #6604 (comment) Fixes #6605 RELNOTES=n/a PiperOrigin-RevId: 544384609
Hello again from Gradle 👋
The following is a suggestion open for discussion. I provide this as a PR, instead of an issue only, as a proof of concept to aid the discussion.
Background
With the release of Gradle 6, Gradle fully supports a new metadata format called Gradle Module Metadata (GMM) - docs and spec. Such a metadata (
.module
) file can be published in addition to a.pom
file and Gradle 6 will use it to obtain additional information about a library, its artifacts and transitive dependencies which can not be represented in the pom format. The GMM format has been under development at Gradle since about two years. With the release of Gradle 6, we are now at a point where it is ready to be adopted in larger scale. That's why we have been looking at popular JVM libraries, like Guava, to evaluate if Gradle users depending on a library would profit if the library would publish GMM.Why should Guava care?
There are a couple of things Guava struggles with to express in their poms. Many of which have already been discussed at length here in other issues. I set up a repository with samples demonstrating these use cases. In detail, GMM offers modeling concepts, and with that solutions, for the following:
Multiple variants in one component
Use case with description: guava-jdk-variants
At the core of Gradle 6 dependency management and GMM is the notion that a component can have multiple, and arbitrary many, variants. A concept Gradle has been adopting since 3.x internally and which is heavily used already by all Gradle JVM plugins to distinguish between runtime and compile time variants and to a larger extend, for example, by the Android build system or by Kotlin Native.
Guava publishes two prominent variants -
android
andjre
. It is solved in probably the most elegant way possible with POM by encoding the variant name in the version number. Still this often causes issues as built tools do not know that the versions also present variants.Each variant in GMM is identified by attributes. These attributes are used by Gradle to select the best fitting variant. Gradle defines a number of default attributes for JVM libraries. One of them is
org.gradle.jvm.version
. In the presented solution, we set the attribute to6
for theandroid
variants and to8
for thejre
variants. Gradle, knowing about the attribute, can now use that information. If you do in your build:Gradle will automatically select
android
, if you set it to version 8 or higher, Gradle will selectjre
. If you set it to a lower version, Gradle will fail.It does not matter if you depend on Guava directly or if it is brought in transitively. The central idea behind the variant-aware mechanism is that dependencies are defined between modules, but the variant selection is done when the dependencies are resolved based on the current context. For example, Gradle can say "give me everything I need to compile this library for Java 6" or "give me everything I need to run this application with Java 9".
Note: The presented solution keeps the way how Guava is publishing two "versions" at once. Only for Gradle now both versions are identical (both provide all variants). So in Gradle it would not matter if
com.google.guava:guava:29.0-jre
orcom.google.guava:guava:29.0-android
is in your dependency graph, as both provide the same variants. For Maven users, nothing changes as the poms are no different.Compile-only API dependencies / annotation processor libraries on runtime classpath
Use case with description: guava-compile-only-dependencies
This addresses what is being discussed in #2824. With the solution presented here, you would get the following classpathes:
It can be discussed if that is what is desired, but the essential bit is that you can have things on the compile classpath but not on the runtime classpath (not possible with poms currently).
A component/variant can provide multiple capabilities
Use case 1: guava-listenablefuture-conflict
Use case 2: guava-vs-google-collections
Another concept introduced with variants is capabilities. A capability is essentially an identifier for a "feature" that is implemented by a component. This allows you to express that:
To be more concrete, each component has a default capability corresponding to its GAV coordinates:
com.google.guava:guava
has thecom.google.guava:guava
capabilitycom.google.collections:google-collections
has thecom.google.collections:google-collections
capabilitycom.google.guava:listenablefuture
has thecom.google.guava:listenablefuture
capabilityNow we can say that
guava
also has thecom.google.collections:google-collections
andcom.google.guava:listenablefuture
capabilities. Gradle will then not allow Guava together with these modules on the classpath and offers simple options to resolve the conflict. The9999.0-empty-to-avoid-conflict-with-guava
workaround is then no longer needed to avoid undesired old versions ofcom.google.guava:listenablefuture
on the classpath.Note: since the pom is not changed, the workaround will stay in place for Maven.
How does publishing GMM work with Maven builds?
Usually, we would recommend using Gradle 6 as your build system if your library wants to publish individual variants or other GMM specifics, as it is not easy to manually set this up in a Maven build. However, the Guava build is already setup to do it - by combining two Maven builds. So everything is already in place, the only thing is that certain metadata information cannot be expressed/published in the poms.
As you can see in this PR, I added the
.module
file to the repo. (In Gradle, these files are generated during publishing as are the poms.) Since it mostly contains the additional information and Guava does not have many dependencies (and also does not want to add more), the file is of reasonable size to be maintained manually along the poms. Since the naming of themodule
file is following Maven conventions, it can be treated and published as an additional artifact by Maven - as you can see in this PR.If you consider adopting this solution, I am happy to provide some kind of integration test (maybe based on the samples linked above) that validates that the
module
file is valid and that the information does not contradict the information in the pom files.Further details
Sorry for this rather long text here. My goal is to illustrating the use cases and provide enough detail for a basic understanding of why we think many Gradle users would profit from having Guava publish Gradle Module Metadata. If you are interested in more details, here are a few links to sections in the Gradle user manual:
We also recently did a Webinar on publishing that covers GMM:
https://gradle.com/blog/dependency-management-with-gradle-part-3-publishing-and-release-strategies/ (you can directly jump to 'Section 5 - Metadata' in the video)
And others have started to publish
module
files to central, so that is working :)https://repo1.maven.org/maven2/org/junit/jupiter/junit-jupiter-api/5.6.0-M1/junit-jupiter-api-5.6.0-M1.module
I am happy to answer any questions on the topic and help working out a solution if there is interest in this on your side.