From 0cf3ac2b4158f5c7f6d9410eaaf09f706e4e2881 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 4 Jun 2026 10:22:14 +0200 Subject: [PATCH] Use request properties consistently for CI-friendly version interpolation (#12184) The model builder inconsistently used session properties for CI-friendly version replacement and model property merging, while model interpolation used request properties. When user properties on the ModelBuilderRequest differ from the Session (e.g. -Drevision=2.0.0 overriding a POM-defined revision), ${revision} in dependency versions and distributionManagement was not properly resolved. Fix three places in DefaultModelBuilder to use request properties: - getPropertiesWithProfiles(): use request system+user properties instead of session.getEffectiveProperties() - doReadFileModel(): use request.getUserProperties() instead of session.getUserProperties() for model property and profile merging Also fix DefaultConsumerPomBuilder.buildModel() to pass API Session properties (iSession) instead of RepositorySystemSession properties to the model builder request, ensuring consumer POM builds have access to the full set of user properties. Co-authored-by: Claude Opus 4.6 --- .../impl/DefaultConsumerPomBuilder.java | 4 +- .../maven/impl/model/DefaultModelBuilder.java | 14 +++-- .../impl/model/DefaultModelBuilderTest.java | 63 +++++++++++++++++++ .../poms/factory/ci-friendly-deps-no-prop.xml | 19 ++++++ .../poms/factory/ci-friendly-deps.xml | 30 +++++++++ 5 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 impl/maven-impl/src/test/resources/poms/factory/ci-friendly-deps-no-prop.xml create mode 100644 impl/maven-impl/src/test/resources/poms/factory/ci-friendly-deps.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 4bda69d040d5..d597b15ad39c 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 @@ -249,8 +249,8 @@ private ModelBuilderResult buildModel(RepositorySystemSession session, ModelSour request.session(iSession); request.source(src); request.locationTracking(false); - request.systemProperties(session.getSystemProperties()); - request.userProperties(session.getUserProperties()); + request.systemProperties(iSession.getSystemProperties()); + request.userProperties(iSession.getUserProperties()); request.lifecycleBindingsInjector(lifecycleBindingsInjector::injectLifecycleBindings); ModelBuilder.ModelBuilderSession mbSession = iSession.getData().get(SessionData.key(ModelBuilder.ModelBuilderSession.class)); 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 3c4cd3c4b08f..60e41950c3d0 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 @@ -723,8 +723,10 @@ private Map getPropertiesWithProfiles(Model model, Map newProps = merge(model.getProperties(), session.getUserProperties()); + // Override model properties with user properties (use request properties + // to ensure consistency with model interpolation) + Map userProps = request.getUserProperties(); + Map newProps = merge(model.getProperties(), userProps); if (newProps != null) { model = model.withProperties(newProps); } - model = model.withProfiles(merge(model.getProfiles(), session.getUserProperties())); + model = model.withProfiles(merge(model.getProfiles(), userProps)); } for (var transformer : transformers) { diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java index d361faad123a..a0133344ae6f 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java @@ -214,6 +214,69 @@ public void testDirectoryPropertiesInProfilesAndRepositories() { expectedUrl, result.getEffectiveModel().getRepositories().get(0).getUrl()); } + @Test + public void testCiFriendlyDependencyVersionInterpolation() { + // Test that ${revision} in dependency versions is interpolated using model properties + ModelBuilderRequest request = ModelBuilderRequest.builder() + .session(session) + .requestType(ModelBuilderRequest.RequestType.BUILD_PROJECT) + .source(Sources.buildSource(getPom("ci-friendly-deps"))) + .build(); + ModelBuilderResult result = builder.newSession().build(request); + assertNotNull(result); + Model effective = result.getEffectiveModel(); + assertEquals("1.0.0-SNAPSHOT", effective.getVersion()); + assertEquals(1, effective.getDependencies().size()); + assertEquals( + "1.0.0-SNAPSHOT", + effective.getDependencies().get(0).getVersion(), + "${revision} in dependency version should be interpolated"); + assertNotNull(effective.getDistributionManagement()); + assertEquals( + "releases-1.0.0-SNAPSHOT", + effective.getDistributionManagement().getRepository().getId(), + "${revision} in distributionManagement repository id should be interpolated"); + } + + @Test + public void testCiFriendlyDependencyVersionWithUserProperties() { + // Test that ${revision} in dependency versions is interpolated using user properties override + ModelBuilderRequest request = ModelBuilderRequest.builder() + .session(session) + .requestType(ModelBuilderRequest.RequestType.BUILD_PROJECT) + .userProperties(Map.of("revision", "2.0.0")) + .source(Sources.buildSource(getPom("ci-friendly-deps"))) + .build(); + ModelBuilderResult result = builder.newSession().build(request); + assertNotNull(result); + Model effective = result.getEffectiveModel(); + assertEquals("2.0.0", effective.getVersion()); + assertEquals(1, effective.getDependencies().size()); + assertEquals( + "2.0.0", + effective.getDependencies().get(0).getVersion(), + "${revision} in dependency version should be interpolated with user property"); + } + + @Test + public void testCiFriendlyDependencyVersionWithUserPropertiesOnly() { + ModelBuilderRequest request = ModelBuilderRequest.builder() + .session(session) + .requestType(ModelBuilderRequest.RequestType.BUILD_PROJECT) + .userProperties(Map.of("revision", "3.0.0")) + .source(Sources.buildSource(getPom("ci-friendly-deps-no-prop"))) + .build(); + ModelBuilderResult result = builder.newSession().build(request); + assertNotNull(result); + Model effective = result.getEffectiveModel(); + assertEquals("3.0.0", effective.getVersion(), "project version should use user property"); + assertEquals(1, effective.getDependencies().size()); + assertEquals( + "3.0.0", + effective.getDependencies().get(0).getVersion(), + "${revision} in dependency version should be interpolated with user-only property"); + } + @Test public void testMissingDependencyGroupIdInference() throws Exception { // Test that dependencies with missing groupId but present version are inferred correctly in model 4.1.0 diff --git a/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-deps-no-prop.xml b/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-deps-no-prop.xml new file mode 100644 index 000000000000..63f3b290e89f --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-deps-no-prop.xml @@ -0,0 +1,19 @@ + + + 4.0.0 + + com.test + ci-friendly-deps-no-prop + ${revision} + pom + + + + com.test + some-dep + ${revision} + + + diff --git a/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-deps.xml b/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-deps.xml new file mode 100644 index 000000000000..f501eb35864b --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/factory/ci-friendly-deps.xml @@ -0,0 +1,30 @@ + + + 4.0.0 + + com.test + ci-friendly-deps + ${revision} + pom + + + 1.0.0-SNAPSHOT + + + + + com.test + some-dep + ${revision} + + + + + + releases-${revision} + https://repo.example.com/releases + + +