sunchao commented on code in PR #6678:
URL: https://github.com/apache/datafusion-comet/pull/6678#discussion_r4186309692


##########
mvnw:
##########
@@ -19,290 +19,277 @@
 # ----------------------------------------------------------------------------
 
 # ----------------------------------------------------------------------------
-# Apache Maven Wrapper startup batch script, version 3.2.0
-#
-# Required ENV vars:
-# ------------------
-#   JAVA_HOME - location of a JDK home dir
+# Apache Maven Wrapper startup batch script, version 3.3.4
 #
 # Optional ENV vars
 # -----------------
-#   MAVEN_OPTS - parameters passed to the Java VM when running Maven
-#     e.g. to debug Maven itself, use
-#       set MAVEN_OPTS=-Xdebug 
-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=8000
-#   MAVEN_SKIP_RC - flag to disable loading of mavenrc files
+#   JAVA_HOME - location of a JDK home dir, required when download maven via 
java source
+#   MVNW_REPOURL - repo url base for downloading maven distribution
+#   MVNW_USERNAME/MVNW_PASSWORD - user and password for downloading maven
+#   MVNW_VERBOSE - true: enable verbose log; debug: trace the mvnw script; 
others: silence the output
 # ----------------------------------------------------------------------------
 
-if [ -z "$MAVEN_SKIP_RC" ] ; then
-
-  if [ -f /usr/local/etc/mavenrc ] ; then
-    . /usr/local/etc/mavenrc
-  fi
-
-  if [ -f /etc/mavenrc ] ; then
-    . /etc/mavenrc
-  fi
-
-  if [ -f "$HOME/.mavenrc" ] ; then
-    . "$HOME/.mavenrc"
-  fi
+set -euf
+[ "${MVNW_VERBOSE-}" != debug ] || set -x
 
-fi
-
-# OS specific support.  $var _must_ be set to either true or false.
-cygwin=false;
-darwin=false;
-mingw=false
+# OS specific support.
+native_path() { printf %s\\n "$1"; }
 case "$(uname)" in
-  CYGWIN*) cygwin=true ;;
-  MINGW*) mingw=true;;
-  Darwin*) darwin=true
-    # Use /usr/libexec/java_home if available, otherwise fall back to 
/Library/Java/Home
-    # See https://developer.apple.com/library/mac/qa/qa1170/_index.html
-    if [ -z "$JAVA_HOME" ]; then
-      if [ -x "/usr/libexec/java_home" ]; then
-        JAVA_HOME="$(/usr/libexec/java_home)"; export JAVA_HOME
-      else
-        JAVA_HOME="/Library/Java/Home"; export JAVA_HOME
-      fi
-    fi
-    ;;
+CYGWIN* | MINGW*)
+  [ -z "${JAVA_HOME-}" ] || JAVA_HOME="$(cygpath --unix "$JAVA_HOME")"
+  native_path() { cygpath --path --windows "$1"; }
+  ;;
 esac
 
-if [ -z "$JAVA_HOME" ] ; then
-  if [ -r /etc/gentoo-release ] ; then
-    JAVA_HOME=$(java-config --jre-home)
-  fi
-fi
-
-# For Cygwin, ensure paths are in UNIX format before anything is touched
-if $cygwin ; then
-  [ -n "$JAVA_HOME" ] &&
-    JAVA_HOME=$(cygpath --unix "$JAVA_HOME")
-  [ -n "$CLASSPATH" ] &&
-    CLASSPATH=$(cygpath --path --unix "$CLASSPATH")
-fi
-
-# For Mingw, ensure paths are in UNIX format before anything is touched
-if $mingw ; then
-  [ -n "$JAVA_HOME" ] && [ -d "$JAVA_HOME" ] &&
-    JAVA_HOME="$(cd "$JAVA_HOME" || (echo "cannot cd into $JAVA_HOME."; exit 
1); pwd)"
-fi
-
-if [ -z "$JAVA_HOME" ]; then
-  javaExecutable="$(which javac)"
-  if [ -n "$javaExecutable" ] && ! [ "$(expr "\"$javaExecutable\"" : '\([^ 
]*\)')" = "no" ]; then
-    # readlink(1) is not available as standard on Solaris 10.
-    readLink=$(which readlink)
-    if [ ! "$(expr "$readLink" : '\([^ ]*\)')" = "no" ]; then
-      if $darwin ; then
-        javaHome="$(dirname "\"$javaExecutable\"")"
-        javaExecutable="$(cd "\"$javaHome\"" && pwd -P)/javac"
-      else
-        javaExecutable="$(readlink -f "\"$javaExecutable\"")"
-      fi
-      javaHome="$(dirname "\"$javaExecutable\"")"
-      javaHome=$(expr "$javaHome" : '\(.*\)/bin')
-      JAVA_HOME="$javaHome"
-      export JAVA_HOME
-    fi
-  fi
-fi
-
-if [ -z "$JAVACMD" ] ; then
-  if [ -n "$JAVA_HOME"  ] ; then
-    if [ -x "$JAVA_HOME/jre/sh/java" ] ; then
+# set JAVACMD and JAVACCMD
+set_java_home() {
+  # For Cygwin and MinGW, ensure paths are in Unix format before anything is 
touched
+  if [ -n "${JAVA_HOME-}" ]; then
+    if [ -x "$JAVA_HOME/jre/sh/java" ]; then
       # IBM's JDK on AIX uses strange locations for the executables
       JAVACMD="$JAVA_HOME/jre/sh/java"
+      JAVACCMD="$JAVA_HOME/jre/sh/javac"
     else
       JAVACMD="$JAVA_HOME/bin/java"
+      JAVACCMD="$JAVA_HOME/bin/javac"
+
+      if [ ! -x "$JAVACMD" ] || [ ! -x "$JAVACCMD" ]; then
+        echo "The JAVA_HOME environment variable is not defined correctly, so 
mvnw cannot run." >&2
+        echo "JAVA_HOME is set to \"$JAVA_HOME\", but \"\$JAVA_HOME/bin/java\" 
or \"\$JAVA_HOME/bin/javac\" does not exist." >&2
+        return 1
+      fi
     fi
   else
-    JAVACMD="$(\unset -f command 2>/dev/null; \command -v java)"
-  fi
-fi
+    JAVACMD="$(
+      'set' +e
+      'unset' -f command 2>/dev/null
+      'command' -v java
+    )" || :
+    JAVACCMD="$(
+      'set' +e
+      'unset' -f command 2>/dev/null
+      'command' -v javac
+    )" || :
 
-if [ ! -x "$JAVACMD" ] ; then
-  echo "Error: JAVA_HOME is not defined correctly." >&2
-  echo "  We cannot execute $JAVACMD" >&2
-  exit 1
-fi
-
-if [ -z "$JAVA_HOME" ] ; then
-  echo "Warning: JAVA_HOME environment variable is not set."
-fi
-
-# traverses directory structure from process work directory to filesystem root
-# first directory with .mvn subdirectory is considered project base directory
-find_maven_basedir() {
-  if [ -z "$1" ]
-  then
-    echo "Path not specified to find_maven_basedir"
-    return 1
+    if [ ! -x "${JAVACMD-}" ] || [ ! -x "${JAVACCMD-}" ]; then
+      echo "The java/javac command does not exist in PATH nor is JAVA_HOME 
set, so mvnw cannot run." >&2
+      return 1
+    fi
   fi
+}
 
-  basedir="$1"
-  wdir="$1"
-  while [ "$wdir" != '/' ] ; do
-    if [ -d "$wdir"/.mvn ] ; then
-      basedir=$wdir
-      break
-    fi
-    # workaround for JBEAP-8937 (on Solaris 10/Sparc)
-    if [ -d "${wdir}" ]; then
-      wdir=$(cd "$wdir/.." || exit 1; pwd)
-    fi
-    # end of workaround
+# hash string like Java String::hashCode
+hash_string() {
+  str="${1:-}" h=0
+  while [ -n "$str" ]; do
+    char="${str%"${str#?}"}"
+    h=$(((h * 31 + $(LC_CTYPE=C printf %d "'$char")) % 4294967296))
+    str="${str#?}"
   done
-  printf '%s' "$(cd "$basedir" || exit 1; pwd)"
+  printf %x\\n $h
 }
 
-# concatenates all lines of a file
-concat_lines() {
-  if [ -f "$1" ]; then
-    # Remove \r in case we run on Windows within Git Bash
-    # and check out the repository with auto CRLF management
-    # enabled. Otherwise, we may read lines that are delimited with
-    # \r\n and produce $'-Xarg\r' rather than -Xarg due to word
-    # splitting rules.
-    tr -s '\r\n' ' ' < "$1"
-  fi
+verbose() { :; }
+[ "${MVNW_VERBOSE-}" != true ] || verbose() { printf %s\\n "${1-}"; }
+
+die() {
+  printf %s\\n "$1" >&2
+  exit 1
 }
 
-log() {
-  if [ "$MVNW_VERBOSE" = true ]; then
-    printf '%s\n' "$1"
-  fi
+trim() {
+  # MWRAPPER-139:
+  #   Trims trailing and leading whitespace, carriage returns, tabs, and 
linefeeds.
+  #   Needed for removing poorly interpreted newline sequences when running in 
more
+  #   exotic environments such as mingw bash on Windows.
+  printf "%s" "${1}" | tr -d '[:space:]'
+}
+
+scriptDir="$(dirname "$0")"
+scriptName="$(basename "$0")"
+
+# parse distributionUrl and optional distributionSha256Sum, requires 
.mvn/wrapper/maven-wrapper.properties
+while IFS="=" read -r key value; do
+  case "${key-}" in
+  distributionUrl) distributionUrl=$(trim "${value-}") ;;
+  distributionSha256Sum) distributionSha256Sum=$(trim "${value-}") ;;
+  esac
+done <"$scriptDir/.mvn/wrapper/maven-wrapper.properties"
+[ -n "${distributionUrl-}" ] || die "cannot read distributionUrl property in 
$scriptDir/.mvn/wrapper/maven-wrapper.properties"
+
+case "${distributionUrl##*/}" in
+maven-mvnd-*bin.*)
+  MVN_CMD=mvnd.sh _MVNW_REPO_PATTERN=/maven/mvnd/
+  case "${PROCESSOR_ARCHITECTURE-}${PROCESSOR_ARCHITEW6432-}:$(uname -a)" in
+  *AMD64:CYGWIN* | *AMD64:MINGW*) distributionPlatform=windows-amd64 ;;
+  :Darwin*x86_64) distributionPlatform=darwin-amd64 ;;
+  :Darwin*arm64) distributionPlatform=darwin-aarch64 ;;
+  :Linux*x86_64*) distributionPlatform=linux-amd64 ;;
+  *)
+    echo "Cannot detect native platform for mvnd on $(uname)-$(uname -m), use 
pure java version" >&2
+    distributionPlatform=linux-amd64
+    ;;
+  esac
+  distributionUrl="${distributionUrl%-bin.*}-$distributionPlatform.zip"
+  ;;
+maven-mvnd-*) MVN_CMD=mvnd.sh _MVNW_REPO_PATTERN=/maven/mvnd/ ;;
+*) MVN_CMD="mvn${scriptName#mvnw}" _MVNW_REPO_PATTERN=/org/apache/maven/ ;;
+esac
+
+# apply MVNW_REPOURL and calculate MAVEN_HOME
+# maven home pattern: 
~/.m2/wrapper/dists/{apache-maven-<version>,maven-mvnd-<version>-<platform>}/<hash>
+[ -z "${MVNW_REPOURL-}" ] || 
distributionUrl="$MVNW_REPOURL$_MVNW_REPO_PATTERN${distributionUrl#*"$_MVNW_REPO_PATTERN"}"
+distributionUrlName="${distributionUrl##*/}"
+distributionUrlNameMain="${distributionUrlName%.*}"
+distributionUrlNameMain="${distributionUrlNameMain%-bin}"
+MAVEN_USER_HOME="${MAVEN_USER_HOME:-${HOME}/.m2}"

Review Comment:
   [P2] Align the CI cache location with this new default. With 
`MAVEN_USER_HOME` unset, Linux job containers have shell `$HOME=/github/home` 
but Java `user.home=/root`. This line installs Maven under 
`/github/home/.m2/wrapper/dists`, while 
`.github/actions/maven-bootstrap/action.yaml:53-54` still selects 
`/root/.m2/wrapper/dists` for restore/save. Consequently, the cache never 
captures the new installation and each fresh container must download Maven 
again, losing the cache protection against Maven Central failures. Update the 
action to use the shell-home default, or explicitly share `MAVEN_USER_HOME` 
between cache selection and bootstrap.
   
   Evidence: Evaluated the exact head wrapper initialization with 
`MAVEN_USER_HOME` unset and 
`JAVA_TOOL_OPTIONS=-Duser.home=/tmp/comet-pr6678-review.hw8lQ1/simulated-jvm-home`.
 The wrapper selected `/home/dev-user/.m2/wrapper/dists`, while the action’s 
Java-property calculation selected 
`/tmp/comet-pr6678-review.hw8lQ1/simulated-jvm-home/.m2/wrapper/dists`. The 
action itself documents the production `/root` versus `/github/home` split at 
lines 37-45. Linux workflows use job containers without overriding 
`MAVEN_USER_HOME`. Upstream Wrapper 3.2.0 `MavenWrapperMain` confirms the 
previous default used Java `user.home`, matching the existing action.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to