[Backport 4.0.x] Add mvnup SourceStrategy for migrating to <source> elements by gnodet · Pull Request #12357 · apache/maven

gnodet · 2026-06-23T21:01:19Z

Summary

Backport of #12355 to maven-4.0.x.

Add SourceStrategy to mvnup that migrates legacy source configuration to Maven 4.1.0+ <source> elements
Handles compiler properties (maven.compiler.release, maven.compiler.source/target), compiler plugin configuration, source/test directories, and resources
Applicable when --model-version 4.1.0+ or --all is specified

Test plan

26 unit tests covering all migration phases cherry-picked cleanly
Tests pass on maven-4.0.x

🤖 Generated with Claude Code

@priority

Adds a new mvnup upgrade strategy that migrates legacy source configuration to Maven 4.1.0+ <source> elements. Handles four migration phases: - Compiler properties (maven.compiler.release, source/target) to <source><targetVersion> - Compiler plugin configuration (<release>, <source>/<target>) to <source><targetVersion> - Custom source/test directories to <source><directory> - Resource/testResource sections to <source> with lang=resources Applies when --model-version is 4.1.0+ or --all is set. Runs at @priority(20), after all other strategies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Check groupId in findCompilerPlugin (not just artifactId) - Clean up compiler plugin config when properties already provide targetVersion (previously left stale release/source/target elements) - Clean up empty pluginManagement after plugin removal - Clean up empty build element after all children removed - Add test for pluginManagement compiler plugin migration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

desruisseaux · 2026-06-24T18:46:13Z

+                }
+            });
+        });
+


It is a small detail, but the lambda below seems almost identical to the lambda above. Maybe it could be written only once in a separated function.

Fully automatic review from Claude Code

Done — extracted a copyPatternList(source, target, containerName, elementName) static method that both includes and excludes now delegate to.

desruisseaux · 2026-06-24T18:48:03Z

+        return true;
+    }
+
+    private void copyIncludesExcludes(Element source, Element target) {


Maybe the method could be static (I'm not sure, I didn't opened the file in an IDE for making sure that I didn't missed a field).

Fully automatic review from Claude Code

Good catch — copyIncludesExcludes (and the new copyPatternList) are now both static.

desruisseaux

I did not verified in details, but it seems nice by a quick look and the purpose is nice too.

Address review feedback from desruisseaux: extract shared logic from copyIncludesExcludes into a static copyPatternList method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Backports a new mvnup upgrade strategy to migrate legacy compiler/source/resource configuration into Maven 4.1.0+ <build><sources><source> model, gated by --model-version 4.1.0+ or --all.

Changes:

Adds SourceStrategy (@Priority(20)) to extract target Java version from compiler properties / maven-compiler-plugin config and migrate source/test directories and (test)resources into <source> elements.
Cleans up migrated legacy elements/containers (properties, plugin config, sourceDirectory/testSourceDirectory, resources/testResources) and removes empty <build> when applicable.
Adds a comprehensive JUnit test suite covering applicability and all migration phases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/SourceStrategy.java`	Implements the new migration strategy that writes Maven 4.1+ `<source>` entries and removes legacy configuration.
`impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/SourceStrategyTest.java`	Adds unit tests validating applicability and each migration phase (compiler props/plugin, directories, resources, version filtering, combined scenarios).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gnodet · 2026-06-25T04:47:39Z

+        if (targetVersion != null) {
+            Element sourcesElement = ensureSourcesElement(root);
+            Element sourceElement = DomUtils.insertNewElement("source", sourcesElement);
+            DomUtils.insertContentElement(sourceElement, "targetVersion", targetVersion);
+            context.detail("Set targetVersion: " + targetVersion);
+            hasChanges = true;
+        }


Fixed. Now uses findOrCreateMainJavaSource(sourcesElement) instead of DomUtils.insertNewElement(\"source\", sourcesElement) when adding targetVersion. This reuses an existing main/java <source> element (e.g. one with a custom <directory>) instead of creating a duplicate. Added a test that verifies a POM with an existing <source><directory>...</directory></source> gets targetVersion added to the same element.

— Claude Opus 4.6, on behalf of Guillaume Nodet

gnodet · 2026-06-25T04:47:47Z

+        String targetVersion = extractTargetVersionFromProperties(context, root);
+
+        if (targetVersion == null) {
+            targetVersion = extractTargetVersionFromCompilerPlugin(context, root);
+        } else {
+            cleanupCompilerPluginConfig(context, root);
+        }


Fixed. cleanupPluginSectionConfig() now receives the migrated value and only removes <release>, <source>, or <target> from the compiler plugin configuration when the value matches what was migrated to properties. If the plugin config intentionally differs (e.g. property has release=17 but plugin overrides with release=21), it is left in place. Added a test verifying that differing plugin config is preserved.

— Claude Opus 4.6, on behalf of Guillaume Nodet

- Reuse existing main/java <source> element when adding targetVersion instead of always creating a new one (avoids duplicate source elements) - Only remove compiler plugin config (release/source/target) when values match the migrated property value (preserves intentional overrides) - Add tests for both edge cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@priority

* Add mvnup SourceStrategy for migrating to <source> elements Adds a new mvnup upgrade strategy that migrates legacy source configuration to Maven 4.1.0+ <source> elements. Handles four migration phases: - Compiler properties (maven.compiler.release, source/target) to <source><targetVersion> - Compiler plugin configuration (<release>, <source>/<target>) to <source><targetVersion> - Custom source/test directories to <source><directory> - Resource/testResource sections to <source> with lang=resources Applies when --model-version is 4.1.0+ or --all is set. Runs at @priority(20), after all other strategies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Use domtrip path() API in SourceStrategy tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix review issues in SourceStrategy - Check groupId in findCompilerPlugin (not just artifactId) - Clean up compiler plugin config when properties already provide targetVersion (previously left stale release/source/target elements) - Clean up empty pluginManagement after plugin removal - Clean up empty build element after all children removed - Add test for pluginManagement compiler plugin migration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Extract copyPatternList to deduplicate includes/excludes handling Address review feedback from desruisseaux: extract shared logic from copyIncludesExcludes into a static copyPatternList method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address Copilot review comments on SourceStrategy (port from #12357) - Reuse existing main/java <source> element when adding targetVersion instead of always creating a new one (avoids duplicate source elements) - Only remove compiler plugin config (release/source/target) when values match the migrated property value (preserves intentional overrides) - Add tests for both edge cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

gnodet and others added 2 commits June 23, 2026 23:00

Use domtrip path() API in SourceStrategy tests

3db084d

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

gnodet added this to the 4.0.0-rc-6 milestone Jun 23, 2026

gnodet added enhancement New feature or request mvn40 backport labels Jun 23, 2026

desruisseaux reviewed Jun 24, 2026

View reviewed changes

desruisseaux approved these changes Jun 24, 2026

View reviewed changes

gnodet requested a review from Copilot June 24, 2026 21:25

Copilot started reviewing on behalf of gnodet June 24, 2026 21:25 View session

Extract copyPatternList to deduplicate includes/excludes handling

2f4a615

Address review feedback from desruisseaux: extract shared logic from copyIncludesExcludes into a static copyPatternList method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI reviewed Jun 24, 2026

View reviewed changes

gnodet merged commit 2a721dd into maven-4.0.x Jun 25, 2026
23 checks passed

gnodet deleted the backport/mvnup-source-strategy branch June 25, 2026 18:02

github-actions Bot assigned gnodet Jun 25, 2026

Navigation Menu

[Backport 4.0.x] Add mvnup SourceStrategy for migrating to <source> elements - #12357

[Backport 4.0.x] Add mvnup SourceStrategy for migrating to <source> elements#12357
gnodet merged 5 commits into
maven-4.0.xfrom
backport/mvnup-source-strategy

gnodet commented Jun 23, 2026

Uh oh!

desruisseaux Jun 24, 2026

Uh oh!

gnodet Jun 24, 2026

Uh oh!

desruisseaux Jun 24, 2026

Uh oh!

gnodet Jun 24, 2026

Uh oh!

desruisseaux left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

gnodet Jun 25, 2026

Uh oh!

gnodet Jun 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

gnodet commented Jun 23, 2026

Summary

Test plan

Uh oh!

desruisseaux Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

gnodet Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

desruisseaux Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

gnodet Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

desruisseaux left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

gnodet Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

gnodet Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants