[jira] [Commented] (MWRAPPER-88) mvnw is not POSIX compatible

[ASF GitHub Bot (Jira)]

    [ 
https://issues.apache.org/jira/browse/MWRAPPER-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17655424#comment-17655424
 ] 

ASF GitHub Bot commented on MWRAPPER-88:
----------------------------------------

bmarwell commented on code in PR #79:
URL: https://github.com/apache/maven-wrapper/pull/79#discussion_r1063397662


##########
maven-wrapper-distribution/src/resources/mvnw:
##########
@@ -71,38 +71,38 @@ esac
 
 if [ -z "$JAVA_HOME" ] ; then
   if [ -r /etc/gentoo-release ] ; then
-    JAVA_HOME=`java-config --jre-home`
+    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"`
+    JAVA_HOME=$(cygpath --unix "$JAVA_HOME")
   [ -n "$CLASSPATH" ] &&
-    CLASSPATH=`cygpath --path --unix "$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" ] &&
-    JAVA_HOME="`(cd "$JAVA_HOME"; pwd)`"
+  [ -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
+  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
+    readLink=$(which readlink)
+    if [ ! "$(expr "$readLink" : '\([^ ]*\)')" = "no" ]; then
       if $darwin ; then
-        javaHome="`dirname \"$javaExecutable\"`"
-        javaExecutable="`cd \"$javaHome\" && pwd -P`/javac"
+        javaHome="$(dirname "\"$javaExecutable\"")"
+        javaExecutable="$(cd "\"$javaHome\"" && pwd -P)/javac"
       else
-        javaExecutable="`readlink -f \"$javaExecutable\"`"
+        javaExecutable="$(readlink -f "\"$javaExecutable\"")"
       fi
-      javaHome="`dirname \"$javaExecutable\"`"
-      javaHome=`expr "$javaHome" : '\(.*\)/bin'`
+      javaHome="$(dirname "\"$javaExecutable\"")"
+      javaHome=$(expr "$javaHome" : '\(.*\)/bin')

Review Comment:
   As from the `expr' readme, expr doesn't add quotes itself. However, it will 
never match `"no"`, but only `"\"no\""` - I am not going to fix that in this PR.
   Anyway, adding surrounding quotes is correct. Consider this path:
   
   `/home/user/java installations/current/bin/javac`
   
   ```
   user@host ~ $ javaExecutable="/home/user/java 
installations/current/bin/javac"
   user@host ~ $ expr \"$javaExecutable\" : '\([^ ]*\)'
   "/home/user/java
   user@host ~ $ expr "\"$javaExecutable\"" : '\([^ ]*\)'   
   "/home/user/java
   ```
   
   Whatever it does it fails on paths with spaces.
   





> mvnw is not POSIX compatible
> ----------------------------
>
>                 Key: MWRAPPER-88
>                 URL: https://issues.apache.org/jira/browse/MWRAPPER-88
>             Project: Maven Wrapper
>          Issue Type: Bug
>          Components: Maven Wrapper Scripts
>    Affects Versions: 3.1.1
>            Reporter: Benjamin Marwell
>            Assignee: Benjamin Marwell
>            Priority: Major
>              Labels: posix
>
> * POSIX scripts should not use bash-specific features
>  * Fix suggestions from shellcheck 
> ([https://github.com/apache/maven-wrapper/pull/74])



--
This message was sent by Atlassian Jira
(v8.20.10#820010)