From ca7669433c815fd549b37c79fcd32c9825a5e16d Mon Sep 17 00:00:00 2001 From: Jendrik Johannes Date: Thu, 25 Jun 2026 09:14:49 +0200 Subject: [PATCH] Handle classifiers in FilePathToModuleCoordinates --- CHANGELOG.md | 4 + build.gradle.kts | 4 - .../ExtraJavaModuleInfoTransform.java | 22 +---- .../FilePathToModuleCoordinates.java | 86 +++++++++++++++---- .../FilePathToModuleCoordinatesTest.groovy | 28 ++++-- .../test/ClassifiedJarsFunctionalTest.groovy | 37 ++++++++ .../test/EdgeCasesFunctionalTest.groovy | 2 +- 7 files changed, 138 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba240cc..11e9f5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Extra Java Module Info Gradle Plugin - Changelog +## Version 1.14.1 +* [Fixed] [#246](https://github.com/gradlex-org/extra-java-module-info/issues/246) - More correct classifier handling (Thanks to [Geolykt](https://github.com/Geolykt) for reporting) + * ⚠️ For matching a Jar with classifier, the _identifier_ now needs to be included - e.g. `org.dojotoolkit:dojo|distribution` instead of `org.dojotoolkit:dojo` + ## Version 1.14 * [New] [#207](https://github.com/gradlex-org/extra-java-module-info/issues/207) - Add 'exportAllPackagesExcept("org.exception", ...)' (Thanks [Tim Hurman](https://github.com/timhamoni) for contributing) * [New] [#212](https://github.com/gradlex-org/extra-java-module-info/issues/212) - Add 'provides("service", "service.impl", ...)' (Thanks [Sandy Dunlop](https://github.com/sandydunlop) for contributing) diff --git a/build.gradle.kts b/build.gradle.kts index ec320ed..b1edd22 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -19,10 +19,6 @@ publishingConventions { testingConventions { testGradleVersions("6.8.3", "6.9.4", "7.6.5", "8.14.2") } -// Turn off classfile lint as long as we still compile with Java 8 -// /org/objectweb/asm/ClassReader.class: Cannot find annotation method 'forRemoval()' in type 'Deprecated' -tasks.compileJava { options.compilerArgs.add("-Xlint:-classfile") } - // === the following custom configuration should be removed once tests are migrated to Java apply(plugin = "groovy") diff --git a/src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java b/src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java index 4e236d6..3165fec 100644 --- a/src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java +++ b/src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java @@ -39,7 +39,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; -import java.util.stream.Stream; import java.util.zip.ZipEntry; import java.util.zip.ZipException; import javax.annotation.Nullable; @@ -224,22 +223,11 @@ private ModuleSpec findModuleSpec(File originalJar) { Map moduleSpecs = getParameters().getModuleSpecs().get(); Optional moduleSpec = moduleSpecs.values().stream() - .filter(spec -> gaCoordinatesFromFilePathMatch(originalJar.toPath(), spec.getIdentifier())) + .filter(spec -> gaCoordinatesFromFilePathMatch( + originalJar.toPath(), spec.getIdentifier(), spec.getClassifier())) .findFirst(); if (moduleSpec.isPresent()) { - String ga = moduleSpec.get().getIdentifier(); - if (moduleSpecs.containsKey(ga)) { - return moduleSpecs.get(ga); - } else { - // maybe with classifier - Stream idsWithClassifier = moduleSpecs.keySet().stream().filter(id -> id.startsWith(ga + "|")); - for (String idWithClassifier : idsWithClassifier.collect(Collectors.toList())) { - if (nameHasClassifier(originalJar, moduleSpecs.get(idWithClassifier))) { - return moduleSpecs.get(idWithClassifier); - } - } - } - return null; + return moduleSpec.get(); } String originalJarName = originalJar.getName(); @@ -749,10 +737,6 @@ private String moduleNameFromSharedMapping(String ga) { return null; } - private boolean nameHasClassifier(File jar, ModuleSpec spec) { - return jar.getName().endsWith("-" + spec.getClassifier() + ".jar"); - } - private static boolean isModuleInfoClass(String jarEntryName) { return "module-info.class".equals(jarEntryName) || MODULE_INFO_CLASS_MRJAR_PATH.matcher(jarEntryName).matches(); diff --git a/src/main/java/org/gradlex/javamodule/moduleinfo/FilePathToModuleCoordinates.java b/src/main/java/org/gradlex/javamodule/moduleinfo/FilePathToModuleCoordinates.java index c541803..39b4d51 100644 --- a/src/main/java/org/gradlex/javamodule/moduleinfo/FilePathToModuleCoordinates.java +++ b/src/main/java/org/gradlex/javamodule/moduleinfo/FilePathToModuleCoordinates.java @@ -4,8 +4,8 @@ import java.nio.file.Path; import java.util.stream.Collectors; import java.util.stream.StreamSupport; -import javax.annotation.Nullable; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; /** * Attempts to parse 'group', 'name', 'version' coordinates from a paths like: @@ -17,31 +17,57 @@ final class FilePathToModuleCoordinates { @Nullable static String versionFromFilePath(Path path) { - if (isInGradleCache(path)) { + if (null != isInGradleCache(path)) { return getVersionFromGradleCachePath(path); } - if (isInM2Cache(path)) { + if (null != isInM2Cache(path)) { return getVersionFromM2CachePath(path); } return null; } - static boolean gaCoordinatesFromFilePathMatch(Path path, String ga) { + static boolean gaCoordinatesFromFilePathMatch(Path path, String gaAndClassifier) { + if (gaAndClassifier.contains("|")) { + String[] split = gaAndClassifier.split("\\|"); + return gaCoordinatesFromFilePathMatch(path, split[0], split[1]); + } + return gaCoordinatesFromFilePathMatch(path, gaAndClassifier, null); + } + + static boolean gaCoordinatesFromFilePathMatch(Path path, String ga, @Nullable String classifier) { String name = nameCoordinateFromFilePath(path); String group = groupCoordinateFromFilePath(path); if (name == null || group == null) { return false; } - return (isInGradleCache(path) && ga.equals(group + ":" + name)) - || (isInM2Cache(path) && (group + ":" + name).endsWith("." + ga)); + + String coordinatesFromPath = group + ":" + name; + + String classifierFromGradleCache = isInGradleCache(path); + if (classifierFromGradleCache != null) { + if (classifierFromGradleCache.isEmpty()) { + return coordinatesFromPath.equals(ga); + } + return coordinatesFromPath.equals(ga) && classifierFromGradleCache.equals(classifier); + } + + String classifierFromM2Cache = isInM2Cache(path); + if (classifierFromM2Cache != null) { + if (classifierFromM2Cache.isEmpty()) { + return coordinatesFromPath.endsWith(ga); + } + return coordinatesFromPath.endsWith(ga) && classifierFromM2Cache.equals(classifier); + } + + return false; } @Nullable private static String groupCoordinateFromFilePath(Path path) { - if (isInGradleCache(path)) { + if (null != isInGradleCache(path)) { return path.getName(path.getNameCount() - 5).toString(); } - if (isInM2Cache(path)) { + if (null != isInM2Cache(path)) { return StreamSupport.stream(path.subpath(0, path.getNameCount() - 3).spliterator(), false) .map(Path::toString) .collect(Collectors.joining(".")); @@ -49,19 +75,21 @@ private static String groupCoordinateFromFilePath(Path path) { return null; } - static boolean isInGradleCache(Path path) { + @Nullable + static String isInGradleCache(Path path) { String name = nameCoordinateFromFilePath(path); if (name == null) { - return false; + return null; } String version = getVersionFromGradleCachePath(path); return matchesPath(path, name, version); } - static boolean isInM2Cache(Path path) { + @Nullable + static String isInM2Cache(Path path) { String name = nameCoordinateFromFilePath(path); if (name == null) { - return false; + return null; } String version = getVersionFromM2CachePath(path); return matchesPath(path, name, version); @@ -75,12 +103,12 @@ private static String nameCoordinateFromFilePath(Path path) { String nameFromGradleCachePath = path.getName(path.getNameCount() - 4).toString(); String versionFromGradleCachePath = getVersionFromGradleCachePath(path); - if (matchesPath(path, nameFromGradleCachePath, versionFromGradleCachePath)) { + if (null != matchesPath(path, nameFromGradleCachePath, versionFromGradleCachePath)) { return nameFromGradleCachePath; } String nameFromM2CachePath = path.getName(path.getNameCount() - 3).toString(); String versionFromM2CachePath = getVersionFromM2CachePath(path); - if (matchesPath(path, nameFromM2CachePath, versionFromM2CachePath)) { + if (null != matchesPath(path, nameFromM2CachePath, versionFromM2CachePath)) { return nameFromM2CachePath; } @@ -95,8 +123,34 @@ private static String getVersionFromM2CachePath(Path path) { return path.getName(path.getNameCount() - 2).toString(); } - private static boolean matchesPath(Path path, String name, String version) { + /** + * @return the classifier or 'null' if there is no match. + */ + @Nullable + private static String matchesPath(Path path, String potentialName, String potentialVersion) { String jarFileName = path.getFileName().toString(); - return jarFileName.startsWith(name + "-") && !jarFileName.startsWith(version); + boolean fuzzyMatchVersion = potentialVersion.contains("-"); + String potentialVersionNormalized = + fuzzyMatchVersion ? potentialVersion.substring(0, potentialVersion.indexOf("-")) : potentialVersion; + + if (jarFileName.startsWith(potentialVersionNormalized)) { + return null; + } + + String baseName = potentialName + "-" + potentialVersionNormalized; + if (jarFileName.startsWith(baseName)) { + if (fuzzyMatchVersion) { + // Potential classifiers are ignored as we do not know what is version and what is classifier. + // For example in '33.2.1-android' is '-android' part of the version or a classifier? + return ""; + } else { + if (jarFileName.startsWith(baseName + ".")) { + return ""; + } + return jarFileName.substring(baseName.length() + 1, jarFileName.length() - 4); + } + } + + return null; } } diff --git a/src/test/groovy/org/gradlex/javamodule/moduleinfo/FilePathToModuleCoordinatesTest.groovy b/src/test/groovy/org/gradlex/javamodule/moduleinfo/FilePathToModuleCoordinatesTest.groovy index 41b0003..a12e494 100644 --- a/src/test/groovy/org/gradlex/javamodule/moduleinfo/FilePathToModuleCoordinatesTest.groovy +++ b/src/test/groovy/org/gradlex/javamodule/moduleinfo/FilePathToModuleCoordinatesTest.groovy @@ -30,7 +30,7 @@ class FilePathToModuleCoordinatesTest extends Specification { def path = path('/Users/someone/.gradle/caches/modules-2/files-2.1/org.slf4j/slf4j-api/1.7.36/6c62681a2f655b49963a5983b8b0950a6120ae14/slf4j-api-1.7.36.jar') expect: - gaCoordinatesFromFilePathMatch(path, "org.slf4j:slf4j-api") + gaCoordinatesFromFilePathMatch(path, "org.slf4j:slf4j-api", null) } def "ga coordinates from gradle cache file path (version in file name does not match)"() { @@ -38,7 +38,16 @@ class FilePathToModuleCoordinatesTest extends Specification { def path = path('/Users/jendrik/.gradle/caches/modules-2/files-2.1/com.google.guava/guava/33.2.1-jre/818e780da2c66c63bbb6480fef1f3855eeafa3e4/guava-33.2.1-android.jar') expect: - gaCoordinatesFromFilePathMatch(path, "com.google.guava:guava") + gaCoordinatesFromFilePathMatch(path, "com.google.guava:guava", null) + } + + def "ga coordinates from gradle cache file path with classifier"() { + given: + def path = path('/Users/jendrik/.gradle/caches/modules-2/files-2.1/org.lwjgl/lwjgl-glfw/3.3.6/399b42b491c5dfa6595ae6fd79dcba61b93538a7/lwjgl-glfw-3.3.6-natives-macos-arm64.jar') + + expect: + !gaCoordinatesFromFilePathMatch(path, "org.lwjgl:lwjgl-glfw", null) + gaCoordinatesFromFilePathMatch(path, "org.lwjgl:lwjgl-glfw", "natives-macos-arm64") } def "version from m2 repo file path"() { @@ -64,13 +73,13 @@ class FilePathToModuleCoordinatesTest extends Specification { jarPath = path('/Users/someone/.m2/repository/com/google/code/findbugs/jsr305/3.0.2/jsr305-3.0.2.jar') then: - gaCoordinatesFromFilePathMatch(jarPath, "com.google.code.findbugs:jsr305") + gaCoordinatesFromFilePathMatch(jarPath, "com.google.code.findbugs:jsr305", null) when: jarPath = path('/Users/someone/.m2/repository/de/odysseus/juel/juel-impl/2.2.7/juel-impl-2.2.7.jar') then: - gaCoordinatesFromFilePathMatch(jarPath, "de.odysseus.juel:juel-impl") + gaCoordinatesFromFilePathMatch(jarPath, "de.odysseus.juel:juel-impl", null) } def "ga coordinates from m2 repo file path (version in file name does not match)"() { @@ -78,7 +87,16 @@ class FilePathToModuleCoordinatesTest extends Specification { def path = path('/Users/someone/.m2/repository/com/google/guava/guava/33.2.1-jre/guava-33.2.1-android.jar') expect: - gaCoordinatesFromFilePathMatch(path, "com.google.guava:guava") + gaCoordinatesFromFilePathMatch(path, "com.google.guava:guava", null) + } + + def "ga coordinates from m2 repo file path with classifier"() { + given: + def path = path('/Users/someone/.m2/repository/org/lwjgl/lwjgl-glfw/3.3.6/lwjgl-glfw-3.3.6-natives-macos-arm64.jar') + + expect: + !gaCoordinatesFromFilePathMatch(path, "org.lwjgl:lwjgl-glfw", null) + gaCoordinatesFromFilePathMatch(path, "org.lwjgl:lwjgl-glfw", "natives-macos-arm64") } private Path path(String path) { diff --git a/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/ClassifiedJarsFunctionalTest.groovy b/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/ClassifiedJarsFunctionalTest.groovy index 6778723..13dc3ed 100644 --- a/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/ClassifiedJarsFunctionalTest.groovy +++ b/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/ClassifiedJarsFunctionalTest.groovy @@ -63,4 +63,41 @@ class ClassifiedJarsFunctionalTest extends Specification { build().task(':compileJava').outcome == TaskOutcome.SUCCESS } + def "does not accidentally patch classified Jar"() { + given: + file("src/main/java/org/gradle/sample/app/Main.java") << """ + package org.gradle.sample.app; + public class Main { + public static void main(String[] args) { + org.lwjgl.glfw.GLFW.glfwInit(); + } + } + """ + file("src/main/java/module-info.java") << """ + module org.gradle.sample.app { + requires org.lwjgl.glfw; + } + """ + buildFile << """ + dependencies { + implementation("org.lwjgl:lwjgl-glfw:3.2.2") + implementation("org.lwjgl:lwjgl-glfw:3.2.2:natives-macos") + implementation("org.lwjgl:lwjgl:3.2.2:natives-macos") + } + extraJavaModuleInfo { + module("org.lwjgl:lwjgl", "org.lwjgl") { + preserveExisting() + requiresStatic("org.lwjgl.natives") + } + module("org.lwjgl:lwjgl-glfw", "org.lwjgl.glfw") { + preserveExisting() + requiresStatic("org.lwjgl.glfw.natives") + } + } + """ + + expect: + build().task(':compileJava').outcome == TaskOutcome.SUCCESS + } + } diff --git a/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/EdgeCasesFunctionalTest.groovy b/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/EdgeCasesFunctionalTest.groovy index 6b12338..9f5eb62 100644 --- a/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/EdgeCasesFunctionalTest.groovy +++ b/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/EdgeCasesFunctionalTest.groovy @@ -74,7 +74,7 @@ class EdgeCasesFunctionalTest extends Specification { failOnMissingModuleInfo.set(false) automaticModule("org.apache.qpid:qpid-broker-core", "org.apache.qpid.broker") { mergeJar("org.apache.qpid:qpid-broker-plugins-management-http") - mergeJar("org.dojotoolkit:dojo") // This is a Zip, selected by 'distribution' classifier in dependencies of 'qpid-broker-plugins-management-http' + mergeJar("org.dojotoolkit:dojo|distribution") // This is a Zip, selected by 'distribution' classifier in dependencies of 'qpid-broker-plugins-management-http' mergeJar("org.webjars.bower:dgrid") mergeJar("org.webjars.bower:dstore") }