gnodet-bot commented on code in PR #13652:
URL: https://github.com/apache/maven/pull/13652#discussion_r4346832531
##########
apache-maven/src/assembly/maven/bin/mvn.cmd:
##########
@@ -336,17 +336,21 @@ if exist "%JVM_CONFIG_TEMP%" (
echo [DEBUG] Temp file contents:
type "%JVM_CONFIG_TEMP%"
)
+ setlocal EnableDelayedExpansion
set /p JVM_CONFIG_MAVEN_OPTS=<"%JVM_CONFIG_TEMP%" 2>nul
rem Retry once after a brief delay if the read returned empty (Windows
Defender file lock).
rem Note: use string comparison instead of 'if not defined' because 'set
JVM_CONFIG_MAVEN_OPTS='
rem (empty assignment) counts as defined, so 'if not defined' would never
trigger the retry.
- if "%JVM_CONFIG_MAVEN_OPTS%"=="" (
+ rem Note: use delayed expansion (!VAR! instead of %VAR%) because cmd.exe
expands %VAR% at
+ rem parse time for the entire compound block, before 'set /p' above has run
at runtime.
+ if "!JVM_CONFIG_MAVEN_OPTS!"=="" (
if defined MAVEN_DEBUG_SCRIPT (
echo [DEBUG] First read returned empty, retrying after delay...
)
ping -n 2 127.0.0.1 >nul 2>nul
set /p JVM_CONFIG_MAVEN_OPTS=<"%JVM_CONFIG_TEMP%" 2>nul
)
+ endlocal & set "JVM_CONFIG_MAVEN_OPTS=%JVM_CONFIG_MAVEN_OPTS%"
del "%JVM_CONFIG_TEMP%" 2>nul
Review Comment:
`endlocal & set "JVM_CONFIG_MAVEN_OPTS=%JVM_CONFIG_MAVEN_OPTS%"` is inside
the `if exist "%JVM_CONFIG_TEMP%" (` compound block. cmd.exe expands **all**
`%VAR%` in the entire compound block at parse time — before any line inside has
executed. At parse time, `JVM_CONFIG_MAVEN_OPTS` holds the empty string from
the earlier `set JVM_CONFIG_MAVEN_OPTS=` (line 283), so this line becomes
`endlocal & set "JVM_CONFIG_MAVEN_OPTS="` unconditionally. The value written by
`set /p` is never exported from the `setlocal` scope.
**Reference:** [cmd.exe parse-time expansion in compound
blocks](https://stackoverflow.com/questions/16852503): *"When a compound
statement enclosed in parentheses is to be executed, the statement is first
parsed… At this time, any `%var%` is replaced by that var's value… (its
PARSE-TIME value)."*
Note that using `!JVM_CONFIG_MAVEN_OPTS!` here would not help: in the
`endlocal & set` idiom, `endlocal` runs first, destroying the inner scope, so
`!VAR!` would then expand to the outer scope's (empty) value.
**Fix:** Move `endlocal & set` **outside** the `if exist` compound block
(after the closing `)`). When it is a standalone line,
`%JVM_CONFIG_MAVEN_OPTS%` is expanded at line-execution time, after `set /p`
has run and `setlocal`'s scope is still active for the expansion:
```suggestion
del "%JVM_CONFIG_TEMP%" 2>nul
)
endlocal & set "JVM_CONFIG_MAVEN_OPTS=%JVM_CONFIG_MAVEN_OPTS%"
```
--
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]
