Skip to content

Commit 565c5b2

Browse files
gnodetclaude
gnodet
and
claude
authored
Use effective model to resolve properties from remote parent POMs in mvnup (#12158)
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 <noreply@anthropic.com>
1 parent 13f1608 commit 565c5b2

2 files changed

Lines changed: 140 additions & 0 deletions

File tree

‎impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
129129
Set<Path> errorPoms = new HashSet<>();
130130

131131
Set<String> allDefinedProperties = collectAllDefinedProperties(pomMap);
132+
allDefinedProperties.addAll(collectEffectiveProperties(context, pomMap));
132133

133134
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
134135
Path pomPath = entry.getKey();
@@ -380,6 +381,19 @@ private void collectPropertiesFromDom(Document document, Set<String> properties)
380381
propsElement.childElements().forEach(child -> properties.add(child.name())))));
381382
}
382383

384+
private Set<String> collectEffectiveProperties(UpgradeContext context, Map<Path, Document> pomMap) {
385+
Set<String> properties = new HashSet<>();
386+
for (Path pomPath : pomMap.keySet()) {
387+
try {
388+
org.apache.maven.api.model.Model effectiveModel = buildEffectiveModel(pomPath);
389+
properties.addAll(effectiveModel.getProperties().keySet());
390+
} catch (Exception e) {
391+
context.debug("Failed to build effective model for " + pomPath + ": " + e.getMessage());
392+
}
393+
}
394+
return properties;
395+
}
396+
383397
/**
384398
* Fixes dependencies with undefined property expressions by commenting them out.
385399
*/

‎impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,6 +1329,132 @@ void shouldRecognizePropertyFromGrandparent() throws Exception {
13291329
deps.childElements("dependency").count(),
13301330
"Dependency should not be commented out when property is inherited from grandparent");
13311331
}
1332+
1333+
@Test
1334+
@DisplayName("should not comment out when property is defined in external parent not in reactor")
1335+
void shouldNotCommentOutWhenPropertyFromExternalParentNotInReactor() throws Exception {
1336+
String externalParentPom = """
1337+
<?xml version="1.0" encoding="UTF-8"?>
1338+
<project xmlns="http://maven.apache.org/POM/4.0.0"
1339+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
1340+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
1341+
<modelVersion>4.0.0</modelVersion>
1342+
<groupId>com.example</groupId>
1343+
<artifactId>external-parent</artifactId>
1344+
<version>1.0.0</version>
1345+
<packaging>pom</packaging>
1346+
<properties>
1347+
<oak.version>1.62.0</oak.version>
1348+
</properties>
1349+
</project>
1350+
""";
1351+
1352+
String childPom = """
1353+
<?xml version="1.0" encoding="UTF-8"?>
1354+
<project xmlns="http://maven.apache.org/POM/4.0.0"
1355+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
1356+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
1357+
<modelVersion>4.0.0</modelVersion>
1358+
<parent>
1359+
<groupId>com.example</groupId>
1360+
<artifactId>external-parent</artifactId>
1361+
<version>1.0.0</version>
1362+
<relativePath>../external/pom.xml</relativePath>
1363+
</parent>
1364+
<artifactId>child</artifactId>
1365+
<dependencyManagement>
1366+
<dependencies>
1367+
<dependency>
1368+
<groupId>org.apache.jackrabbit</groupId>
1369+
<artifactId>oak-core</artifactId>
1370+
<version>${oak.version}</version>
1371+
</dependency>
1372+
</dependencies>
1373+
</dependencyManagement>
1374+
</project>
1375+
""";
1376+
1377+
Path externalDir = Files.createDirectories(tempDir.resolve("external"));
1378+
Path externalParentPath = externalDir.resolve("pom.xml");
1379+
Path childDir = Files.createDirectories(tempDir.resolve("child"));
1380+
Path childPomPath = childDir.resolve("pom.xml");
1381+
1382+
Files.writeString(externalParentPath, externalParentPom);
1383+
Files.writeString(childPomPath, childPom);
1384+
1385+
Document childDoc = Document.of(childPom);
1386+
Map<Path, Document> pomMap = Map.of(childPomPath, childDoc);
1387+
1388+
UpgradeContext context = createMockContext(childDir);
1389+
strategy.doApply(context, pomMap);
1390+
1391+
Element root = childDoc.root();
1392+
Element depMgmt = DomUtils.findChildElement(root, "dependencyManagement");
1393+
Element deps = DomUtils.findChildElement(depMgmt, "dependencies");
1394+
assertEquals(
1395+
1,
1396+
deps.childElements("dependency").count(),
1397+
"Managed dependency should not be commented out when property is inherited from external parent");
1398+
}
1399+
1400+
@Test
1401+
@DisplayName("should comment out when property is truly undefined even in effective model")
1402+
void shouldCommentOutWhenPropertyTrulyUndefinedInEffectiveModel() throws Exception {
1403+
String parentPom = """
1404+
<?xml version="1.0" encoding="UTF-8"?>
1405+
<project xmlns="http://maven.apache.org/POM/4.0.0"
1406+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
1407+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
1408+
<modelVersion>4.0.0</modelVersion>
1409+
<groupId>com.example</groupId>
1410+
<artifactId>parent</artifactId>
1411+
<version>1.0.0</version>
1412+
<packaging>pom</packaging>
1413+
</project>
1414+
""";
1415+
1416+
String childPom = """
1417+
<?xml version="1.0" encoding="UTF-8"?>
1418+
<project xmlns="http://maven.apache.org/POM/4.0.0"
1419+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
1420+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
1421+
<modelVersion>4.0.0</modelVersion>
1422+
<parent>
1423+
<groupId>com.example</groupId>
1424+
<artifactId>parent</artifactId>
1425+
<version>1.0.0</version>
1426+
<relativePath>../pom.xml</relativePath>
1427+
</parent>
1428+
<artifactId>child</artifactId>
1429+
<dependencyManagement>
1430+
<dependencies>
1431+
<dependency>
1432+
<groupId>com.google.guava</groupId>
1433+
<artifactId>guava</artifactId>
1434+
<version>${truly.undefined.prop}</version>
1435+
</dependency>
1436+
</dependencies>
1437+
</dependencyManagement>
1438+
</project>
1439+
""";
1440+
1441+
Path parentPath = tempDir.resolve("pom.xml");
1442+
Path childDir = Files.createDirectories(tempDir.resolve("child"));
1443+
Path childPomPath = childDir.resolve("pom.xml");
1444+
1445+
Files.writeString(parentPath, parentPom);
1446+
Files.writeString(childPomPath, childPom);
1447+
1448+
Document childDoc = Document.of(childPom);
1449+
Map<Path, Document> pomMap = Map.of(childPomPath, childDoc);
1450+
1451+
UpgradeContext context = createMockContext(childDir);
1452+
strategy.doApply(context, pomMap);
1453+
1454+
String xml = DomUtils.toXml(childDoc);
1455+
assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
1456+
assertTrue(xml.contains("truly.undefined.prop"), "Should mention the undefined property");
1457+
}
13321458
}
13331459

13341460
@Nested

0 commit comments

Comments
 (0)