gnodet commented on code in PR #13056: URL: https://github.com/apache/maven/pull/13056#discussion_r3944571229
########## apache-maven/src/assembly/maven/bin/mvn.cmd: ########## @@ -74,14 +74,67 @@ if not exist "%JAVACMD%" ( goto error ) -@REM Check Java version by testing the Java 17+ flag -"%JAVACMD%" --enable-native-access=ALL-UNNAMED -version >nul 2>&1 -if ERRORLEVEL 1 ( - echo Error: Apache Maven 4.x requires Java 17 or newer to run. >&2 - "%JAVACMD%" -version >&2 - echo Please upgrade your Java installation or set JAVA_HOME to point to a compatible JDK. >&2 - goto error -) +@REM Scan the arguments for version/quiet flags so that version-only +@REM invocations can be answered without starting Maven itself; the Java-17 +@REM gate below then doubles as the settings probe used to render the banner. +@REM Arguments are inspected via :getFlagArg instead of "for %%a in (%*)" so +@REM that quoted argument boundaries are preserved: both the "--" and the +@REM "for" tokenizers split values like -Dfoo=-v into "-Dfoo" and "-v", which +@REM would false-trigger the version fast path. The scan leaves the positional +@REM parameters intact so the regular argument handling below (-f/--file and +@REM Maven goal args) is unaffected. +set "IS_VERSION_AND_EXIT=" +set "IS_SHOW_VERSION=" +set "IS_QUIET=" +set "IS_VERBOSE=" +set "IS_MAIN_OVERRIDE=" +set "_ARG_IDX=0" +:parseFlags +set /a _ARG_IDX+=1 +call :getFlagArg %_ARG_IDX% %* +if "%_FLAG_ARG%"=="" goto parseFlagsDone +if "%_FLAG_ARG%"=="--" goto parseFlagsDone +if "%_FLAG_ARG%"=="-v" set "IS_VERSION_AND_EXIT=1" +if "%_FLAG_ARG%"=="--version" set "IS_VERSION_AND_EXIT=1" +if "%_FLAG_ARG%"=="-V" set "IS_SHOW_VERSION=1" +if "%_FLAG_ARG%"=="--show-version" set "IS_SHOW_VERSION=1" +if "%_FLAG_ARG%"=="-q" set "IS_QUIET=1" +if "%_FLAG_ARG%"=="--quiet" set "IS_QUIET=1" +if "%_FLAG_ARG%"=="-X" set "IS_VERBOSE=1" +if "%_FLAG_ARG%"=="--debug" set "IS_VERBOSE=1" +if "%_FLAG_ARG%"=="--enc" set "IS_MAIN_OVERRIDE=1" +if "%_FLAG_ARG%"=="--shell" set "IS_MAIN_OVERRIDE=1" +if "%_FLAG_ARG%"=="--up" set "IS_MAIN_OVERRIDE=1" +@REM Compact single-dash tokens (e.g. -qv, -vX) mirror the Unix script's +@REM -[qvVXe]* handling, but only when the part after '-' is made exclusively +@REM of the safe chars v V q X e; otherwise (e.g. -f, -D...) the token is +@REM skipped so property values like -Dfoo=-v never trigger the fast path. +@REM Each set is a separate top-level line so %var% is expanded after the +@REM previous assignment, avoiding the need for delayed expansion. +if not "%_FLAG_ARG:~0,1%"=="-" goto parseFlags +set "_FLAG_REST=%_FLAG_ARG:~1%" +set "_FLAG_CHECK=%_FLAG_REST:v=%" +set "_FLAG_CHECK=%_FLAG_CHECK:V=%" +set "_FLAG_CHECK=%_FLAG_CHECK:q=%" +set "_FLAG_CHECK=%_FLAG_CHECK:X=%" +set "_FLAG_CHECK=%_FLAG_CHECK:e=%" +if not "%_FLAG_CHECK%"=="" goto parseFlags +if not "%_FLAG_ARG:v=%"=="%_FLAG_ARG%" set "IS_VERSION_AND_EXIT=1" +if not "%_FLAG_ARG:V=%"=="%_FLAG_ARG%" set "IS_SHOW_VERSION=1" +if not "%_FLAG_ARG:q=%"=="%_FLAG_ARG%" set "IS_QUIET=1" +if not "%_FLAG_ARG:X=%"=="%_FLAG_ARG%" set "IS_VERBOSE=1" +if not "%_FLAG_ARG:e=%"=="%_FLAG_ARG%" set "IS_MAIN_OVERRIDE=1" +goto parseFlags Review Comment: ⚠️ **Warning:** `-e` (`--errors`) maps to `IS_MAIN_OVERRIDE` here, which causes the fast version path to be bypassed. But `-e` just controls error stack trace output — it does NOT change the main class (unlike `--enc`, `--shell`, `--up`). The Unix script correctly includes `e` in the compact pattern `-[qvVXe]*` as a "safe" character but does **not** set any bypass flag for it. So on Unix, `-ve` uses the fast path; on Windows, `-ve` falls through to the full Maven JVM — a behavioral inconsistency. Fix: remove this line. The `e` character is already included in the `_FLAG_CHECK` guard (line 120) so it won't reject compact tokens containing `e`. ########## apache-maven/src/assembly/maven/bin/mvn.cmd: ########## @@ -74,14 +74,67 @@ if not exist "%JAVACMD%" ( goto error ) -@REM Check Java version by testing the Java 17+ flag -"%JAVACMD%" --enable-native-access=ALL-UNNAMED -version >nul 2>&1 -if ERRORLEVEL 1 ( - echo Error: Apache Maven 4.x requires Java 17 or newer to run. >&2 - "%JAVACMD%" -version >&2 - echo Please upgrade your Java installation or set JAVA_HOME to point to a compatible JDK. >&2 - goto error -) +@REM Scan the arguments for version/quiet flags so that version-only +@REM invocations can be answered without starting Maven itself; the Java-17 +@REM gate below then doubles as the settings probe used to render the banner. +@REM Arguments are inspected via :getFlagArg instead of "for %%a in (%*)" so +@REM that quoted argument boundaries are preserved: both the "--" and the +@REM "for" tokenizers split values like -Dfoo=-v into "-Dfoo" and "-v", which +@REM would false-trigger the version fast path. The scan leaves the positional +@REM parameters intact so the regular argument handling below (-f/--file and +@REM Maven goal args) is unaffected. +set "IS_VERSION_AND_EXIT=" +set "IS_SHOW_VERSION=" +set "IS_QUIET=" +set "IS_VERBOSE=" +set "IS_MAIN_OVERRIDE=" +set "_ARG_IDX=0" +:parseFlags +set /a _ARG_IDX+=1 +call :getFlagArg %_ARG_IDX% %* +if "%_FLAG_ARG%"=="" goto parseFlagsDone +if "%_FLAG_ARG%"=="--" goto parseFlagsDone +if "%_FLAG_ARG%"=="-v" set "IS_VERSION_AND_EXIT=1" +if "%_FLAG_ARG%"=="--version" set "IS_VERSION_AND_EXIT=1" +if "%_FLAG_ARG%"=="-V" set "IS_SHOW_VERSION=1" +if "%_FLAG_ARG%"=="--show-version" set "IS_SHOW_VERSION=1" +if "%_FLAG_ARG%"=="-q" set "IS_QUIET=1" +if "%_FLAG_ARG%"=="--quiet" set "IS_QUIET=1" +if "%_FLAG_ARG%"=="-X" set "IS_VERBOSE=1" +if "%_FLAG_ARG%"=="--debug" set "IS_VERBOSE=1" +if "%_FLAG_ARG%"=="--enc" set "IS_MAIN_OVERRIDE=1" +if "%_FLAG_ARG%"=="--shell" set "IS_MAIN_OVERRIDE=1" +if "%_FLAG_ARG%"=="--up" set "IS_MAIN_OVERRIDE=1" +@REM Compact single-dash tokens (e.g. -qv, -vX) mirror the Unix script's +@REM -[qvVXe]* handling, but only when the part after '-' is made exclusively +@REM of the safe chars v V q X e; otherwise (e.g. -f, -D...) the token is +@REM skipped so property values like -Dfoo=-v never trigger the fast path. +@REM Each set is a separate top-level line so %var% is expanded after the +@REM previous assignment, avoiding the need for delayed expansion. +if not "%_FLAG_ARG:~0,1%"=="-" goto parseFlags +set "_FLAG_REST=%_FLAG_ARG:~1%" +set "_FLAG_CHECK=%_FLAG_REST:v=%" +set "_FLAG_CHECK=%_FLAG_CHECK:V=%" +set "_FLAG_CHECK=%_FLAG_CHECK:q=%" +set "_FLAG_CHECK=%_FLAG_CHECK:X=%" +set "_FLAG_CHECK=%_FLAG_CHECK:e=%" +if not "%_FLAG_CHECK%"=="" goto parseFlags +if not "%_FLAG_ARG:v=%"=="%_FLAG_ARG%" set "IS_VERSION_AND_EXIT=1" +if not "%_FLAG_ARG:V=%"=="%_FLAG_ARG%" set "IS_SHOW_VERSION=1" Review Comment: 🔴 **Bug:** CMD `%var:str=%` substitution is **case-insensitive**. This means `%_FLAG_ARG:v=%` removes both `v` AND `V` from the string, and `%_FLAG_ARG:V=%` also removes both. Consequence: for a compact option like `-qV` (show-version + quiet): - Line 122: `%_FLAG_ARG:v=%` strips the `V` → not equal → incorrectly sets `IS_VERSION_AND_EXIT=1` - Line 123: `%_FLAG_ARG:V=%` strips the `V` → not equal → correctly sets `IS_SHOW_VERSION=1` So `-qV` would print the version and **exit** instead of printing the banner and continuing the build. Similarly, `-qv` would also set `IS_SHOW_VERSION` (wrong). The same case-insensitivity affects `q`/`Q` and `x`/`X`, but those are less problematic since the lowercase/uppercase variants aren't used for different flags. Since delayed expansion (`!var:str=!`) has the same case-insensitive behavior, one approach would be to use `findstr` with case-sensitive matching, or skip compact flag detection entirely on Windows (the exact-match single-flag checks at lines 95–107 are already correct and sufficient). -- 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]
