Fix #12507: dont wipe unresolved ${...} expressions in CLI-supplied property values#12515
Fix #12507: dont wipe unresolved ${...} expressions in CLI-supplied property values#12515ascheman wants to merge 2 commits into
Conversation
MavenITgh12507CliParamNestedInterpolationTest shows that ${user.dir}
and ${project.build.directory} inside a -D-supplied plugin parameter
value are replaced with the empty string at CLI parse time
(BaseParser.populateUserProperties interpolates user-specified
properties with defaultsToEmpty=true against a paths-only callback),
while Maven 3 resolves such expressions at mojo configuration time.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BaseParser.populateUserProperties interpolated user-specified
properties with defaultsToEmpty=true against a paths-only callback,
silently replacing every expression it could not resolve (system
properties, project.* references) with the empty string. Pass
defaultsToEmpty=false so unresolved placeholders stay literal and are
evaluated later with full session/project context by the plugin
parameter expression evaluator -- restoring Maven 3 semantics for
values like -DaltDeploymentRepository=id::file://${project.build.directory}/x.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a Maven 4 CLI parsing regression where user-supplied -D property values containing ${...} placeholders were interpolated too early (during CLI parse) with defaultsToEmpty=true, causing any placeholders not resolvable from the early session.*Directory map to be silently replaced with "". The change preserves unresolved placeholders so they can be evaluated later with full session/project context (matching Maven 3 behavior and unblocking common patterns like -DaltDeploymentRepository=...${project.build.directory}...).
Changes:
- Adjust
BaseParser.populateUserProperties()to interpolate CLI-Dvalues withdefaultsToEmpty=false, preventing destructive empty-substitution for unknown expressions. - Add a new core integration test covering nested interpolation in CLI-supplied plugin parameter values for both system (
${user.dir}) and project (${project.build.directory}) expressions. - Add a new IT resource project used by the integration test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| its/core-it-suite/src/test/resources/gh-12507-cli-param-nested-interpolation/pom.xml | New IT project that writes evaluated plugin parameters to a properties file for assertion. |
| its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12507CliParamNestedInterpolationTest.java | New IT validating that ${...} embedded in CLI-supplied plugin parameter values remains resolvable at mojo configuration time. |
| impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java | Prevents early CLI parse interpolation from wiping unresolved ${...} by setting defaultsToEmpty=false. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you for the excellent root-cause analysis and fix @ascheman! Unfortunately I cannot push to your fork branch (token permissions), so I've created #12523 which includes your two commits (rebased) plus the |
Fixes #12507.
Root cause
BaseParser.populateUserProperties()interpolates user-specified-Dvalues at CLI parse time via theInterpolatorconvenience overload, which defaults todefaultsToEmpty=true, against a callback that only knows theextraInterpolationSource()paths (session.topDirectory/session.rootDirectory). Every other expression — system properties like${user.dir}, project references like${project.build.directory}— was silently replaced with the empty string before any session/project context existed.Maven 3 resolves such expressions at mojo configuration time; the v3-compat
PluginParameterExpressionEvaluatorin Maven 4 would too, but it only ever saw the already-wiped value.This is independent of the recent launcher quoting fixes (#11978 / #11983): those fixed the shell layer so
${...}arguments reach the JVM intact. On rc-5 the launcher bug masked this issue with a loudbad substitutionshell error; with the launcher fix shipping in rc-6, users hit this silent empty-substitution instead.Fix
Pass
defaultsToEmpty=falsefor the user-specified properties: placeholders that cannot be resolved from the early paths map stay literal and are evaluated later with full session/project context. The${session.topDirectory}/${session.rootDirectory}support from #2480 is unaffected.Testing
MavenITgh12507CliParamNestedInterpolationTest(two cases:${user.dir}and${project.build.directory}in a CLI-supplied plugin parameter): red against the current master distro (expected: <PRE-…-POST> but was: <PRE--POST>), green with the fix.maven-clitest suites pass.mvn deploy '-DaltDeploymentRepository=test::file://${project.build.directory}/deploy'deploys totarget/deploy/…again (was:file:///deploy→Read-only file system).🤖 Generated with Claude Code