This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch mvnup
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/mvnup by this push:
new d599d7e5e8 Improve BaseUpgradeGoal to fix Maven 4 compatibility issues
d599d7e5e8 is described below
commit d599d7e5e820d3442f53a6a081b85a0d2adbe475
Author: Guillaume Nodet <gno...@gmail.com>
AuthorDate: Fri May 30 08:25:05 2025 +0200
Improve BaseUpgradeGoal to fix Maven 4 compatibility issues
- Add fixDuplicatePlugins() to handle duplicate plugin declarations
- Add fixIncorrectParentRelativePaths() to correct invalid parent paths
- Enhance fixUnsupportedRepositoryExpressions() for better error handling
- Add comprehensive unit tests for new functionality
- Exclude FileLength checkstyle check for BaseUpgradeGoal
Fixes issues identified in Maven 4 testing:
- Duplicate plugin declarations (e.g., axis-axis1-java)
- Unsupported repository URL expressions (e.g., spark with ${asf.staging})
- Incorrect parent relative paths in multi-module projects
---
impl/maven-cli/pom.xml | 5 +
.../cling/invoker/mvnup/goals/BaseUpgradeGoal.java | 262 ++++++++++++++++++---
.../mvnup/goals/Maven4CompatibilityFixesTest.java | 127 ++++++++++
3 files changed, 362 insertions(+), 32 deletions(-)
diff --git a/impl/maven-cli/pom.xml b/impl/maven-cli/pom.xml
index cabe1f0eee..0c9c703dca 100644
--- a/impl/maven-cli/pom.xml
+++ b/impl/maven-cli/pom.xml
@@ -31,6 +31,11 @@ under the License.
<name>Maven 4 CLI</name>
<description>Maven 4 CLI component, with CLI and logging
support.</description>
+ <properties>
+ <!-- in: BaseUpgradeGoal -->
+ <checkstyle.violation.ignore>FileLength</checkstyle.violation.ignore>
+ </properties>
+
<dependencies>
<!-- Maven4 API -->
<dependency>
diff --git
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/BaseUpgradeGoal.java
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/BaseUpgradeGoal.java
index 38f810b92e..1163b1942a 100644
---
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/BaseUpgradeGoal.java
+++
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/BaseUpgradeGoal.java
@@ -405,11 +405,21 @@ protected void
applyMaven4CompatibilityFixes(UpgradeContext context, Map<Path, D
hasIssues = true;
}
+ // Fix duplicate plugins
+ if (fixDuplicatePlugins(context, pomDocument)) {
+ hasIssues = true;
+ }
+
// Fix unsupported expressions in repository URLs
if (fixUnsupportedRepositoryExpressions(context, pomDocument))
{
hasIssues = true;
}
+ // Fix incorrect parent relative paths
+ if (fixIncorrectParentRelativePaths(context, pomDocument,
pomPath, pomMap)) {
+ hasIssues = true;
+ }
+
if (hasIssues) {
fixedCount++;
context.logger.info(" ✓ Maven 4 compatibility issues
fixed");
@@ -1845,40 +1855,66 @@ protected boolean
fixUnsupportedRepositoryExpressions(UpgradeContext context, Do
*/
protected boolean fixRepositoryExpressions(
UpgradeContext context, Element repositoriesElement, Namespace
namespace) {
+ if (repositoriesElement == null) {
+ return false;
+ }
+
boolean fixed = false;
+ String elementType =
repositoriesElement.getName().equals("repositories") ? "repository" :
"pluginRepository";
+ List<Element> repositories =
repositoriesElement.getChildren(elementType, namespace);
+
+ for (Element repository : repositories) {
+ Element urlElement = repository.getChild("url", namespace);
+ if (urlElement != null) {
+ String url = urlElement.getTextTrim();
+ if (url.contains("${")
+ && !url.contains("${project.basedir}")
+ && !url.contains("${project.rootDirectory}")) {
+ String repositoryId = getChildText(repository, "id",
namespace);
+ context.logger.warn(" • Found unsupported expression
in " + elementType + " URL (id: "
+ + repositoryId + "): " + url);
+ context.logger.warn(
+ " Maven 4 only supports ${project.basedir}
and ${project.rootDirectory} expressions in repository URLs");
+
+ // Comment out the problematic repository
+ org.jdom2.Comment comment = new org.jdom2.Comment(
+ " Repository disabled due to unsupported
expression in URL: " + url + " ");
+ Element parent = repository.getParentElement();
+ parent.addContent(parent.indexOf(repository), comment);
+ removeElementWithFormatting(repository);
+
+ context.logger.info(" • Commented out " + elementType
+ + " with unsupported URL expression (id: " +
repositoryId + ")");
+ fixed = true;
+ }
+ }
+ }
- if (repositoriesElement != null) {
- String elementType =
- repositoriesElement.getName().equals("repositories") ?
"repository" : "pluginRepository";
- List<Element> repositories =
repositoriesElement.getChildren(elementType, namespace);
-
- for (Element repository : repositories) {
- Element urlElement = repository.getChild("url", namespace);
- if (urlElement != null) {
- String url = urlElement.getTextTrim();
- if (url.contains("${")
- && !url.contains("${project.basedir}")
- && !url.contains("${project.rootDirectory}")) {
- // Replace unsupported expressions with a comment
- String repositoryId = getChildText(repository, "id",
namespace);
- context.logger.warn(" • Found unsupported
expression in " + elementType + " URL (id: "
- + repositoryId + "): " + url);
- context.logger.warn(
- " Maven 4 only supports
${project.basedir} and ${project.rootDirectory} expressions in repository
URLs");
-
- // For now, we'll comment out the problematic
repository
- // In a real implementation, you might want to ask the
user how to handle this
- org.jdom2.Comment comment = new org.jdom2.Comment(
- " Repository disabled due to unsupported
expression in URL: " + url + " ");
- Element parent = repository.getParentElement();
- int index = parent.indexOf(repository);
- parent.addContent(index, comment);
- removeElementWithFormatting(repository);
-
- context.logger.info(" • Commented out " +
elementType
- + " with unsupported URL expression (id: " +
repositoryId + ")");
- fixed = true;
- }
+ return fixed;
+ }
+
+ /**
+ * Fixes duplicate plugin declarations.
+ * Maven 4 enforces stricter validation for duplicate plugins in the same
section.
+ */
+ protected boolean fixDuplicatePlugins(UpgradeContext context, Document
pomDocument) {
+ boolean fixed = false;
+ Element root = pomDocument.getRootElement();
+ Namespace namespace = root.getNamespace();
+
+ // Check main build plugins
+ Element buildElement = root.getChild("build", namespace);
+ if (buildElement != null) {
+ fixed |= fixPluginsInBuildElement(context, buildElement,
namespace, "build");
+ }
+
+ // Check profile plugins
+ Element profilesElement = root.getChild("profiles", namespace);
+ if (profilesElement != null) {
+ for (Element profileElement :
profilesElement.getChildren("profile", namespace)) {
+ Element profileBuildElement = profileElement.getChild("build",
namespace);
+ if (profileBuildElement != null) {
+ fixed |= fixPluginsInBuildElement(context,
profileBuildElement, namespace, "profile build");
}
}
}
@@ -1886,6 +1922,168 @@ protected boolean fixRepositoryExpressions(
return fixed;
}
+ /**
+ * Fixes duplicate plugins in a build element (both plugins and
pluginManagement).
+ */
+ protected boolean fixPluginsInBuildElement(
+ UpgradeContext context, Element buildElement, Namespace namespace,
String prefix) {
+ boolean fixed = false;
+
+ Element pluginsElement = buildElement.getChild("plugins", namespace);
+ if (pluginsElement != null) {
+ fixed |= fixDuplicatePluginsInSection(context, pluginsElement,
namespace, prefix + "/plugins");
+ }
+
+ Element pluginManagementElement =
buildElement.getChild("pluginManagement", namespace);
+ if (pluginManagementElement != null) {
+ Element managedPluginsElement =
pluginManagementElement.getChild("plugins", namespace);
+ if (managedPluginsElement != null) {
+ fixed |= fixDuplicatePluginsInSection(
+ context, managedPluginsElement, namespace, prefix +
"/pluginManagement/plugins");
+ }
+ }
+
+ return fixed;
+ }
+
+ /**
+ * Fixes duplicate plugins within a specific plugins section.
+ */
+ protected boolean fixDuplicatePluginsInSection(
+ UpgradeContext context, Element pluginsElement, Namespace
namespace, String sectionName) {
+ boolean fixed = false;
+ List<Element> plugins = pluginsElement.getChildren("plugin",
namespace);
+ Map<String, Element> seenPlugins = new HashMap<>();
+ List<Element> toRemove = new ArrayList<>();
+
+ for (Element plugin : plugins) {
+ String groupId = getChildText(plugin, "groupId", namespace);
+ String artifactId = getChildText(plugin, "artifactId", namespace);
+
+ // Default groupId for Maven plugins
+ if (groupId == null && artifactId != null &&
artifactId.startsWith("maven-")) {
+ groupId = "org.apache.maven.plugins";
+ }
+
+ if (groupId != null && artifactId != null) {
+ // Create a key for uniqueness check (groupId:artifactId)
+ String key = groupId + ":" + artifactId;
+
+ if (seenPlugins.containsKey(key)) {
+ // Found duplicate - remove it
+ toRemove.add(plugin);
+ context.logger.info(" • Removed duplicate plugin: " +
key + " in " + sectionName);
+ fixed = true;
+ } else {
+ seenPlugins.put(key, plugin);
+ }
+ }
+ }
+
+ // Remove duplicates while preserving formatting
+ for (Element duplicate : toRemove) {
+ removeElementWithFormatting(duplicate);
+ }
+
+ return fixed;
+ }
+
+ /**
+ * Fixes incorrect parent relative paths by searching for the correct
parent POM in the project structure.
+ */
+ protected boolean fixIncorrectParentRelativePaths(
+ UpgradeContext context, Document pomDocument, Path pomPath,
Map<Path, Document> pomMap) {
+ Element root = pomDocument.getRootElement();
+ Namespace namespace = root.getNamespace();
+ Element parentElement = root.getChild("parent", namespace);
+
+ if (parentElement == null) {
+ return false;
+ }
+
+ // Get current relativePath (default is "../pom.xml")
+ Element relativePathElement = parentElement.getChild("relativePath",
namespace);
+ String currentRelativePath = relativePathElement != null ?
relativePathElement.getTextTrim() : "../pom.xml";
+ if (currentRelativePath == null ||
currentRelativePath.trim().isEmpty()) {
+ currentRelativePath = "../pom.xml";
+ }
+
+ // Check if current path is valid
+ Path resolvedParentPath = resolveParentPomPath(pomPath,
currentRelativePath);
+ if (resolvedParentPath != null && Files.exists(resolvedParentPath)) {
+ return false; // Path is correct
+ }
+
+ // Find correct parent in pomMap
+ String parentGroupId = getChildText(parentElement, "groupId",
namespace);
+ String parentArtifactId = getChildText(parentElement, "artifactId",
namespace);
+ String parentVersion = getChildText(parentElement, "version",
namespace);
+
+ if (parentGroupId == null || parentArtifactId == null) {
+ return false;
+ }
+
+ Path correctParentPath = findParentPomInMap(parentGroupId,
parentArtifactId, parentVersion, pomMap);
+ if (correctParentPath == null) {
+ return false;
+ }
+
+ String correctRelativePath = calculateRelativePath(pomPath,
correctParentPath);
+ if (correctRelativePath == null ||
correctRelativePath.equals(currentRelativePath)) {
+ return false;
+ }
+
+ // Update relativePath element
+ if (relativePathElement == null) {
+ relativePathElement = new Element("relativePath", namespace);
+ Element insertAfter = parentElement.getChild("version", namespace);
+ if (insertAfter == null) {
+ insertAfter = parentElement.getChild("artifactId", namespace);
+ }
+ if (insertAfter != null) {
+ parentElement.addContent(parentElement.indexOf(insertAfter) +
1, relativePathElement);
+ } else {
+ parentElement.addContent(relativePathElement);
+ }
+ }
+
+ relativePathElement.setText(correctRelativePath);
+ context.logger.info(" • Fixed parent relativePath from '" +
currentRelativePath + "' to '"
+ + correctRelativePath + "'");
+ return true;
+ }
+
+ /**
+ * Finds a parent POM in the pomMap by GAV coordinates.
+ */
+ protected Path findParentPomInMap(String groupId, String artifactId,
String version, Map<Path, Document> pomMap) {
+ return pomMap.entrySet().stream()
+ .filter(entry -> {
+ GAV gav = extractGAVWithParentResolution(entry.getValue(),
null);
+ return gav != null
+ && java.util.Objects.equals(gav.groupId, groupId)
+ && java.util.Objects.equals(gav.artifactId,
artifactId)
+ && (version == null ||
java.util.Objects.equals(gav.version, version));
+ })
+ .map(Map.Entry::getKey)
+ .findFirst()
+ .orElse(null);
+ }
+
+ /**
+ * Calculates the relative path from a child POM to a parent POM.
+ */
+ protected String calculateRelativePath(Path childPomPath, Path
parentPomPath) {
+ try {
+ Path childDir = childPomPath.getParent();
+ return childDir != null
+ ?
childDir.relativize(parentPomPath).toString().replace('\\', '/')
+ : null;
+ } catch (Exception e) {
+ return null;
+ }
+ }
+
/**
* Recursively finds all elements with a specific attribute value.
*/
diff --git
a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/Maven4CompatibilityFixesTest.java
b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/Maven4CompatibilityFixesTest.java
index 73e2426c74..1957a5864b 100644
---
a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/Maven4CompatibilityFixesTest.java
+++
b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/Maven4CompatibilityFixesTest.java
@@ -19,6 +19,7 @@
package org.apache.maven.cling.invoker.mvnup.goals;
import java.io.StringReader;
+import java.util.List;
import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
import org.jdom2.Document;
@@ -28,6 +29,7 @@
import org.mockito.Mockito;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
@@ -205,6 +207,117 @@ void testDefaultBehavior() throws Exception {
assertTrue(defaultFixModel, "Default behavior should enable
--fix-model");
}
+ @Test
+ void testFixDuplicatePlugins() throws Exception {
+ String pomXml =
+ """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>com.example</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.1</version>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.9.0</version>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ SAXBuilder builder = new SAXBuilder();
+ Document document = builder.build(new StringReader(pomXml));
+
+ TestableBaseUpgradeGoal goal = new TestableBaseUpgradeGoal();
+ UpgradeContext context = Mockito.mock(UpgradeContext.class);
+ context.logger = Mockito.mock(org.apache.maven.api.cli.Logger.class);
+
+ boolean fixed = goal.fixDuplicatePlugins(context, document);
+
+ assertTrue(fixed, "Should have fixed duplicate plugins");
+
+ // Verify only one plugin remains
+ Element buildElement = document.getRootElement()
+ .getChild("build", document.getRootElement().getNamespace());
+ Element pluginsElement = buildElement.getChild("plugins",
buildElement.getNamespace());
+ List<Element> plugins = pluginsElement.getChildren("plugin",
pluginsElement.getNamespace());
+
+ assertEquals(1, plugins.size(), "Should have only one plugin after
removing duplicates");
+ }
+
+ @Test
+ void testFixIncorrectParentRelativePaths() throws Exception {
+ String pomXml =
+ """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>com.example</groupId>
+ <artifactId>parent</artifactId>
+ <version>1.0.0</version>
+ <relativePath>../wrong-path/pom.xml</relativePath>
+ </parent>
+ <artifactId>child</artifactId>
+ </project>
+ """;
+
+ String parentPomXml =
+ """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>com.example</groupId>
+ <artifactId>parent</artifactId>
+ <version>1.0.0</version>
+ <packaging>pom</packaging>
+ </project>
+ """;
+
+ SAXBuilder builder = new SAXBuilder();
+ Document childDocument = builder.build(new StringReader(pomXml));
+ Document parentDocument = builder.build(new
StringReader(parentPomXml));
+
+ // Create mock pomMap with parent POM
+ java.util.Map<java.nio.file.Path, Document> pomMap = new
java.util.HashMap<>();
+ java.nio.file.Path childPath =
java.nio.file.Paths.get("child/pom.xml");
+ java.nio.file.Path parentPath = java.nio.file.Paths.get("pom.xml");
+ pomMap.put(childPath, childDocument);
+ pomMap.put(parentPath, parentDocument);
+
+ TestableBaseUpgradeGoal goal = new TestableBaseUpgradeGoal();
+ UpgradeContext context = Mockito.mock(UpgradeContext.class);
+ context.logger = Mockito.mock(org.apache.maven.api.cli.Logger.class);
+
+ boolean fixed = goal.fixIncorrectParentRelativePaths(context,
childDocument, childPath, pomMap);
+
+ assertTrue(fixed, "Should have fixed incorrect parent relative path");
+
+ // Verify the relativePath was updated
+ Element parentElement = childDocument
+ .getRootElement()
+ .getChild("parent",
childDocument.getRootElement().getNamespace());
+ Element relativePathElement = parentElement.getChild("relativePath",
parentElement.getNamespace());
+
+ assertNotNull(relativePathElement, "relativePath element should
exist");
+ assertEquals("../pom.xml", relativePathElement.getTextTrim(),
"relativePath should be corrected");
+ }
+
/**
* Testable subclass that exposes protected methods for testing.
*/
@@ -224,6 +337,20 @@ public boolean
fixUnsupportedCombineSelfAttributes(UpgradeContext context, Docum
return super.fixUnsupportedCombineSelfAttributes(context,
pomDocument);
}
+ @Override
+ public boolean fixDuplicatePlugins(UpgradeContext context, Document
pomDocument) {
+ return super.fixDuplicatePlugins(context, pomDocument);
+ }
+
+ @Override
+ public boolean fixIncorrectParentRelativePaths(
+ UpgradeContext context,
+ Document pomDocument,
+ java.nio.file.Path pomPath,
+ java.util.Map<java.nio.file.Path, Document> pomMap) {
+ return super.fixIncorrectParentRelativePaths(context, pomDocument,
pomPath, pomMap);
+ }
+
// Test helper methods to expose internal logic
public String testDoUpgradeLogic(
UpgradeContext context,
