Skip to content

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

Merged
gnodet merged 3 commits into
masterfrom
gnodet/gh-12288-lrm-profile-properties
Jun 17, 2026
Merged

[#12288] Pass settings.xml profile properties to LRM#12297
gnodet merged 3 commits into
masterfrom
gnodet/gh-12288-lrm-profile-properties

Conversation

@gnodet

@gnodet gnodet commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #12288.

Supersedes #12291 — preserves the original commits by @ascheman with review fixes applied on top.

Original fix (by @ascheman):
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.

Review fixes applied:

  1. Replaced redundant request.getActiveProfiles() call with inactive profile collection, so -P !profileId properly excludes activeByDefault profiles
  2. Renamed IT class to MavenITgh12288... following project naming convention

ascheman and others added 3 commits June 17, 2026 15:29
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 #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.
- Replace redundant request.getActiveProfiles() call with inactive
  profile collection so -P !profileId properly excludes activeByDefault
  profiles
- Rename IT class to MavenITgh12288... following project naming convention
@github-actions

Copy link
Copy Markdown

@gnodet Please assign appropriate label to PR according to the type of change.

@github-actions github-actions Bot added this to the 4.1.0 milestone Jun 17, 2026
ascheman added a commit that referenced this pull request Jun 22, 2026
Backport of the master fix (#12297) and gnodet's review hardening to
the maven-3.10.x line. On 3.x, request.getActiveProfiles() already
covers both CLI -P and settings.xml <activeProfiles>, so only the
<activeByDefault> channel was broken. The activeByDefault filter is
guarded against -P !id deactivation per gnodet's review feedback.

Cherry-picks could not be applied verbatim due to path and API
divergence:

* Path: 3.x has maven-core/src/main/java/..., master has
  impl/maven-core/src/main/java/...
* API: 3.x exposes request.getActiveProfiles() /
  getInactiveProfiles() returning List<String>; master uses
  request.getProfileActivation().get{Required,Optional}{Active,Inactive}
  ProfileIds().

The minimal-diff adaptation here only adds the activeByDefault branch
and -P !id guard; the existing settings.xml <activeProfiles>
propagation is unchanged.

Master origin commits:
* 3e2c756 ([#12288] fix)
* 5785235 (gnodet review hardening)

Integration tests live in apache/maven-integration-testing and are
addressed in a separate PR.

Co-Authored-By: Guillaume Nodet <gnodet@gmail.com>
@gnodet gnodet added the bug Something isn't working label Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants