Fix glob patterns in exists()/missing() profile conditions on Windows - #13163
Conversation
DefaultProfileActivationContext aligned the whole pattern to the project
directory before splitting it into a fixed directory and a glob. On
Windows this breaks both kinds of pattern:
- an absolute pattern such as '${project.basedir}/src/**/*.xsd' leaves a
glob with backslash separators ('**\*.xsd'), and the glob syntax reads
'\' as an escape character, so nothing ever matches: exists() is always
false and missing() always true;
- a relative pattern such as '**/*.xsd' is resolved with Path.resolve(),
which throws InvalidPathException because '*' is not a valid Windows
path character.
Split the interpolated pattern first, align only the fixed part, and use
'/' as the separator in the glob. testFileWilcards, disabled since the
condition activator was added, passes on Linux and Windows now and is
enabled again.
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
The fix is correct. The root cause was clear: alignToBaseDirectory() normalises separators to File.separatorChar, which makes * and ? characters illegal on Windows paths before the OS can resolve them. Splitting the pattern at the wildcard boundary first (via the new interpolate() helper), then aligning only the fixed prefix, then normalising the glob suffix to / is the right approach. The lastSep == -1 edge case (pure-glob patterns like **/*.xsd) is handled correctly — alignToBaseDirectory("", basedir) resolves to basedir itself, matching the expected behaviour tested on line 408.
One nit on the test:
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Re-review after 475f726 — previous nit (duplicate assertActivation for ${project.basedir}/**/*.xsd) has been addressed: the duplicate line is gone, replaced by two new assertions (missing('**/*.xsd') → false, exists('**/*.xml') → false) that better exercise the fix by verifying the correct .xml-vs-.xsd discrimination.
The core logic is solid:
alignToBaseDirectoryis now called only on the fixed prefix (before the first wildcard), avoiding the separator-normalisation problem that broke Windows paths.- The
lastSep < 0edge case (pure-glob patterns like**/*.xsd) producesfixed = "", whichalignToBaseDirectory("", basedir)correctly resolves tobasedir. - The
replace('\\', '/')normalisation on the glob suffix is correct — Java's glob engine always uses/as the path separator regardless of OS. java.io.Fileimport removed cleanly; noFile.separatorCharusage remains.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
3 participants
Backport of #13160 to
maven-4.0.x.Cherry-pick of faddbd3, with minor conflict resolution:
DefaultPathTranslator.alignToBaseDirectoryis not yet a static method onmaven-4.0.x, so the static calls were replaced withpathTranslator.alignToBaseDirectoryinstance calls.Related: #13164 (forward-port of sandbox review findings, also targets master).