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/06/23 10:40:52 UTC
(maven) branch fix-11606 created (now 5290442231)
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a change to branch fix-11606
in repository https://gitbox.apache.org/repos/asf/maven.git
at 5290442231 [#11606] Fix mvnup spurious pluginManagement injection for plugins inherited from remote parent POM
This branch includes the following new commits:
new 5290442231 [#11606] Fix mvnup spurious pluginManagement injection for plugins inherited from remote parent POM
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(maven) 01/01: [#11606] Fix mvnup spurious pluginManagement injection for plugins inherited from remote parent POM
Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch fix-11606
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 529044223192ececf38db09fbcaec431a785455b
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Tue Jun 23 10:40:39 2026 +0000
[#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 <no...@anthropic.com>
---
.../invoker/mvnup/goals/PluginUpgradeStrategy.java | 63 +++++++++-
.../mvnup/goals/PluginUpgradeStrategyTest.java | 129 ++++++++++++++-------
2 files changed, 145 insertions(+), 47 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..2d6044aa9e 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
@@ -475,6 +475,7 @@ private PluginAnalysisResults analyzePluginsUsingEffectiveModels(
Map<Path, Set<String>> managementResult = new HashMap<>();
Map<Path, Set<String>> directOverrideResult = new HashMap<>();
Map<String, PluginUpgrade> pluginUpgrades = getPluginUpgradesAsMap();
+ Set<String> localPluginKeys = collectLocallyDeclaredPluginKeys(pomMap);
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Path originalPomPath = entry.getKey();
@@ -486,7 +487,8 @@ private PluginAnalysisResults analyzePluginsUsingEffectiveModels(
Path tempPomPath = tempDir.resolve(relativePath);
// Build effective model using Maven 4 API
- PluginAnalysis analysis = analyzeEffectiveModelForPlugins(context, tempPomPath, pluginUpgrades);
+ PluginAnalysis analysis =
+ analyzeEffectiveModelForPlugins(context, tempPomPath, pluginUpgrades, localPluginKeys);
// Determine where to add plugin management (last local parent)
Path targetPom =
@@ -528,9 +530,12 @@ private Map<String, PluginUpgrade> getPluginUpgradesAsMap() {
}
private PluginAnalysis analyzeEffectiveModelForPlugins(
- UpgradeContext context, Path tempPomPath, Map<String, PluginUpgrade> pluginUpgrades) {
+ UpgradeContext context,
+ Path tempPomPath,
+ Map<String, PluginUpgrade> pluginUpgrades,
+ Set<String> localPluginKeys) {
Model effectiveModel = buildEffectiveModel(tempPomPath);
- return analyzePluginsFromEffectiveModel(context, effectiveModel, pluginUpgrades);
+ return analyzePluginsFromEffectiveModel(context, effectiveModel, pluginUpgrades, localPluginKeys);
}
/**
@@ -540,7 +545,10 @@ private PluginAnalysis analyzeEffectiveModelForPlugins(
* parent's build/plugins, not via pluginManagement).
*/
private PluginAnalysis analyzePluginsFromEffectiveModel(
- UpgradeContext context, Model effectiveModel, Map<String, PluginUpgrade> pluginUpgrades) {
+ UpgradeContext context,
+ Model effectiveModel,
+ Map<String, PluginUpgrade> pluginUpgrades,
+ Set<String> localPluginKeys) {
Set<String> needsManagement = new HashSet<>();
Set<String> needsDirectOverride = new HashSet<>();
@@ -561,6 +569,13 @@ private PluginAnalysis analyzePluginsFromEffectiveModel(
String pluginKey = getPluginKey(plugin);
PluginUpgrade upgrade = pluginUpgrades.get(pluginKey);
if (upgrade != null) {
+ // Skip plugins not declared in any local POM — they are inherited
+ // from a remote parent POM that the project does not control
+ if (!localPluginKeys.contains(pluginKey)) {
+ context.debug(
+ "Plugin " + pluginKey + " is inherited from a remote parent POM — skipping upgrade");
+ continue;
+ }
String effectiveVersion = plugin.getVersion();
if (isVersionBelow(effectiveVersion, upgrade.minVersion())) {
needsManagement.add(pluginKey);
@@ -587,6 +602,10 @@ private PluginAnalysis analyzePluginsFromEffectiveModel(
String pluginKey = getPluginKey(plugin);
PluginUpgrade upgrade = pluginUpgrades.get(pluginKey);
if (upgrade != null && !needsManagement.contains(pluginKey)) {
+ // Skip plugins not declared in any local POM
+ if (!localPluginKeys.contains(pluginKey)) {
+ continue;
+ }
String effectiveVersion = plugin.getVersion();
if (isVersionBelow(effectiveVersion, upgrade.minVersion())) {
needsManagement.add(pluginKey);
@@ -808,6 +827,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 3702a07e05..4350fcd15a 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 not inject pluginManagement for plugins only inherited from remote parent")
+ void shouldNotInjectPluginManagementForPluginsOnlyInRemoteParent() 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 NOT
+ // get pluginManagement overrides — the version is locked by the remote parent
+ // that the project does not control.
String pomXml = """
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0">
@@ -806,12 +807,15 @@ 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");
+ assertEquals(
+ 0,
+ result.modifiedCount(),
+ "Should not modify POM when plugins are only inherited from remote parent");
String xml = DomUtils.toXml(document);
- assertTrue(
- xml.contains("<artifactId>maven-enforcer-plugin</artifactId>"),
- "Should add pluginManagement for maven-enforcer-plugin inherited from parent");
+ assertFalse(
+ xml.contains("<pluginManagement>"),
+ "Should not inject pluginManagement for plugins only from remote parent");
} finally {
try (var walk = Files.walk(tempDir)) {
walk.sorted(java.util.Comparator.reverseOrder()).forEach(p -> {
@@ -825,12 +829,12 @@ void shouldDetectInheritedPluginsFromRemoteParent() throws Exception {
}
@Test
- @DisplayName("should not add direct build/plugins override when plugin version comes from pluginManagement")
- void shouldNotAddDirectOverrideWhenVersionFromPluginManagement() 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.
+ @DisplayName("should not inject any overrides for plugins only from remote parent")
+ void shouldNotInjectAnyOverridesForPluginsOnlyFromRemoteParent() throws Exception {
+ // org.apache:apache:23 has maven-enforcer-plugin in build/plugins and
+ // pluginManagement. Since the child POM does not declare the plugin itself,
+ // mvnup should not inject any overrides — neither pluginManagement nor
+ // direct build/plugins entries.
String pomXml = """
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0">
@@ -855,34 +859,9 @@ void shouldNotAddDirectOverrideWhenVersionFromPluginManagement() throws Exceptio
assertTrue(result.success(), "Strategy should succeed");
- Editor editor = new Editor(document);
- Element root = editor.root();
-
- // Verify pluginManagement entry exists for enforcer
- Element pmPlugins =
- root.path("build", "pluginManagement", "plugins").orElse(null);
- assertNotNull(pmPlugins, "Should have pluginManagement/plugins");
- boolean hasEnforcerInPM = pmPlugins.childElements("plugin").anyMatch(p -> "maven-enforcer-plugin"
- .equals(p.childElement("artifactId")
- .map(Element::textContentTrimmed)
- .orElse("")));
- assertTrue(hasEnforcerInPM, "Should have enforcer in pluginManagement");
-
- // Verify NO direct build/plugins entry for enforcer (PM override is sufficient)
- Element buildPlugins = root.childElement("build")
- .flatMap(b -> b.childElement("plugins"))
- .orElse(null);
- if (buildPlugins != null) {
- boolean hasEnforcerInPlugins = buildPlugins
- .childElements("plugin")
- .anyMatch(p -> "maven-enforcer-plugin"
- .equals(p.childElement("artifactId")
- .map(Element::textContentTrimmed)
- .orElse("")));
- assertFalse(
- hasEnforcerInPlugins,
- "Should NOT add enforcer in build/plugins when pluginManagement override suffices");
- }
+ String xml = DomUtils.toXml(document);
+ assertFalse(
+ xml.contains("<build>"), "Should not add any build elements for plugins only from remote parent");
}
@Test
@@ -949,6 +928,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