This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 10ba35b706 Fix mvnup spurious pluginManagement injection for remote
parent plugins (#12350)
10ba35b706 is described below
commit 10ba35b7063c528a61a3b85db2c4eeb2c99fc854
Author: Guillaume Nodet <[email protected]>
AuthorDate: Thu Jun 25 15:17:40 2026 +0200
Fix mvnup spurious pluginManagement injection for remote parent plugins
(#12350)
* [#11606] Fix mvnup spurious pluginManagement injection for plugins
inherited 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 <[email protected]>
* Override remote parent plugins with comment instead of skipping
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 <[email protected]>
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
---
.../invoker/mvnup/goals/PluginUpgradeStrategy.java | 79 ++++++++++++++---
.../mvnup/goals/PluginUpgradeStrategyTest.java | 98 +++++++++++++++++++---
2 files changed, 153 insertions(+), 24 deletions(-)
diff --git
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
index 3402b78c2f..8df694b2e2 100644
---
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
+++
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
@@ -26,6 +26,7 @@
import java.util.Set;
import java.util.stream.Collectors;
+import eu.maveniverse.domtrip.Comment;
import eu.maveniverse.domtrip.Document;
import eu.maveniverse.domtrip.Editor;
import eu.maveniverse.domtrip.Element;
@@ -132,6 +133,9 @@ public UpgradeResult doApply(UpgradeContext context,
Map<Path, Document> pomMap)
// Phase 2: For each POM, build effective model using the session
and analyze plugins
PluginAnalysisResults analysisResults =
analyzePluginsUsingEffectiveModels(context, pomMap, tempDir);
+ // Collect locally declared plugin keys so we can add comments for
remote-parent overrides
+ Set<String> localPluginKeys =
collectLocallyDeclaredPluginKeys(pomMap);
+
// Phase 3: Add plugin management and direct overrides to the last
local parent in hierarchy
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Path pomPath = entry.getKey();
@@ -151,8 +155,8 @@ public UpgradeResult doApply(UpgradeContext context,
Map<Path, Document> pomMap)
Set<String> pluginsForManagement =
analysisResults.pluginsNeedingManagement().get(pomPath);
if (pluginsForManagement != null &&
!pluginsForManagement.isEmpty()) {
- hasUpgrades |=
-
addPluginManagementForEffectivePlugins(context, pomDocument,
pluginsForManagement);
+ hasUpgrades |= addPluginManagementForEffectivePlugins(
+ context, pomDocument, pluginsForManagement,
localPluginKeys);
context.detail("Added plugin management to " + pomPath
+ " (target parent for "
+ pluginsForManagement.size() + " plugins)");
}
@@ -162,7 +166,8 @@ public UpgradeResult doApply(UpgradeContext context,
Map<Path, Document> pomMap)
Set<String> pluginsForDirectOverride =
analysisResults.pluginsNeedingDirectOverride().get(pomPath);
if (pluginsForDirectOverride != null &&
!pluginsForDirectOverride.isEmpty()) {
- hasUpgrades |= addDirectPluginOverrides(context,
pomDocument, pluginsForDirectOverride);
+ hasUpgrades |= addDirectPluginOverrides(
+ context, pomDocument,
pluginsForDirectOverride, localPluginKeys);
}
if (hasUpgrades) {
@@ -697,7 +702,7 @@ private Path findParentInPomMap(Parent parent, Map<Path,
Document> pomMap) {
* Adds plugin management entries for plugins found through effective
model analysis.
*/
private boolean addPluginManagementForEffectivePlugins(
- UpgradeContext context, Document pomDocument, Set<String>
pluginKeys) {
+ UpgradeContext context, Document pomDocument, Set<String>
pluginKeys, Set<String> localPluginKeys) {
Map<String, PluginUpgrade> pluginUpgrades = getPluginUpgradesAsMap();
boolean hasUpgrades = false;
@@ -728,7 +733,8 @@ private boolean addPluginManagementForEffectivePlugins(
if (upgrade != null) {
// Check if plugin is already managed
if (!isPluginAlreadyManagedInElement(managedPluginsElement,
upgrade)) {
- addPluginManagementEntryFromUpgrade(managedPluginsElement,
upgrade, context);
+ boolean fromRemoteParent =
!localPluginKeys.contains(pluginKey);
+ addPluginManagementEntryFromUpgrade(managedPluginsElement,
upgrade, context, fromRemoteParent);
hasUpgrades = true;
}
}
@@ -762,12 +768,18 @@ private boolean isPluginAlreadyManagedInElement(Element
pluginsElement, PluginUp
* Adds a plugin management entry from a PluginUpgrade.
*/
private void addPluginManagementEntryFromUpgrade(
- Element managedPluginsElement, PluginUpgrade upgrade,
UpgradeContext context) {
- // Create plugin element using DomUtils convenience method for proper
formatting
- DomUtils.createPlugin(managedPluginsElement, upgrade.groupId(),
upgrade.artifactId(), upgrade.minVersion());
-
- context.detail("Added plugin management for " + upgrade.groupId() +
":" + upgrade.artifactId() + " version "
- + upgrade.minVersion() + " (found through effective model
analysis)");
+ Element managedPluginsElement, PluginUpgrade upgrade,
UpgradeContext context, boolean fromRemoteParent) {
+ Element plugin = DomUtils.createPlugin(
+ managedPluginsElement, upgrade.groupId(),
upgrade.artifactId(), upgrade.minVersion());
+
+ if (fromRemoteParent) {
+ managedPluginsElement.insertChildBefore(plugin, Comment.of("
Override version inherited from parent "));
+ context.detail("Added plugin management for " + upgrade.groupId()
+ ":" + upgrade.artifactId() + " version "
+ + upgrade.minVersion() + " (overrides version inherited
from parent)");
+ } else {
+ context.detail("Added plugin management for " + upgrade.groupId()
+ ":" + upgrade.artifactId() + " version "
+ + upgrade.minVersion() + " (found through effective model
analysis)");
+ }
}
/**
@@ -775,7 +787,8 @@ private void addPluginManagementEntryFromUpgrade(
* This is necessary when a parent POM sets an explicit version in its
build/plugins
* that pluginManagement alone cannot override.
*/
- private boolean addDirectPluginOverrides(UpgradeContext context, Document
pomDocument, Set<String> pluginKeys) {
+ private boolean addDirectPluginOverrides(
+ UpgradeContext context, Document pomDocument, Set<String>
pluginKeys, Set<String> localPluginKeys) {
Map<String, PluginUpgrade> pluginUpgrades = getPluginUpgradesAsMap();
boolean hasUpgrades = false;
@@ -795,8 +808,12 @@ private boolean addDirectPluginOverrides(UpgradeContext
context, Document pomDoc
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());
+ if (!localPluginKeys.contains(pluginKey)) {
+ pluginsElement.insertChildBefore(
+ plugin, Comment.of(" Override version
inherited from parent "));
+ }
hasUpgrades = true;
context.detail("Added " + upgrade.groupId() + ":" +
upgrade.artifactId() + " version "
+ upgrade.minVersion()
@@ -808,6 +825,42 @@ private boolean addDirectPluginOverrides(UpgradeContext
context, Document pomDoc
return hasUpgrades;
}
+ private Set<String> collectLocallyDeclaredPluginKeys(Map<Path, Document>
pomMap) {
+ Set<String> localPluginKeys = new HashSet<>();
+ for (Document doc : pomMap.values()) {
+ Element root = doc.root();
+ Element buildElement = root.childElement(BUILD).orElse(null);
+ if (buildElement != null) {
+ Element pluginsElement =
buildElement.childElement(PLUGINS).orElse(null);
+ if (pluginsElement != null) {
+ collectPluginKeysFromElement(pluginsElement,
localPluginKeys);
+ }
+ Element pmElement =
buildElement.childElement(PLUGIN_MANAGEMENT).orElse(null);
+ if (pmElement != null) {
+ Element managedPluginsElement =
+ pmElement.childElement(PLUGINS).orElse(null);
+ if (managedPluginsElement != null) {
+ collectPluginKeysFromElement(managedPluginsElement,
localPluginKeys);
+ }
+ }
+ }
+ }
+ return localPluginKeys;
+ }
+
+ private void collectPluginKeysFromElement(Element pluginsElement,
Set<String> keys) {
+ pluginsElement.childElements(PLUGIN).forEach(pluginElement -> {
+ String groupId = getChildText(pluginElement, GROUP_ID);
+ String artifactId = getChildText(pluginElement, ARTIFACT_ID);
+ if (groupId == null && artifactId != null &&
artifactId.startsWith(MAVEN_PLUGIN_PREFIX)) {
+ groupId = DEFAULT_MAVEN_PLUGIN_GROUP_ID;
+ }
+ if (groupId != null && artifactId != null) {
+ keys.add(groupId + ":" + artifactId);
+ }
+ });
+ }
+
private record PluginAnalysis(Set<String> needsManagement, Set<String>
needsDirectOverride) {}
private record PluginAnalysisResults(
diff --git
a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java
b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java
index a5824e7b9f..9ba2d04812 100644
---
a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java
+++
b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java
@@ -773,11 +773,12 @@ void shouldHaveValidPluginUpgradeDefinitions() throws
Exception {
class InheritedPluginDetectionTests {
@Test
- @DisplayName("should detect inherited plugins from remote parent POM
and add pluginManagement")
- void shouldDetectInheritedPluginsFromRemoteParent() throws Exception {
- // org.apache:apache:23 defines maven-enforcer-plugin:1.4.1 in
pluginManagement.
- // A child POM that inherits from this parent should get
pluginManagement overrides
- // added by mvnup for plugins that need Maven 4 compatibility
upgrades.
+ @DisplayName("should inject pluginManagement with comment for plugins
inherited from remote parent")
+ void shouldInjectPluginManagementWithCommentForRemoteParentPlugins()
throws Exception {
+ // org.apache:apache:23 defines maven-enforcer-plugin in
pluginManagement and
+ // build/plugins. A child POM that does NOT itself declare the
plugin should
+ // still get a pluginManagement override with a comment explaining
it overrides
+ // the parent, so that Maven 4 incompatible plugin versions get
upgraded.
String pomXml = """
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0";>
@@ -806,12 +807,18 @@ void shouldDetectInheritedPluginsFromRemoteParent()
throws Exception {
UpgradeResult result = strategy.doApply(context, pomMap);
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(
+ xml.contains("<pluginManagement>"),
+ "Should inject pluginManagement for plugins from
remote parent");
+ assertTrue(
+ xml.contains("Override version inherited from parent"),
+ "Should add comment explaining the override");
assertTrue(
xml.contains("<artifactId>maven-enforcer-plugin</artifactId>"),
- "Should add pluginManagement for maven-enforcer-plugin
inherited from parent");
+ "Should add pluginManagement for
maven-enforcer-plugin");
} finally {
try (var walk = Files.walk(tempDir)) {
walk.sorted(java.util.Comparator.reverseOrder()).forEach(p
-> {
@@ -825,12 +832,12 @@ void shouldDetectInheritedPluginsFromRemoteParent()
throws Exception {
}
@Test
- @DisplayName("should not add direct build/plugins override when plugin
version comes from pluginManagement")
- void shouldNotAddDirectOverrideWhenVersionFromPluginManagement()
throws Exception {
+ @DisplayName("should override remote parent plugin via
pluginManagement with comment, not direct build/plugins")
+ void shouldOverrideRemoteParentPluginViaPluginManagementWithComment()
throws Exception {
// org.apache:apache:23 has maven-enforcer-plugin in build/plugins
WITHOUT
// an explicit version — the version (1.4.1) comes from
pluginManagement.
- // In this case, adding a pluginManagement override in the child
is sufficient;
- // no direct build/plugins entry should be added for enforcer.
+ // In this case, adding a pluginManagement override with a comment
in the child
+ // is sufficient; no direct build/plugins entry should be added
for enforcer.
String pomXml = """
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0";>
@@ -870,6 +877,11 @@ void
shouldNotAddDirectOverrideWhenVersionFromPluginManagement() throws Exceptio
.orElse("")));
assertTrue(hasEnforcerInPM, "Should have enforcer in
pluginManagement");
+ String xml = DomUtils.toXml(document);
+ assertTrue(
+ xml.contains("Override version inherited from parent"),
+ "Should add comment explaining the override");
+
// Verify NO direct build/plugins entry for enforcer (PM override
is sufficient)
Element buildPlugins = root.childElement("build")
.flatMap(b -> b.childElement("plugins"))
@@ -951,6 +963,70 @@ void
shouldNotDuplicatePluginInBuildPluginsWhenAlreadyDeclared() throws Exceptio
.orElse(null);
assertEquals("3.5.0", version, "Existing enforcer-plugin version
should be upgraded to 3.5.0");
}
+
+ @Test
+ @DisplayName("should inject pluginManagement when plugin is locally
declared without version")
+ void
shouldInjectPluginManagementForLocallyDeclaredPluginWithoutVersion() throws
Exception {
+ // Child POM explicitly declares maven-enforcer-plugin in
build/plugins without
+ // a version. The version comes from the remote parent's
pluginManagement.
+ // Since the child does declare the plugin, mvnup should add a
pluginManagement
+ // entry to override the inherited version.
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0";>
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache</groupId>
+ <artifactId>apache</artifactId>
+ <version>23</version>
+ </parent>
+ <groupId>org.example</groupId>
+ <artifactId>test-child</artifactId>
+ <version>1.0.0-SNAPSHOT</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-enforcer-plugin</artifactId>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Path tempDir = Files.createTempDirectory("mvnup-test-");
+ try {
+ Files.createDirectories(tempDir.resolve(".mvn"));
+ Path pomPath = tempDir.resolve("pom.xml");
+ Files.writeString(pomPath, pomXml);
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(pomPath, document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Strategy should succeed");
+ assertTrue(
+ result.modifiedCount() > 0, "Should have added
pluginManagement for locally declared plugin");
+
+ String xml = DomUtils.toXml(document);
+ assertTrue(
+ xml.contains("<pluginManagement>"), "Should add
pluginManagement for locally declared plugin");
+ assertTrue(
+
xml.contains("<artifactId>maven-enforcer-plugin</artifactId>"),
+ "Should add pluginManagement for
maven-enforcer-plugin");
+ } finally {
+ try (var walk = Files.walk(tempDir)) {
+ walk.sorted(java.util.Comparator.reverseOrder()).forEach(p
-> {
+ try {
+ Files.delete(p);
+ } catch (IOException ignored) {
+ }
+ });
+ }
+ }
+ }
}
@Nested
