Skip to content

Commit 504785b

Browse files
gnodetclaude
gnodet
and
claude
authored
Fix mvnup effective model analysis for CI-friendly parent versions (#12205)
* Fix mvnup effective model analysis for CI-friendly parent versions mvnup's PluginUpgradeStrategy copied POMs to a temp directory for effective model analysis. That temp directory lacked .mvn, so root detection failed for child modules with ${revision} parent versions, producing "Parent POM is located above the root directory" errors. Eliminate the temp directory entirely — build effective models from the original file paths, which already have proper .mvn and project structure for root detection. Also fix DefaultModelBuilder.doReadFileModel() to: - Enter the parent resolution block when parent version contains expressions (${revision}, etc.) - Only enforce isParentWithinRootDirectory when rootDirectory came from the session, not the fallback heuristic - Accept parent version match when version contains an expression Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix Spotless formatting violation Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c7d96d3 commit 504785b

6 files changed

Lines changed: 192 additions & 17 deletions

File tree

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

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
*/
1919
package org.apache.maven.cling.invoker.mvnup.goals;
2020

21+
import java.io.IOException;
22+
import java.nio.file.Files;
2123
import java.nio.file.Path;
2224
import java.nio.file.Paths;
2325
import java.util.List;
@@ -776,8 +778,6 @@ void shouldDetectInheritedPluginsFromRemoteParent() throws Exception {
776778
// org.apache:apache:23 defines maven-enforcer-plugin:1.4.1 in pluginManagement.
777779
// A child POM that inherits from this parent should get pluginManagement overrides
778780
// added by mvnup for plugins that need Maven 4 compatibility upgrades.
779-
// Uses an absolute path because the effective model analysis path resolution
780-
// requires it to match between phases.
781781
String pomXml = """
782782
<?xml version="1.0" encoding="UTF-8"?>
783783
<project xmlns="http://maven.apache.org/POM/4.0.0">
@@ -793,20 +793,35 @@ void shouldDetectInheritedPluginsFromRemoteParent() throws Exception {
793793
</project>
794794
""";
795795

796-
Document document = Document.of(pomXml);
797-
Path pomPath = Paths.get("/project/pom.xml").toAbsolutePath();
798-
Map<Path, Document> pomMap = Map.of(pomPath, document);
796+
Path tempDir = Files.createTempDirectory("mvnup-test-");
797+
try {
798+
Files.createDirectories(tempDir.resolve(".mvn"));
799+
Path pomPath = tempDir.resolve("pom.xml");
800+
Files.writeString(pomPath, pomXml);
799801

800-
UpgradeContext context = createMockContext();
801-
UpgradeResult result = strategy.doApply(context, pomMap);
802+
Document document = Document.of(pomXml);
803+
Map<Path, Document> pomMap = Map.of(pomPath, document);
802804

803-
assertTrue(result.success(), "Strategy should succeed");
804-
assertTrue(result.modifiedCount() > 0, "Should have added plugin management for inherited plugins");
805+
UpgradeContext context = createMockContext();
806+
UpgradeResult result = strategy.doApply(context, pomMap);
805807

806-
String xml = DomUtils.toXml(document);
807-
assertTrue(
808-
xml.contains("<artifactId>maven-enforcer-plugin</artifactId>"),
809-
"Should add pluginManagement for maven-enforcer-plugin inherited from parent");
808+
assertTrue(result.success(), "Strategy should succeed");
809+
assertTrue(result.modifiedCount() > 0, "Should have added plugin management for inherited plugins");
810+
811+
String xml = DomUtils.toXml(document);
812+
assertTrue(
813+
xml.contains("<artifactId>maven-enforcer-plugin</artifactId>"),
814+
"Should add pluginManagement for maven-enforcer-plugin inherited from parent");
815+
} finally {
816+
try (var walk = Files.walk(tempDir)) {
817+
walk.sorted(java.util.Comparator.reverseOrder()).forEach(p -> {
818+
try {
819+
Files.delete(p);
820+
} catch (IOException ignored) {
821+
}
822+
});
823+
}
824+
}
810825
}
811826

812827
@Test

‎impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,12 +1502,14 @@ Model doReadFileModel() throws ModelBuilderException {
15021502
ModelSource modelSource = request.getSource();
15031503
Model model;
15041504
Path rootDirectory;
1505+
boolean rootDirectoryFromSession = false;
15051506
setSource(modelSource.getLocation());
15061507
logger.debug("Reading file model from " + modelSource.getLocation());
15071508
try {
15081509
boolean strict = isBuildRequest();
15091510
try {
15101511
rootDirectory = request.getSession().getRootDirectory();
1512+
rootDirectoryFromSession = true;
15111513
} catch (IllegalStateException ignore) {
15121514
rootDirectory = modelSource.getPath();
15131515
while (rootDirectory != null && !Files.isDirectory(rootDirectory)) {
@@ -1586,7 +1588,8 @@ Model doReadFileModel() throws ModelBuilderException {
15861588
String artifactId = parent.getArtifactId();
15871589
String version = parent.getVersion();
15881590
String path = parent.getRelativePath();
1589-
if ((groupId == null || artifactId == null || version == null)
1591+
boolean versionContainsExpression = version != null && version.contains("${");
1592+
if ((groupId == null || artifactId == null || version == null || versionContainsExpression)
15901593
&& (path == null || !path.isEmpty())) {
15911594
Path pomFile = model.getPomFile();
15921595
Path relativePath = Paths.get(path != null ? path : "..");
@@ -1595,8 +1598,7 @@ Model doReadFileModel() throws ModelBuilderException {
15951598
pomPath = modelProcessor.locateExistingPom(pomPath);
15961599
}
15971600
if (pomPath != null && Files.isRegularFile(pomPath)) {
1598-
// Check if parent POM is above the root directory
1599-
if (!isParentWithinRootDirectory(pomPath, rootDirectory)) {
1601+
if (rootDirectoryFromSession && !isParentWithinRootDirectory(pomPath, rootDirectory)) {
16001602
add(
16011603
Severity.FATAL,
16021604
Version.BASE,
@@ -1614,7 +1616,9 @@ Model doReadFileModel() throws ModelBuilderException {
16141616
String parentVersion = getVersion(parentModel);
16151617
if ((groupId == null || groupId.equals(parentGroupId))
16161618
&& (artifactId == null || artifactId.equals(parentArtifactId))
1617-
&& (version == null || version.equals(parentVersion))) {
1619+
&& (version == null
1620+
|| version.equals(parentVersion)
1621+
|| versionContainsExpression)) {
16181622
model = model.withParent(parent.with()
16191623
.groupId(parentGroupId)
16201624
.artifactId(parentArtifactId)

‎impl/maven-impl/src/test/resources/poms/factory/ci-friendly-version.xml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!---
3+
Licensed to the Apache Software Foundation (ASF) under one or more
4+
contributor license agreements. See the NOTICE file distributed with
5+
this work for additional information regarding copyright ownership.
6+
The ASF licenses this file to You under the Apache License, Version 2.0
7+
(the "License"); you may not use this file except in compliance with
8+
the License. You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
-->
18+
<project xmlns="http://maven.apache.org/POM/4.0.0"
19+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
20+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
21+
<modelVersion>4.0.0</modelVersion>
22+
23+
<groupId>org.apache.maven.test</groupId>
24+
<artifactId>ci-friendly-version-test</artifactId>
25+
<version>${revision}</version>
26+
<packaging>pom</packaging>
27+
28+
<properties>
29+
<revision>1.0.0-SNAPSHOT</revision>
30+
</properties>
31+
</project>

‎its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12184CIFriendlyParentVersionTest.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.it;
20+
21+
import java.io.File;
22+
23+
import org.junit.jupiter.api.Test;
24+
25+
/**
26+
* Integration test for <a href="https://github.com/apache/maven/issues/12184">#12184</a>.
27+
* Verifies that CI-friendly {@code ${revision}} in parent version works in Maven 4
28+
* native mode (without maven3Personality) for model version 4.0.0 projects.
29+
*/
30+
class MavenITgh12184CIFriendlyParentVersionTest extends AbstractMavenIntegrationTestCase {
31+
32+
/**
33+
* Verify that a multi-module project with {@code ${revision}} in parent version
34+
* builds successfully when the property is defined in the parent POM properties.
35+
*/
36+
@Test
37+
void testCiFriendlyParentVersionFromProperties() throws Exception {
38+
File testDir = extractResources("/gh-12184-ci-friendly-parent-version");
39+
40+
Verifier verifier = newVerifier(testDir.getAbsolutePath());
41+
verifier.addCliArgument("validate");
42+
verifier.execute();
43+
verifier.verifyErrorFreeLog();
44+
}
45+
46+
/**
47+
* Verify that a multi-module project with {@code ${revision}} in parent version
48+
* builds successfully when the property is overridden via command line.
49+
*/
50+
@Test
51+
void testCiFriendlyParentVersionFromCli() throws Exception {
52+
File testDir = extractResources("/gh-12184-ci-friendly-parent-version");
53+
54+
Verifier verifier = newVerifier(testDir.getAbsolutePath());
55+
verifier.addCliArgument("-Drevision=2.0.0");
56+
verifier.addCliArgument("validate");
57+
verifier.execute();
58+
verifier.verifyErrorFreeLog();
59+
}
60+
}

‎its/core-it-suite/src/test/resources/gh-12184-ci-friendly-parent-version/child/pom.xml‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
Licensed to the Apache Software Foundation (ASF) under one
4+
or more contributor license agreements. See the NOTICE file
5+
distributed with this work for additional information
6+
regarding copyright ownership. The ASF licenses this file
7+
to you under the Apache License, Version 2.0 (the
8+
"License"); you may not use this file except in compliance
9+
with the License. You may obtain a copy of the License at
10+
11+
http://www.apache.org/licenses/LICENSE-2.0
12+
13+
Unless required by applicable law or agreed to in writing,
14+
software distributed under the License is distributed on an
15+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
KIND, either express or implied. See the License for the
17+
specific language governing permissions and limitations
18+
under the License.
19+
-->
20+
<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">
21+
<modelVersion>4.0.0</modelVersion>
22+
23+
<parent>
24+
<groupId>org.apache.maven.its.gh12184</groupId>
25+
<artifactId>ci-friendly-parent</artifactId>
26+
<version>${revision}</version>
27+
</parent>
28+
29+
<artifactId>ci-friendly-child</artifactId>
30+
</project>

‎its/core-it-suite/src/test/resources/gh-12184-ci-friendly-parent-version/pom.xml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
Licensed to the Apache Software Foundation (ASF) under one
4+
or more contributor license agreements. See the NOTICE file
5+
distributed with this work for additional information
6+
regarding copyright ownership. The ASF licenses this file
7+
to you under the Apache License, Version 2.0 (the
8+
"License"); you may not use this file except in compliance
9+
with the License. You may obtain a copy of the License at
10+
11+
http://www.apache.org/licenses/LICENSE-2.0
12+
13+
Unless required by applicable law or agreed to in writing,
14+
software distributed under the License is distributed on an
15+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
KIND, either express or implied. See the License for the
17+
specific language governing permissions and limitations
18+
under the License.
19+
-->
20+
<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">
21+
<modelVersion>4.0.0</modelVersion>
22+
23+
<groupId>org.apache.maven.its.gh12184</groupId>
24+
<artifactId>ci-friendly-parent</artifactId>
25+
<version>${revision}</version>
26+
<packaging>pom</packaging>
27+
28+
<modules>
29+
<module>child</module>
30+
</modules>
31+
32+
<properties>
33+
<revision>1.0.0-SNAPSHOT</revision>
34+
</properties>
35+
</project>

0 commit comments

Comments
 (0)