[MNG-3309] Cascading profile activation

[gnodet]

JIRA issue: https://issues.apache.org/jira/browse/MNG-3309

[cstamas]

Is this whole PR only about cascading profile activation? There are mvnup changes for example...

[gnodet]

AI Review — PR #1774: [MNG-3309] Cascading profile activation

This PR implements cascading profile activation — a long-requested feature where activating one profile can trigger activation of another via properties set by the first. The core design is sound (the loop terminates correctly), but there are several issues to address.

Bug

ReportSet merge result discarded — In MavenModelMerger.java, the change from:

element = mergeReportSet(element, existing, sourceDominant, context);

to:

mergeReportSet(element, existing, sourceDominant, context);

discards the merged result. In the Maven 4 immutable model, mergeReportSet returns a new object via builder.build() — it does not mutate in place. The original unmerged element is put into the map, silently breaking ReportSet merging during inheritance. (The compat ModelMerger uses void returns, so this may have been copied from the wrong pattern.)

Correctness Concerns

1. NPE in ConditionProfileActivator — doGetProperty(Profile profile, ...) calls Profile.SOURCE_POM.equals(profile.getSource()) but ConditionParserTest passes null as the profile parameter. If any property resolution reaches the model-property fallback path, this will NPE. Current tests don't trigger it (they resolve system properties), but the code is fragile.

2. instanceof coupling for cascading API — addProfileProperties was correctly added to the ProfileActivationContext interface, but clearCascadedProfileProperties() and getModelPropertyForActivation() are only on DefaultProfileActivationContext and accessed via instanceof downcasts (4 places). Alternative implementations won't crash (they fall back), but the asymmetry is a code smell.

Behavioral Changes

• CI-friendly parent version support removed — The deleted MavenITgh12184CIFriendlyParentVersionTest confirms ${revision} in parent version is no longer handled the same way. This needs migration guide documentation.
• Repository validation upgraded WARNING → ERROR — Uninterpolated expressions in repository id/url now fail the build instead of warning. May break real-world builds.
• Property lookup order changed — ConditionProfileActivator.doGetProperty now checks user → system → model (previously user → model → system). System properties now take precedence over model properties during activation.

Scope Concern

As committer cstamas noted, this PR bundles many unrelated changes (mvnup fixes, plugin version changes, consumer POM simplification, GitHub Actions downgrades) with the cascading activation feature. Splitting into focused PRs would improve reviewability.

What Works Well

The cascading activation core design is correct: the do...while loop removes activated profiles from remainingProfiles on each iteration, guaranteeing monotonic progress and termination. Circular dependencies are handled correctly (both profiles activate in the first pass). Test coverage for the cascading behavior is adequate.

---

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet