[#12288] Pass settings.xml profile properties to LRM by ascheman · Pull Request #12291 · apache/maven

ascheman · 2026-06-17T10:34:19Z

Profiles activated through settings.xml (<activeProfiles> or
<activation><activeByDefault>true</activeByDefault>) had their
<properties> block dropped before the resolver session was built, so
aether.* configuration declared in such a profile silently failed.
The same <properties> on the same <profile> worked when activated
via CLI -P.

Changes

Two ITs in core-it-suite covering both settings.xml activation
channels — testActiveByDefaultProfile (red on master) and
testActiveProfilesList (green on master, regression guard).
DefaultRepositorySystemSessionFactory#getPropertiesFromRequestedProfiles
extended to also consider request.getActiveProfiles() and
<activeByDefault>true</activeByDefault> profiles.

Locally verified against the core-it-suite:

master + new ITs: 1 of 2 fail (testActiveByDefaultProfile)
master + new ITs + fix: 2 of 2 pass

Scope

Only the profile's <properties> block is affected — other profile
content (<plugins>, <dependencies>, <repositories>, …) flows
through its existing channels unchanged.

Limitation: this does not re-implement the full DefaultProfileSelector
semantics (e.g. activeByDefault deactivation when other profiles of
the same source are active). For the property-propagation use case in
scope of #12288 this is sufficient; injecting ProfileSelector here
would be a larger refactor in a session-build hot path and is left to a
follow-up if needed.

Affected versions

Reproduces on Maven 4.x master and on Maven 3.9.16; backport candidate.

Test plan

mvn install on apache-maven from the branch builds cleanly
mvn verify -Prun-its -DmavenDistro=… -Dit.test=MavenITSettingsProfileAetherPropertiesTest on the patched dist: both ITs green
Same command on vanilla master dist: testActiveByDefaultProfile red, testActiveProfilesList green
core-it-suite full run (not done locally — CI takes it from here)

Adds two integration tests asserting that <properties> declared in a settings.xml <profile> reach the resolver session config before the LocalRepositoryManager is initialized. The fixtures use aether.enhancedLocalRepository.{split,localPrefix} because those have an observable effect on the install path, but the propagation gap covered by the tests applies to any profile property. Only the profile's <properties> block is consumed; the profile's <plugins>, <dependencies>, <repositories>, etc. are not in scope here. Both settings.xml-only activation channels are covered: * testActiveByDefaultProfile -- <activation><activeByDefault>true</... * testActiveProfilesList -- <activeProfiles><activeProfile>... On unmodified master, testActiveByDefaultProfile is RED (the bug) and testActiveProfilesList is GREEN (acts as a regression guard for the channel that already propagates correctly).

DefaultRepositorySystemSessionFactory#getPropertiesFromRequestedProfiles previously only considered CLI-activated profile ids (-P). Profiles activated through settings.xml (<activeProfiles> or <activeByDefault>) were filtered out, so any <properties> they declared were dropped before the resolver session was built. aether.* configuration was the most visible casualty (LRM split/localPrefix etc.), but the gap affects every property declared in such a profile. The fix only touches profile <properties>; other profile content (<plugins>, <dependencies>, <repositories>, ...) is unchanged and keeps flowing through its existing project/build channels. Extend the active-profile-id set with request.getActiveProfiles() and add an isActiveByDefault() branch to the filter. Note: this does not re-implement the full DefaultProfileSelector semantics (e.g. activeByDefault deactivation when other profiles of the same source are active). For the property-propagation use case in scope of apache#12288 this is sufficient; injecting ProfileSelector here would be a larger refactor in a session-build hot path and is left to a follow-up if needed.

gnodet

Thank you for this well-documented PR, @ascheman — the bug report and reproducer are thorough, and the fix targets the right method.

Two issues to address, one correctness gap and one redundancy:

Missing deactivation check: The activeByDefault branch doesn't exclude profiles explicitly deactivated via -P !profileId. A user running mvn -P !aether-split-via-settings would expect the profile to be deactivated, but the current code still includes its properties.
Redundant request.getActiveProfiles() call: request.getActiveProfiles() returns the exact union of getRequiredActiveProfileIds() + getOptionalActiveProfileIds() (already collected above). The settings.xml <activeProfiles> list flows via overwriteActiveProfiles() → activateOptionalProfile(), so they're already in getOptionalActiveProfileIds(). The real fix is the activeByDefault check — the getActiveProfiles() line adds nothing.

See inline suggestions for concrete fixes.

Note: this review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).

Fully automatic review from Claude Code

gnodet · 2026-06-17T13:30:54Z

+        // Also include profiles activated via settings.xml <activeProfiles> so that their
+        // properties (notably aether.* configuration) reach the resolver session config in
+        // time for LocalRepositoryManager initialization.
+        activeProfileId.addAll(request.getActiveProfiles());


request.getActiveProfiles() delegates to profileActivation.getActiveProfiles(), which returns all IDs where active() == true — the exact union of getRequiredActiveProfileIds() + getOptionalActiveProfileIds() already collected above. The settings.xml <activeProfiles> list is wired via overwriteActiveProfiles() → activateOptionalProfile(), so those IDs are already in getOptionalActiveProfileIds().

This block should instead collect inactive profile IDs, so the activeByDefault filter below can respect explicit deactivation (-P !profileId):

Suggested change

// Also include profiles activated via settings.xml <activeProfiles> so that their

// properties (notably aether.* configuration) reach the resolver session config in

// time for LocalRepositoryManager initialization.

activeProfileId.addAll(request.getActiveProfiles());

// Profiles explicitly deactivated via -P !id must be excluded even if they

// declare activeByDefault=true.

HashSet<String> inactiveProfileId =

new HashSet<>(request.getProfileActivation().getRequiredInactiveProfileIds());

inactiveProfileId.addAll(request.getProfileActivation().getOptionalInactiveProfileIds());

gnodet · 2026-06-17T13:30:54Z

+                .filter(profile -> activeProfileId.contains(profile.getId())
+                        || (profile.getActivation() != null
+                                && profile.getActivation().isActiveByDefault()))


The activeByDefault branch should exclude profiles that have been explicitly deactivated:

Suggested change

.filter(profile -> activeProfileId.contains(profile.getId())

|| (profile.getActivation() != null

&& profile.getActivation().isActiveByDefault()))

.filter(profile -> activeProfileId.contains(profile.getId())

|| (!inactiveProfileId.contains(profile.getId())

&& profile.getActivation() != null

&& profile.getActivation().isActiveByDefault()))

gnodet · 2026-06-17T13:30:55Z

+ * settings.xml activation channels fail, which is what these tests guard
+ * against.
+ */
+public class MavenITSettingsProfileAetherPropertiesTest extends AbstractMavenIntegrationTestCase {


Nit: other ITs fixing tracked issues typically follow the MavenITgh<number>...Test pattern (e.g. MavenITgh10210SettingsXmlDecryptTest). Consider renaming to MavenITgh12288SettingsProfileAetherPropertiesTest for consistency and traceability.

ascheman · 2026-06-17T16:13:27Z

Superseded by #12297 — closing in favor of @gnodet's reroll. Thanks for picking it up and preserving attribution.

Fixes #12288. Supersedes #12291. Profiles activated through settings.xml (<activeProfiles> or <activation><activeByDefault>true</activeByDefault>) had their <properties> block dropped before the resolver session was built, so aether.* configuration declared in such a profile silently failed. Changes: - Collect activeByDefault profiles for property propagation to LRM - Respect -P !profileId deactivation for activeByDefault profiles - Add ITs for both settings.xml activation channels Co-authored-by: Gerd Aschemann <gerd@aschemann.net>

…#12299) Fixes #12288. Supersedes #12291. Profiles activated through settings.xml (<activeProfiles> or <activation><activeByDefault>true</activeByDefault>) had their <properties> block dropped before the resolver session was built, so aether.* configuration declared in such a profile silently failed. Changes: - Collect activeByDefault profiles for property propagation to LRM - Respect -P !profileId deactivation for activeByDefault profiles - Add ITs for both settings.xml activation channels Co-authored-by: Gerd Aschemann <gerd@aschemann.net>

ascheman added 2 commits June 17, 2026 12:28

ascheman mentioned this pull request Jun 17, 2026

Profile-level properties from a settings.xml-activated profile are dropped before LRM init #12288

Closed

gnodet requested changes Jun 17, 2026

View reviewed changes

gnodet mentioned this pull request Jun 17, 2026

[#12288] Pass settings.xml profile properties to LRM #12297

Merged

ascheman closed this Jun 17, 2026

ascheman mentioned this pull request Jun 17, 2026

[#12288] Add -P !profile deactivation regression guard #12298

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[#12288] Pass settings.xml profile properties to LRM#12291

[#12288] Pass settings.xml profile properties to LRM#12291
ascheman wants to merge 2 commits into
apache:masterfrom
aschemaven:gh-12288-lrm-profile-properties

ascheman commented Jun 17, 2026

Uh oh!

gnodet left a comment

Uh oh!

gnodet Jun 17, 2026

Uh oh!

gnodet Jun 17, 2026

Uh oh!

gnodet Jun 17, 2026

Uh oh!

ascheman commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        // Also include profiles activated via settings.xml <activeProfiles> so that their
-        // properties (notably aether.* configuration) reach the resolver session config in
-        // time for LocalRepositoryManager initialization.
-        activeProfileId.addAll(request.getActiveProfiles());
+        // Profiles explicitly deactivated via -P !id must be excluded even if they
+        // declare activeByDefault=true.
+        HashSet<String> inactiveProfileId =
+                new HashSet<>(request.getProfileActivation().getRequiredInactiveProfileIds());
+        inactiveProfileId.addAll(request.getProfileActivation().getOptionalInactiveProfileIds());

Uh oh!

Conversation

ascheman commented Jun 17, 2026

Changes

Scope

Affected versions

Test plan

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

gnodet Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

gnodet Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

gnodet Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

ascheman commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants