gnodet commented on PR #13652:
URL: https://github.com/apache/maven/pull/13652#issuecomment-6339274779

   Reverted the FOR /F approach and went back to `endlocal & set 
"JVM_CONFIG_MAVEN_OPTS=!JVM_CONFIG_MAVEN_OPTS!"` (commit `dac74767f8`).
   
   The FOR /F approaches had compounding issues:
   
   1. `""!VAR!""` + `%%~V` -- breaks when the value is empty (FOR /F skips 
empty token -> `endlocal` never called -> `setlocal` scope leaks)
   2. `"#""!VAR!"""` + `:~1` strip -- JvmConfigParser wraps every arg in 
`"..."`, so `JVM_CONFIG_MAVEN_OPTS` is e.g. `"-Xmx512m" "-Dprop=foo bar"` -- 
the embedded quotes and spaces corrupt the `FOR /F in(...)` clause regardless 
of the sentinel
   
   The bot's concern about `!VAR!` not working after `endlocal` is incorrect. 
On a single `cmd1 & cmd2` line, cmd.exe parses (and expands `!VAR!`) the entire 
line while `EnableDelayedExpansion` is still active, then executes `endlocal`, 
then runs `set` with the already-expanded value. This is the canonical pattern 
for exporting variables from a `setlocal EnableDelayedExpansion` scope -- see 
https://stackoverflow.com/a/13871514 (dbenham).
   


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

Reply via email to