gnodet commented on code in PR #13141:
URL: https://github.com/apache/maven/pull/13141#discussion_r4018736748


##########
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

Review Comment:
   Fixed in a2460d375f: comment now attributes the guarantee to the actual 
`hasFileOrPropertyOrConditionActivation` filter rather than the not-yet-merged 
#13112 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.
+        assertTrue(model.getRepositories().stream().anyMatch(r -> 
"profile-repo".equals(r.getId())));

Review Comment:
   Fixed in a2460d375f: added `testActiveByDefaultProfileRepositoryHonored` 
using a dedicated fixture (`active-by-default-profile.xml`) that contains only 
the `activeByDefault=true` profile. The existing fixture couldn't be reused 
directly — the JDK-activated profile there causes Maven to suppress 
`activeByDefault` per the standard rule, which would have made the test pass 
vacuously on a broken implementation.



-- 
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]

Reply via email to