diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java index fe508dd4a6fd..a5824e7b9f05 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java @@ -18,6 +18,8 @@ */ package org.apache.maven.cling.invoker.mvnup.goals; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; @@ -776,8 +778,6 @@ void shouldDetectInheritedPluginsFromRemoteParent() throws Exception { // org.apache:apache:23 defines maven-enforcer-plugin:1.4.1 in pluginManagement. // A child POM that inherits from this parent should get pluginManagement overrides // added by mvnup for plugins that need Maven 4 compatibility upgrades. - // Uses an absolute path because the effective model analysis path resolution - // requires it to match between phases. String pomXml = """ @@ -793,20 +793,35 @@ void shouldDetectInheritedPluginsFromRemoteParent() throws Exception { """; - Document document = Document.of(pomXml); - Path pomPath = Paths.get("/project/pom.xml").toAbsolutePath(); - Map pomMap = Map.of(pomPath, document); + Path tempDir = Files.createTempDirectory("mvnup-test-"); + try { + Files.createDirectories(tempDir.resolve(".mvn")); + Path pomPath = tempDir.resolve("pom.xml"); + Files.writeString(pomPath, pomXml); - UpgradeContext context = createMockContext(); - UpgradeResult result = strategy.doApply(context, pomMap); + Document document = Document.of(pomXml); + Map pomMap = Map.of(pomPath, document); - assertTrue(result.success(), "Strategy should succeed"); - assertTrue(result.modifiedCount() > 0, "Should have added plugin management for inherited plugins"); + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); - String xml = DomUtils.toXml(document); - assertTrue( - xml.contains("maven-enforcer-plugin"), - "Should add pluginManagement for maven-enforcer-plugin inherited from parent"); + assertTrue(result.success(), "Strategy should succeed"); + assertTrue(result.modifiedCount() > 0, "Should have added plugin management for inherited plugins"); + + String xml = DomUtils.toXml(document); + assertTrue( + xml.contains("maven-enforcer-plugin"), + "Should add pluginManagement for maven-enforcer-plugin inherited from parent"); + } finally { + try (var walk = Files.walk(tempDir)) { + walk.sorted(java.util.Comparator.reverseOrder()).forEach(p -> { + try { + Files.delete(p); + } catch (IOException ignored) { + } + }); + } + } } @Test diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java index 0e48da1a02b4..5e917c76fd8e 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java @@ -1502,12 +1502,14 @@ Model doReadFileModel() throws ModelBuilderException { ModelSource modelSource = request.getSource(); Model model; Path rootDirectory; + boolean rootDirectoryFromSession = false; setSource(modelSource.getLocation()); logger.debug("Reading file model from " + modelSource.getLocation()); try { boolean strict = isBuildRequest(); try { rootDirectory = request.getSession().getRootDirectory(); + rootDirectoryFromSession = true; } catch (IllegalStateException ignore) { rootDirectory = modelSource.getPath(); while (rootDirectory != null && !Files.isDirectory(rootDirectory)) { @@ -1586,7 +1588,8 @@ Model doReadFileModel() throws ModelBuilderException { String artifactId = parent.getArtifactId(); String version = parent.getVersion(); String path = parent.getRelativePath(); - if ((groupId == null || artifactId == null || version == null) + boolean versionContainsExpression = version != null && version.contains("${"); + if ((groupId == null || artifactId == null || version == null || versionContainsExpression) && (path == null || !path.isEmpty())) { Path pomFile = model.getPomFile(); Path relativePath = Paths.get(path != null ? path : ".."); @@ -1595,8 +1598,7 @@ Model doReadFileModel() throws ModelBuilderException { pomPath = modelProcessor.locateExistingPom(pomPath); } if (pomPath != null && Files.isRegularFile(pomPath)) { - // Check if parent POM is above the root directory - if (!isParentWithinRootDirectory(pomPath, rootDirectory)) { + if (rootDirectoryFromSession && !isParentWithinRootDirectory(pomPath, rootDirectory)) { add( Severity.FATAL, Version.BASE, @@ -1614,7 +1616,9 @@ Model doReadFileModel() throws ModelBuilderException { String parentVersion = getVersion(parentModel); if ((groupId == null || groupId.equals(parentGroupId)) && (artifactId == null || artifactId.equals(parentArtifactId)) - && (version == null || version.equals(parentVersion))) { + && (version == null + || version.equals(parentVersion) + || versionContainsExpression)) { model = model.withParent(parent.with() .groupId(parentGroupId) .artifactId(parentArtifactId) diff --git a/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-version.xml b/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-version.xml new file mode 100644 index 000000000000..c33bbe3af0a2 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-version.xml @@ -0,0 +1,31 @@ + + + + 4.0.0 + + org.apache.maven.test + ci-friendly-version-test + ${revision} + pom + + + 1.0.0-SNAPSHOT + + diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12184CIFriendlyParentVersionTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12184CIFriendlyParentVersionTest.java new file mode 100644 index 000000000000..2ba11172c0a9 --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12184CIFriendlyParentVersionTest.java @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.it; + +import java.io.File; + +import org.junit.jupiter.api.Test; + +/** + * Integration test for #12184. + * Verifies that CI-friendly {@code ${revision}} in parent version works in Maven 4 + * native mode (without maven3Personality) for model version 4.0.0 projects. + */ +class MavenITgh12184CIFriendlyParentVersionTest extends AbstractMavenIntegrationTestCase { + + /** + * Verify that a multi-module project with {@code ${revision}} in parent version + * builds successfully when the property is defined in the parent POM properties. + */ + @Test + void testCiFriendlyParentVersionFromProperties() throws Exception { + File testDir = extractResources("/gh-12184-ci-friendly-parent-version"); + + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + verifier.addCliArgument("validate"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + } + + /** + * Verify that a multi-module project with {@code ${revision}} in parent version + * builds successfully when the property is overridden via command line. + */ + @Test + void testCiFriendlyParentVersionFromCli() throws Exception { + File testDir = extractResources("/gh-12184-ci-friendly-parent-version"); + + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + verifier.addCliArgument("-Drevision=2.0.0"); + verifier.addCliArgument("validate"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + } +} diff --git a/its/core-it-suite/src/test/resources/gh-12184-ci-friendly-parent-version/child/pom.xml b/its/core-it-suite/src/test/resources/gh-12184-ci-friendly-parent-version/child/pom.xml new file mode 100644 index 000000000000..c826300db9e6 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-12184-ci-friendly-parent-version/child/pom.xml @@ -0,0 +1,30 @@ + + + + 4.0.0 + + + org.apache.maven.its.gh12184 + ci-friendly-parent + ${revision} + + + ci-friendly-child + diff --git a/its/core-it-suite/src/test/resources/gh-12184-ci-friendly-parent-version/pom.xml b/its/core-it-suite/src/test/resources/gh-12184-ci-friendly-parent-version/pom.xml new file mode 100644 index 000000000000..07ff1a29d9dc --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-12184-ci-friendly-parent-version/pom.xml @@ -0,0 +1,35 @@ + + + + 4.0.0 + + org.apache.maven.its.gh12184 + ci-friendly-parent + ${revision} + pom + + + child + + + + 1.0.0-SNAPSHOT + +