Skip to content

Commit 2941bf3

Browse files
gnodetclaude
gnodet
and
claude
authored
Fix mvnup effective model analysis for CI-friendly parent versions (#12205) (#12230)
* 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 * Fix Spotless formatting violation --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3c62e2e commit 2941bf3

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
@@ -1464,12 +1464,14 @@ Model doReadFileModel() throws ModelBuilderException {
14641464
ModelSource modelSource = request.getSource();
14651465
Model model;
14661466
Path rootDirectory;
1467+
boolean rootDirectoryFromSession = false;
14671468
setSource(modelSource.getLocation());
14681469
logger.debug("Reading file model from " + modelSource.getLocation());
14691470
try {
14701471
boolean strict = isBuildRequest();
14711472
try {
14721473
rootDirectory = request.getSession().getRootDirectory();
1474+
rootDirectoryFromSession = true;
14731475
} catch (IllegalStateException ignore) {
14741476
rootDirectory = modelSource.getPath();
14751477
while (rootDirectory != null && !Files.isDirectory(rootDirectory)) {
@@ -1559,7 +1561,8 @@ Model doReadFileModel() throws ModelBuilderException {
15591561
String artifactId = parent.getArtifactId();
15601562
String version = parent.getVersion();
15611563
String path = parent.getRelativePath();
1562-
if ((groupId == null || artifactId == null || version == null)
1564+
boolean versionContainsExpression = version != null && version.contains("${");
1565+
if ((groupId == null || artifactId == null || version == null || versionContainsExpression)
15631566
&& (path == null || !path.isEmpty())) {
15641567
Path pomFile = model.getPomFile();
15651568
Path relativePath = Paths.get(path != null ? path : "..");
@@ -1568,8 +1571,7 @@ Model doReadFileModel() throws ModelBuilderException {
15681571
pomPath = modelProcessor.locateExistingPom(pomPath);
15691572
}
15701573
if (pomPath != null && Files.isRegularFile(pomPath)) {
1571-
// Check if parent POM is above the root directory
1572-
if (!isParentWithinRootDirectory(pomPath, rootDirectory)) {
1574+
if (rootDirectoryFromSession && !isParentWithinRootDirectory(pomPath, rootDirectory)) {
15731575
add(
15741576
Severity.FATAL,
15751577
Version.BASE,
@@ -1587,7 +1589,9 @@ Model doReadFileModel() throws ModelBuilderException {
15871589
String parentVersion = getVersion(parentModel);
15881590
if ((groupId == null || groupId.equals(parentGroupId))
15891591
&& (artifactId == null || artifactId.equals(parentArtifactId))
1590-
&& (version == null || version.equals(parentVersion))) {
1592+
&& (version == null
1593+
|| version.equals(parentVersion)
1594+
|| versionContainsExpression)) {
15911595
model = model.withParent(parent.with()
15921596
.groupId(parentGroupId)
15931597
.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)