fix: interpolate properties in module/subproject path before filesystem resolution

[waterWang]

Problem

Maven 4.0.0-rc6 does not interpolate properties in <module> (or <subproject>) paths of aggregator POMs. For example:

<properties>
    <version-discriminator>-v14</version-discriminator>
</properties>
<modules>
    <module>./../module/pom${version-discriminator}.xml</module>
</modules>

Maven 3 correctly interpolates ${version-discriminator} to produce ./../module/pom-v14.xml, but Maven 4 fails with:

[ERROR] Child subproject .../pom${version-discriminator}.xml of ... does not exist

Root Cause

In DefaultModelBuilder.loadFilePom(), the subproject/module path strings are resolved against the filesystem before model-wide property interpolation runs. The path ${version-discriminator} is never resolved, so Maven 4 treats it as a literal directory name.

Fix

Interpolate each subproject path string against the model, user, and system properties before resolving it against the filesystem. This is a targeted interpolation of the path only — the full model interpolation still runs later in the build pipeline, unchanged.

Testing

• Maven 3 behavior: module paths with ${property} references are correctly interpolated
• This fix restores that behavior in Maven 4 by interpolating the path before the filesystem lookup
• Non-interpolated paths (the common case) pass through unchanged

Fixes #12729

[gnodet]

The fix correctly identifies the root cause — Maven 4 doesn't interpolate properties in module/subproject paths before filesystem resolution, breaking ${property}-based module paths that worked in Maven 3. The approach (targeted early interpolation) is sound.

However, two issues should be addressed:

Property lookup precedence is inverted

The callback checks activated.getProperties() (model/POM properties) first, then falls back to request.getUserProperties(). Maven convention is that user properties (-Dprop=value) override POM-defined properties.

Compare with the established patterns in the same file:

• interpolateModel() (~line 2448): Interpolator.chain(userProps, modelProps, systemProps) — user properties first
• getPropertiesWithProfiles() (~lines 798-799): putAll(request.getUserProperties()) called last so user properties override everything

With the current code, if a user passes -Dversion-discriminator=-v15, the POM-defined value would still be used instead of the user override.

Suggested fix — swap the order:

subproject = interpolator.interpolate(
        subproject,
        Interpolator.chain(
                request.getUserProperties()::get,
                activated.getProperties()::get,
                request.getSystemProperties()::get));

Missing test

No test accompanies this regression fix. The issue (#12729) provides a reproducible project structure that could be adapted into an integration test under its/core-it-suite/ to prevent future regression.

Minor nits

• The comment references MNG-XXXX — should be #12729
• The if (interpolated != null) guard is redundant since subproject is guaranteed non-null at that point

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

Claude Code on behalf of gnodet

[gnodet]

The fix correctly identifies the root cause (module paths need interpolation before filesystem resolution), but several issues need to be addressed.

Findings:

1. [high] Property lookup precedence is inverted — The code checks activated.getProperties().get(key) (model) before request.getUserProperties().get(key), but Maven convention is user → model → system. Compare with interpolateModel() at line 2398 in the same file which uses Interpolator.chain(userProps::get, modelProps::get, systemProps::get). With the current code, a user passing -Dprop=value would be silently ignored if the POM also defines that property.

2. [medium] Use Interpolator.chain() instead of manual lambda — The manual key -> { ... } lambda duplicates what Interpolator.chain() already provides. The same file uses Interpolator.chain() at line 2398 for the same purpose. Suggested replacement:

   subproject = interpolator.interpolate(
       subproject,
       Interpolator.chain(
           request.getUserProperties()::get,
           activated.getProperties()::get,
           request.getSystemProperties()::get));

3. [medium] No regression test — This is a fix for issue #12729 (Maven 3 behavior broken in Maven 4). A regression test should accompany this fix to prevent recurrence.

4. [low] Placeholder issue reference — Comment says MNG-XXXX which should be replaced with the actual issue reference #12729.

5. [low] Redundant null guard — The if (interpolated != null) check is unnecessary since interpolator.interpolate() returns non-null for non-null input, consistent with how interpolateModel() uses the result directly without a null check.

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

Claude Code on behalf of Guillaume Nodet