eclipseMavenCentral: Version of "com.ibm.icu"

[jmini]

An Eclipse release seems to fix the value of the com.ibm.icu:

Example from from the 4.14.0 release (XML artifacts-4.14.0.xml is present as test case in the test suite):

    <artifact classifier='osgi.bundle' id='com.ibm.icu' version='64.2.0.v20190507-1337'>

Meaning that in my opinion the com.ibm.icu version should be fixed to:

<dependency>
    <groupId>com.ibm.icu</groupId>
    <artifactId>icu4j</artifactId>
    <version>64.2</version>
</dependency>

But if I ask for the dependencies of my project using:

eclipseMavenCentral {
    release '4.14.0', {
        implementation 'org.eclipse.jdt.ui'
        //...
        
        constrainTransitivesToThisRelease()
    }
}

with ./gradlew <my-project>:dependencies I get:

+--- org.eclipse.jdt:org.eclipse.jdt.ui:[3.17.100,4.0.0) -> 3.20.0
|    +--- com.ibm.icu:icu4j:[4.4.2,) -> 70.1

Which is not what I expect.

---

What do you think?

Should eclipseMavenCentral set only the versions of the artifact with an Eclipse groupId?

Or can we also make a rule to set the version of that component because it is defined in the Eclipse Release and it is a dependency for components like JDT.

[jmini]

In the POM file on maven central:
https://search.maven.org/artifact/org.eclipse.jdt/org.eclipse.jdt.ui/3.20.0/jar

We see following dependency declaration:

    <dependency>
      <groupId>com.ibm.icu</groupId>
      <artifactId>icu4j</artifactId>
      <version>[4.4.2,)</version>
    </dependency>

This matches exactly the open-range as defined in the MANIFEST.MF

But in Maven central, where you have access to all the version you get the latest version an not the one from a specific maven release.

---

To provide more context, we have detected this, because we got a

java.lang.NoSuchMethodError: 'boolean com.ibm.icu.text.UTF16.isSurrogate(int)'

The signature has changed between versions:

Version 71.0 (we are compiling against this version because eclipseMavenCentral is not pinning the version):
boolean isSurrogate(int codePoint)
Source https://github.com/unicode-org/icu/blob/a56dde820dc35665a66f2e9ee8ba58e75049b668/icu4j/main/classes/core/src/com/ibm/icu/text/UTF16.java#L606-L608

Version 67.1 (we have this version in our Eclipse IDE distribution):
boolean isSurrogate(char char16)
Source https://github.com/unicode-org/icu/blob/125e29d54990e74845e1546851b5afa3efab06ce/icu4j/main/classes/core/src/com/ibm/icu/text/UTF16.java#L605-L607

[nedtwigg]

Have you tried

eclipseMavenCentral {
  constrainTransitivesToThisRelease()
  ...
}

That should probably just be the default behavior, but I didn't add the feature until later on so I forced it to be a "turn-on" kind of thing for backward-compatibility.

[jmini]

Yes sorry I forgot to mention it in the bug description. I will edit it.

---

The problem is line 138:

goomph/src/main/java/com/diffplug/gradle/eclipse/MavenCentralExtension.java

Lines 134 to 147 in 40f9c82

	public void constrainTransitivesToThisRelease() {
	project.getConfigurations().forEach(config -> {
	config.getResolutionStrategy().eachDependency(dep -> {
	ModuleVersionSelector mod = dep.getRequested();
	if (MavenCentralMapping.isEclipseGroup(mod.getGroup())) {
	String version = bundleToVersion.get(mod.getName());
	if (version != null) {
	dep.useVersion(version);
	}
	}
	});
	});
	}

The current implementation of isEclipseGroup(String) is not considering ibm icu:

goomph/src/main/java/com/diffplug/gradle/eclipse/MavenCentralMapping.java

Lines 49 to 51 in 40f9c82

	public static boolean isEclipseGroup(String group) {
	return group.equals(PLATFORM) || group.equals(JDT) || group.equals(PDE) || group.equals(EMF) || group.equals(ECF);
	}

---

For me, I expect eclipseMavenCentral to pin also the ibm icu dependency, but since this is not a straight forward change I prefer to ask if this is also your vision.

[nedtwigg]

Sounds like isEclipseGroup("com.ibm.icu") should return true. If you submit a PR with that change, I'll happily merge and release.

[jmini]

There is a little bit more work to do for a PR…

The special rule to define the maven mapping is defined here:

  <mavenMappings namePattern="com\.ibm\.icu" groupId="com.ibm.icu" artifactId="icu4j" versionPattern="([^.]+)\.([^.]+)\.0(?:\..*)?" versionTemplate="$1.$2"/>

Source https://git.eclipse.org/c/platform/eclipse.platform.releng.git/tree/publish-to-maven-central/SDK4Mvn.aggr#n41

If you agree that this is a valid addition, I can give it a try.

[nedtwigg]

I agree it's a valid addition, happy to merge anything that works. It's okay to "break" relative to previous behavior, seems like it was the previous behavior that was broken. At DiffPlug we have pinned our icu4j version manually, was never sure why we needed to, I guess this is why.

[jmini]

I have something ready, but I am fighting with the build (see #168) right now.

[nedtwigg]

Fixed in 3.33.2.