You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "choksi2212 (via GitHub)" <gi...@apache.org> on 2026/10/12 07:05:29 UTC

[PR] Verify the cygpath on the PATH converts to usable Windows paths [maven]

choksi2212 opened a new pull request, #13429:
URL: https://github.com/apache/maven/pull/13429

   ## Description
   
   `bin/mvn` finds the first `cygpath(1)` on the `PATH` and trusts its output for
   every Windows path conversion. That binary does not have to be the one shipped
   with the POSIX emulation layer the shell runs on. When, for example, an
   Anaconda installation comes first on the `PATH`, its MSYS `cygpath` re-roots
   every POSIX path under its own installation prefix:
   
   ```
   $JAVA_HOME  => C:\Users\[username]\anaconda3\Library\c\tools\dev\java\...
   $MAVEN_HOME => C:\Users\[username]\anaconda3\Library\c\tools\dev\java\maven\...
   ```
   
   All converted values then point to directories that do not exist, and the
   launch fails with a bare `ClassNotFoundException`:
   
   ```
   Erreur : impossible de trouver ou de charger la classe principale
   org.codehaus.plexus.classworlds.launcher.Launcher
   Causé par : java.lang.ClassNotFoundException: ...
   ```
   
   The guard introduced with #12567 verifies that a `cygpath` binary is present,
   but not that its output is usable. This change adds a second check right next
   to it: convert the current directory — the one path that is known to exist at
   this point of the script — and require its Windows form to exist on disk. When
   the probe fails, the launcher warns with the `cygpath` that was found, shows
   the conversion it produced, and keeps the POSIX paths, exactly like the
   existing missing-`cygpath` fallback. On a real Git Bash the shell then
   auto-converts the POSIX arguments when it spawns the JVM, so the launch
   succeeds, and the user gets an explanation instead of a bare class loading
   error.
   
   ## Steps to reproduce (from the issue)
   
   1. Install an Anaconda distribution on Windows; its `Library\bin` (which ships
      an MSYS `cygpath`) comes first on the `PATH`.
   2. Download an Apache Maven 3.10.x distribution and unzip it.
   3. In a Git Bash / MinGW shell: `./bin/mvn --help`.
   4. Before this change: `ClassNotFoundException` for the classworlds launcher,
      with no hint about the actual problem.
   
   ## Type of Bug
   
   Diagnostic/robustness bug in the POSIX launcher introduced by #12567: a
   `cygpath` binary from an unrelated installation is trusted blindly.
   
   ## Fix and Implementation
   
   * Convert the current directory with the `cygpath` found on the `PATH` and
     verify the result exists on disk (`[ -d ]`) before doing any of the later
     conversions.
   * On failure, print a warning that names the offending `cygpath` and the
     non-existing Windows form it produced, and disable the conversion, falling
     back to the POSIX paths — the same posture as the missing-`cygpath` case
     that #12567 already handles.
   
   The current directory is used because it is the only path the script can rely
   on at that point (it runs before `MAVEN_HOME` is even resolved, and any of the
   user-provided values may point anywhere).
   
   ## Testing and Verification
   
   **All verification is empirical; every step was run on real Windows 11 (Git
   Bash/MSYS) and real Linux (Debian/kali via WSL).**
   
   1. `sh -n apache-maven/src/bin/mvn` and `bash -n` — clean.
   2. `test-mvn-path-conversion.sh` — the existing harness, extended with a third
      stub set whose `cygpath` re-roots paths like the Anaconda one:
      * on Linux: **27/27 assertions pass** — all 21 pre-existing assertions stay
        green (POSIX untouched, CYGWIN/MINGW32/MINGW64/MSYS conversions,
        missing-`cygpath` fallback), and the 6 new assertions pass:
        foreign `cygpath` → paths fall back to POSIX form, no empty
        `-Dmaven.home=`, the warning names the offending `cygpath`, shows the
        bogus Windows form, and points at the discovered binary.
      * on Windows Git Bash: all assertions pass as well (the stubbed probe target
        is materialized as a literal `C:`-prefixed directory name, which POSIX
        filesystems accept; on Windows the real directory already exists).
   3. **Negative control**: with `bin/mvn` reverted to the branch state, the new
      harness fails exactly the 5 assertions the fix introduces (`foreign
      cygpath: … falls back to a POSIX path` ×2, warning ×3), 12 assertions fail
      in total (the conversions silently produce garbage before the fix) —
      i.e. the tests are load-bearing for this change.
   4. **Real Windows end-to-end, faithful `cygpath`**: the fixed launcher on a
      real Apache Maven distribution (3.9.16 layout) with the real `cygpath` and
      a real JDK 17 prints the full `mvn --version` banner (Maven home, Java
      version, locale, `OS name: "windows 11"`), even in a shell where MSYS
      argument auto-conversion is disabled (`MSYS_NO_PATHCONV=1`) — the explicit
      conversions make the launcher robust in that environment too.
   5. **Real Windows end-to-end, foreign `cygpath`**: with an Anaconda-style
      re-rooting `cygpath` first on the `PATH`, the fixed launcher prints the
      four-line warning naming the binary and the non-existing conversion, and
      skips the conversions.
   
   Before this change the same launch in (4)'s `MSYS_NO_PATHCONV` shell dies with
   the issue's `ClassNotFoundException` — reproduced.
   
   Note: the fix only uses POSIX shell built-ins and `cygpath` itself; no new
   external commands, so the launcher keeps working with the minimal `PATH`
   assumptions it already documents.
   
   ## Documentation
   
   The change is behavior-preserving for every environment where the previous
   launcher worked (the probe only ever *disables* conversions that would have
   produced non-existing paths). The new warning text is the documentation the
   user sees.
   
   ## Backport
   
   Filed against `maven-3.10.x` because the issue is reported for 3.10.0 and
   #12567 landed on this branch. Happy to open the same change for `master`
   (cling launcher, same guard block) as a follow-up if wanted.
   
   Closes #13414
   
   - [X] I hereby declare this contribution to be licenced under the [Apache License Version 2.0](http://www.apache.org/licenses/LICENSE-2.0)
   


-- 
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] Verify the cygpath on the PATH converts to usable Windows paths [maven]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #13429:
URL: https://github.com/apache/maven/pull/13429


-- 
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] Verify the cygpath on the PATH converts to usable Windows paths [maven]

Posted by "choksi2212 (via GitHub)" <gi...@apache.org>.
choksi2212 commented on PR #13429:
URL: https://github.com/apache/maven/pull/13429#issuecomment-6122464318

   Good catch, thank you — reproduced and fixed in 4883fca.
   
   I confirmed the truncation empirically before changing anything: running the new harness under `dash` with a `c`-prefixed component in the temp directory (`TMPDIR=/tmp/mvnprobe/cache`) makes the exact assertion you cited fail on the pre-fix code — the warning is cut to `...converts to "C:\Anaconda\Library\tmp\mvnprobe` (everything after `\c` dropped), so `which does not exist` never reaches the user. The same run with the `printf '%s\n'` form passes.
   
   The block now uses `printf` for all four warning lines, verbatim as suggested. The remaining `echo` calls in the launcher only print plain literals without backslashes, which the header note (echo vs printf) already documents as safe.
   
   Verified after the change, both directions:
   - harness under `dash`, c-free temp dir: all assertions pass
   - harness under `dash`, `cache` temp dir: all assertions pass (was: 1 failure, truncated warning)
   - pre-fix code (echo restored) + `cache` temp dir: the failure reproduces, so the assertion is load-bearing for this follow-up too
   
   `sh -n` on the modified launcher is clean as well.
   


-- 
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] Verify the cygpath on the PATH converts to usable Windows paths [maven]

Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13429:
URL: https://github.com/apache/maven/pull/13429#discussion_r4245494765


##########
apache-maven/src/bin/mvn:
##########
@@ -75,6 +75,26 @@ if $cygwin || $mingw ; then
   fi
 fi
 
+# A cygpath(1) from an unrelated POSIX emulation layer (e.g. the one shipped in
+# an Anaconda installation) can be found on the PATH first. It re-roots every
+# POSIX path under its own installation prefix, so all converted values point to
+# non-existing directories and the JVM dies with a bare ClassNotFoundException.
+# cygpath can convert relative paths too, but the current directory is the only
+# path that is known to exist at this point: verify that its Windows form can be
+# found on disk before trusting any of the later conversions.
+if $cygwin || $mingw ; then
+  _cygpath_bin=`( \unset -f command; \command -v cygpath )` || _cygpath_bin=
+  _probe_win=`cygpath --windows "$PWD"` || _probe_win=
+  if [ -z "$_probe_win" ] || [ ! -d "$_probe_win" ] ; then
+    echo "Warning: the cygpath found on the PATH (\"$_cygpath_bin\") does not convert paths to" >&2
+    echo "usable Windows paths (\"$PWD\" converts to \"$_probe_win\", which does not exist)," >&2
+    echo "so Apache Maven cannot convert paths to the Windows format." >&2
+    echo "Some plugins may misbehave when they receive POSIX paths." >&2

Review Comment:
   ⚠️ **`echo` with Windows paths silently truncates the warning under `dash`.**
   
   The script runs under `#!/bin/sh` — on Debian/Ubuntu (including GitHub Actions `ubuntu-latest`) `/bin/sh` is `dash`, which interprets backslash sequences in its built-in `echo`. `$_probe_win` is a Windows path produced by `cygpath --windows`, so it contains backslashes. When the POSIX path contains a component starting with `c` (e.g. `cache`, `conda`, `config`, `cmake`), the Windows form contains `\c` — and dash's `echo` interprets `\c` as **suppress all further output**, silently truncating the diagnostic. The user sees a broken, incomplete warning in exactly the scenario this fix is designed to explain.
   
   Confirmed: `sh -c 'echo "test \\cache\\scratch rest"'` → `test` (stops at `\c`).
   
   Also confirmed by test run: the assertion `"foreign cygpath: warning shows the bogus Windows form"` (checking for `"which does not exist"`) fails with the fix on any system where the temp dir contains `cache/` in its path — the warning is cut off before that phrase is printed.
   
   Use `printf '%s\n'` instead — `printf` does not interpret backslash sequences in its arguments:
   
   ```suggestion
       printf '%s\n' "Warning: the cygpath found on the PATH (\"$_cygpath_bin\") does not convert paths to" >&2
       printf '%s\n' "usable Windows paths (\"$PWD\" converts to \"$_probe_win\", which does not exist)," >&2
       printf '%s\n' "so Apache Maven cannot convert paths to the Windows format." >&2
       printf '%s\n' "Some plugins may misbehave when they receive POSIX paths." >&2
   ```



-- 
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] Verify the cygpath on the PATH converts to usable Windows paths [maven]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13429:
URL: https://github.com/apache/maven/pull/13429#issuecomment-6119437532

   <!-- license-declaration-check -->
   ### ⚠️ License declaration missing
   
   The pull request description must contain at least one of the following lines with a **checked** box (`[x]`):
   
   ```markdown
   - [ ] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
   - [ ] I hereby declare that I have filed an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   ```
   
   This comment will be hidden automatically once the description is fixed.


-- 
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] Verify the cygpath on the PATH converts to usable Windows paths [maven]

Posted by "choksi2212 (via GitHub)" <gi...@apache.org>.
choksi2212 commented on code in PR #13429:
URL: https://github.com/apache/maven/pull/13429#discussion_r4247006956


##########
apache-maven/src/bin/mvn:
##########
@@ -75,6 +75,26 @@ if $cygwin || $mingw ; then
   fi
 fi
 
+# A cygpath(1) from an unrelated POSIX emulation layer (e.g. the one shipped in
+# an Anaconda installation) can be found on the PATH first. It re-roots every
+# POSIX path under its own installation prefix, so all converted values point to
+# non-existing directories and the JVM dies with a bare ClassNotFoundException.
+# cygpath can convert relative paths too, but the current directory is the only
+# path that is known to exist at this point: verify that its Windows form can be
+# found on disk before trusting any of the later conversions.
+if $cygwin || $mingw ; then
+  _cygpath_bin=`( \unset -f command; \command -v cygpath )` || _cygpath_bin=
+  _probe_win=`cygpath --windows "$PWD"` || _probe_win=
+  if [ -z "$_probe_win" ] || [ ! -d "$_probe_win" ] ; then
+    echo "Warning: the cygpath found on the PATH (\"$_cygpath_bin\") does not convert paths to" >&2
+    echo "usable Windows paths (\"$PWD\" converts to \"$_probe_win\", which does not exist)," >&2
+    echo "so Apache Maven cannot convert paths to the Windows format." >&2
+    echo "Some plugins may misbehave when they receive POSIX paths." >&2

Review Comment:
   Applied in 4883fca — you were right, I reproduced the truncation under dash with a c-prefixed temp-dir component (assertion fails on the pre-fix code, passes with printf). Comment above has the full evidence.



-- 
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