[Forward port] mvnup: skip dedup inside plugin <configuration> elements - #12614
Conversation
Move the plugin POM to src/test/resources-filtered/ (which already
has <filtering>true</filtering> in the core-it-suite build) and
replace the hardcoded version with ${maven-version}, defined in the
ITs parent POM.
Maven resource filtering resolves the token at build time, so the
version always matches the Maven build under test — including during
the release process, where the release plugin bumps the version.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Clean forward port of #12582. The fix correctly skips deduplication inside plugin <configuration> elements in DuplicateElementStrategy, preventing the tool from silently removing valid list entries like <arg>, <module>, <exclude>. The behavioral distinction between FREEFORM_ELEMENTS (skip entire subtree) and LIST_CONTAINER_ELEMENTS (skip dedup but recurse children) is correct and well-motivated.
The IT fixture move from resources/ to resources-filtered/ properly leverages the existing filtering configuration, replacing the hardcoded 4.1.0-SNAPSHOT with ${maven-version} so the fixture version tracks the build automatically.
The 6 new tests in PluginConfigurationSkippingTests provide excellent coverage of real-world scenarios (compiler args, exec plugin arguments, checkstyle modules/properties, mixed POM-level dedup + configuration preservation, RAT excludes, and m2e lifecycle mapping).
One minor observation: the FREEFORM_ELEMENTS constant is declared mid-class (line 150) while the analogous LIST_CONTAINER_ELEMENTS is at the top (line 73) — grouping them together would improve readability. Not blocking.
(Posting as COMMENT because GitHub does not allow self-approval — this would otherwise be an APPROVE.)
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c1c2ff3 to
d214fa3
Compare
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Re-reviewed after new commit (a9c4f26 — use resource filtering for IT plugin mavenVersion). The change is clean: moves plugin/pom.xml from resources/ to resources-filtered/ to leverage the existing <filtering>true</filtering> configuration, replacing the hardcoded 4.1.0-SNAPSHOT with ${maven-version}. This eliminates a version that would go stale during releases. The main fix (skip dedup inside <configuration>) remains correct. LGTM ✅
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
1 participant
Summary
Forward port of #12582 (
maven-4.0.x→master).${maven-version}) for the mng-12534 IT plugin fixture so the version tracks the build automatically<configuration>elements inDuplicateElementStrategy, fixing builds that use repeated same-named elements as list entries (e.g.<arg>,<module>,<exclude>)Test plan
DuplicateElementStrategyTest.PluginConfigurationSkippingTestspassDuplicateElementStrategyandUpgradeGoalTesttests still passmaven-4.0.xPR mvnup: skip dedup inside plugin <configuration> elements #12582🤖 Generated with Claude Code