This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch mulberry-motion in repository https://gitbox.apache.org/repos/asf/maven.git
commit 63121bc4911317e292cd63ccb361ce661b2d6b47 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 8739d17667..4fad1a81db 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(); @@ -375,6 +376,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
