gnodet commented on code in PR #1774:
URL: https://github.com/apache/maven/pull/1774#discussion_r4291741742
##########
api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileActivationContext.java:
##########
@@ -141,4 +144,12 @@ public interface ProfileActivationContext {
* @throws InterpolatorException if an error occurs during interpolation
*/
boolean exists(@Nullable String path, boolean glob);
+
+ /**
+ * Inject properties from newly activated profiles in order to trigger the
cascading mechanism.
+ * This method allows profiles to contribute properties that can trigger
the activation of other profiles.
+ *
+ * @param activatedProfiles The collection of profiles that have been
activated that may trigger the cascading effect.
+ */
+ void addProfileProperties(Collection<Profile> activatedProfiles);
}
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Already fixed in the previous round — `addProfileProperties` has a `default`
no-op.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileSelector.java:
##########
@@ -63,32 +63,54 @@ public DefaultProfileSelector
addProfileActivator(ProfileActivator profileActiva
@Override
public List<Profile> getActiveProfiles(
Collection<Profile> profiles, ProfileActivationContext context,
ModelProblemCollector problems) {
- List<Profile> activeProfiles = new ArrayList<>(profiles.size());
+
+ List<Profile> activeSettingsProfiles = new ArrayList<>();
+ List<Profile> activePomProfiles = new ArrayList<>();
List<Profile> activePomProfilesByDefault = new ArrayList<>();
- boolean activatedPomProfileNotByDefault = false;
- for (Profile profile : profiles) {
- if (!context.isProfileInactive(profile.getId())) {
- if (context.isProfileActive(profile.getId()) ||
isActive(profile, context, problems)) {
- activeProfiles.add(profile);
- if (Profile.SOURCE_POM.equals(profile.getSource())) {
- activatedPomProfileNotByDefault = true;
- }
- } else if (isActiveByDefault(profile)) {
- if (Profile.SOURCE_POM.equals(profile.getSource())) {
- activePomProfilesByDefault.add(profile);
- } else {
- activeProfiles.add(profile);
+ // Cascading mode: iterate until no more profiles are activated
+ List<Profile> remainingProfiles = new ArrayList<>(profiles);
+ List<Profile> activatedProfiles;
+ do {
+ activatedProfiles = new ArrayList<>();
+ for (Profile profile : List.copyOf(remainingProfiles)) {
+ if (!context.isProfileInactive(profile.getId())) {
+ boolean activated =
context.isProfileActive(profile.getId());
+ boolean active = isActive(profile, context, problems);
+ boolean activeByDefault = isActiveByDefault(profile);
+ if (activated || active || activeByDefault) {
+ if (Profile.SOURCE_POM.equals(profile.getSource())) {
+ if (activated || active) {
+ activePomProfiles.add(profile);
+ } else {
+ activePomProfilesByDefault.add(profile);
+ }
+ } else {
+ activeSettingsProfiles.add(profile);
+ }
+ remainingProfiles.remove(profile);
+ activatedProfiles.add(profile);
}
}
}
+ // Add profile properties for cascading activation
+ context.addProfileProperties(activatedProfiles);
+ } while (!activatedProfiles.isEmpty());
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Already fixed in the previous round — `activeByDefault` profiles are
excluded from `activatedProfiles` so they don't inject cascading properties.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileSelector.java:
##########
@@ -63,32 +63,54 @@ public DefaultProfileSelector
addProfileActivator(ProfileActivator profileActiva
@Override
public List<Profile> getActiveProfiles(
Collection<Profile> profiles, ProfileActivationContext context,
ModelProblemCollector problems) {
- List<Profile> activeProfiles = new ArrayList<>(profiles.size());
+
+ List<Profile> activeSettingsProfiles = new ArrayList<>();
+ List<Profile> activePomProfiles = new ArrayList<>();
List<Profile> activePomProfilesByDefault = new ArrayList<>();
- boolean activatedPomProfileNotByDefault = false;
- for (Profile profile : profiles) {
- if (!context.isProfileInactive(profile.getId())) {
- if (context.isProfileActive(profile.getId()) ||
isActive(profile, context, problems)) {
- activeProfiles.add(profile);
- if (Profile.SOURCE_POM.equals(profile.getSource())) {
- activatedPomProfileNotByDefault = true;
- }
- } else if (isActiveByDefault(profile)) {
- if (Profile.SOURCE_POM.equals(profile.getSource())) {
- activePomProfilesByDefault.add(profile);
- } else {
- activeProfiles.add(profile);
+ // Cascading mode: iterate until no more profiles are activated
+ List<Profile> remainingProfiles = new ArrayList<>(profiles);
+ List<Profile> activatedProfiles;
+ do {
+ activatedProfiles = new ArrayList<>();
+ for (Profile profile : List.copyOf(remainingProfiles)) {
+ if (!context.isProfileInactive(profile.getId())) {
+ boolean activated =
context.isProfileActive(profile.getId());
+ boolean active = isActive(profile, context, problems);
+ boolean activeByDefault = isActiveByDefault(profile);
+ if (activated || active || activeByDefault) {
+ if (Profile.SOURCE_POM.equals(profile.getSource())) {
+ if (activated || active) {
+ activePomProfiles.add(profile);
+ } else {
+ activePomProfilesByDefault.add(profile);
+ }
+ } else {
+ activeSettingsProfiles.add(profile);
+ }
+ remainingProfiles.remove(profile);
+ activatedProfiles.add(profile);
}
}
}
+ // Add profile properties for cascading activation
+ context.addProfileProperties(activatedProfiles);
+ } while (!activatedProfiles.isEmpty());
+
+ // Clear cascaded properties to prevent them from leaking into
subsequent uses of the context
+ if (context instanceof
org.apache.maven.impl.model.DefaultProfileActivationContext dctx) {
+ dctx.clearCascadedProfileProperties();
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Fixed — `clearCascadedProfileProperties()` is now a `default` method on
`ProfileActivationContext` SPI. `DefaultProfileSelector` calls it through the
interface instead of `instanceof`.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionProfileActivator.java:
##########
@@ -182,16 +182,23 @@ static String doGetProperty(ProfileActivationContext
context, String name) {
// Check user properties
String v = context.getUserProperty(name);
- if (v == null) {
- // Check project properties
- // TODO: this may leads to instability between file model
activation and effective model activation
- // as the effective model properties may be different from
the file model
- v = context.getModelProperty(name);
- }
if (v == null) {
// Check system properties
v = context.getSystemProperty(name);
}
+ if (v == null) {
+ // Check project properties (with cascading support if available)
+ // ONLY for POM profiles - settings profiles should not use model
properties
+ // TODO: this may leads to instability between file model
activation and effective model activation
+ // as the effective model properties may be different from
the file model
+ if (Profile.SOURCE_POM.equals(profile.getSource())) {
+ if (context instanceof
org.apache.maven.impl.model.DefaultProfileActivationContext dctx) {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Already fixed in the previous round — added `profile != null` guard.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionProfileActivator.java:
##########
@@ -182,16 +182,23 @@ static String doGetProperty(ProfileActivationContext
context, String name) {
// Check user properties
String v = context.getUserProperty(name);
- if (v == null) {
- // Check project properties
- // TODO: this may leads to instability between file model
activation and effective model activation
- // as the effective model properties may be different from
the file model
- v = context.getModelProperty(name);
- }
if (v == null) {
// Check system properties
v = context.getSystemProperty(name);
}
+ if (v == null) {
+ // Check project properties (with cascading support if available)
+ // ONLY for POM profiles - settings profiles should not use model
properties
+ // TODO: this may leads to instability between file model
activation and effective model activation
+ // as the effective model properties may be different from
the file model
+ if (Profile.SOURCE_POM.equals(profile.getSource())) {
+ if (context instanceof
org.apache.maven.impl.model.DefaultProfileActivationContext dctx) {
+ v = dctx.getModelPropertyForActivation(name);
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Fixed — `getCascadedProfileProperty()` is now a `default` method on the
`ProfileActivationContext` SPI (returns `null` by default). Both
`ConditionProfileActivator` and `PropertyProfileActivator` now call it through
the interface.
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/PropertyProfileActivator.java:
##########
@@ -99,6 +99,15 @@ public boolean isActive(Profile profile,
ProfileActivationContext context, Model
if (sysValue == null) {
sysValue = context.getSystemProperty(name);
}
+ // Check cascaded profile properties last (for cascading profile
activation).
+ // Only cascaded properties (from previously-activated profiles) are
checked — the
+ // POM's own <properties> must NOT self-activate profiles via
PropertyProfileActivator.
+ // ONLY for POM profiles - settings profiles should not use cascaded
properties.
+ if (sysValue == null &&
Profile.SOURCE_POM.equals(profile.getSource())) {
+ if (context instanceof
org.apache.maven.impl.model.DefaultProfileActivationContext dctx) {
+ sysValue = dctx.getCascadedProfileProperty(name);
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Fixed — same as above, `PropertyProfileActivator` now calls
`context.getCascadedProfileProperty(name)` directly through the SPI.
--
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]
