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 <[email protected]> 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 <[email protected]> --- .../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
