diffplug / goomph

IDE as build artifact
Apache License 2.0
130 stars 30 forks source link

eclipseMavenCentral constrainTransitivesToThisRelease() and multimodule build #154

Closed jmini closed 1 year ago

jmini commented 3 years ago

I did the setup of com.diffplug.eclipse.mavencentral with constrainTransitivesToThisRelease() inside the eclipseMavenCentral {} section. See https://github.com/diffplug/goomph/issues/149 for more details.

In a multi-module setup I have noticed that consumer of a library using constrainTransitivesToThisRelease() needs the same section as well.


Example:

In this the jdt-gradle-example folder, I have two modules (inspired by the gradle init task):

The project is built with Java 8, so if there is no constraint on the JDT version the newest will be used and this version is built with Java 11. Therefore the tests are failing with:

> Task :app:test FAILED

AppTest > testGetMessage() FAILED
    java.lang.UnsupportedClassVersionError at AppTest.java:20

The module utilities is using the JDT so I would like to specify the version in this project:

https://github.com/jmini/jdt-experiments/blob/ff0e464daed3a126df1c983c080c91f1e657ded5/jdt-gradle-example/utilities/build.gradle#L1-L11

The module app in only using utilities, but specifying the JDT version is necessary. If you remove following block:

https://github.com/jmini/jdt-experiments/blob/ff0e464daed3a126df1c983c080c91f1e657ded5/jdt-gradle-example/app/build.gradle#L5-L13

You get the Error mentioned above.


Is this intended?

nedtwigg commented 3 years ago

Define "intended", haha ;-)

I do think this is the expected behavior. Eclipse publishes their jars with wide version compatibility ranges - wider than their JRE compatibility. You could make the argument that bumping the required JRE from 8 to 11 constitutes a breaking change.

The wide version compatibility makes sense in the p2 resolution model where the primary way of setting versions is to specify a version-specific repository. In the mavenCentral resolution model of a single global repository, it means that you have to fight those version compatibility ranges if you want a repeatable build, or if you want a consistent set of artifacts from a specific eclipse release.

Because eclipse artifacts are published with broad guarantees of future-compatibility, you will need to do eclipseMavenCentral { constrainTransitivesToThisRelease() } for any and every module which is downloading eclipse artifacts where you want a specific release.

jmini commented 3 years ago

Thank you for this quick reply.

This behavior is not what I get if I use the BOM approach using platform (..) in the dependencies section:

https://github.com/jmini/jdt-experiments/blob/671e499c538d70c850d69999b1ca3ca13a2faea8/jdt-gradle-example/utilities/build.gradle#L2

Currently the BOM pom xml is not on maven central so you need to declare its repository as well: https://github.com/jmini/jdt-experiments/blob/671e499c538d70c850d69999b1ca3ca13a2faea8/jdt-gradle-example/build.gradle#L20-L22

In that case the version selection is done in the utilities module and the app module just consume the utilities with implementation project(':utilities') without having to care about the version selection for the transitive dependencies of utilities.

This is why I have asked you about this. This gives me pro and cons for the 2 approaches.

I consider my question as answered.

nedtwigg commented 3 years ago

I haven't heard of the BOM approach, and I've never seen the platform syntax you're using there. But if you're satisfied, I'm satisfied :)

jmini commented 3 years ago

From the Gradle docs:

Importing Maven BOMs https://docs.gradle.org/current/userguide/platforms.html#sub:bom_import

Projects like Spring or Quarkus are big users of BOM files.


But if you're satisfied, I'm satisfied :)

I would prefer that using eclipseMavenCentral {} with constrainTransitivesToThisRelease() would set the version of all eclipse transitive dependencies so that for the consumer, this gets transparent. But I can understand if this is not possible or not a feature on the roadmap…

nedtwigg commented 3 years ago

Interesting! I had not heard of those features. I do not plan to explore this further, but I'm happy to review and merge a PR which adds something along these lines.

nedtwigg commented 1 year ago

I'm still happy to merge improvements to Goomph, but I think dev.equo.p2deps is the way forward on this.