From 4032fc8225c879c86245a232787802efcece5d74 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 30 Jul 2026 13:41:36 +0200 Subject: [PATCH 1/2] Fix BOM consumer POM leaving property references unresolved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route all BOMs through buildBom() which uses getEffectiveModel() (fully interpolated), instead of buildBomWithoutFlatten() which used getRawModel(). The latter was incompatible with transformBom() which strips parent and properties — leaving dangling ${...} references in the consumer POM. Delete the now-dead buildBomWithoutFlatten() method. Reported by Karl Heinz Marbaise during the 4.0.0-rc-6 vote. Closes #12625 --- .../impl/DefaultConsumerPomBuilder.java | 16 +- ...TBomConsumerPomPropertyResolutionTest.java | 184 ++++++++++++++++++ .../bom/pom.xml | 61 ++++++ .../mod-1/pom.xml | 35 ++++ .../pom.xml | 41 ++++ 5 files changed, 326 insertions(+), 11 deletions(-) create mode 100644 its/core-it-suite/src/test/java/org/apache/maven/it/MavenITBomConsumerPomPropertyResolutionTest.java create mode 100644 its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/bom/pom.xml create mode 100644 its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/mod-1/pom.xml create mode 100644 its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/pom.xml diff --git a/impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/DefaultConsumerPomBuilder.java b/impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/DefaultConsumerPomBuilder.java index 2de90988a190..c9c13f89e22a 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/DefaultConsumerPomBuilder.java +++ b/impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/DefaultConsumerPomBuilder.java @@ -89,9 +89,12 @@ public Model build(RepositorySystemSession session, MavenProject project, ModelS if (!flattenEnabled) { // When flattening is disabled, treat non-POM projects like parent POMs // Apply only basic transformations without flattening dependency management - // However, BOMs still need special handling to transform packaging from "bom" to "pom" + // BOMs always need the effective (interpolated) model because transformBom() + // strips parent and properties — any ${...} references would become dangling. + // The flatten flag has no semantic effect on BOMs (transformBom always produces + // a self-contained POM), so we use the same buildBom() path regardless. if (isBom) { - return buildBomWithoutFlatten(session, project, src); + return buildBom(session, project, src); } else { Model result = buildPom(session, project, src); // Validate POM-packaged projects (parent POMs): if the consumer POM cannot be @@ -136,15 +139,6 @@ protected Model buildPom(RepositorySystemSession session, MavenProject project, return transformPom(model, project); } - protected Model buildBomWithoutFlatten(RepositorySystemSession session, MavenProject project, ModelSource src) - throws ModelBuilderException { - ModelBuilderResult result = buildModel(session, project, src); - Model model = result.getRawModel(); - // For BOMs without flattening, we just need to transform the packaging from "bom" to "pom" - // but keep everything else from the raw model (including unresolved versions) - return transformBom(model, project); - } - protected Model buildBom(RepositorySystemSession session, MavenProject project, ModelSource src) throws ModelBuilderException { ModelBuilderResult result = buildModel(session, project, src); diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITBomConsumerPomPropertyResolutionTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITBomConsumerPomPropertyResolutionTest.java new file mode 100644 index 000000000000..eaca1545f515 --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITBomConsumerPomPropertyResolutionTest.java @@ -0,0 +1,184 @@ +/* + * 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.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that the BOM consumer POM resolves property-referenced dependency + * versions, especially when the property is defined in a parent POM. + *

+ * Reproducer for the issue reported by Karl Heinz Marbaise during the + * Maven 4.0.0-rc-6 vote: when a BOM's {@code } uses a + * property like {@code ${junit.version}} for an external dependency version, + * and that property is defined in the parent POM, the consumer POM leaves the + * property reference unresolved. This happens because + * {@code DefaultConsumerPomBuilder.buildBomWithoutFlatten()} uses + * {@code getRawModel()} (no interpolation) and then {@code transformBom()} + * strips the parent and properties — leaving dangling {@code ${...}} references. + * + * @see Vote thread + * @since 4.0.0 + */ +class MavenITBomConsumerPomPropertyResolutionTest extends AbstractMavenIntegrationTestCase { + + /** + * Verify that the BOM consumer POM (default, no flatten) resolves + * property-referenced versions in dependencyManagement. + *

+ * The parent POM defines {@code junit.version=4.13.2}. The BOM child + * references it as {@code ${junit.version}}. The consumer POM must + * contain the resolved literal {@code 4.13.2}, not the raw property + * expression, because the consumer POM has no parent or properties + * section to resolve it from. + */ + @Test + void testBomConsumerPomResolvesParentProperties() throws Exception { + Path basedir = extractResources("/bom-consumer-pom-property-resolution"); + + Verifier verifier = newVerifier(basedir); + verifier.deleteArtifacts("org.apache.maven.its.bom-property"); + verifier.addCliArguments("install"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // Read the consumer POM that was installed to the local repo + Path consumerPomPath = + verifier.getArtifactPath("org.apache.maven.its.bom-property", "bom", "1.0.0-SNAPSHOT", "pom"); + + assertTrue(Files.exists(consumerPomPath), "Consumer POM not found at " + consumerPomPath); + + List lines = Files.readAllLines(consumerPomPath); + String content = String.join("\n", lines); + + // 1. Packaging must be "pom" (not "bom") + assertTrue( + content.contains("pom"), + "Consumer POM packaging should be 'pom', not 'bom'"); + + // 2. The consumer POM strips parent and properties, so inherited + // fields must be inlined: groupId and version must be present + assertTrue( + content.contains("org.apache.maven.its.bom-property"), + "Consumer POM must have groupId inlined (parent is stripped).\nActual:\n" + content); + assertTrue( + content.contains("1.0.0-SNAPSHOT"), + "Consumer POM must have version inlined (parent is stripped).\nActual:\n" + content); + + // 3. The key assertion: no unresolved ${...} property references + // anywhere in the consumer POM. The parent and properties sections + // are removed, so any remaining ${...} would be unresolvable. + assertFalse( + content.contains("${"), + "Consumer POM must not contain unresolved property references.\nActual:\n" + content); + + // 4. Specifically: junit version must be the resolved literal "4.13.2" + assertTrue( + content.contains("4.13.2"), + "Consumer POM must contain resolved junit version '4.13.2'.\nActual:\n" + content); + } + + /** + * Verify that the BOM consumer POM with flatten=true also resolves + * property-referenced versions (this path uses getEffectiveModel() + * and should already work). + */ + @Test + void testBomConsumerPomWithFlattenResolvesParentProperties() throws Exception { + Path basedir = extractResources("/bom-consumer-pom-property-resolution"); + + Verifier verifier = newVerifier(basedir); + verifier.deleteArtifacts("org.apache.maven.its.bom-property"); + verifier.addCliArguments("install", "-Dmaven.consumer.pom.flatten=true"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + Path consumerPomPath = + verifier.getArtifactPath("org.apache.maven.its.bom-property", "bom", "1.0.0-SNAPSHOT", "pom"); + + assertTrue(Files.exists(consumerPomPath), "Consumer POM not found at " + consumerPomPath); + + List lines = Files.readAllLines(consumerPomPath); + String content = String.join("\n", lines); + + // Packaging must be "pom" + assertTrue(content.contains("pom"), "Consumer POM packaging should be 'pom'"); + + // No unresolved property references + assertFalse( + content.contains("${"), + "Consumer POM must not contain unresolved property references.\nActual:\n" + content); + + // junit version must be the resolved literal + assertTrue( + content.contains("4.13.2"), + "Consumer POM must contain resolved junit version '4.13.2'.\nActual:\n" + content); + } + + /** + * Same test as {@link #testBomConsumerPomResolvesParentProperties} but verifies + * the consumer POM that gets written to the project-local repo (not user-home), + * using the "-consumer" classifier POM. + */ + @Test + void testConsumerPomInProjectLocalRepo() throws Exception { + Path basedir = extractResources("/bom-consumer-pom-property-resolution"); + + Verifier verifier = newVerifier(basedir); + verifier.deleteArtifacts("org.apache.maven.its.bom-property"); + verifier.addCliArguments("install"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // Check the consumer POM in the project-local-repo + Path consumerPom = basedir.resolve(Path.of( + "target", + "project-local-repo", + "org.apache.maven.its.bom-property", + "bom", + "1.0.0-SNAPSHOT", + "bom-1.0.0-SNAPSHOT-consumer.pom")); + + assertTrue(Files.exists(consumerPom), "Consumer POM not found at " + consumerPom); + + String content = Files.readString(consumerPom); + + // Must not contain raw property references + assertFalse( + content.contains("${"), + "Consumer POM must not contain unresolved property references.\nActual:\n" + content); + + // Must contain the resolved version + assertTrue( + content.contains("4.13.2"), + "Consumer POM must contain resolved junit version '4.13.2'.\nActual:\n" + content); + + // Must have packaging=pom + assertEquals(-1, content.indexOf("bom"), "Consumer POM must not have bom packaging"); + assertTrue(content.contains("pom"), "Consumer POM must have pom packaging"); + } +} diff --git a/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/bom/pom.xml b/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/bom/pom.xml new file mode 100644 index 000000000000..e1fba27622be --- /dev/null +++ b/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/bom/pom.xml @@ -0,0 +1,61 @@ + + + + 4.0.0 + + + org.apache.maven.its.bom-property + parent + 1.0.0-SNAPSHOT + + + bom + bom + + BOM with parent-defined property versions + + + + + + + + org.apache.maven.its.bom-property + mod-1 + + + + junit + junit + ${junit.version} + + + + diff --git a/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/mod-1/pom.xml b/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/mod-1/pom.xml new file mode 100644 index 000000000000..a2de3e622ae6 --- /dev/null +++ b/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/mod-1/pom.xml @@ -0,0 +1,35 @@ + + + + 4.0.0 + + + org.apache.maven.its.bom-property + parent + 1.0.0-SNAPSHOT + + + mod-1 + jar + + Module 1 + diff --git a/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/pom.xml b/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/pom.xml new file mode 100644 index 000000000000..b2f2df320223 --- /dev/null +++ b/its/core-it-suite/src/test/resources/bom-consumer-pom-property-resolution/pom.xml @@ -0,0 +1,41 @@ + + + + 4.0.0 + + org.apache.maven.its.bom-property + parent + 1.0.0-SNAPSHOT + pom + + BOM Consumer POM Property Resolution Test + + + + 4.13.2 + + + + mod-1 + bom + + From cdcca89ff72d399d22ea9a4db2442023b9afa1f0 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 30 Jul 2026 15:01:04 +0200 Subject: [PATCH 2/2] Adapt IT test for maven-4.0.x AbstractMavenIntegrationTestCase API The maven-4.0.x branch uses the older IT helper API where extractResources() returns File, newVerifier() takes String, and getArtifactPath() returns String. The test also needs a constructor with a version range. Adapt accordingly. Co-Authored-By: Claude Opus 4.6 --- ...TBomConsumerPomPropertyResolutionTest.java | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITBomConsumerPomPropertyResolutionTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITBomConsumerPomPropertyResolutionTest.java index eaca1545f515..359d54bee0e6 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITBomConsumerPomPropertyResolutionTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITBomConsumerPomPropertyResolutionTest.java @@ -20,7 +20,9 @@ import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -46,6 +48,10 @@ */ class MavenITBomConsumerPomPropertyResolutionTest extends AbstractMavenIntegrationTestCase { + MavenITBomConsumerPomPropertyResolutionTest() { + super("[4.0.0-rc-4,)"); + } + /** * Verify that the BOM consumer POM (default, no flatten) resolves * property-referenced versions in dependencyManagement. @@ -58,21 +64,25 @@ class MavenITBomConsumerPomPropertyResolutionTest extends AbstractMavenIntegrati */ @Test void testBomConsumerPomResolvesParentProperties() throws Exception { - Path basedir = extractResources("/bom-consumer-pom-property-resolution"); + Path basedir = + extractResources("/bom-consumer-pom-property-resolution").getAbsoluteFile().toPath(); - Verifier verifier = newVerifier(basedir); + Verifier verifier = newVerifier(basedir.toString()); verifier.deleteArtifacts("org.apache.maven.its.bom-property"); verifier.addCliArguments("install"); verifier.execute(); verifier.verifyErrorFreeLog(); // Read the consumer POM that was installed to the local repo - Path consumerPomPath = - verifier.getArtifactPath("org.apache.maven.its.bom-property", "bom", "1.0.0-SNAPSHOT", "pom"); + Path consumerPomPath = Paths.get( + verifier.getArtifactPath("org.apache.maven.its.bom-property", "bom", "1.0.0-SNAPSHOT", "pom")); assertTrue(Files.exists(consumerPomPath), "Consumer POM not found at " + consumerPomPath); - List lines = Files.readAllLines(consumerPomPath); + List lines; + try (Stream stream = Files.lines(consumerPomPath)) { + lines = stream.toList(); + } String content = String.join("\n", lines); // 1. Packaging must be "pom" (not "bom") @@ -109,20 +119,24 @@ void testBomConsumerPomResolvesParentProperties() throws Exception { */ @Test void testBomConsumerPomWithFlattenResolvesParentProperties() throws Exception { - Path basedir = extractResources("/bom-consumer-pom-property-resolution"); + Path basedir = + extractResources("/bom-consumer-pom-property-resolution").getAbsoluteFile().toPath(); - Verifier verifier = newVerifier(basedir); + Verifier verifier = newVerifier(basedir.toString()); verifier.deleteArtifacts("org.apache.maven.its.bom-property"); verifier.addCliArguments("install", "-Dmaven.consumer.pom.flatten=true"); verifier.execute(); verifier.verifyErrorFreeLog(); - Path consumerPomPath = - verifier.getArtifactPath("org.apache.maven.its.bom-property", "bom", "1.0.0-SNAPSHOT", "pom"); + Path consumerPomPath = Paths.get( + verifier.getArtifactPath("org.apache.maven.its.bom-property", "bom", "1.0.0-SNAPSHOT", "pom")); assertTrue(Files.exists(consumerPomPath), "Consumer POM not found at " + consumerPomPath); - List lines = Files.readAllLines(consumerPomPath); + List lines; + try (Stream stream = Files.lines(consumerPomPath)) { + lines = stream.toList(); + } String content = String.join("\n", lines); // Packaging must be "pom" @@ -146,9 +160,10 @@ void testBomConsumerPomWithFlattenResolvesParentProperties() throws Exception { */ @Test void testConsumerPomInProjectLocalRepo() throws Exception { - Path basedir = extractResources("/bom-consumer-pom-property-resolution"); + Path basedir = + extractResources("/bom-consumer-pom-property-resolution").getAbsoluteFile().toPath(); - Verifier verifier = newVerifier(basedir); + Verifier verifier = newVerifier(basedir.toString()); verifier.deleteArtifacts("org.apache.maven.its.bom-property"); verifier.addCliArguments("install"); verifier.execute();