diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a4c7f8..741bfd2 100644 --- 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 index d8ad574..e243917 100644 --- 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 index e0cd475..0cb70b8 100644 --- 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> providers = new LinkedHashMap<>(); Set 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> providers, Set packages) throws IOException { + @Nullable + private byte[] copyAndExtractProviders(JarInputStream inputStream, JarOutputStream outputStream, boolean willMergeJars, Map> providers, Set 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 extractImplementations(byte[] content) { @@ -366,13 +374,40 @@ private List extractImplementations(byte[] content) { .collect(Collectors.toList()); } - private byte[] addModuleInfo(ModuleInfo moduleInfo, Map> providers, @Nullable String version, Set autoExportedPackages) { - ClassWriter classWriter = new ClassWriter(0); - classWriter.visit(Opcodes.V9, Opcodes.ACC_MODULE, "module-info", null, null, null); + private byte[] addModuleInfo(ModuleInfo moduleInfo, Map> providers, @Nullable String version, Set 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> providers, Set autoExportedPackages, ModuleVisitor moduleVisitor) { for (String packageName : autoExportedPackages) { moduleVisitor.visitExport(packageName, 0); } @@ -388,8 +423,6 @@ private byte[] addModuleInfo(ModuleInfo moduleInfo, Map> 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> 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> providers, Set 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 index 9122a6c..57b118f 100644 --- 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 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 index d21d949..1353d99 100644 --- 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 index 7030936..6f6f644 100644 --- 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/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/RealModuleJarPatchingFunctionalTest.groovy b/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/RealModuleJarPatchingFunctionalTest.groovy index b12d606..387faca 100644 --- a/src/test/groovy/org/gradlex/javamodule/moduleinfo/test/RealModuleJarPatchingFunctionalTest.groovy +++ b/src/test/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().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() + } + }