[Backport 4.0.x] Fix mvnup spurious pluginManagement injection for remote parent plugins#12351
Open
gnodet wants to merge 2 commits into
Open
[Backport 4.0.x] Fix mvnup spurious pluginManagement injection for remote parent plugins#12351gnodet wants to merge 2 commits into
gnodet wants to merge 2 commits into
Conversation
…herited from remote parent POM When a project inherits from a remote parent POM, plugins declared only in the remote parent's build/plugins or pluginManagement should not have local pluginManagement entries injected by mvnup. The project does not control the remote parent, so overriding its plugin versions locally is spurious. The fix collects all plugin keys declared in local POMs (build/plugins and build/pluginManagement/plugins) and skips plugins in the effective model analysis that are not locally declared — they come from remote parents that the project does not control. Plugins that ARE declared locally (even without a version, inheriting from a remote parent's pluginManagement) are still eligible for pluginManagement injection, since the project explicitly uses them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of skipping plugins inherited from remote parent POMs, mvnup now adds pluginManagement overrides with a comment "Override version inherited from parent". This ensures Maven 4 incompatible plugin versions get upgraded even when they come from a parent POM the project does not control. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Backport intended to prevent mvnup from injecting unwanted plugin overrides for plugins that originate only from remote parent POMs, by tracking which plugins are declared in the local project and adjusting effective-model-based upgrades accordingly.
Changes:
- Added collection of locally declared plugin keys (
collectLocallyDeclaredPluginKeys) and threaded that information into plugin-management/direct-override insertion paths. - Added logic to insert an XML comment when an override is deemed to come from a remote parent.
- Updated and added tests around inherited plugin handling and locally declared plugins without explicit versions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java |
Collects local plugin keys and uses them during effective-model-driven insertion of pluginManagement/build/plugins entries. |
impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java |
Updates inherited-plugin tests and adds a test for locally declared plugins without versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
731
to
735
| for (String pluginKey : pluginKeys) { | ||
| PluginUpgrade upgrade = pluginUpgrades.get(pluginKey); | ||
| if (upgrade != null) { | ||
| // Check if plugin is already managed | ||
| if (!isPluginAlreadyManagedInElement(managedPluginsElement, upgrade)) { |
Comment on lines
807
to
812
| for (String pluginKey : pluginKeys) { | ||
| PluginUpgrade upgrade = pluginUpgrades.get(pluginKey); | ||
| if (upgrade != null) { | ||
| if (!isPluginAlreadyManagedInElement(pluginsElement, upgrade)) { | ||
| DomUtils.createPlugin( | ||
| Element plugin = DomUtils.createPlugin( | ||
| pluginsElement, upgrade.groupId(), upgrade.artifactId(), upgrade.minVersion()); |
Comment on lines
+136
to
+137
| // Collect locally declared plugin keys so we can add comments for remote-parent overrides | ||
| Set<String> localPluginKeys = collectLocallyDeclaredPluginKeys(pomMap); |
Comment on lines
+776
to
+777
| @DisplayName("should inject pluginManagement with comment for plugins inherited from remote parent") | ||
| void shouldInjectPluginManagementWithCommentForRemoteParentPlugins() throws Exception { |
Comment on lines
809
to
+813
| assertTrue(result.success(), "Strategy should succeed"); | ||
| assertTrue(result.modifiedCount() > 0, "Should have added plugin management for inherited plugins"); | ||
| assertTrue(result.modifiedCount() > 0, "Should have modified POM for remote parent plugin upgrade"); | ||
|
|
||
| String xml = DomUtils.toXml(document); | ||
| assertTrue( |
Comment on lines
+835
to
+836
| @DisplayName("should override remote parent plugin via pluginManagement with comment, not direct build/plugins") | ||
| void shouldOverrideRemoteParentPluginViaPluginManagementWithComment() throws Exception { |
Comment on lines
+878
to
+881
| String xml = DomUtils.toXml(document); | ||
| assertTrue( | ||
| xml.contains("Override version inherited from parent"), | ||
| "Should add comment explaining the override"); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backport of #12350 to maven-4.0.x.
<pluginManagement>entries for plugins inherited from remote parent POMs that the project does not controlFixes #11606
Test plan
🤖 Generated with Claude Code