From 1e3ec8ebac6636ba8130b77fafcbff1d75655739 Mon Sep 17 00:00:00 2001 From: "Henning P. Schmiedehausen" Date: Thu, 17 Aug 2023 18:29:28 -0700 Subject: [PATCH 1/2] Allow defined and automatic modules to co-exist In a maven project, a maven module creates two artifacts: - a main artifact with a module-info.class file that defines a JPMS module "foo.bar". The jar is called foo-bar-1.0.jar - a test artifact which is not a JPMS module. It is called foo-bar-1.0-tests.jar Another module declares dependencies on both modules: xxx foo-bar 1.0 xxx foo-bar 1.0 tests test This is a common use case in large projects. The LocationManager now creates two JavaModuleDescriptors for the "foo.bar" JPMS module. One from the main artifact and its module-info.class (automatic == false) and one from the test artifact (automatic == true). The current code considers these duplicates and drops one. As a result, the test code no longer compiles because a dependency is missing. The patch separates out modules with a module descriptor and automatic modules. Then it adds the modules with module descriptors to the module path. Any duplicate is dropped for modules with module descriptors. Finally, it adds the automatic modules; if a module with the same module id already exists on the module path, it adds it to the class path. In the case above, this will allow the compile to successfully compile test code (the tests dependency drops to the class path, while the main artifact is on the module path). --- .../languages/java/jpms/LocationManager.java | 50 +++++++++++---- .../java/jpms/LocationManagerTest.java | 57 +++++++++++++++++- .../plexus-java-1.0.0-SNAPSHOT-tests.jar | Bin 0 -> 371 bytes .../jar.tests/plexus-java-1.0.0-SNAPSHOT.jar | Bin 0 -> 855 bytes 4 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 plexus-java/src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT-tests.jar create mode 100644 plexus-java/src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT.jar diff --git a/plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java b/plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java index 9a51ae6..4bdbacb 100644 --- a/plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java +++ b/plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.function.Consumer; import org.codehaus.plexus.languages.java.jpms.JavaModuleDescriptor.JavaProvides; @@ -249,22 +250,47 @@ public String extract(Path path) throws IOException { request.isIncludeStatic()); } - // in case of identical module names, first one wins + Map definedModules = new HashMap<>(); + Map automaticModules = new HashMap<>(); + + pathElements.forEach((k, v) -> { + if (v != null && !v.isAutomatic()) { + definedModules.put(k, v); + } else { + automaticModules.put(k, v); + } + }); + + // in case of identical module names, first one wins, others drop onto classpath + // if they are automatic modules. Set collectedModules = new HashSet<>(requiredNamedModules.size()); - for (Entry entry : pathElements.entrySet()) { - if (entry.getValue() != null - && requiredNamedModules.contains(entry.getValue().name())) { - if (collectedModules.add(entry.getValue().name())) { - result.getModulepathElements() - .put( - entry.getKey(), - moduleNameSources.get(entry.getValue().name())); + Consumer> moduleAcceptor = moduleSet -> { + for (Entry entry : moduleSet.entrySet()) { + if (entry.getValue() != null + && requiredNamedModules.contains(entry.getValue().name())) { + if (collectedModules.add(entry.getValue().name())) { + result.getModulepathElements() + .put( + entry.getKey(), + moduleNameSources.get(entry.getValue().name())); + // if the module is an automatic module, add it to the classpath + } else if (entry.getValue().isAutomatic()) { + result.getClasspathElements().add(entry.getKey()); + } + } else { + result.getClasspathElements().add(entry.getKey()); } - } else { - result.getClasspathElements().add(entry.getKey()); } - } + }; + + // process defined modules first. This fixes a corner case where a project creates + // a main artifact that is a JPMS module and a tests artifact that is not. If the + // main artifact and the test artifact happen to have the same module id (one from + // the module-info, one from the automatic module naming), the main artifact will + // be on the module path and the test artifact will be on the class path. + moduleAcceptor.accept(definedModules); + moduleAcceptor.accept(automaticModules); return result; } diff --git a/plexus-java/src/test/java/org/codehaus/plexus/languages/java/jpms/LocationManagerTest.java b/plexus-java/src/test/java/org/codehaus/plexus/languages/java/jpms/LocationManagerTest.java index acd825d..e9b0c90 100644 --- a/plexus-java/src/test/java/org/codehaus/plexus/languages/java/jpms/LocationManagerTest.java +++ b/plexus-java/src/test/java/org/codehaus/plexus/languages/java/jpms/LocationManagerTest.java @@ -161,6 +161,32 @@ void testIdenticalModuleNames() throws Exception { ResolvePathsRequest request = ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava); + when(asmParser.getModuleDescriptor(pj1)) + .thenReturn(JavaModuleDescriptor.newModule("plexus.java").build()); + when(asmParser.getModuleDescriptor(pj2)) + .thenReturn(JavaModuleDescriptor.newModule("plexus.java").build()); + + ResolvePathsResult result = locationManager.resolvePaths(request); + + assertThat(result.getMainModuleDescriptor()).isEqualTo(descriptor); + assertThat(result.getPathElements()).hasSize(2); + assertThat(result.getModulepathElements()).hasSize(1); + assertThat(result.getModulepathElements()).containsKey(pj1); + assertThat(result.getModulepathElements()).doesNotContainKey(pj2); + assertThat(result.getClasspathElements()).isEmpty(); + assertThat(result.getPathExceptions()).isEmpty(); + } + + @Test + public void testIdenticalAutomaticModuleNames() throws Exception { + Path pj1 = Paths.get("src/test/resources/jar.empty/plexus-java-1.0.0-SNAPSHOT.jar"); + Path pj2 = Paths.get("src/test/resources/jar.empty.2/plexus-java-2.0.0-SNAPSHOT.jar"); + JavaModuleDescriptor descriptor = + JavaModuleDescriptor.newModule("base").requires("plexus.java").build(); + when(qdoxParser.fromSourcePath(any(Path.class))).thenReturn(descriptor); + ResolvePathsRequest request = + ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava); + when(asmParser.getModuleDescriptor(pj1)) .thenReturn( JavaModuleDescriptor.newAutomaticModule("plexus.java").build()); @@ -173,8 +199,35 @@ void testIdenticalModuleNames() throws Exception { assertThat(result.getPathElements()).hasSize(2); assertThat(result.getModulepathElements()).containsOnlyKeys(pj1); assertThat(result.getModulepathElements()).doesNotContainKey(pj2); - assertThat(result.getClasspathElements()).hasSize(0); - assertThat(result.getPathExceptions()).hasSize(0); + assertThat(result.getClasspathElements()).hasSize(1); + assertThat(result.getPathExceptions()).isEmpty(); + } + + @Test + public void testMainJarModuleAndTestJarAutomatic() throws Exception { + Path pj1 = Paths.get("src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT.jar"); + Path pj2 = Paths.get("src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT-tests.jar"); + JavaModuleDescriptor descriptor = + JavaModuleDescriptor.newModule("base").requires("plexus.java").build(); + when(qdoxParser.fromSourcePath(any(Path.class))).thenReturn(descriptor); + ResolvePathsRequest request = + ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava); + + when(asmParser.getModuleDescriptor(pj1)) + .thenReturn(JavaModuleDescriptor.newModule("plexus.java").build()); + when(asmParser.getModuleDescriptor(pj2)).thenReturn(null); + + ResolvePathsResult result = locationManager.resolvePaths(request); + + assertThat(result.getMainModuleDescriptor()).isEqualTo(descriptor); + assertThat(result.getPathElements()).hasSize(2); + assertThat(result.getModulepathElements()).hasSize(1); + assertThat(result.getModulepathElements()).containsKey(pj1); + assertThat(result.getModulepathElements()).doesNotContainKey(pj2); + assertThat(result.getClasspathElements()).hasSize(1); + assertThat(result.getClasspathElements()).contains(pj2); + assertThat(result.getClasspathElements()).doesNotContain(pj1); + assertThat(result.getPathExceptions()).isEmpty(); } @Test diff --git a/plexus-java/src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT-tests.jar b/plexus-java/src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT-tests.jar new file mode 100644 index 0000000000000000000000000000000000000000..a6afd0fbf5a82ef0664819fe60f023082ec1e6d6 GIT binary patch literal 371 zcmWIWW@Zs#;Nak3crsBioB;`NGO#fCx`sIFdiuHP|2xINz|0Wf&CUT*!30$nfK#&w zPz7AGucM!*n`>~0p06A4>5Y621_BHh_ImzbsO&7QmgL;i^R0W{lOVWO9x zX{d$l<(;VcJBUlu4&=mr@FaE0S*myjj^ms+fSV7Dzt;aTox%Ia4

*(j{<{BKL=j-;__snS@Z(Y5MyxzK6=gyqp9At3C_`%a6JuhD!Pv48Bt5~>Z zyq0_ssgbcm?Rkor$Z{=z@oRyN$$1T4Nm zNdgtX;~11c0=!YRBF7WRT?k+cWWu!~B@c8HkV6F&p9pXO$VB!dICeqWxfpN<9YXsQ oMp!^1oD2#| Date: Tue, 22 Aug 2023 11:35:58 -0700 Subject: [PATCH 2/2] fix brittle test that relies on order --- .../codehaus/plexus/languages/java/jpms/LocationManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java b/plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java index 4bdbacb..06f5683 100644 --- a/plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java +++ b/plexus-java/src/main/java/org/codehaus/plexus/languages/java/jpms/LocationManager.java @@ -250,8 +250,8 @@ public String extract(Path path) throws IOException { request.isIncludeStatic()); } - Map definedModules = new HashMap<>(); - Map automaticModules = new HashMap<>(); + Map definedModules = new LinkedHashMap<>(); + Map automaticModules = new LinkedHashMap<>(); pathElements.forEach((k, v) -> { if (v != null && !v.isAutomatic()) {