From 994fc0c45343d5edcfee404f03635538213b2dc1 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 28 May 2026 12:03:46 +0200 Subject: [PATCH] Use request properties consistently for CI-friendly version interpolation 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 d631e8fd7e2c..b824ce03f0d4 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 @@ -303,8 +303,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 aa282d272353..0e48da1a02b4 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 @@ -734,8 +734,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 + + +