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/05/27 08:03:52 UTC
(maven) branch maven-4.0.x-test-fixes updated (2bb8344c2f -> 3a2cfcdb0e)
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a change to branch maven-4.0.x-test-fixes
in repository https://gitbox.apache.org/repos/asf/maven.git
discard 2bb8344c2f Merge PR #12100: Fix #12087: add surefire and failsafe plugins to PluginUpgradeStrategy
discard 7ebfd4b1d2 Merge PR #12099: Fix #12080: mvnup - comment out dependencies with undefined property expressions
discard d6b6935a91 Merge PR #12082: Switch mvnup to domtrip on 4.0.x
discard 2dabbdc550 Merge PR #12098: Fix #12086: filter transitive repos and deps with uninterpolated expressions
discard ef4f073094 Merge PR #12096: Fix #12075: skip expression validation for distributionManagement repository IDs
discard fc428351dd Merge PR #12095: Fix mvn script expanding ${...} in CLI arguments
discard 521d71697a Merge PR #12094: restore buildConfiguration() callback in deprecated build() methods
discard 069b51e45c Merge PR #12093: propagate addResource() to model Build for Maven 3 compat
discard af03a7e676 Merge PR #12079: Fix #12074: prevent false parent cycle with shade plugin
discard 8f5368eccb Merge PR #12073: Use ComparableVersion for mvnup plugin version comparison
omit 5963f0abcc Fix #12087: add surefire and failsafe plugins to PluginUpgradeStrategy
discard 1403fc87e5 Fix #12080: mvnup - comment out dependencies with undefined property expressions
discard 41016d48cc Fix #12086: filter transitive repos with uninterpolated expressions
discard a56dfd9997 Filter transitive dependencies with uninterpolated expressions
omit 7595bdd6bc Fix #12075: skip expression validation for distributionManagement repository IDs
omit a45d213586 Add IT for mvn script expanding ${...} in CLI arguments
omit b41303b2b1 Fix mvn script expanding ${...} in CLI arguments
omit 0d81b4d548 fix: restore buildConfiguration() callback in deprecated build() methods
omit 33bfd7931a fix: only sync Build.resources for <sources> projects
omit 1b40da8f01 fix: sync Build.resources from sources set for Maven 3 compat
omit 10e6853152 fix: propagate addResource() to model Build for Maven 3 compat
omit a404c0d093 Fix #12074: prevent false parent cycle with shade plugin's dependency-reduced-pom.xml
omit 26314587cb Use maven-api Version for mvnup plugin version comparison
omit dd67f6b748 Filter transitive repositories with uninterpolated IDs (#12049) (#12050)
omit 2a1cf481ae Port inherited plugin detection tests to domtrip API
omit 0bbb7d3396 Switch mvnup to domtrip (#11432)
add 6b42a8e69b Filter transitive repositories with uninterpolated IDs (#12049) (#12050) (#12070)
add 904bb60ea3 [maven-4.0.x] fix: propagate addResource() to model Build for Maven 3 compat (#12093)
add be7d73b2c5 fix: restore buildConfiguration() callback in deprecated build() methods (#12094)
add aa3b4d33f1 Fix #12075: skip expression validation for distributionManagement repository IDs (#12077)
add 1b53468bfe Fix #12074: prevent false parent cycle with shade plugin's dependency-reduced-pom.xml (#12079)
add b8a0ae05a8 [maven-4.0.x] Backport bug fixes from master to maven-4.0.x (#12091)
add f52d3cae6a Fix #12085: add regression tests for version inheritance from remote parent (#12101)
add 16c00a84f2 Fix #12081: mvnup: use domtrip XML API on 4.0.x branch to align with master (#12082)
add 0a792e8a8a Use maven-api Version for mvnup plugin version comparison (#12073)
add 67d297bf69 [maven-4.0.x] Fix #12087: add surefire and failsafe plugins to PluginUpgradeStrategy (#12109)
add 402facb381 Backport bug fixes from master to maven-4.0.x (batch 2) (#12107)
add ef1ce187aa [maven-4.0.x] Upgrade plugin dependencies (extra-enforcer-rules) in mvnup (#12108)
add 9c8a1ef4fb Fix #11885: Disable ANSI colors when stdout is piped or redirected on JDK 22+ (#11887) (#12111)
add cf510b39e1 [maven-4.0.x] Add maven-surefire-report-plugin to PluginUpgradeStrategy (#12114)
add 7a9947bdd0 [maven-4.0.x] Fix consumer POM serialization of prefixed XML attributes (fixes #11760) (#12110)
add 8f7bdee4c4 [maven-4.0.x] Fix mvn script expanding ${...} in CLI arguments (#12095)
add 94ba23c666 Bump asmVersion from 9.9.1 to 9.10 (#12059)
add aaa1d29b01 Fix #11899: Default addLocationInformation to false in Settings and Toolchains XML writers (#11984) (#12123)
add ae719daa25 Update binary distribution LICENSE with complete Apache License 2.0 text
add 475bd4a202 Bump com.fasterxml.woodstox:woodstox-core from 7.1.1 to 7.2.0 (#12126)
add 7ac961157f Bump org.apache.maven.reporting:maven-reporting-exec from 2.0.0 to 2.0.1 (#12066)
add 537cb7eb83 Resolver 2018 (#12112) (#12133)
add 39879890f8 Bump org.codehaus.plexus:plexus-classworlds from 2.9.0 to 2.12.0 (#12132)
add 9f1b12fcbb Bump jlineVersion from 4.0.14 to 4.1.0 (#12018)
add 2dd2693500 [maven-4.0.x] Filter transitive dependencies with uninterpolated expressions (#12097)
add c106730d1e Fix #12086: filter transitive repos with uninterpolated expressions (#12098)
add d39e57d307 [maven-4.0.x] Fix mvnup to replace deprecated ${basedir} in repository URLs (#12105) (#12106)
add 004d2435ee mvnup: upgrade enforcer plugin minimum and add new plugin upgrades (#12121)
add 34d3b053df Fix incompatible extensions in mvnup for Maven 4 compatibility (#12122)
add 933992eeb0 Bump domtripVersion from 0.4.1 to 1.5.1 (#12131)
add c2073d48e8 Fix domtrip API breakage after 1.5.1 upgrade (#12138)
add 19bd49fe44 [backport maven-4.0.x] Fix logging setup/teardown order (#12137)
add 126a27e3a8 Bump eu.maveniverse.maven.mimir:testing from 0.11.2 to 0.11.4 (#12067)
add f980166475 Bump com.github.siom79.japicmp:japicmp-maven-plugin (#12142)
add 763898b8ee [maven-4.0.x] Backport bug fixes from #11741 and #11733 (#12144)
add 5f099825e3 Bump asmVersion from 9.10 to 9.10.1 (#12152)
add c6af3e3971 Bump jlineVersion from 4.1.0 to 4.1.2 (#12151)
add 686f1b8e76 Extract shared session infrastructure into AbstractUpgradeStrategy
add a0ed03ca9f Fix #12080: mvnup - comment out dependencies with undefined property expressions
new 6f1c53b5a4 Use effective model to resolve properties from remote parent POMs in mvnup
new 8d1f0124fa Fix mvnup plugin upgrade for versions locked by parent's build/plugins
new 680cd1d141 Remove invalid combine.self and combine.children attributes instead of converting them
new 3a2cfcdb0e Fix deadlock in AbstractRequestCache.requests() due to unstable hashCode
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (2bb8344c2f)
\
N -- N -- N refs/heads/maven-4.0.x-test-fixes (3a2cfcdb0e)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 4 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.
Summary of changes:
apache-maven/src/assembly/maven/bin/mvn | 2 +-
.../main/appended-resources/META-INF/LICENSE.vm | 273 ++++-
.../main/java/org/apache/maven/api/Constants.java | 37 +-
.../java/org/apache/maven/api/xml/XmlNode.java | 38 +-
.../legacy/DefaultUpdateCheckManagerTest.java | 4 +-
.../model/validation/DefaultModelValidator.java | 73 +-
.../validation/DefaultModelValidatorTest.java | 4 +
.../java/org/apache/maven/model/ModelTest.java | 13 +
.../apache/maven/cling/invoker/LookupInvoker.java | 8 +-
.../invoker/mvnup/goals/AbstractUpgradeGoal.java | 104 ++
.../mvnup/goals/AbstractUpgradeStrategy.java | 147 ++-
.../mvnup/goals/CompatibilityFixStrategy.java | 205 ++--
.../maven/cling/invoker/mvnup/goals/DomUtils.java | 10 +-
.../invoker/mvnup/goals/InferenceStrategy.java | 81 +-
.../invoker/mvnup/goals/ModelUpgradeStrategy.java | 38 +-
.../invoker/mvnup/goals/ModelVersionUtils.java | 8 +-
.../invoker/mvnup/goals/PluginUpgradeStrategy.java | 461 ++++-----
.../cling/invoker/mvnup/goals/PomDiscovery.java | 4 +-
.../mvnup/goals/AbstractUpgradeGoalTest.java | 261 +++++
.../mvnup/goals/CompatibilityFixStrategyTest.java | 1074 +++++++++++++++++++-
.../cling/invoker/mvnup/goals/DomUtilsTest.java | 8 +-
.../cling/invoker/mvnup/goals/GAVUtilsTest.java | 16 +-
.../invoker/mvnup/goals/InferenceStrategyTest.java | 106 +-
.../mvnup/goals/ModelUpgradeStrategyTest.java | 167 +--
.../invoker/mvnup/goals/ModelVersionUtilsTest.java | 6 +-
.../mvnup/goals/PluginUpgradeStrategyTest.java | 302 +++++-
.../main/java/org/apache/maven/DefaultMaven.java | 1 +
.../DefaultRepositorySystemSessionFactory.java | 100 +-
.../maven/internal/aether/TypeRegistryAdapter.java | 17 +-
.../DefaultPluginDependenciesResolver.java | 2 +
.../maven/project/DefaultProjectBuilder.java | 9 +-
.../maven/project/ProjectBuildingException.java | 84 +-
.../project/collector/DefaultProjectsSelector.java | 2 +-
.../session/scope/internal/SessionScopeModule.java | 4 +
.../DefaultRepositorySystemSessionFactoryTest.java | 96 +-
.../project/DefaultMavenProjectBuilderTest.java | 39 +
.../project/ProjectBuildingExceptionTest.java | 163 +++
.../org/test/parent-pom/1.0/parent-pom-1.0.pom | 31 +
.../child/pom.xml | 12 +
.../pom.xml | 10 +
.../parent-version-inherited-from-remote/pom.xml | 12 +
.../maven/impl/DefaultDependencyResolver.java | 60 +-
.../maven/impl/DefaultSettingsXmlFactory.java | 16 +-
.../maven/impl/DefaultToolchainsXmlFactory.java | 16 +-
.../maven/impl/cache/AbstractRequestCache.java | 4 +-
.../maven/impl/model/DefaultModelValidator.java | 97 +-
.../impl/standalone/RepositorySystemSupplier.java | 22 +-
.../maven/impl/DefaultSettingsXmlFactoryTest.java | 108 ++
.../impl/DefaultToolchainsXmlFactoryTest.java | 108 ++
.../impl/model/DefaultModelValidatorTest.java | 25 +
.../java/org/apache/maven/jline/FastTerminal.java | 6 +
.../testing/stubs/RepositorySystemSupplier.java | 25 +-
.../maven/internal/xml/DefaultXmlService.java | 141 ++-
.../apache/maven/internal/xml/XmlNodeImplTest.java | 799 +++++++++++++++
.../it/MavenITgh11978PlaceholderInCliArgTest.java | 66 +-
.../spy/src/main/java/mng5208/EventLoggerSpy.java | 3 +-
.../core-it-plugins/maven-it-plugin-site/pom.xml | 2 +-
its/pom.xml | 2 +-
pom.xml | 12 +-
src/mdo/java/WrapperProperties.java | 6 +
src/mdo/model-v3.vm | 6 +-
src/mdo/writer-stax.vm | 59 +-
src/mdo/writer.vm | 54 +-
63 files changed, 4713 insertions(+), 956 deletions(-)
create mode 100644 impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuildingExceptionTest.java
create mode 100644 impl/maven-core/src/test/remote-repo/org/test/parent-pom/1.0/parent-pom-1.0.pom
create mode 100644 impl/maven-core/src/test/resources/projects/parent-version-inherited-from-remote-multimodule/child/pom.xml
create mode 100644 impl/maven-core/src/test/resources/projects/parent-version-inherited-from-remote-multimodule/pom.xml
create mode 100644 impl/maven-core/src/test/resources/projects/parent-version-inherited-from-remote/pom.xml
create mode 100644 impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsXmlFactoryTest.java
create mode 100644 impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultToolchainsXmlFactoryTest.java
(maven) 04/04: Fix deadlock in AbstractRequestCache.requests() due to unstable hashCode
Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch maven-4.0.x-test-fixes
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 3a2cfcdb0e4651b9c077b2ca78b772e1ba44ba35
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Wed May 27 09:41:23 2026 +0200
Fix deadlock in AbstractRequestCache.requests() due to unstable hashCode
The `requests()` method uses a HashMap to coordinate batch results between
the batch supplier and the individual CachingSupplier instances. When a
request object's hashCode() changes between HashMap.put() and
HashMap.containsKey() (e.g., because ResolverRequest includes a
RequestTrace with mutable ModelBuilderRequest data), the key is stored
in one bucket but looked up in another, causing containsKey() to return
false. The individualSupplier then waits forever for a result that is
already in the map but unreachable.
Switch to IdentityHashMap which uses System.identityHashCode() and
reference equality (==), both of which are stable regardless of
mutable object state. This is safe because the code always uses the
same object reference for put and get.
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../main/java/org/apache/maven/impl/cache/AbstractRequestCache.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/cache/AbstractRequestCache.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/cache/AbstractRequestCache.java
index 0b7fac393b..0dc5a7d0f0 100644
--- a/impl/maven-impl/src/main/java/org/apache/maven/impl/cache/AbstractRequestCache.java
+++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/cache/AbstractRequestCache.java
@@ -19,7 +19,7 @@
package org.apache.maven.impl.cache;
import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
@@ -85,7 +85,7 @@ public <REQ extends Request<?>, REP extends Result<REQ>> REP request(REQ req, Fu
@SuppressWarnings("unchecked")
public <REQ extends Request<?>, REP extends Result<REQ>> List<REP> requests(
List<REQ> reqs, Function<List<REQ>, List<REP>> supplier) {
- final Map<REQ, Object> nonCachedResults = new HashMap<>();
+ final Map<REQ, Object> nonCachedResults = new IdentityHashMap<>();
List<RequestResult<REQ, REP>> allResults = new ArrayList<>(reqs.size());
Function<REQ, REP> individualSupplier = req -> {
(maven) 01/04: Use effective model to resolve properties from remote parent POMs in mvnup
Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch maven-4.0.x-test-fixes
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 6f1c53b5a4ff45251e0ed580ce1f1470a7d8e5b3
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Wed May 27 00:53:26 2026 +0200
Use effective model to resolve properties from remote parent POMs in mvnup
The undefined property detection in CompatibilityFixStrategy only performed
static analysis of reactor POMs, missing properties inherited from remote
parent POMs. This caused valid dependencyManagement entries to be incorrectly
commented out, breaking child modules relying on them for version resolution.
Now uses buildEffectiveModel to collect properties from the full parent chain,
including remote parents resolved via relativePath or Maven repositories.
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../mvnup/goals/CompatibilityFixStrategy.java | 14 +++
.../mvnup/goals/CompatibilityFixStrategyTest.java | 126 +++++++++++++++++++++
2 files changed, 140 insertions(+)
diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
index f18b5161d7..c8c99d30ac 100644
--- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
+++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
@@ -125,6 +125,7 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
Set<Path> errorPoms = new HashSet<>();
Set<String> allDefinedProperties = collectAllDefinedProperties(pomMap);
+ allDefinedProperties.addAll(collectEffectiveProperties(context, pomMap));
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Path pomPath = entry.getKey();
@@ -374,6 +375,19 @@ private void collectPropertiesFromDom(Document document, Set<String> properties)
propsElement.childElements().forEach(child -> properties.add(child.name())))));
}
+ private Set<String> collectEffectiveProperties(UpgradeContext context, Map<Path, Document> pomMap) {
+ Set<String> properties = new HashSet<>();
+ for (Path pomPath : pomMap.keySet()) {
+ try {
+ org.apache.maven.api.model.Model effectiveModel = buildEffectiveModel(pomPath);
+ properties.addAll(effectiveModel.getProperties().keySet());
+ } catch (Exception e) {
+ context.debug("Failed to build effective model for " + pomPath + ": " + e.getMessage());
+ }
+ }
+ return properties;
+ }
+
/**
* Fixes dependencies with undefined property expressions by commenting them out.
*/
diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
index 00dacbb1c1..7795c25d9d 100644
--- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
+++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
@@ -919,6 +919,132 @@ void shouldRecognizePropertyFromGrandparent() throws Exception {
deps.childElements("dependency").count(),
"Dependency should not be commented out when property is inherited from grandparent");
}
+
+ @Test
+ @DisplayName("should not comment out when property is defined in external parent not in reactor")
+ void shouldNotCommentOutWhenPropertyFromExternalParentNotInReactor() throws Exception {
+ String externalParentPom = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>com.example</groupId>
+ <artifactId>external-parent</artifactId>
+ <version>1.0.0</version>
+ <packaging>pom</packaging>
+ <properties>
+ <oak.version>1.62.0</oak.version>
+ </properties>
+ </project>
+ """;
+
+ String childPom = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>com.example</groupId>
+ <artifactId>external-parent</artifactId>
+ <version>1.0.0</version>
+ <relativePath>../external/pom.xml</relativePath>
+ </parent>
+ <artifactId>child</artifactId>
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>oak-core</artifactId>
+ <version>${oak.version}</version>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+ </project>
+ """;
+
+ Path externalDir = Files.createDirectories(tempDir.resolve("external"));
+ Path externalParentPath = externalDir.resolve("pom.xml");
+ Path childDir = Files.createDirectories(tempDir.resolve("child"));
+ Path childPomPath = childDir.resolve("pom.xml");
+
+ Files.writeString(externalParentPath, externalParentPom);
+ Files.writeString(childPomPath, childPom);
+
+ Document childDoc = Document.of(childPom);
+ Map<Path, Document> pomMap = Map.of(childPomPath, childDoc);
+
+ UpgradeContext context = createMockContext(childDir);
+ strategy.doApply(context, pomMap);
+
+ Element root = childDoc.root();
+ Element depMgmt = DomUtils.findChildElement(root, "dependencyManagement");
+ Element deps = DomUtils.findChildElement(depMgmt, "dependencies");
+ assertEquals(
+ 1,
+ deps.childElements("dependency").count(),
+ "Managed dependency should not be commented out when property is inherited from external parent");
+ }
+
+ @Test
+ @DisplayName("should comment out when property is truly undefined even in effective model")
+ void shouldCommentOutWhenPropertyTrulyUndefinedInEffectiveModel() throws Exception {
+ String parentPom = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>com.example</groupId>
+ <artifactId>parent</artifactId>
+ <version>1.0.0</version>
+ <packaging>pom</packaging>
+ </project>
+ """;
+
+ String childPom = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>com.example</groupId>
+ <artifactId>parent</artifactId>
+ <version>1.0.0</version>
+ <relativePath>../pom.xml</relativePath>
+ </parent>
+ <artifactId>child</artifactId>
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>${truly.undefined.prop}</version>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+ </project>
+ """;
+
+ Path parentPath = tempDir.resolve("pom.xml");
+ Path childDir = Files.createDirectories(tempDir.resolve("child"));
+ Path childPomPath = childDir.resolve("pom.xml");
+
+ Files.writeString(parentPath, parentPom);
+ Files.writeString(childPomPath, childPom);
+
+ Document childDoc = Document.of(childPom);
+ Map<Path, Document> pomMap = Map.of(childPomPath, childDoc);
+
+ UpgradeContext context = createMockContext(childDir);
+ strategy.doApply(context, pomMap);
+
+ String xml = DomUtils.toXml(childDoc);
+ assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
+ assertTrue(xml.contains("truly.undefined.prop"), "Should mention the undefined property");
+ }
}
@Nested
(maven) 02/04: Fix mvnup plugin upgrade for versions locked by parent's build/plugins
Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch maven-4.0.x-test-fixes
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 8d1f0124fafb9186932c84258850646a7189c5dc
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Wed May 27 09:09:08 2026 +0200
Fix mvnup plugin upgrade for versions locked by parent's build/plugins
When a remote parent POM declares a plugin with an explicit version in
build/plugins (not via pluginManagement), adding a pluginManagement
entry in the child is insufficient — the parent's explicit version
takes precedence.
The analysis now compares each plugin's effective version in
build/plugins against build/pluginManagement/plugins:
- Same version: version comes from PM, pluginManagement override works
- Different version (or absent from PM): explicit version in parent's
plugins, needs a direct build/plugins entry to override
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../invoker/mvnup/goals/PluginUpgradeStrategy.java | 152 ++++++++++++++++-----
.../mvnup/goals/PluginUpgradeStrategyTest.java | 128 +++++++++++++++++
2 files changed, 245 insertions(+), 35 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..108cf8dc7d 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,13 @@ 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 analysis results with two maps: plugins needing pluginManagement entries
+ * and plugins needing direct build/plugins overrides.
*/
- 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 +490,27 @@ 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 =
+ Path targetPom =
findLastLocalParentForPluginManagement(context, tempPomPath, pomMap, tempDir, commonRoot);
- if (targetPomForManagement != null) {
- result.computeIfAbsent(targetPomForManagement, k -> new HashSet<>())
- .addAll(pluginsNeedingUpgrade);
-
- if (!pluginsNeedingUpgrade.isEmpty()) {
- context.debug("Will add plugin management to " + targetPomForManagement + " for plugins: "
- + pluginsNeedingUpgrade);
+ if (targetPom != null) {
+ managementResult
+ .computeIfAbsent(targetPom, k -> new HashSet<>())
+ .addAll(analysis.needsManagement());
+ directOverrideResult
+ .computeIfAbsent(targetPom, k -> new HashSet<>())
+ .addAll(analysis.needsDirectOverride());
+
+ if (!analysis.needsManagement().isEmpty()) {
+ context.debug("Will add plugin management to " + targetPom + " for plugins: "
+ + analysis.needsManagement());
+ }
+ if (!analysis.needsDirectOverride().isEmpty()) {
+ context.debug("Will add direct plugin overrides to " + targetPom + " for plugins: "
+ + analysis.needsDirectOverride());
}
}
@@ -504,7 +519,7 @@ private Map<Path, Set<String>> analyzePluginsUsingEffectiveModels(
}
}
- return result;
+ return new PluginAnalysisResults(managementResult, directOverrideResult);
}
/**
@@ -516,7 +531,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,13 +539,27 @@ private Set<String> analyzeEffectiveModelForPlugins(
/**
* Analyzes plugins from the effective model and determines which ones need upgrades.
+ * Separates plugins into those overridable via pluginManagement and those requiring
+ * a direct build/plugins entry (because the version is set explicitly in an inherited
+ * parent's build/plugins, not via pluginManagement).
*/
- private Set<String> analyzePluginsFromEffectiveModel(
+ private PluginAnalysis analyzePluginsFromEffectiveModel(
UpgradeContext context, Model effectiveModel, Map<String, PluginUpgrade> pluginUpgrades) {
- Set<String> pluginsNeedingUpgrade = new HashSet<>();
+ Set<String> needsManagement = new HashSet<>();
+ Set<String> needsDirectOverride = new HashSet<>();
Build build = effectiveModel.getBuild();
if (build != null) {
+ // Collect managed plugin versions for comparison
+ Map<String, String> managedVersions = new HashMap<>();
+ PluginManagement pluginManagement = build.getPluginManagement();
+ if (pluginManagement != null) {
+ for (Plugin plugin : pluginManagement.getPlugins()) {
+ String pluginKey = getPluginKey(plugin);
+ managedVersions.put(pluginKey, plugin.getVersion());
+ }
+ }
+
// Check build/plugins - these are the actual plugins used in the build
for (Plugin plugin : build.getPlugins()) {
String pluginKey = getPluginKey(plugin);
@@ -538,23 +567,33 @@ private Set<String> analyzePluginsFromEffectiveModel(
if (upgrade != null) {
String effectiveVersion = plugin.getVersion();
if (isVersionBelow(effectiveVersion, upgrade.minVersion())) {
- pluginsNeedingUpgrade.add(pluginKey);
- context.debug("Plugin " + pluginKey + " version " + effectiveVersion + " needs upgrade to "
- + upgrade.minVersion());
+ needsManagement.add(pluginKey);
+ String managedVersion = managedVersions.get(pluginKey);
+ if (managedVersion == null || !managedVersion.equals(effectiveVersion)) {
+ // Version differs from pluginManagement (or not in PM at all):
+ // the parent sets an explicit version in build/plugins that
+ // pluginManagement alone cannot override
+ needsDirectOverride.add(pluginKey);
+ context.debug("Plugin " + pluginKey + " version " + effectiveVersion
+ + " has explicit version in inherited build/plugins"
+ + " — needs direct override to " + upgrade.minVersion());
+ } else {
+ context.debug("Plugin " + pluginKey + " version " + effectiveVersion
+ + " is managed via pluginManagement — needs upgrade to " + upgrade.minVersion());
+ }
}
}
}
- // Check build/pluginManagement/plugins - these provide version management
- PluginManagement pluginManagement = build.getPluginManagement();
+ // Check build/pluginManagement/plugins for managed-only plugins
if (pluginManagement != null) {
for (Plugin plugin : pluginManagement.getPlugins()) {
String pluginKey = getPluginKey(plugin);
PluginUpgrade upgrade = pluginUpgrades.get(pluginKey);
- if (upgrade != null) {
+ if (upgrade != null && !needsManagement.contains(pluginKey)) {
String effectiveVersion = plugin.getVersion();
if (isVersionBelow(effectiveVersion, upgrade.minVersion())) {
- pluginsNeedingUpgrade.add(pluginKey);
+ needsManagement.add(pluginKey);
context.debug("Managed plugin " + pluginKey + " version " + effectiveVersion
+ " needs upgrade to " + upgrade.minVersion());
}
@@ -563,7 +602,7 @@ private Set<String> analyzePluginsFromEffectiveModel(
}
}
- return pluginsNeedingUpgrade;
+ return new PluginAnalysis(needsManagement, needsDirectOverride);
}
/**
@@ -735,6 +774,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 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) {
+ 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 " + upgrade.groupId() + ":" + upgrade.artifactId() + " version "
+ + upgrade.minVersion()
+ + " in build/plugins (overrides version locked by parent)");
+ }
+ }
+ }
+
+ return hasUpgrades;
+ }
+
+ private record PluginAnalysis(Set<String> needsManagement, Set<String> needsDirectOverride) {}
+
+ 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..91d7420304 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,134 @@ void shouldDetectInheritedPluginsFromRemoteParent() throws Exception {
xml.contains("<artifactId>maven-enforcer-plugin</artifactId>"),
"Should add pluginManagement for maven-enforcer-plugin inherited from parent");
}
+
+ @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.
+ 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");
+
+ 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");
+ }
+ }
+
+ @Test
+ @DisplayName("should not duplicate plugin in build/plugins when already locally declared")
+ void shouldNotDuplicatePluginInBuildPluginsWhenAlreadyDeclared() throws Exception {
+ 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");
+
+ Editor editor = new Editor(document);
+ Element root = editor.root();
+ Element buildPlugins = root.childElement("build")
+ .flatMap(b -> b.childElement("plugins"))
+ .orElse(null);
+ assertNotNull(buildPlugins, "Should have build/plugins section");
+
+ long enforcerCount = buildPlugins
+ .childElements("plugin")
+ .filter(p -> "maven-enforcer-plugin"
+ .equals(p.childElement("artifactId")
+ .map(Element::textContentTrimmed)
+ .orElse("")))
+ .count();
+ assertEquals(1, enforcerCount, "Should have exactly one maven-enforcer-plugin in build/plugins");
+
+ String version = buildPlugins
+ .childElements("plugin")
+ .filter(p -> "maven-enforcer-plugin"
+ .equals(p.childElement("artifactId")
+ .map(Element::textContentTrimmed)
+ .orElse("")))
+ .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
(maven) 03/04: Remove invalid combine.self and combine.children attributes instead of converting them
Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch maven-4.0.x-test-fixes
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 680cd1d14129a1d1d42f2f137da8513d80d9c7a4
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Wed May 27 00:55:07 2026 +0200
Remove invalid combine.self and combine.children attributes instead of converting them
Maven 3 silently ignored invalid combine.self and combine.children
attribute values, so removing them entirely preserves actual Maven 3
behavior. Previously mvnup converted specific invalid values (e.g.
combine.self="append" → "merge"), which could miss other invalid values
and subtly changed semantics. Now both fixers detect any invalid value
against the full set of valid values (combine.self: override/merge/remove;
combine.children: append/merge), remove the attribute, and collect to a
list before mutating to avoid potential stream processing issues.
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../mvnup/goals/CompatibilityFixStrategy.java | 66 ++--
.../mvnup/goals/CompatibilityFixStrategyTest.java | 410 +++++++++++++++++++++
2 files changed, 452 insertions(+), 24 deletions(-)
diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
index c8c99d30ac..343caf147b 100644
--- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
+++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
@@ -77,6 +77,10 @@ public class CompatibilityFixStrategy extends AbstractUpgradeStrategy {
private static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{([^}]+)}");
+ private static final Set<String> VALID_COMBINE_SELF_VALUES = Set.of(COMBINE_OVERRIDE, COMBINE_MERGE, "remove");
+
+ private static final Set<String> VALID_COMBINE_CHILDREN_VALUES = Set.of(COMBINE_APPEND, COMBINE_MERGE);
+
@Override
public boolean isApplicable(UpgradeContext context) {
UpgradeOptions options = getOptions(context);
@@ -165,44 +169,44 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
/**
* Fixes unsupported combine.children attribute values.
- * Maven 4 only supports 'append' and 'merge', not 'override'.
+ * Maven 4 only supports 'append' and 'merge' (default is merge).
+ * Invalid values are removed entirely since Maven 3 silently ignored them.
*/
private boolean fixUnsupportedCombineChildrenAttributes(Document pomDocument, UpgradeContext context) {
- boolean fixed = false;
Element root = pomDocument.root();
- // Find all elements with combine.children="override" and change to "merge"
- long fixedCombineChildrenCount = findElementsWithAttribute(root, COMBINE_CHILDREN, COMBINE_OVERRIDE)
- .peek(element -> {
- element.attributeObject(COMBINE_CHILDREN).value(COMBINE_MERGE);
- context.detail("Fixed: " + COMBINE_CHILDREN + "='" + COMBINE_OVERRIDE + "' → '" + COMBINE_MERGE
- + "' in " + element.name());
- })
- .count();
- fixed |= fixedCombineChildrenCount > 0;
+ List<Element> invalidElements = findElementsWithInvalidAttribute(
+ root, COMBINE_CHILDREN, VALID_COMBINE_CHILDREN_VALUES)
+ .toList();
- return fixed;
+ for (Element element : invalidElements) {
+ String invalidValue = element.attribute(COMBINE_CHILDREN);
+ element.removeAttribute(COMBINE_CHILDREN);
+ context.detail(
+ "Fixed: removed invalid " + COMBINE_CHILDREN + "='" + invalidValue + "' from " + element.name());
+ }
+
+ return !invalidElements.isEmpty();
}
/**
* Fixes unsupported combine.self attribute values.
- * Maven 4 only supports 'override', 'merge', and 'remove' (default is merge), not 'append'.
+ * Maven 4 only supports 'override', 'merge', and 'remove' (default is merge).
+ * Invalid values are removed entirely since Maven 3 silently ignored them.
*/
private boolean fixUnsupportedCombineSelfAttributes(Document pomDocument, UpgradeContext context) {
- boolean fixed = false;
Element root = pomDocument.root();
- // Find all elements with combine.self="append" and change to "merge"
- long fixedCombineSelfCount = findElementsWithAttribute(root, COMBINE_SELF, COMBINE_APPEND)
- .peek(element -> {
- element.attributeObject(COMBINE_SELF).value(COMBINE_MERGE);
- context.detail("Fixed: " + COMBINE_SELF + "='" + COMBINE_APPEND + "' → '" + COMBINE_MERGE + "' in "
- + element.name());
- })
- .count();
- fixed |= fixedCombineSelfCount > 0;
+ List<Element> invalidElements = findElementsWithInvalidAttribute(root, COMBINE_SELF, VALID_COMBINE_SELF_VALUES)
+ .toList();
- return fixed;
+ for (Element element : invalidElements) {
+ String invalidValue = element.attribute(COMBINE_SELF);
+ element.removeAttribute(COMBINE_SELF);
+ context.detail("Fixed: removed invalid " + COMBINE_SELF + "='" + invalidValue + "' from " + element.name());
+ }
+
+ return !invalidElements.isEmpty();
}
/**
@@ -527,6 +531,20 @@ private Stream<Element> findElementsWithAttribute(Element element, String attrib
.flatMap(child -> findElementsWithAttribute(child, attributeName, attributeValue)));
}
+ /**
+ * Recursively finds all elements with an attribute whose value is not in the set of valid values.
+ */
+ private Stream<Element> findElementsWithInvalidAttribute(
+ Element element, String attributeName, Set<String> validValues) {
+ return Stream.concat(
+ Stream.of(element).filter(e -> {
+ String attr = e.attribute(attributeName);
+ return attr != null && !validValues.contains(attr);
+ }),
+ element.childElements()
+ .flatMap(child -> findElementsWithInvalidAttribute(child, attributeName, validValues)));
+ }
+
/**
* Helper methods extracted from BaseUpgradeGoal for compatibility fixes.
*/
diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
index 7795c25d9d..051a33fd34 100644
--- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
+++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
@@ -140,6 +140,416 @@ void shouldHandleAllOptionsDisabled() {
}
}
+ @Nested
+ @DisplayName("Unsupported combine.children Attribute Fixes")
+ class UnsupportedCombineChildrenFixesTests {
+
+ @Test
+ @DisplayName("should remove combine.children='override' attribute")
+ void shouldRemoveCombineChildrenOverride() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs combine.children="override">
+ <arg>-Xlint:all</arg>
+ </compilerArgs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertTrue(result.modifiedCount() > 0, "Should have fixed combine.children attribute");
+
+ String xml = DomUtils.toXml(document);
+ assertFalse(xml.contains("combine.children"), "combine.children attribute should be removed entirely");
+ assertTrue(xml.contains("compilerArgs"), "Element should still exist");
+ }
+
+ @Test
+ @DisplayName("should remove any invalid combine.children value")
+ void shouldRemoveAnyCombineChildrenInvalidValue() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs combine.children="invalid_value">
+ <arg>-Xlint:all</arg>
+ </compilerArgs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertTrue(result.modifiedCount() > 0, "Should have fixed combine.children attribute");
+
+ String xml = DomUtils.toXml(document);
+ assertFalse(xml.contains("combine.children"), "combine.children attribute should be removed entirely");
+ }
+
+ @Test
+ @DisplayName("should not remove valid combine.children values")
+ void shouldNotRemoveValidCombineChildrenValues() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs combine.children="append">
+ <arg>-Xlint:all</arg>
+ </compilerArgs>
+ <includes combine.children="merge">
+ <include>**/*.java</include>
+ </includes>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertEquals(0, result.modifiedCount(), "Should not have modified any POMs");
+
+ String xml = DomUtils.toXml(document);
+ assertTrue(xml.contains("combine.children=\"append\""), "append should be preserved");
+ assertTrue(xml.contains("combine.children=\"merge\""), "merge should be preserved");
+ }
+
+ @Test
+ @DisplayName("should remove invalid combine.children in profile plugin configuration")
+ void shouldRemoveInvalidCombineChildrenInProfile() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <profiles>
+ <profile>
+ <id>release</id>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-assembly-plugin</artifactId>
+ <configuration>
+ <descriptorRefs combine.children="override">
+ <descriptorRef>src</descriptorRef>
+ </descriptorRefs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
+ </profiles>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertTrue(result.modifiedCount() > 0, "Should have fixed combine.children in profile");
+
+ String xml = DomUtils.toXml(document);
+ assertFalse(xml.contains("combine.children"), "combine.children attribute should be removed from profile");
+ }
+ }
+
+ @Nested
+ @DisplayName("Unsupported combine.self Attribute Fixes")
+ class UnsupportedCombineSelfFixesTests {
+
+ @Test
+ @DisplayName("should remove combine.self='append' attribute")
+ void shouldRemoveCombineSelfAppend() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-assembly-plugin</artifactId>
+ <configuration>
+ <descriptorRefs combine.self="append">
+ <descriptorRef>jar-with-dependencies</descriptorRef>
+ </descriptorRefs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertTrue(result.modifiedCount() > 0, "Should have fixed combine.self attribute");
+
+ String xml = DomUtils.toXml(document);
+ assertFalse(xml.contains("combine.self"), "combine.self attribute should be removed entirely");
+ assertTrue(xml.contains("descriptorRefs"), "Element should still exist");
+ }
+
+ @Test
+ @DisplayName("should remove any invalid combine.self value")
+ void shouldRemoveAnyCombineSelfInvalidValue() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs combine.self="invalid_value">
+ <arg>-Xlint:all</arg>
+ </compilerArgs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertTrue(result.modifiedCount() > 0, "Should have fixed combine.self attribute");
+
+ String xml = DomUtils.toXml(document);
+ assertFalse(xml.contains("combine.self"), "combine.self attribute should be removed entirely");
+ }
+
+ @Test
+ @DisplayName("should not remove valid combine.self values")
+ void shouldNotRemoveValidCombineSelfValues() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <source combine.self="override">11</source>
+ <target combine.self="merge">11</target>
+ <compilerArgs combine.self="remove"/>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertEquals(0, result.modifiedCount(), "Should not have modified any POMs");
+
+ String xml = DomUtils.toXml(document);
+ assertTrue(xml.contains("combine.self=\"override\""), "override should be preserved");
+ assertTrue(xml.contains("combine.self=\"merge\""), "merge should be preserved");
+ assertTrue(xml.contains("combine.self=\"remove\""), "remove should be preserved");
+ }
+
+ @Test
+ @DisplayName("should remove invalid combine.self in profile plugin configuration")
+ void shouldRemoveInvalidCombineSelfInProfile() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <profiles>
+ <profile>
+ <id>release</id>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-assembly-plugin</artifactId>
+ <configuration>
+ <descriptorRefs combine.self="append">
+ <descriptorRef>src</descriptorRef>
+ </descriptorRefs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
+ </profiles>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertTrue(result.modifiedCount() > 0, "Should have fixed combine.self in profile");
+
+ String xml = DomUtils.toXml(document);
+ assertFalse(xml.contains("combine.self"), "combine.self attribute should be removed from profile");
+ }
+
+ @Test
+ @DisplayName("should remove all occurrences of invalid combine.self across the POM")
+ void shouldRemoveAllOccurrencesOfInvalidCombineSelf() throws Exception {
+ String pomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0.0</version>
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs combine.self="append">
+ <arg>-Xlint:all</arg>
+ </compilerArgs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-assembly-plugin</artifactId>
+ <configuration>
+ <descriptorRefs combine.self="append">
+ <descriptorRef>jar-with-dependencies</descriptorRef>
+ </descriptorRefs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ <profiles>
+ <profile>
+ <id>release</id>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-assembly-plugin</artifactId>
+ <configuration>
+ <descriptorRefs combine.self="append">
+ <descriptorRef>src</descriptorRef>
+ </descriptorRefs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
+ </profiles>
+ </project>
+ """;
+
+ Document document = Document.of(pomXml);
+ Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
+
+ UpgradeContext context = createMockContext();
+ UpgradeResult result = strategy.doApply(context, pomMap);
+
+ assertTrue(result.success(), "Compatibility fix should succeed");
+ assertTrue(result.modifiedCount() > 0, "Should have fixed combine.self attributes");
+
+ String xml = DomUtils.toXml(document);
+ assertFalse(xml.contains("combine.self"), "All combine.self attributes should be removed");
+ }
+ }
+
@Nested
@DisplayName("Duplicate Dependency Fixes")
class DuplicateDependencyFixesTests {