gnodet commented on code in PR #13141:
URL: https://github.com/apache/maven/pull/13141#discussion_r4018754934
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java:
##########
@@ -1722,10 +1722,14 @@ private List<Profile> getActiveProfiles(
// A model resolved to satisfy dependency resolution -- a
dependency POM
// itself, or one of its parents, reached transitively --
evaluates only
// platform-derived activation (JDK version, operating
system,
- // activeByDefault); its profiles contribute no
repositories.
+ // activeByDefault). Repository stripping is intentionally
omitted here:
+ // the sandbox activation context (see #13112) already
suppresses consumer
+ // -D flags and file conditions, so only
legitimately-active profiles reach
+ // injection. Stripping their repositories would break the
established
+ // project → dep1 → dep2 pattern where dep1 declares
dep2's repository
+ // inside an activeByDefault or JDK-activated profile.
eligibleProfiles = interpolatedProfiles.stream()
.filter(profile ->
!hasFileOrPropertyOrConditionActivation(profile))
Review Comment:
Fixed in a2460d375f: the comment now explicitly attributes the guarantee to
the `hasFileOrPropertyOrConditionActivation` filter below ("file and property
activation is already excluded by the hasFileOrPropertyOrConditionActivation
filter below, so only legitimately-active profiles reach injection"). This
makes clear that property-activated profiles are excluded at that point, not by
the sandbox.
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java:
##########
@@ -180,7 +180,9 @@ public void
testDependencyModelActivatesOnlyEnvironmentIndependentProfiles() {
assertNull(model.getProperties().get("profile.property"));
assertNull(model.getProperties().get("profile.condition"));
assertEquals("activated", model.getProperties().get("profile.jdk"));
- assertTrue(model.getRepositories().stream().noneMatch(r ->
"profile-repo".equals(r.getId())));
+ // Repositories from legitimately-active profiles (JDK-activated) must
be honored:
+ // stripping them would break the project → dep1 → dep2 pattern. See
#13100, #13116.
Review Comment:
Fixed in a2460d375f: added `testActiveByDefaultProfileRepositoryHonored`
using a dedicated fixture (`active-by-default-profile.xml`) with only an
`activeByDefault=true` profile, so the "other profile activates" suppression
rule doesn't interfere. The test asserts the `always-active-repo` repository is
retained in the resolved external model.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
