From fc894ff7fcbb9384b097934d5d610873f23b1853 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 28 Jan 2025 11:49:36 +0100 Subject: [PATCH 1/3] [MNG-8544] Conflicting extensions went unnoticed Now that Maven4 has several extension sources, we need to be able to detect conflicts. For now, we allow only one GA and fail the bootstrapping, if conflict discovered. User should fix the situation, with the help of detailed error message enumerating all the conflicts. --- https://issues.apache.org/jira/browse/MNG-8544 --- .../maven/cling/invoker/BaseParser.java | 37 ++++++++++++++++--- .../cling/invoker/mvn/MavenInvokerTest.java | 33 +++++++++++++++++ .../invoker/mvn/MavenInvokerTestSupport.java | 2 +- 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java index 27f4add0afd9..e9b2546e4235 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java @@ -295,20 +295,47 @@ protected Map populateUserProperties(LocalContext context) throw protected List readCoreExtensionsDescriptor(LocalContext context) throws ParserException, IOException { - ArrayList extensions = new ArrayList<>(); String installationExtensionsFile = context.userProperties.get(Constants.MAVEN_INSTALLATION_EXTENSIONS); - extensions.addAll(readCoreExtensionsDescriptorFromFile( + ArrayList installationExtensions = new ArrayList<>(readCoreExtensionsDescriptorFromFile( context.installationDirectory.resolve(installationExtensionsFile))); + String userExtensionsFile = context.userProperties.get(Constants.MAVEN_USER_EXTENSIONS); + ArrayList userExtensions = new ArrayList<>( + readCoreExtensionsDescriptorFromFile(context.userHomeDirectory.resolve(userExtensionsFile))); + String projectExtensionsFile = context.userProperties.get(Constants.MAVEN_PROJECT_EXTENSIONS); - extensions.addAll(readCoreExtensionsDescriptorFromFile(context.cwd.resolve(projectExtensionsFile))); + ArrayList projectExtensions = + new ArrayList<>(readCoreExtensionsDescriptorFromFile(context.cwd.resolve(projectExtensionsFile))); - String userExtensionsFile = context.userProperties.get(Constants.MAVEN_USER_EXTENSIONS); - extensions.addAll(readCoreExtensionsDescriptorFromFile(context.userHomeDirectory.resolve(userExtensionsFile))); + // merge these 3 but check for GA uniqueness; we don't want to load up same extension w/ different Vs + ArrayList extensions = + new ArrayList<>(installationExtensions.size() + userExtensions.size() + projectExtensions.size()); + HashMap gas = new HashMap<>(); + ArrayList conflicts = new ArrayList<>(); + + mergeExtensions(installationExtensions, installationExtensionsFile, gas, conflicts); + mergeExtensions(userExtensions, userExtensionsFile, gas, conflicts); + mergeExtensions(projectExtensions, projectExtensionsFile, gas, conflicts); + + if (!conflicts.isEmpty()) { + throw new ParserException("Extension conflicts: " + String.join("; ", conflicts)); + } return extensions; } + private void mergeExtensions( + List extensions, String extensionsSource, Map gas, List conflicts) { + for (CoreExtension extension : extensions) { + String ga = extension.getGroupId() + ":" + extension.getArtifactId(); + if (gas.containsKey(ga)) { + conflicts.add(ga + " from " + extensionsSource + " already specified in " + gas.get(ga)); + } else { + gas.put(ga, extensionsSource); + } + } + } + protected List readCoreExtensionsDescriptorFromFile(Path extensionsFile) throws ParserException, IOException { try { diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java index bae4f7015f51..bd7b8bb160af 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java @@ -19,6 +19,7 @@ package org.apache.maven.cling.invoker.mvn; import java.nio.file.FileSystem; +import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; @@ -26,6 +27,7 @@ import com.google.common.jimfs.Jimfs; import org.apache.maven.api.cli.Invoker; import org.apache.maven.api.cli.Parser; +import org.apache.maven.api.cli.ParserException; import org.apache.maven.cling.invoker.ProtoLookup; import org.codehaus.plexus.classworlds.ClassWorld; import org.junit.jupiter.api.Disabled; @@ -34,6 +36,8 @@ import org.junit.jupiter.api.io.CleanupMode; import org.junit.jupiter.api.io.TempDir; +import static org.junit.jupiter.api.Assertions.assertThrows; + /** * Local UT. */ @@ -59,6 +63,35 @@ void defaultFs( invoke(cwd, userHome, Arrays.asList("clean", "verify")); } + @Test + void conflictingExtensions( + @TempDir(cleanup = CleanupMode.ON_SUCCESS) Path cwd, + @TempDir(cleanup = CleanupMode.ON_SUCCESS) Path userHome) + throws Exception { + String extensionsXml = + """ + + + + eu.maveniverse.maven.mimir + extension3 + 0.3.4 + + + """; + Path dotMvn = cwd.resolve(".mvn"); + Files.createDirectories(dotMvn); + Path projectExtensions = dotMvn.resolve("extensions.xml"); + Files.writeString(projectExtensions, extensionsXml); + + Path userConf = userHome.resolve(".m2"); + Path userExtensions = userConf.resolve("extensions.xml"); + Files.createDirectories(userConf); + Files.writeString(userExtensions, extensionsXml); + + assertThrows(ParserException.class, () -> invoke(cwd, userHome, Arrays.asList("clean", "verify"))); + } + @Disabled("Until we move off fully from File") @Test void jimFs() throws Exception { diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTestSupport.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTestSupport.java index 89b33df73fe8..15caa92f674f 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTestSupport.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTestSupport.java @@ -88,7 +88,7 @@ protected void invoke(Path cwd, Path userHome, Collection goals) throws .resolve("maven.properties")), "${maven.home}/conf/maven.properties must be a file"); - Files.createDirectory(cwd.resolve(".mvn")); + Files.createDirectories(cwd.resolve(".mvn")); Path pom = cwd.resolve("pom.xml").toAbsolutePath(); Files.writeString(pom, POM_STRING); Path appJava = cwd.resolve("src/main/java/org/apache/maven/samples/sample/App.java"); From 0820a874e6913e9d7ffcd97ca440a5f256e11a43 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 28 Jan 2025 11:54:48 +0100 Subject: [PATCH 2/3] Do merge them as well --- .../apache/maven/cling/invoker/BaseParser.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java index e9b2546e4235..982b1320b757 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java @@ -308,23 +308,23 @@ protected List readCoreExtensionsDescriptor(LocalContext context) new ArrayList<>(readCoreExtensionsDescriptorFromFile(context.cwd.resolve(projectExtensionsFile))); // merge these 3 but check for GA uniqueness; we don't want to load up same extension w/ different Vs - ArrayList extensions = - new ArrayList<>(installationExtensions.size() + userExtensions.size() + projectExtensions.size()); HashMap gas = new HashMap<>(); ArrayList conflicts = new ArrayList<>(); - mergeExtensions(installationExtensions, installationExtensionsFile, gas, conflicts); - mergeExtensions(userExtensions, userExtensionsFile, gas, conflicts); - mergeExtensions(projectExtensions, projectExtensionsFile, gas, conflicts); + ArrayList coreExtensions = + new ArrayList<>(installationExtensions.size() + userExtensions.size() + projectExtensions.size()); + coreExtensions.addAll(mergeExtensions(installationExtensions, installationExtensionsFile, gas, conflicts)); + coreExtensions.addAll(mergeExtensions(userExtensions, userExtensionsFile, gas, conflicts)); + coreExtensions.addAll(mergeExtensions(projectExtensions, projectExtensionsFile, gas, conflicts)); if (!conflicts.isEmpty()) { throw new ParserException("Extension conflicts: " + String.join("; ", conflicts)); } - return extensions; + return coreExtensions; } - private void mergeExtensions( + private List mergeExtensions( List extensions, String extensionsSource, Map gas, List conflicts) { for (CoreExtension extension : extensions) { String ga = extension.getGroupId() + ":" + extension.getArtifactId(); @@ -334,6 +334,7 @@ private void mergeExtensions( gas.put(ga, extensionsSource); } } + return extensions; } protected List readCoreExtensionsDescriptorFromFile(Path extensionsFile) From ff90b90d2954871a3a56911555f36f3db5061c66 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 28 Jan 2025 11:58:10 +0100 Subject: [PATCH 3/3] Align methods --- .../org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java index bd7b8bb160af..21a24bed57ce 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/MavenInvokerTest.java @@ -85,8 +85,8 @@ void conflictingExtensions( Files.writeString(projectExtensions, extensionsXml); Path userConf = userHome.resolve(".m2"); - Path userExtensions = userConf.resolve("extensions.xml"); Files.createDirectories(userConf); + Path userExtensions = userConf.resolve("extensions.xml"); Files.writeString(userExtensions, extensionsXml); assertThrows(ParserException.class, () -> invoke(cwd, userHome, Arrays.asList("clean", "verify")));