You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/10/23 12:00:03 UTC
(maven) 02/03: Fix mvnup plugin upgrade for plugins inherited in build/plugins
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to tag backup/maven-4.0.x-test-fixes
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 6adef7a8fa9bbc8c3587c646819b1770ae7b61a2
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Wed May 27 00:51:44 2026 +0200
Fix mvnup plugin upgrade for plugins inherited in build/plugins
When a remote parent POM declares a plugin with an explicit version in
<build><plugins>, adding a <pluginManagement> entry in the child is
insufficient to override it. The parent's <plugins> version takes
precedence over the child's <pluginManagement>.
Now the plugin upgrade strategy adds plugins to both <pluginManagement>
(for downstream children) and <build><plugins> (to override the
inherited version) when the effective model shows outdated plugins in
the build/plugins section.
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../invoker/mvnup/goals/PluginUpgradeStrategy.java | 118 ++++++++++++++-----
.../mvnup/goals/PluginUpgradeStrategyTest.java | 127 +++++++++++++++++++++
2 files changed, 219 insertions(+), 26 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 552ab1c2cf..8550d4a594 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
@@ -134,10 +134,9 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
Path tempDir = createTempProjectStructure(context, pomMap);
// Phase 2: For each POM, build effective model using the session and analyze plugins
- Map<Path, Set<String>> pluginsNeedingManagement =
- analyzePluginsUsingEffectiveModels(context, pomMap, tempDir);
+ PluginAnalysisResults analysisResults = analyzePluginsUsingEffectiveModels(context, pomMap, tempDir);
- // Phase 3: Add plugin management to the last local parent in hierarchy
+ // 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();
Document pomDocument = entry.getValue();
@@ -153,14 +152,21 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
hasUpgrades |= upgradePluginsInDocument(pomDocument, context);
// Add plugin management based on effective model analysis
- // Note: pluginsNeedingManagement only contains entries for POMs that should receive plugin
- // management
- // (i.e., the "last local parent" for each plugin that needs management)
- Set<String> pluginsForThisPom = pluginsNeedingManagement.get(pomPath);
- if (pluginsForThisPom != null && !pluginsForThisPom.isEmpty()) {
- hasUpgrades |= addPluginManagementForEffectivePlugins(context, pomDocument, pluginsForThisPom);
+ Set<String> pluginsForManagement =
+ analysisResults.pluginsNeedingManagement().get(pomPath);
+ if (pluginsForManagement != null && !pluginsForManagement.isEmpty()) {
+ hasUpgrades |=
+ addPluginManagementForEffectivePlugins(context, pomDocument, pluginsForManagement);
context.detail("Added plugin management to " + pomPath + " (target parent for "
- + pluginsForThisPom.size() + " plugins)");
+ + pluginsForManagement.size() + " plugins)");
+ }
+
+ // Add direct plugin overrides in build/plugins for inherited plugins
+ // whose versions cannot be overridden via pluginManagement alone
+ Set<String> pluginsForDirectOverride =
+ analysisResults.pluginsNeedingDirectOverride().get(pomPath);
+ if (pluginsForDirectOverride != null && !pluginsForDirectOverride.isEmpty()) {
+ hasUpgrades |= addDirectPluginOverrides(context, pomDocument, pluginsForDirectOverride);
}
if (hasUpgrades) {
@@ -465,11 +471,15 @@ public static List<PluginUpgrade> getPluginUpgrades() {
/**
* Analyzes plugins using effective models built from the temp directory.
- * Returns a map of POM path to the set of plugin keys that need management.
+ * Returns a PluginAnalysisResults with two maps:
+ * - pluginsNeedingManagement: plugins that need pluginManagement entries
+ * - pluginsNeedingDirectOverride: plugins from build/plugins that need direct overrides
+ * (because pluginManagement alone cannot override versions inherited from a parent's build/plugins)
*/
- private Map<Path, Set<String>> analyzePluginsUsingEffectiveModels(
+ private PluginAnalysisResults analyzePluginsUsingEffectiveModels(
UpgradeContext context, Map<Path, Document> pomMap, Path tempDir) {
- Map<Path, Set<String>> result = new HashMap<>();
+ Map<Path, Set<String>> managementResult = new HashMap<>();
+ Map<Path, Set<String>> directOverrideResult = new HashMap<>();
Map<String, PluginUpgrade> pluginUpgrades = getPluginUpgradesAsMap();
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
@@ -482,20 +492,28 @@ private Map<Path, Set<String>> analyzePluginsUsingEffectiveModels(
Path tempPomPath = tempDir.resolve(relativePath);
// Build effective model using Maven 4 API
- Set<String> pluginsNeedingUpgrade =
- analyzeEffectiveModelForPlugins(context, tempPomPath, pluginUpgrades);
+ PluginAnalysis analysis = analyzeEffectiveModelForPlugins(context, tempPomPath, pluginUpgrades);
// Determine where to add plugin management (last local parent)
Path targetPomForManagement =
findLastLocalParentForPluginManagement(context, tempPomPath, pomMap, tempDir, commonRoot);
if (targetPomForManagement != null) {
- result.computeIfAbsent(targetPomForManagement, k -> new HashSet<>())
- .addAll(pluginsNeedingUpgrade);
+ managementResult
+ .computeIfAbsent(targetPomForManagement, k -> new HashSet<>())
+ .addAll(analysis.allPluginsNeedingUpgrade());
- if (!pluginsNeedingUpgrade.isEmpty()) {
+ directOverrideResult
+ .computeIfAbsent(targetPomForManagement, k -> new HashSet<>())
+ .addAll(analysis.buildPluginsNeedingUpgrade());
+
+ if (!analysis.allPluginsNeedingUpgrade().isEmpty()) {
context.debug("Will add plugin management to " + targetPomForManagement + " for plugins: "
- + pluginsNeedingUpgrade);
+ + analysis.allPluginsNeedingUpgrade());
+ }
+ if (!analysis.buildPluginsNeedingUpgrade().isEmpty()) {
+ context.debug("Will add direct plugin overrides to " + targetPomForManagement + " for plugins: "
+ + analysis.buildPluginsNeedingUpgrade());
}
}
@@ -504,7 +522,7 @@ private Map<Path, Set<String>> analyzePluginsUsingEffectiveModels(
}
}
- return result;
+ return new PluginAnalysisResults(managementResult, directOverrideResult);
}
/**
@@ -516,7 +534,7 @@ private Map<String, PluginUpgrade> getPluginUpgradesAsMap() {
upgrade -> upgrade.groupId() + ":" + upgrade.artifactId(), upgrade -> upgrade));
}
- private Set<String> analyzeEffectiveModelForPlugins(
+ private PluginAnalysis analyzeEffectiveModelForPlugins(
UpgradeContext context, Path tempPomPath, Map<String, PluginUpgrade> pluginUpgrades) {
Model effectiveModel = buildEffectiveModel(tempPomPath);
return analyzePluginsFromEffectiveModel(context, effectiveModel, pluginUpgrades);
@@ -524,10 +542,14 @@ private Set<String> analyzeEffectiveModelForPlugins(
/**
* Analyzes plugins from the effective model and determines which ones need upgrades.
+ * Returns a PluginAnalysis with two sets:
+ * - allPluginsNeedingUpgrade: plugins from both build/plugins and build/pluginManagement/plugins
+ * - buildPluginsNeedingUpgrade: plugins from build/plugins only (these may need direct overrides)
*/
- private Set<String> analyzePluginsFromEffectiveModel(
+ private PluginAnalysis analyzePluginsFromEffectiveModel(
UpgradeContext context, Model effectiveModel, Map<String, PluginUpgrade> pluginUpgrades) {
- Set<String> pluginsNeedingUpgrade = new HashSet<>();
+ Set<String> allPluginsNeedingUpgrade = new HashSet<>();
+ Set<String> buildPluginsNeedingUpgrade = new HashSet<>();
Build build = effectiveModel.getBuild();
if (build != null) {
@@ -538,7 +560,8 @@ private Set<String> analyzePluginsFromEffectiveModel(
if (upgrade != null) {
String effectiveVersion = plugin.getVersion();
if (isVersionBelow(effectiveVersion, upgrade.minVersion())) {
- pluginsNeedingUpgrade.add(pluginKey);
+ allPluginsNeedingUpgrade.add(pluginKey);
+ buildPluginsNeedingUpgrade.add(pluginKey);
context.debug("Plugin " + pluginKey + " version " + effectiveVersion + " needs upgrade to "
+ upgrade.minVersion());
}
@@ -554,7 +577,7 @@ private Set<String> analyzePluginsFromEffectiveModel(
if (upgrade != null) {
String effectiveVersion = plugin.getVersion();
if (isVersionBelow(effectiveVersion, upgrade.minVersion())) {
- pluginsNeedingUpgrade.add(pluginKey);
+ allPluginsNeedingUpgrade.add(pluginKey);
context.debug("Managed plugin " + pluginKey + " version " + effectiveVersion
+ " needs upgrade to " + upgrade.minVersion());
}
@@ -563,7 +586,7 @@ private Set<String> analyzePluginsFromEffectiveModel(
}
}
- return pluginsNeedingUpgrade;
+ return new PluginAnalysis(allPluginsNeedingUpgrade, buildPluginsNeedingUpgrade);
}
/**
@@ -735,6 +758,49 @@ private void addPluginManagementEntryFromUpgrade(
+ upgrade.minVersion() + " (found through effective model analysis)");
}
+ /**
+ * Adds direct plugin entries in build/plugins for plugins inherited from remote parents.
+ * This is necessary because pluginManagement alone cannot override plugin versions
+ * that are explicitly set in an inherited parent's build/plugins section.
+ */
+ private boolean addDirectPluginOverrides(UpgradeContext context, Document pomDocument, Set<String> pluginKeys) {
+ Map<String, PluginUpgrade> pluginUpgrades = getPluginUpgradesAsMap();
+ boolean hasUpgrades = false;
+
+ Element root = pomDocument.root();
+
+ Element buildElement = root.childElement(BUILD).orElse(null);
+ if (buildElement == null) {
+ buildElement = DomUtils.insertNewElement(BUILD, root);
+ }
+
+ Element pluginsElement = buildElement.childElement(PLUGINS).orElse(null);
+ if (pluginsElement == null) {
+ pluginsElement = DomUtils.insertNewElement(PLUGINS, buildElement);
+ }
+
+ for (String pluginKey : pluginKeys) {
+ PluginUpgrade upgrade = pluginUpgrades.get(pluginKey);
+ if (upgrade != null) {
+ if (!isPluginAlreadyManagedInElement(pluginsElement, upgrade)) {
+ DomUtils.createPlugin(
+ pluginsElement, upgrade.groupId(), upgrade.artifactId(), upgrade.minVersion());
+ hasUpgrades = true;
+ context.detail("Added plugin " + upgrade.groupId() + ":" + upgrade.artifactId() + " version "
+ + upgrade.minVersion()
+ + " in build/plugins (overrides inherited version from remote parent)");
+ }
+ }
+ }
+
+ return hasUpgrades;
+ }
+
+ private record PluginAnalysis(Set<String> allPluginsNeedingUpgrade, Set<String> buildPluginsNeedingUpgrade) {}
+
+ private record PluginAnalysisResults(
+ Map<Path, Set<String>> pluginsNeedingManagement, Map<Path, Set<String>> pluginsNeedingDirectOverride) {}
+
/**
* Holds plugin upgrade information for Maven 4 compatibility.
* This class contains the minimum version requirements for plugins
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 2328e905d1..5c9f9e6bc0 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
@@ -808,6 +808,133 @@ void shouldDetectInheritedPluginsFromRemoteParent() throws Exception {
xml.contains("<artifactId>maven-enforcer-plugin</artifactId>"),
"Should add pluginManagement for maven-enforcer-plugin inherited from parent");
}
+
+ @Test
+ @DisplayName("should add direct build/plugins override for inherited plugins from remote parent")
+ void shouldAddDirectBuildPluginsOverrideForInheritedPlugins() throws Exception {
+ // When a remote parent declares a plugin in build/plugins (not just pluginManagement),
+ // adding a pluginManagement entry in the child is insufficient to override the version.
+ // mvnup should also add the plugin directly in the child's build/plugins section.
+ // org.apache:apache:23 has maven-enforcer-plugin in its effective build/plugins,
+ // so the child should get BOTH pluginManagement AND build/plugins entries.
+ 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>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Path pomPath = Paths.get("/project/pom.xml").toAbsolutePath();
+ 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 overrides for inherited plugins");
+
+ Editor editor = new Editor(document);
+ Element root = editor.root();
+
+ // Verify pluginManagement entry exists
+ Element pmPlugin =
+ root.path("build", "pluginManagement", "plugins", "plugin").orElse(null);
+ assertNotNull(pmPlugin, "Should have a pluginManagement/plugins/plugin entry");
+
+ // Verify build/plugins entry exists (direct override for inherited plugin)
+ Element buildPluginsElement = root.childElement("build")
+ .flatMap(b -> b.childElement("plugins"))
+ .orElse(null);
+ assertNotNull(buildPluginsElement, "Should have a build/plugins section");
+
+ boolean hasEnforcerInBuildPlugins = buildPluginsElement
+ .childElements("plugin")
+ .anyMatch(p -> p.childElement("artifactId")
+ .map(Element::textContentTrimmed)
+ .orElse("")
+ .equals("maven-enforcer-plugin"));
+ assertTrue(
+ hasEnforcerInBuildPlugins,
+ "Should add maven-enforcer-plugin in build/plugins to override inherited version");
+ }
+
+ @Test
+ @DisplayName("should not duplicate plugin in build/plugins when already locally declared")
+ void shouldNotDuplicatePluginInBuildPluginsWhenAlreadyDeclared() throws Exception {
+ // When the child POM already declares the plugin in build/plugins,
+ // the strategy should NOT add a duplicate entry.
+ 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>
+ <version>3.0.0</version>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Path pomPath = Paths.get("/project/pom.xml").toAbsolutePath();
+ Map<Path, Document> pomMap = Map.of(pomPath, document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Strategy should succeed");
+
+ // Count enforcer-plugin entries in build/plugins - should be exactly 1
+ Editor editor = new Editor(document);
+ Element root = editor.root();
+ Element buildPluginsElement = root.childElement("build")
+ .flatMap(b -> b.childElement("plugins"))
+ .orElse(null);
+ assertNotNull(buildPluginsElement, "Should have build/plugins section");
+
+ long enforcerCount = buildPluginsElement
+ .childElements("plugin")
+ .filter(p -> p.childElement("artifactId")
+ .map(Element::textContentTrimmed)
+ .orElse("")
+ .equals("maven-enforcer-plugin"))
+ .count();
+ assertEquals(1, enforcerCount, "Should have exactly one maven-enforcer-plugin in build/plugins");
+
+ // Verify the version was upgraded (not the original 3.0.0)
+ String version = buildPluginsElement
+ .childElements("plugin")
+ .filter(p -> p.childElement("artifactId")
+ .map(Element::textContentTrimmed)
+ .orElse("")
+ .equals("maven-enforcer-plugin"))
+ .findFirst()
+ .flatMap(p -> p.childElement("version"))
+ .map(Element::textContentTrimmed)
+ .orElse(null);
+ assertEquals("3.5.0", version, "Existing enforcer-plugin version should be upgraded to 3.5.0");
+ }
}
@Nested