Skip to content

[Forward port] mvnup: skip dedup inside plugin <configuration> elements #12614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gnodet merged 2 commits into from
Jul 29, 2026
+317 −4
Merged

[Forward port] mvnup: skip dedup inside plugin <configuration> elements #12614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a9c4f26
Use resource filtering for mng-12534 IT plugin mavenVersion
gnodet Jul 29, 2026
d214fa3
mvnup: skip dedup inside plugin <configuration> elements
gnodet Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions ...li/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand All @@ -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
Expand Down
290 changes: 290 additions & 0 deletions ...rc/test/java/org/apache/maven/cling/invoker/mvnup/goals/DuplicateElementStrategyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion ...mng-12534-after-annotation/plugin/pom.xml → ...mng-12534-after-annotation/plugin/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

<properties>
<javaVersion>17</javaVersion>
<mavenVersion>4.1.0-SNAPSHOT</mavenVersion>
<mavenVersion>${maven-version}</mavenVersion>
</properties>

<dependencies>
Expand Down
Loading