gnodet commented on code in PR #13411:
URL: https://github.com/apache/maven/pull/13411#discussion_r4242578977
##########
apache-maven/src/assembly/maven/bin/mvn:
##########
@@ -397,27 +397,41 @@ concat_lines() {
return 1
}
- # Run the parser using source-launch mode
- # Capture both stdout and stderr for comprehensive error reporting
- parser_output=$("$JAVACMD" "$MAVEN_HOME/bin/JvmConfigParser.java" "$1"
"$MAVEN_PROJECTBASEDIR_NATIVE" 2>&1)
+ # Run the parser using source-launch mode. Keep stderr separate because
the JVM
+ # writes JAVA_TOOL_OPTIONS / JDK_JAVA_OPTIONS notices to stderr on
successful
+ # startup; those notices must not become JVM arguments.
+ parser_stderr_file=$(mktemp "${TMPDIR:-/tmp}/maven-jvm-config.XXXXXX") || {
+ echo "ERROR: Unable to create temporary file for JvmConfigParser stderr"
>&2
+ return 1
+ }
+ parser_output=$("$JAVACMD" "$MAVEN_HOME/bin/JvmConfigParser.java" "$1"
"$MAVEN_PROJECTBASEDIR_NATIVE" 2>"$parser_stderr_file")
parser_exit=$?
if [ -n "$MAVEN_DEBUG_SCRIPT" ]; then
echo "[DEBUG] JvmConfigParser exit code: $parser_exit" >&2
- printf '[DEBUG] JvmConfigParser output: %s\n' "$parser_output" >&2
+ printf '[DEBUG] JvmConfigParser stdout: %s\n' "$parser_output" >&2
+ if [ -s "$parser_stderr_file" ]; then
+ echo "[DEBUG] JvmConfigParser stderr:" >&2
+ sed 's/^/[DEBUG] /' "$parser_stderr_file" >&2
+ fi
fi
if [ $parser_exit -ne 0 ]; then
- # Parser failed - print comprehensive error information
+ # Parser failed - print comprehensive error information, including
stderr.
echo "ERROR: JvmConfigParser failed with exit code $parser_exit" >&2
printf ' jvm.config path: %s\n' "$1" >&2
printf ' Maven basedir: %s\n' "$MAVEN_PROJECTBASEDIR" >&2
printf ' Java command: %s\n' "$JAVACMD" >&2
echo " Parser output:" >&2
printf '%s\n' "$parser_output" | sed 's/^/ /' >&2
+ if [ -s "$parser_stderr_file" ]; then
Review Comment:
⚠️ **Missing label for parser stderr on error path**
The debug path (lines 412–415) correctly labels its stderr output with
`[DEBUG] JvmConfigParser stderr:`. But on the error path, the stderr block has
no label — it's silently appended after `" Parser output:"`, making it
indistinguishable from stdout in the error message.
```suggestion
if [ -s "$parser_stderr_file" ]; then
echo " Parser stderr:" >&2
sed 's/^/ /' "$parser_stderr_file" >&2
fi
```
##########
apache-maven/src/assembly/maven/bin/mvn:
##########
@@ -397,27 +397,41 @@ concat_lines() {
return 1
}
- # Run the parser using source-launch mode
- # Capture both stdout and stderr for comprehensive error reporting
- parser_output=$("$JAVACMD" "$MAVEN_HOME/bin/JvmConfigParser.java" "$1"
"$MAVEN_PROJECTBASEDIR_NATIVE" 2>&1)
+ # Run the parser using source-launch mode. Keep stderr separate because
the JVM
+ # writes JAVA_TOOL_OPTIONS / JDK_JAVA_OPTIONS notices to stderr on
successful
+ # startup; those notices must not become JVM arguments.
Review Comment:
💡 **Minor:** Consider adding `$$` to the `mktemp` prefix for easier
debugging/tracing.
```suggestion
parser_stderr_file=$(mktemp
"${TMPDIR:-/tmp}/maven-jvm-config.$$.XXXXXX") || {
```
The `XXXXXX` suffix already guarantees uniqueness, but including the PID
makes it trivial to correlate the temp file with the Maven process in `lsof` or
`strace` output.
--
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]
