You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/07/29 10:01:30 UTC
(maven) 01/01: [MNG-8507] mvnup: skip dedup inside plugin elements
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch fix/mvnup-dedup-configuration
in repository https://gitbox.apache.org/repos/asf/maven.git
commit bda05be036fbde344c64ccf55017404a4f583261
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Wed Jul 29 12:01:17 2026 +0200
[MNG-8507] mvnup: skip dedup inside plugin <configuration> elements
DuplicateElementStrategy removes XML elements that share the same tag
name within a parent, keeping only the last occurrence (last-wins).
This is correct for POM schema elements (e.g. two <version> inside a
<dependency>), but breaks plugin <configuration> sections whose XML
is free-form and plugin-specific.
For example, in accumulo-access the compiler plugin declares:
<compilerArgs>
<arg>-Xlint:all</arg>
<arg>-Xlint:-processing</arg>
<arg>-Xmaxwarns</arg>
<arg>5</arg>
</compilerArgs>
The strategy treated the first three <arg> elements as "duplicates"
of the last one and removed them, leaving only <arg>5</arg>. This
caused `error: invalid flag: 5` during compilation.
The fix stops recursion at <configuration> boundaries. Everything
above <configuration> is well-defined POM schema where deduplication
is safe; everything inside is plugin-owned and must not be touched.
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../mvnup/goals/DuplicateElementStrategy.java | 29 ++-
.../mvnup/goals/DuplicateElementStrategyTest.java | 290 +++++++++++++++++++++
2 files changed, 316 insertions(+), 3 deletions(-)
diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java
index d85c745f18..f66c3c959b 100644
--- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java
+++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java
@@ -43,13 +43,20 @@
* each POM element's children and removes duplicates, keeping only the last
* occurrence of each element name.
*
- * <p>This strategy only targets "scalar" elements — elements that should appear
- * at most once within their parent according to the Maven POM schema. Elements
- * inside list containers (e.g., {@code <dependency>} inside {@code <dependencies>},
+ * <p>This strategy only targets "scalar" elements at well-known POM schema
+ * positions — elements that should appear at most once within their parent
+ * according to the Maven POM schema. Elements inside list containers
+ * (e.g., {@code <dependency>} inside {@code <dependencies>},
* {@code <plugin>} inside {@code <plugins>}) are not affected by this strategy
* since duplicate dependencies and plugins are handled by
* {@link DeduplicateDependenciesStrategy}.
*
+ * <p>Plugin {@code <configuration>} elements are skipped entirely, because they
+ * contain free-form, plugin-specific XML whose schema is not known to this tool.
+ * Treating same-named children as duplicates in configuration sections (e.g.,
+ * multiple {@code <arg>} entries inside {@code <compilerArgs>}) would silently
+ * remove valid list entries and break builds.
+ *
* @see <a href="https://github.com/apache/maven/issues/12530">#12530</a>
*/
@Named
@@ -134,6 +141,14 @@ protected UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomM
return new UpgradeResult(processedPoms, modifiedPoms, errorPoms);
}
+ /**
+ * Parent element names whose contents are free-form, plugin-specific XML.
+ * Deduplication is skipped entirely for these elements and their descendants,
+ * because same-named children (e.g., multiple {@code <arg>} in
+ * {@code <compilerArgs>}) are list entries, not schema-level duplicates.
+ */
+ static final Set<String> FREEFORM_ELEMENTS = Set.of("configuration");
+
/**
* Recursively scans an element's children for duplicates and removes them.
* Uses last-wins semantics (consistent with Maven 3's behavior).
@@ -145,6 +160,14 @@ protected UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomM
private boolean removeDuplicateElements(Element element, UpgradeContext context) {
boolean removed = false;
+ // Skip free-form plugin configuration elements — their XML schema is
+ // plugin-specific and unknown to this tool. Treating same-named children
+ // as duplicates here would silently remove valid list entries (e.g.,
+ // <arg> elements inside <compilerArgs>) and break builds.
+ if (FREEFORM_ELEMENTS.contains(element.name())) {
+ return false;
+ }
+
// Skip list container elements — their children naturally repeat
if (LIST_CONTAINER_ELEMENTS.contains(element.name())) {
// Still recurse into each child to check for duplicates within them
diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java
index 602cc06269..4ae2d795d9 100644
--- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java
+++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java
@@ -457,6 +457,296 @@ void shouldNotRemoveModulesInList() {
}
}
+ @Nested
+ @DisplayName("Plugin Configuration Skipping")
+ class PluginConfigurationSkippingTests {
+
+ @Test
+ @DisplayName("should not remove multiple arg elements inside compilerArgs in configuration")
+ void shouldNotRemoveCompilerArgsInConfiguration() {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs>
+ <arg>-Xlint:all</arg>
+ <arg>-Xlint:-processing</arg>
+ <arg>-Xmaxwarns</arg>
+ <arg>5</arg>
+ </compilerArgs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Strategy should succeed");
+ assertEquals(0, result.modifiedCount(), "Should not have modified any POM");
+
+ String xml = DomUtils.toXml(document);
+ assertTrue(xml.contains("<arg>-Xlint:all</arg>"), "Should keep -Xlint:all arg");
+ assertTrue(xml.contains("<arg>-Xlint:-processing</arg>"), "Should keep -Xlint:-processing arg");
+ assertTrue(xml.contains("<arg>-Xmaxwarns</arg>"), "Should keep -Xmaxwarns arg");
+ assertTrue(xml.contains("<arg>5</arg>"), "Should keep 5 arg");
+ }
+
+ @Test
+ @DisplayName("should not remove elements inside execution configuration")
+ void shouldNotRemoveElementsInsideExecutionConfiguration() {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>exec-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <configuration>
+ <arguments>
+ <argument>-classpath</argument>
+ <argument>${project.build.outputDirectory}</argument>
+ <argument>com.example.Main</argument>
+ </arguments>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Strategy should succeed");
+ assertEquals(0, result.modifiedCount(), "Should not have modified any POM");
+
+ String xml = DomUtils.toXml(document);
+ // Count argument occurrences
+ int count = xml.split("<argument>", -1).length - 1;
+ assertEquals(3, count, "Should still have 3 argument elements");
+ }
+
+ @Test
+ @DisplayName("should not remove checkstyle module elements inside configuration")
+ void shouldNotRemoveCheckstyleModulesInConfiguration() {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <artifactId>maven-checkstyle-plugin</artifactId>
+ <configuration>
+ <checkstyleRules>
+ <module name="Checker">
+ <property name="charset" value="UTF-8" />
+ <property name="severity" value="warning" />
+ <module name="FileTabCharacter" />
+ <module name="TreeWalker">
+ <module name="AvoidStarImport" />
+ <module name="NeedBraces" />
+ </module>
+ </module>
+ </checkstyleRules>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Strategy should succeed");
+ assertEquals(0, result.modifiedCount(), "Should not have modified any POM");
+
+ String xml = DomUtils.toXml(document);
+ // Count module occurrences (Checker, FileTabCharacter, TreeWalker, AvoidStarImport, NeedBraces = 5)
+ int moduleCount = xml.split("<module ", -1).length - 1;
+ assertEquals(5, moduleCount, "Should still have 5 module elements");
+ // Count property occurrences (charset, severity = 2)
+ int propertyCount = xml.split("<property ", -1).length - 1;
+ assertEquals(2, propertyCount, "Should still have 2 property elements");
+ }
+
+ @Test
+ @DisplayName("should still remove POM-level duplicates when configuration is present")
+ void shouldStillRemovePomDuplicatesWithConfiguration() {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>old-name</artifactId>
+ <artifactId>new-name</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs>
+ <arg>-Xlint:all</arg>
+ <arg>-Xmaxwarns</arg>
+ <arg>5</arg>
+ </compilerArgs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Strategy should succeed");
+ assertEquals(1, result.modifiedCount(), "Should have modified 1 POM");
+
+ String xml = DomUtils.toXml(document);
+ // POM-level duplicate should be removed
+ assertTrue(xml.contains("<artifactId>new-name</artifactId>"), "Should keep last artifactId");
+ assertFalse(xml.contains("<artifactId>old-name</artifactId>"), "Should remove first artifactId");
+ // Configuration args should be preserved
+ assertTrue(xml.contains("<arg>-Xlint:all</arg>"), "Should keep -Xlint:all arg");
+ assertTrue(xml.contains("<arg>-Xmaxwarns</arg>"), "Should keep -Xmaxwarns arg");
+ assertTrue(xml.contains("<arg>5</arg>"), "Should keep 5 arg");
+ }
+
+ @Test
+ @DisplayName("should not remove exclude elements inside inputExcludes in configuration")
+ void shouldNotRemoveExcludesInConfiguration() {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <artifactId>apache-rat-plugin</artifactId>
+ <configuration>
+ <inputExcludes>
+ <exclude>.github/**</exclude>
+ <exclude>src/main/antlr4/Abnf.g4</exclude>
+ </inputExcludes>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Strategy should succeed");
+ assertEquals(0, result.modifiedCount(), "Should not have modified any POM");
+
+ String xml = DomUtils.toXml(document);
+ assertTrue(xml.contains("<exclude>.github/**</exclude>"), "Should keep first exclude");
+ assertTrue(xml.contains("<exclude>src/main/antlr4/Abnf.g4</exclude>"), "Should keep second exclude");
+ }
+
+ @Test
+ @DisplayName("should not remove pluginExecution elements inside lifecycleMappingMetadata in configuration")
+ void shouldNotRemovePluginExecutionsInConfiguration() {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.eclipse.m2e</groupId>
+ <artifactId>lifecycle-mapping</artifactId>
+ <configuration>
+ <lifecycleMappingMetadata>
+ <pluginExecutions>
+ <pluginExecution>
+ <pluginExecutionFilter>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-dependency-plugin</artifactId>
+ </pluginExecutionFilter>
+ </pluginExecution>
+ <pluginExecution>
+ <pluginExecutionFilter>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>exec-maven-plugin</artifactId>
+ </pluginExecutionFilter>
+ </pluginExecution>
+ </pluginExecutions>
+ </lifecycleMappingMetadata>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Strategy should succeed");
+ assertEquals(0, result.modifiedCount(), "Should not have modified any POM");
+
+ String xml = DomUtils.toXml(document);
+ int count = xml.split("<pluginExecution>", -1).length - 1;
+ assertEquals(2, count, "Should still have 2 pluginExecution elements");
+ }
+ }
+
@Nested
@DisplayName("Strategy Description")
class StrategyDescriptionTests {