gnodet-bot commented on code in PR #13447:
URL: https://github.com/apache/maven/pull/13447#discussion_r4267623887


##########
apache-maven/src/test/scripts/test-mvn-jvm-config.sh:
##########
@@ -0,0 +1,190 @@
+#!/bin/sh
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# -----------------------------------------------------------------------------
+# Tests that JVM options from .mvn/jvm.config are correctly parsed and that
+# JVM notices printed to stderr (e.g. JAVA_TOOL_OPTIONS, JDK_JAVA_OPTIONS,
+# _JAVA_OPTIONS) do not pollute MAVEN_OPTS.
+#
+# Usage: sh apache-maven/src/test/scripts/test-mvn-jvm-config.sh
+# -----------------------------------------------------------------------------
+
+set -e
+
+script_dir=`cd "\`dirname "$0"\`" && pwd`
+mvn_script="$script_dir/../../assembly/maven/bin/mvn"
+
+if [ ! -f "$mvn_script" ]; then
+  echo "Cannot locate the mvn script at $mvn_script" >&2
+  exit 1
+fi
+
+sh_bin=`unset -f command; command -v sh`
+
+work_dir=`mktemp -d "${TMPDIR:-/tmp}/mvn-jvm-config.XXXXXX"`
+work_dir=`cd "$work_dir" && pwd`
+trap 'rm -rf "$work_dir"' EXIT INT TERM
+
+stub_dir="$work_dir/stubs"
+maven_home="$work_dir/maven-home"
+project_dir="$work_dir/project"
+
+mkdir -p "$stub_dir" "$maven_home/bin" "$maven_home/boot" "$project_dir/.mvn"
+
+cp "$mvn_script" "$maven_home/bin/mvn"
+touch "$maven_home/bin/m2.conf" "$maven_home/boot/plexus-classworlds-9.9.9.jar"
+
+# uname stub
+cat > "$stub_dir/uname" <<'STUB'
+#!/bin/sh
+echo "Linux"
+STUB
+
+# dirname stub
+cat > "$stub_dir/dirname" <<'STUB'
+#!/bin/sh
+path="$1"
+case "$path" in
+  */*)
+    path="${path%/*}"
+    [ -n "$path" ] || path="/"
+    ;;
+  *) path="." ;;
+esac
+printf '%s\n' "$path"
+STUB
+
+# java stub: simulates Java runtime notices on stderr when tool options env 
vars are set
+cat > "$stub_dir/java" <<'STUB'
+#!/bin/sh
+
+# If tool options are set, print JVM notices on stderr just like real JDK
+if [ -n "$JAVA_TOOL_OPTIONS" ]; then
+  echo "Picked up JAVA_TOOL_OPTIONS: $JAVA_TOOL_OPTIONS" >&2
+fi
+if [ -n "$JDK_JAVA_OPTIONS" ]; then
+  echo "NOTE: Picked up JDK_JAVA_OPTIONS: $JDK_JAVA_OPTIONS" >&2
+fi
+if [ -n "$_JAVA_OPTIONS" ]; then
+  echo "Picked up _JAVA_OPTIONS: $_JAVA_OPTIONS" >&2
+fi
+
+case " $* " in
+  *" -version "*) exit 0 ;;
+esac
+
+case "$1" in
+  *JvmConfigParser.java)
+    if [ "$FAIL_PARSER" = "1" ]; then
+      echo "Error: simulated parser failure" >&2
+      exit 1
+    fi
+    # Emulate JvmConfigParser output
+    while IFS= read -r line || [ -n "$line" ]; do
+      line="${line%%#*}"
+      for token in $line; do
+        printf '"%s" ' "$token"
+      done
+    done < "$2"
+    printf '\n'
+    exit 0 ;;
+esac

Review Comment:
   ⚠️ **Broken stub: `JvmConfigParser` emulation never fires**
   
   The java stub matches `$1` against `*JvmConfigParser.java)` — the old 
source-launch convention. The updated `mvn` script (the fix in this very PR) 
invokes the precompiled class:
   
   ```sh
   parser_output=$("$JAVACMD" -cp "$MAVEN_HOME/bin" JvmConfigParser "$1" 
"$MAVEN_PROJECTBASEDIR_NATIVE" ...)
   ```
   
   When the stub is called, `$1` is `-cp`, `$2` is `$MAVEN_HOME/bin`, `$3` is 
`JvmConfigParser`, and `$4` is the jvm.config path. The pattern 
`*JvmConfigParser.java)` can never match `-cp`. The stub falls through to the 
generic arg-printer, so jvm.config options are never parsed into MAVEN_OPTS and 
the `assert_contains "[-Xmx512m]"` assertions always fail — the test cannot 
verify the primary invariant.
   
   Fix: match on `$3` and read from `$4`:
   
   ```suggestion
   case "$3" in
     JvmConfigParser)
       if [ "$FAIL_PARSER" = "1" ]; then
         echo "Error: simulated parser failure" >&2
         exit 1
       fi
       # Emulate JvmConfigParser output
       while IFS= read -r line || [ -n "$line" ]; do
         line="${line%%#*}"
         for token in $line; do
           printf '"%s" ' "$token"
         done
       done < "$4"
       printf '\n'
       exit 0 ;;
   esac
   ```



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