Fix macOS CI failure in test-mvn-path-conversion.sh (PR #12546) - #12563
Closed
gnodet wants to merge 2 commits into
Closed
Fix macOS CI failure in test-mvn-path-conversion.sh (PR #12546)#12563gnodet wants to merge 2 commits into
gnodet wants to merge 2 commits into
Conversation
The launcher already converted POSIX paths to their native Windows form for Cygwin and MinGW (GH-7958), but a few paths still reached the JVM in a mangled or mixed form: * uname(1) reports MSYS_NT-* for the MSYS2 shell, which did not match the MINGW* pattern, so no conversion happened at all there. The MSYS2 argument conversion then rewrote the POSIX path into the mixed form D:/a/... instead of D:\a\... * ${MAVEN_PROJECTBASEDIR} placeholders in .mvn/jvm.config were expanded with the POSIX path, since JvmConfigParser was invoked before the conversion took place. The base directory is now converted upfront and both representations are kept: the POSIX one to locate jvm.config from the shell, the native one for everything handed over to the JVM. * -Dlibrary.jline.path was built by appending /lib/jline-native to the already converted MAVEN_HOME, producing C:\maven/lib/jline-native. It is now converted on its own. Values holding a path are printed with printf rather than echo, including the debug logging: several POSIX shells (dash and the other Almquist derivatives commonly installed as /bin/sh) expand backslash escapes in echo, so C:\tmp\build was emitted with a tab in place of \t. The conversion is also guarded, so a missing cygpath(1) warns and falls back to POSIX paths rather than passing empty paths to the JVM. A shell test covering all of the above is added and wired into the build through exec-maven-plugin, so mvn verify runs it. It stubs uname, cygpath and java and runs the launcher with a PATH holding nothing but those stubs, so the Windows-only code paths are exercised identically on every platform; keeping the ambient PATH would let the genuine cygpath(1) of an actual Cygwin/MSYS2 installation be picked up and the missing-cygpath fallback would never be exercised there. Related to GH-12537
On macOS the $TMPDIR variable ends with a trailing slash, so
mktemp -d "${TMPDIR}/mvn-path-conversion.XXXXXX" produces a path
containing a double slash (e.g. .../T//mvn-path-conversion.XNBzZw).
The pwd(1) calls inside the mvn launcher collapse the double slash,
causing all 22 path assertions to fail on macOS CI runners.
Resolve the work directory through cd/pwd right after creation so
the test's reference paths stay in sync with the launcher output.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8 tasks
Contributor
|
Confirmed — this is a bug in my test harness, not the launcher. I reproduced it locally with TMPDIR=/tmp/foo/ and got exactly the same 22 failures: pwd inside the launcher collapses the // that mktemp returns, so the test's reference paths never matched. Verified your fix passes with a trailing-slash TMPDIR, a normal one, and with TMPDIR unset, across sh/dash/bash/busybox sh. It also still reports failures against the unpatched launcher, so the normalisation hasn't made the test vacuous. Thanks for catching and fixing it — happy for this to be merged in place of #12546. |
Contributor
Author
|
Pushed the fix commit directly to PR #12546's branch instead. The macOS CI fix is now part of that PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test-mvn-path-conversion.shthat affect all macOS CI jobs in PR Convert paths for MSYS2 and jvm.config placeholders in bin/mvn #12546$TMPDIRends with a trailing/(e.g./var/folders/.../T/), somktemp -d "${TMPDIR}/..."produces a path with a double slash (/T//mvn-path-...). Thepwdbuiltin inside themvnlauncher normalizes this to a single slash, causing every path assertion in the test to mismatch.work_dirthroughcd/pwdimmediately aftermktemp, so the test's reference paths match what the launcher produces.This is a one-line fix (plus comment) that resolves all 22 failures without changing the launcher script itself.
Applies on top of PR #12546.
Test plan
🤖 Generated with Claude Code