gnodet-bot commented on code in PR #13239:
URL: https://github.com/apache/maven/pull/13239#discussion_r4070427096
##########
apache-maven/src/assembly/maven/bin/mvn:
##########
@@ -462,6 +462,42 @@ if $cygwin || $mingw ; then
fi
MAVEN_PROJECTBASEDIR="$MAVEN_PROJECTBASEDIR_NATIVE"
+# MNG-8056: Under Cygwin, the JVM is a native Windows process and cannot
resolve
+# Cygwin-style POSIX paths (e.g. /cygdrive/c/...) directly. The internal paths
+# (MAVEN_HOME, CLASSWORLDS_CONF, JAVA_HOME, …) are already converted above, but
+# user-supplied paths passed via -f/--file, -s/--settings,
-gs/--global-settings,
+# -st/--toolchains and -gt/--global-toolchains reach the JVM unconverted.
+# The loop below rewrites only those argument values that begin with '/'
(absolute
+# POSIX paths); relative paths and already-Windows paths are left unchanged.
+# MinGW/MSYS2 perform automatic path mangling, so the fix is Cygwin-only.
+if $cygwin ; then
+ _np=false
+ _count=$#
+ while [ $_count -gt 0 ]; do
+ _a="$1"
+ shift
+ if $_np; then
+ case "$_a" in
+ /*) _a=$(cygpath --windows "$_a") ;;
+ esac
+ _np=false
+ else
+ case "$_a" in
+
-f|--file|-s|--settings|-gs|--global-settings|-st|--toolchains|-gt|--global-toolchains)
+ _np=true ;;
+
--file=/*|--settings=/*|--global-settings=/*|--toolchains=/*|--global-toolchains=/*)
+ _flag="${_a%%=*}"
+ _path="${_a#*=}"
+ _a="${_flag}=$(cygpath --windows "${_path}")"
+ ;;
+ esac
+ fi
+ set -- "$@" "$_a"
+ _count=$(( _count - 1 ))
+ done
+fi
+
+
Review Comment:
🔧 **Nit: spurious blank line.** Two blank lines between the `fi` closing the
MNG-8056 block and `handle_args()`. The rest of the script uses a single blank
line between top-level blocks.
```suggestion
fi
handle_args() {
```
##########
apache-maven/src/test/scripts/test-mvn-path-conversion.sh:
##########
@@ -176,9 +186,18 @@ failures=0
# run_mvn <uname-output> <stub-dir>
run_mvn() {
+ run_mvn_with_args "$1" "$2" verify
+}
+
+# run_mvn_with_args <uname-output> <stub-dir> [args...]
+run_mvn_with_args() {
+ uname_out="$1"
+ shift
+ stub_dir="$1"
+ shift
( cd "$project_dir/module" &&
- FAKE_UNAME="$1" PATH="$2" JAVA_HOME= MAVEN_SKIP_RC=1 \
- "$sh_bin" "$maven_home/bin/mvn" verify 2>/dev/null )
+ FAKE_UNAME="$uname_out" PATH="$stub_dir" JAVA_HOME= MAVEN_SKIP_RC=1 \
+ "$sh_bin" "$maven_home/bin/mvn" "$@" )
Review Comment:
⚠️ **`run_mvn_with_args` drops `2>/dev/null` — existing tests now emit noise
to stderr.**
The original `run_mvn()` had `2>/dev/null`, which was intentional: the stub
environment generates warnings (`POM file ... does not exist`, `cygpath was not
found on the PATH`) that pollute test output and CI logs. By routing
`run_mvn()` through `run_mvn_with_args()`, that suppression is now removed for
all callers.
The 5 new test cases each invoke `run_mvn_with_args` directly and
deliberately pass a non-existent POM path, producing 4 warning lines on stderr
when CI runs this. The new tests all pass, but the warnings appear
unconditionally.
Simplest fix — suppress stderr in `run_mvn_with_args` and let
`run_mvn_debug` (which intentionally captures stderr) remain the exception:
```suggestion
"$sh_bin" "$maven_home/bin/mvn" "$@" 2>/dev/null )
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
