gradlex-org · jjohannes · Feb 3, 2025 · Feb 3, 2025
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Extra Java Module Info Gradle Plugin - Changelog
 
+## Version 1.10
+* [New] [#160](https://github.com/gradlex-org/extra-java-module-info/pull/160) - Add 'preserveExisting' option to patch real modules
+
 ## Version 1.9
 * [New] [#137](https://github.com/gradlex-org/extra-java-module-info/pull/137) - Configuration option for 'versionsProvidingConfiguration'
 * [New] [#130](https://github.com/gradlex-org/extra-java-module-info/pull/130) - Support classifier in coordinates notation

diff --git a/README.md b/README.md
@@ -313,12 +313,15 @@ Note: The merged Jar will include the *first* appearance of duplicated files (li
 
 ## How can I fix a library with a broken `module-info.class`?
 
-To fix a library with a broken `module-info.class`, you can override the modular descriptor in the same way it is done with non-modular JARs. However, you need to specify `patchRealModule()` in order to avoid unintentional overrides.
+To fix a library with a broken `module-info.class`, you can override the modular descriptor in the same way it is done with non-modular JARs.
+However, you need to specify `patchRealModule()` to overwrite the existing `module-info.class`.
+You can also use `preserveExisting()`, if the exiting `module-info.class` is working in general, but misses entries.
 
 ```
 extraJavaModuleInfo {                
     module("org.apache.tomcat.embed:tomcat-embed-core", "org.apache.tomcat.embed.core") {
-        patchRealModule()
+        patchRealModule()   // overwrite existing module-info.class
+        preserveExisting()  // extend existing module-info.class 
         requires("java.desktop")
         requires("java.instrument")
         ...

diff --git a/src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java b/src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java
@@ -30,6 +30,8 @@
 import org.gradle.api.provider.Provider;
 import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.InputFiles;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.ModuleVisitor;
 import org.objectweb.asm.Opcodes;
@@ -144,7 +146,7 @@ public void transform(TransformOutputs outputs) {
         boolean realModule = isModule(originalJar);
         if (moduleSpec instanceof ModuleInfo) {
             if (realModule && !((ModuleInfo) moduleSpec).patchRealModule) {
-                throw new RuntimeException("Patching of real modules must be explicitly enabled with 'patchRealModule()'");
+                throw new RuntimeException("Patching of real modules must be explicitly enabled with 'patchRealModule()' or 'preserveExisting()'");
             }
             String definedName = moduleSpec.getModuleName();
             String expectedName = autoModuleName(originalJar);
@@ -287,12 +289,14 @@ private void addModuleDescriptor(File originalJar, File moduleJar, ModuleInfo mo
             try (JarOutputStream outputStream = newJarOutputStream(Files.newOutputStream(moduleJar.toPath()), inputStream.getManifest())) {
                 Map<String, List<String>> providers = new LinkedHashMap<>();
                 Set<String> packages = new TreeSet<>();
-                copyAndExtractProviders(inputStream, outputStream, !moduleInfo.getMergedJars().isEmpty(), providers, packages);
+                byte[] existingModuleInfo = copyAndExtractProviders(inputStream, outputStream, !moduleInfo.getMergedJars().isEmpty(), providers, packages);
                 mergeJars(moduleInfo, outputStream, providers, packages);
                 outputStream.putNextEntry(newReproducibleEntry("module-info.class"));
                 outputStream.write(addModuleInfo(moduleInfo, providers, versionFromFilePath(originalJar.toPath()),
-                        moduleInfo.exportAllPackages ? packages : Collections.emptySet()));
+                        moduleInfo.exportAllPackages ? packages : Collections.emptySet(),
+                        existingModuleInfo));
                 outputStream.closeEntry();
+                System.out.println("AAA: " + moduleJar);
             }
         } catch (IOException e) {
             throw new RuntimeException(e);
@@ -310,8 +314,10 @@ private JarOutputStream newJarOutputStream(OutputStream out, @Nullable Manifest
         return jar;
     }
 
-    private void copyAndExtractProviders(JarInputStream inputStream, JarOutputStream outputStream, boolean willMergeJars, Map<String, List<String>> providers, Set<String> packages) throws IOException {
+    @Nullable
+    private byte[] copyAndExtractProviders(JarInputStream inputStream, JarOutputStream outputStream, boolean willMergeJars, Map<String, List<String>> providers, Set<String> packages) throws IOException {
         JarEntry jarEntry = inputStream.getNextJarEntry();
+        byte[] existingModuleInfo = null;
         while (jarEntry != null) {
             byte[] content = readAllBytes(inputStream);
             String entryName = jarEntry.getName();
@@ -325,8 +331,9 @@ private void copyAndExtractProviders(JarInputStream inputStream, JarOutputStream
                 }
                 providers.get(key).addAll(extractImplementations(content));
             }
-
-            if (!JAR_SIGNATURE_PATH.matcher(entryName).matches() && !"META-INF/MANIFEST.MF".equals(entryName) && !isModuleInfoClass(entryName)) {
+            if (isModuleInfoClass(entryName)) {
+                existingModuleInfo = content;
+            } else if (!JAR_SIGNATURE_PATH.matcher(entryName).matches() && !"META-INF/MANIFEST.MF".equals(entryName)) {
                 if (!willMergeJars || !isFileInServicesFolder) { // service provider files will be merged later
                     jarEntry.setCompressedSize(-1);
                     try {
@@ -354,6 +361,7 @@ private void copyAndExtractProviders(JarInputStream inputStream, JarOutputStream
             }
             jarEntry = inputStream.getNextJarEntry();
         }
+        return existingModuleInfo;
     }
 
     private List<String> extractImplementations(byte[] content) {
@@ -366,13 +374,40 @@ private List<String> extractImplementations(byte[] content) {
                 .collect(Collectors.toList());
     }
 
-    private byte[] addModuleInfo(ModuleInfo moduleInfo, Map<String, List<String>> providers, @Nullable String version, Set<String> autoExportedPackages) {
-        ClassWriter classWriter = new ClassWriter(0);
-        classWriter.visit(Opcodes.V9, Opcodes.ACC_MODULE, "module-info", null, null, null);
+    private byte[] addModuleInfo(ModuleInfo moduleInfo, Map<String, List<String>> providers, @Nullable String version, Set<String> autoExportedPackages,
+                                 @Nullable byte[] existingModuleInfo) {
+        ClassReader classReader = moduleInfo.preserveExisting && existingModuleInfo != null ? new ClassReader(existingModuleInfo) : null;
+        ClassWriter classWriter = new ClassWriter(classReader, 0);
         int openModule = moduleInfo.openModule ? Opcodes.ACC_OPEN : 0;
         String moduleVersion = moduleInfo.getModuleVersion() == null ? version : moduleInfo.getModuleVersion();
-        ModuleVisitor moduleVisitor = classWriter.visitModule(moduleInfo.getModuleName(), openModule, moduleVersion);
 
+        if (classReader == null) {
+            classWriter.visit(Opcodes.V9, Opcodes.ACC_MODULE, "module-info", null, null, null);
+            ModuleVisitor moduleVisitor = classWriter.visitModule(moduleInfo.getModuleName(), openModule, moduleVersion);
+            moduleVisitor.visitRequire("java.base", 0, null);
+            addModuleInfoEntires(moduleInfo, providers, autoExportedPackages, moduleVisitor);
+            moduleVisitor.visitEnd();
+            classWriter.visitEnd();
+        } else {
+            ClassVisitor classVisitor = new ClassVisitor(Opcodes.ASM9, classWriter) {
+                @Override
+                public ModuleVisitor visitModule(String name, int access, String version) {
+                    ModuleVisitor moduleVisitor = super.visitModule(name, access, version);
+                    return new ModuleVisitor(Opcodes.ASM9, moduleVisitor) {
+                        @Override
+                        public void visitEnd() {
+                            addModuleInfoEntires(moduleInfo, providers, autoExportedPackages, this);
+                            super.visitEnd();
+                        }
+                    };
+                }
+            };
+            classReader.accept(classVisitor, 0);
+        }
+        return classWriter.toByteArray();
+    }
+
+    private void addModuleInfoEntires(ModuleInfo moduleInfo, Map<String, List<String>> providers, Set<String> autoExportedPackages, ModuleVisitor moduleVisitor) {
         for (String packageName : autoExportedPackages) {
             moduleVisitor.visitExport(packageName, 0);
         }
@@ -388,8 +423,6 @@ private byte[] addModuleInfo(ModuleInfo moduleInfo, Map<String, List<String>> pr
             moduleVisitor.visitOpen(packageName.replace('.', '/'), 0, modules.toArray(new String[0]));
         }
 
-        moduleVisitor.visitRequire("java.base", 0, null);
-
         if (moduleInfo.requireAllDefinedDependencies) {
             String identifier = moduleInfo.getIdentifier();
             PublishedMetadata requires = getParameters().getRequiresFromMetadata().get().get(identifier);
@@ -439,9 +472,6 @@ private byte[] addModuleInfo(ModuleInfo moduleInfo, Map<String, List<String>> pr
                         implementations.stream().map(impl -> impl.replace('.', '/')).toArray(String[]::new));
             }
         }
-        moduleVisitor.visitEnd();
-        classWriter.visitEnd();
-        return classWriter.toByteArray();
     }
 
     private void mergeJars(ModuleSpec moduleSpec, JarOutputStream outputStream, Map<String, List<String>> providers, Set<String> packages) throws IOException {

diff --git a/src/main/java/org/gradlex/javamodule/moduleinfo/ModuleInfo.java b/src/main/java/org/gradlex/javamodule/moduleinfo/ModuleInfo.java
@@ -44,6 +44,7 @@ public class ModuleInfo extends ModuleSpec {
     boolean exportAllPackages;
     boolean requireAllDefinedDependencies;
     boolean patchRealModule;
+    boolean preserveExisting;
 
     ModuleInfo(String identifier, String moduleName, String moduleVersion, ObjectFactory objectFactory) {
         super(identifier, moduleName);
@@ -133,12 +134,20 @@ public void requireAllDefinedDependencies() {
     }
 
     /**
-     * Explicitly allow patching real (JARs with module-info.class) modules
+     * Allow patching real (JARs with module-info.class) modules by overriding the existing module-info.class.
      */
     public void patchRealModule() {
         this.patchRealModule = true;
     }
 
+    /**
+     * Allow patching real (JARs with module-info.class) by extending the existing module-info.class.
+     */
+    public void preserveExisting() {
+        this.patchRealModule = true;
+        this.preserveExisting = true;
+    }
+
     private static void addOrThrow(Set<String> target, String element) {
         if (!target.add(element)) {
             throw new IllegalArgumentException("The element '" + element + "' is already specified");

diff --git a/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/AbstractFunctionalTest.groovy b/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/AbstractFunctionalTest.groovy
@@ -5,8 +5,6 @@ import org.gradlex.javamodule.moduleinfo.test.fixture.LegacyLibraries
 import org.gradle.testkit.runner.TaskOutcome
 import spock.lang.Specification
 
-import static org.gradlex.javamodule.moduleinfo.test.fixture.GradleBuild.gradleVersionUnderTest
-
 abstract class AbstractFunctionalTest extends Specification {
 
     abstract LegacyLibraries getLibs()

diff --git a/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/EdgeCasesFunctionalTest.groovy b/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/EdgeCasesFunctionalTest.groovy
@@ -5,8 +5,6 @@ import org.gradlex.javamodule.moduleinfo.test.fixture.GradleBuild
 import org.gradlex.javamodule.moduleinfo.test.fixture.LegacyLibraries
 import spock.lang.Specification
 
-import static org.gradlex.javamodule.moduleinfo.test.fixture.GradleBuild.gradleVersionUnderTest
-
 class EdgeCasesFunctionalTest extends Specification {
 
     @Delegate

diff --git a/.../groovy/org/gradlex/javamodule/moduleinfo/test/RealModuleJarPatchingFunctionalTest.groovy b/.../groovy/org/gradlex/javamodule/moduleinfo/test/RealModuleJarPatchingFunctionalTest.groovy
@@ -1,6 +1,7 @@
 package org.gradlex.javamodule.moduleinfo.test
 
 import org.gradlex.javamodule.moduleinfo.test.fixture.GradleBuild
+import spock.lang.IgnoreIf
 import spock.lang.Specification
 
 class RealModuleJarPatchingFunctionalTest extends Specification {
@@ -199,4 +200,57 @@ class RealModuleJarPatchingFunctionalTest extends Specification {
         out.output.contains("Patching of real modules must be explicitly enabled with 'patchRealModule()' and can only be done with 'module()'")
     }
 
+    @IgnoreIf({ GradleBuild.gradleVersionUnderTest?.matches("[67]\\..*") }) // requires Gradle to support Java 17
+    def "a real module cannot be extended"() {
+        given:
+        buildFile << ''' 
+            tasks.withType<JavaCompile>().configureEach {
+                options.compilerArgs.add("-Xlint:all")
+                options.compilerArgs.add("-Werror")
+            }
+            dependencies {
+                implementation("org.apache.logging.log4j:log4j-api:2.24.3")
+
+                // required because not declared in LOG4J metadata
+                compileOnly("com.google.errorprone:error_prone_annotations:2.36.0")
+                compileOnly("com.github.spotbugs:spotbugs-annotations:4.9.0")
+                compileOnly("biz.aQute.bnd:biz.aQute.bnd.annotation:7.1.0")
+                compileOnly("org.osgi:osgi.annotation:8.1.0") // this includes 'org.osgi.annotation.bundle' 
+            }
+            extraJavaModuleInfo {
+                failOnMissingModuleInfo.set(false) // transitive dependencies of annotation libs
+
+                module("org.apache.logging.log4j:log4j-api", "org.apache.logging.log4j") {
+                    preserveExisting()
+                    requiresStatic("com.google.errorprone.annotations")
+                    requiresStatic("com.github.spotbugs.annotations")
+                    requiresStatic("biz.aQute.bnd.annotation")
+                    requiresStatic("org.osgi.annotation")
+                }
+                module("biz.aQute.bnd:biz.aQute.bnd.annotation", "biz.aQute.bnd.annotation") {
+                    requiresStatic("org.osgi.annotation")
+                    exportAllPackages()
+                }
+                module("org.osgi:osgi.annotation", "org.osgi.annotation") 
+            }            
+        '''
+        file("src/main/java/module-info.java") << """
+            module org.example {
+                requires org.apache.logging.log4j;
+            }
+        """
+        file("src/main/java/org/example/Main.java") << """
+            package org.example;
+            public class Main {
+                org.apache.logging.log4j.message.ParameterizedMessage m; // needs errorprone
+                org.apache.logging.log4j.status.StatusData d; // needs spotbugs
+                org.apache.logging.log4j.util.SystemPropertiesPropertySource s; // needs aQute.bnd
+                org.apache.logging.log4j.util.Activator a; // needs osgi
+            }
+        """
+
+        expect:
+        build()
+    }
+
 }