Skip to content

Commit 8af997f

Browse files
authored
Add preserveExisting() as option to patch real modules (#162)
1 parent acfd46e commit 8af997f

File tree

7 files changed

+117
-22
lines changed

7 files changed

+117
-22
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# Extra Java Module Info Gradle Plugin - Changelog
22

3+
## Version 1.10
4+
* [New] [#160](https://github.com/gradlex-org/extra-java-module-info/pull/160) - Add 'preserveExisting' option to patch real modules
5+
36
## Version 1.9
47
* [New] [#137](https://github.com/gradlex-org/extra-java-module-info/pull/137) - Configuration option for 'versionsProvidingConfiguration'
58
* [New] [#130](https://github.com/gradlex-org/extra-java-module-info/pull/130) - Support classifier in coordinates notation

README.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,15 @@ Note: The merged Jar will include the *first* appearance of duplicated files (li
313313

314314
## How can I fix a library with a broken `module-info.class`?
315315

316-
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.
316+
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.
317+
However, you need to specify `patchRealModule()` to overwrite the existing `module-info.class`.
318+
You can also use `preserveExisting()`, if the exiting `module-info.class` is working in general, but misses entries.
317319

318320
```
319321
extraJavaModuleInfo {
320322
module("org.apache.tomcat.embed:tomcat-embed-core", "org.apache.tomcat.embed.core") {
321-
patchRealModule()
323+
patchRealModule() // overwrite existing module-info.class
324+
preserveExisting() // extend existing module-info.class
322325
requires("java.desktop")
323326
requires("java.instrument")
324327
...

src/main/java/org/gradlex/javamodule/moduleinfo/ExtraJavaModuleInfoTransform.java

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import org.gradle.api.provider.Provider;
3131
import org.gradle.api.tasks.Input;
3232
import org.gradle.api.tasks.InputFiles;
33+
import org.objectweb.asm.ClassReader;
34+
import org.objectweb.asm.ClassVisitor;
3335
import org.objectweb.asm.ClassWriter;
3436
import org.objectweb.asm.ModuleVisitor;
3537
import org.objectweb.asm.Opcodes;
@@ -144,7 +146,7 @@ public void transform(TransformOutputs outputs) {
144146
boolean realModule = isModule(originalJar);
145147
if (moduleSpec instanceof ModuleInfo) {
146148
if (realModule && !((ModuleInfo) moduleSpec).patchRealModule) {
147-
throw new RuntimeException("Patching of real modules must be explicitly enabled with 'patchRealModule()'");
149+
throw new RuntimeException("Patching of real modules must be explicitly enabled with 'patchRealModule()' or 'preserveExisting()'");
148150
}
149151
String definedName = moduleSpec.getModuleName();
150152
String expectedName = autoModuleName(originalJar);
@@ -287,12 +289,14 @@ private void addModuleDescriptor(File originalJar, File moduleJar, ModuleInfo mo
287289
try (JarOutputStream outputStream = newJarOutputStream(Files.newOutputStream(moduleJar.toPath()), inputStream.getManifest())) {
288290
Map<String, List<String>> providers = new LinkedHashMap<>();
289291
Set<String> packages = new TreeSet<>();
290-
copyAndExtractProviders(inputStream, outputStream, !moduleInfo.getMergedJars().isEmpty(), providers, packages);
292+
byte[] existingModuleInfo = copyAndExtractProviders(inputStream, outputStream, !moduleInfo.getMergedJars().isEmpty(), providers, packages);
291293
mergeJars(moduleInfo, outputStream, providers, packages);
292294
outputStream.putNextEntry(newReproducibleEntry("module-info.class"));
293295
outputStream.write(addModuleInfo(moduleInfo, providers, versionFromFilePath(originalJar.toPath()),
294-
moduleInfo.exportAllPackages ? packages : Collections.emptySet()));
296+
moduleInfo.exportAllPackages ? packages : Collections.emptySet(),
297+
existingModuleInfo));
295298
outputStream.closeEntry();
299+
System.out.println("AAA: " + moduleJar);
296300
}
297301
} catch (IOException e) {
298302
throw new RuntimeException(e);
@@ -310,8 +314,10 @@ private JarOutputStream newJarOutputStream(OutputStream out, @Nullable Manifest
310314
return jar;
311315
}
312316

313-
private void copyAndExtractProviders(JarInputStream inputStream, JarOutputStream outputStream, boolean willMergeJars, Map<String, List<String>> providers, Set<String> packages) throws IOException {
317+
@Nullable
318+
private byte[] copyAndExtractProviders(JarInputStream inputStream, JarOutputStream outputStream, boolean willMergeJars, Map<String, List<String>> providers, Set<String> packages) throws IOException {
314319
JarEntry jarEntry = inputStream.getNextJarEntry();
320+
byte[] existingModuleInfo = null;
315321
while (jarEntry != null) {
316322
byte[] content = readAllBytes(inputStream);
317323
String entryName = jarEntry.getName();
@@ -325,8 +331,9 @@ private void copyAndExtractProviders(JarInputStream inputStream, JarOutputStream
325331
}
326332
providers.get(key).addAll(extractImplementations(content));
327333
}
328-
329-
if (!JAR_SIGNATURE_PATH.matcher(entryName).matches() && !"META-INF/MANIFEST.MF".equals(entryName) && !isModuleInfoClass(entryName)) {
334+
if (isModuleInfoClass(entryName)) {
335+
existingModuleInfo = content;
336+
} else if (!JAR_SIGNATURE_PATH.matcher(entryName).matches() && !"META-INF/MANIFEST.MF".equals(entryName)) {
330337
if (!willMergeJars || !isFileInServicesFolder) { // service provider files will be merged later
331338
jarEntry.setCompressedSize(-1);
332339
try {
@@ -354,6 +361,7 @@ private void copyAndExtractProviders(JarInputStream inputStream, JarOutputStream
354361
}
355362
jarEntry = inputStream.getNextJarEntry();
356363
}
364+
return existingModuleInfo;
357365
}
358366

359367
private List<String> extractImplementations(byte[] content) {
@@ -366,13 +374,40 @@ private List<String> extractImplementations(byte[] content) {
366374
.collect(Collectors.toList());
367375
}
368376

369-
private byte[] addModuleInfo(ModuleInfo moduleInfo, Map<String, List<String>> providers, @Nullable String version, Set<String> autoExportedPackages) {
370-
ClassWriter classWriter = new ClassWriter(0);
371-
classWriter.visit(Opcodes.V9, Opcodes.ACC_MODULE, "module-info", null, null, null);
377+
private byte[] addModuleInfo(ModuleInfo moduleInfo, Map<String, List<String>> providers, @Nullable String version, Set<String> autoExportedPackages,
378+
@Nullable byte[] existingModuleInfo) {
379+
ClassReader classReader = moduleInfo.preserveExisting && existingModuleInfo != null ? new ClassReader(existingModuleInfo) : null;
380+
ClassWriter classWriter = new ClassWriter(classReader, 0);
372381
int openModule = moduleInfo.openModule ? Opcodes.ACC_OPEN : 0;
373382
String moduleVersion = moduleInfo.getModuleVersion() == null ? version : moduleInfo.getModuleVersion();
374-
ModuleVisitor moduleVisitor = classWriter.visitModule(moduleInfo.getModuleName(), openModule, moduleVersion);
375383

384+
if (classReader == null) {
385+
classWriter.visit(Opcodes.V9, Opcodes.ACC_MODULE, "module-info", null, null, null);
386+
ModuleVisitor moduleVisitor = classWriter.visitModule(moduleInfo.getModuleName(), openModule, moduleVersion);
387+
moduleVisitor.visitRequire("java.base", 0, null);
388+
addModuleInfoEntires(moduleInfo, providers, autoExportedPackages, moduleVisitor);
389+
moduleVisitor.visitEnd();
390+
classWriter.visitEnd();
391+
} else {
392+
ClassVisitor classVisitor = new ClassVisitor(Opcodes.ASM9, classWriter) {
393+
@Override
394+
public ModuleVisitor visitModule(String name, int access, String version) {
395+
ModuleVisitor moduleVisitor = super.visitModule(name, access, version);
396+
return new ModuleVisitor(Opcodes.ASM9, moduleVisitor) {
397+
@Override
398+
public void visitEnd() {
399+
addModuleInfoEntires(moduleInfo, providers, autoExportedPackages, this);
400+
super.visitEnd();
401+
}
402+
};
403+
}
404+
};
405+
classReader.accept(classVisitor, 0);
406+
}
407+
return classWriter.toByteArray();
408+
}
409+
410+
private void addModuleInfoEntires(ModuleInfo moduleInfo, Map<String, List<String>> providers, Set<String> autoExportedPackages, ModuleVisitor moduleVisitor) {
376411
for (String packageName : autoExportedPackages) {
377412
moduleVisitor.visitExport(packageName, 0);
378413
}
@@ -388,8 +423,6 @@ private byte[] addModuleInfo(ModuleInfo moduleInfo, Map<String, List<String>> pr
388423
moduleVisitor.visitOpen(packageName.replace('.', '/'), 0, modules.toArray(new String[0]));
389424
}
390425

391-
moduleVisitor.visitRequire("java.base", 0, null);
392-
393426
if (moduleInfo.requireAllDefinedDependencies) {
394427
String identifier = moduleInfo.getIdentifier();
395428
PublishedMetadata requires = getParameters().getRequiresFromMetadata().get().get(identifier);
@@ -439,9 +472,6 @@ private byte[] addModuleInfo(ModuleInfo moduleInfo, Map<String, List<String>> pr
439472
implementations.stream().map(impl -> impl.replace('.', '/')).toArray(String[]::new));
440473
}
441474
}
442-
moduleVisitor.visitEnd();
443-
classWriter.visitEnd();
444-
return classWriter.toByteArray();
445475
}
446476

447477
private void mergeJars(ModuleSpec moduleSpec, JarOutputStream outputStream, Map<String, List<String>> providers, Set<String> packages) throws IOException {

src/main/java/org/gradlex/javamodule/moduleinfo/ModuleInfo.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public class ModuleInfo extends ModuleSpec {
4444
boolean exportAllPackages;
4545
boolean requireAllDefinedDependencies;
4646
boolean patchRealModule;
47+
boolean preserveExisting;
4748

4849
ModuleInfo(String identifier, String moduleName, String moduleVersion, ObjectFactory objectFactory) {
4950
super(identifier, moduleName);
@@ -133,12 +134,20 @@ public void requireAllDefinedDependencies() {
133134
}
134135

135136
/**
136-
* Explicitly allow patching real (JARs with module-info.class) modules
137+
* Allow patching real (JARs with module-info.class) modules by overriding the existing module-info.class.
137138
*/
138139
public void patchRealModule() {
139140
this.patchRealModule = true;
140141
}
141142

143+
/**
144+
* Allow patching real (JARs with module-info.class) by extending the existing module-info.class.
145+
*/
146+
public void preserveExisting() {
147+
this.patchRealModule = true;
148+
this.preserveExisting = true;
149+
}
150+
142151
private static void addOrThrow(Set<String> target, String element) {
143152
if (!target.add(element)) {
144153
throw new IllegalArgumentException("The element '" + element + "' is already specified");

src/test/groovy/org/gradlex/javamodule/moduleinfo/test/AbstractFunctionalTest.groovy

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import org.gradlex.javamodule.moduleinfo.test.fixture.LegacyLibraries
55
import org.gradle.testkit.runner.TaskOutcome
66
import spock.lang.Specification
77

8-
import static org.gradlex.javamodule.moduleinfo.test.fixture.GradleBuild.gradleVersionUnderTest
9-
108
abstract class AbstractFunctionalTest extends Specification {
119

1210
abstract LegacyLibraries getLibs()

src/test/groovy/org/gradlex/javamodule/moduleinfo/test/EdgeCasesFunctionalTest.groovy

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import org.gradlex.javamodule.moduleinfo.test.fixture.GradleBuild
55
import org.gradlex.javamodule.moduleinfo.test.fixture.LegacyLibraries
66
import spock.lang.Specification
77

8-
import static org.gradlex.javamodule.moduleinfo.test.fixture.GradleBuild.gradleVersionUnderTest
9-
108
class EdgeCasesFunctionalTest extends Specification {
119

1210
@Delegate

src/test/groovy/org/gradlex/javamodule/moduleinfo/test/RealModuleJarPatchingFunctionalTest.groovy

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.gradlex.javamodule.moduleinfo.test
22

33
import org.gradlex.javamodule.moduleinfo.test.fixture.GradleBuild
4+
import spock.lang.IgnoreIf
45
import spock.lang.Specification
56

67
class RealModuleJarPatchingFunctionalTest extends Specification {
@@ -199,4 +200,57 @@ class RealModuleJarPatchingFunctionalTest extends Specification {
199200
out.output.contains("Patching of real modules must be explicitly enabled with 'patchRealModule()' and can only be done with 'module()'")
200201
}
201202

203+
@IgnoreIf({ GradleBuild.gradleVersionUnderTest?.matches("[67]\\..*") }) // requires Gradle to support Java 17
204+
def "a real module cannot be extended"() {
205+
given:
206+
buildFile << '''
207+
tasks.withType<JavaCompile>().configureEach {
208+
options.compilerArgs.add("-Xlint:all")
209+
options.compilerArgs.add("-Werror")
210+
}
211+
dependencies {
212+
implementation("org.apache.logging.log4j:log4j-api:2.24.3")
213+
214+
// required because not declared in LOG4J metadata
215+
compileOnly("com.google.errorprone:error_prone_annotations:2.36.0")
216+
compileOnly("com.github.spotbugs:spotbugs-annotations:4.9.0")
217+
compileOnly("biz.aQute.bnd:biz.aQute.bnd.annotation:7.1.0")
218+
compileOnly("org.osgi:osgi.annotation:8.1.0") // this includes 'org.osgi.annotation.bundle'
219+
}
220+
extraJavaModuleInfo {
221+
failOnMissingModuleInfo.set(false) // transitive dependencies of annotation libs
222+
223+
module("org.apache.logging.log4j:log4j-api", "org.apache.logging.log4j") {
224+
preserveExisting()
225+
requiresStatic("com.google.errorprone.annotations")
226+
requiresStatic("com.github.spotbugs.annotations")
227+
requiresStatic("biz.aQute.bnd.annotation")
228+
requiresStatic("org.osgi.annotation")
229+
}
230+
module("biz.aQute.bnd:biz.aQute.bnd.annotation", "biz.aQute.bnd.annotation") {
231+
requiresStatic("org.osgi.annotation")
232+
exportAllPackages()
233+
}
234+
module("org.osgi:osgi.annotation", "org.osgi.annotation")
235+
}
236+
'''
237+
file("src/main/java/module-info.java") << """
238+
module org.example {
239+
requires org.apache.logging.log4j;
240+
}
241+
"""
242+
file("src/main/java/org/example/Main.java") << """
243+
package org.example;
244+
public class Main {
245+
org.apache.logging.log4j.message.ParameterizedMessage m; // needs errorprone
246+
org.apache.logging.log4j.status.StatusData d; // needs spotbugs
247+
org.apache.logging.log4j.util.SystemPropertiesPropertySource s; // needs aQute.bnd
248+
org.apache.logging.log4j.util.Activator a; // needs osgi
249+
}
250+
"""
251+
252+
expect:
253+
build()
254+
}
255+
202256
}

0 commit comments

Comments
 (0)