You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "Hiteshsai007 (via GitHub)" <gi...@apache.org> on 2026/07/27 15:29:27 UTC
[PR] Convert paths for MSYS2 and jvm.config placeholders in bin/mvn [maven]
Hiteshsai007 opened a new pull request, #12546:
URL: https://github.com/apache/maven/pull/12546
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.
Additionally, the output of JvmConfigParser and of the base directory lookup is printed with printf instead of echo. Several POSIX shells (dash and other Almquist derivatives commonly installed as /bin/sh) expand backslash escapes in echo, which corrupts Windows paths, and the conversion is 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; it stubs uname, cygpath and java so the Windows-only code paths can be exercised on any platform, and it can be run on any platform or wired into CI.
Fixes GH-12537
<!--
Please open pull requests against one of the following branches:
- master — default target for all changes (new features + bug fixes).
If the change should also go to maven-4.0.x, request a backport in the PR.
- maven-3.10.x — target for 3.x changes (new features + bug fixes).
If the change should also go to maven-3.9.x, request a backport in the PR.
maven-3.9.x should receive only serious bug fixes, as it is approaching EOL.
Backports are typically handled by maintainers;
however, after agreement you may open a separate backport PR to the target maintenance branch.
-->
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x] Your pull request should address just one issue, without pulling in other changes.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
Note that commits might be squashed by a maintainer on merge.
- [x] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
This may not always be possible but is a best-practice.
- [x] Run `mvn verify` to make sure basic checks pass.
A more thorough check will be performed on your pull request automatically.
- [ ] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert paths for MSYS2 and jvm.config placeholders in bin/mvn [maven]
Posted by "Hiteshsai007 (via GitHub)" <gi...@apache.org>.
Hiteshsai007 commented on PR #12546:
URL: https://github.com/apache/maven/pull/12546#issuecomment-5101327782
The macOS failures were a bug in my test harness (double slash from mktemp when TMPDIR has a trailing slash), not the launcher — which is why only macOS was affected while Linux and Windows passed.
@gnodet has fixed it in #12563, which includes my commit 033146e plus the normalisation. Happy to close this in favour of that PR, or to push the one-line fix here if you'd prefer to keep this one — whichever is easier for you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert paths for MSYS2 and jvm.config placeholders in bin/mvn [maven]
Posted by "Hiteshsai007 (via GitHub)" <gi...@apache.org>.
Hiteshsai007 commented on PR #12546:
URL: https://github.com/apache/maven/pull/12546#issuecomment-5100397641
Thanks for the review, @gnodet — all three points addressed in the force-push to `033146e`.
**1 & 2 — `echo` → `printf` in the debug logging**
You were right, and it was a real inconsistency: the PR argued that `echo` corrupts Windows paths, then kept using `echo` for the debug output. With `MAVEN_DEBUG_SCRIPT=1` under dash, `C:\tmp\build` was printed as `C:<tab>mp\build`.
I went slightly wider than the two lines flagged, since the same flaw applied elsewhere in the script. Every statement that interpolates a path now uses `printf '%s'`:
- `[DEBUG] Parser arguments:` and `[DEBUG] JvmConfigParser output:` (the two you pointed out)
- `[DEBUG] Found jvm.config file at:` and `Running JvmConfigParser with Java:`
- The three error-path lines: `jvm.config path:`, `Maven basedir:`, `Java command:`
- `[DEBUG] Final MAVEN_OPTS:`
- The `JAVA_HOME is set to "..."` diagnostic — pre-existing, same problem
Three `echo` calls remain deliberately: a numeric exit code and two literal strings with no interpolation. The rationale comment moved to the top of the script so it governs the whole file rather than sitting at one call site.
**3 — Test wired into the build**
Rather than just documenting it, the test now runs under `mvn verify` via `exec-maven-plugin`, bound to the `test` phase in a `shell-script-tests` profile activated on `<family>unix</family>` (it drives the POSIX `bin/mvn`; Windows ships `mvn.cmd`). It honours `-DskipTests`.
**Test coverage**
Added an assertion on the debug output that reproduces the exact problem you raised — it fails when those statements are reverted to `echo` and passes with `printf`, so this can't silently regress.
Also fixed a bug in the test harness itself that I'd missed: it appended the ambient `PATH` to the stub directory, so on a real Cygwin/MSYS2 machine the genuine `cygpath` was picked up and the missing-`cygpath` fallback was never exercised. The launcher now runs with a `PATH` containing only the stubs, which makes the result identical on every platform. Consequently the stubs use shell built-ins exclusively.
Now 29 assertions, verified green under `sh`, `dash`, `bash` and `busybox sh`, and still reporting 13 failures against the unpatched launcher.
**One note on scope:** this targets `master` (4.x), whereas #12537 asks specifically for a `maven-3.10.x` backport, so I've changed the trailer to `Related to` rather than `Fixes`. Points 1, 3 and the `echo`/`printf` issue apply equally to the 3.x script — happy to open a separate PR against `maven-3.10.x` if that's useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert paths for MSYS2 and jvm.config placeholders in bin/mvn [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #12546:
URL: https://github.com/apache/maven/pull/12546#issuecomment-5100813048
The macOS CI failures (all 22 assertion failures in `test-mvn-path-conversion.sh`) are caused by a double-slash in the temp directory path.
**Root cause:** On macOS, `$TMPDIR` ends with a trailing `/` (e.g. `/var/folders/.../T/`). When the test runs `mktemp -d "${TMPDIR}/mvn-path-conversion.XXXXXX"`, it produces a path with `//` (e.g. `.../T//mvn-path-conversion.XNBzZw`). The `pwd` builtin inside the `mvn` launcher normalizes this to a single `/`, so every assertion comparing the test's `$project_dir` / `$maven_home` against the actual launcher output mismatches.
**Fix:** Normalize `work_dir` through `cd/pwd` right after `mktemp`:
```sh
work_dir=`mktemp -d "${TMPDIR:-/tmp}/mvn-path-conversion.XXXXXX"`
work_dir=`cd "$work_dir" && pwd`
```
I've opened #12563 with this one-line fix applied on top of this PR's commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert paths for MSYS2 and jvm.config placeholders in bin/mvn [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #12546:
URL: https://github.com/apache/maven/pull/12546
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert paths for MSYS2 and jvm.config placeholders in bin/mvn [maven]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12546:
URL: https://github.com/apache/maven/pull/12546#issuecomment-5102697586
@gnodet Please assign appropriate label to PR according to the type of change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org