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

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

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


##########
maven-wrapper-distribution/src/resources/mvnw:
##########
@@ -193,42 +193,48 @@ else
     else
       
wrapperUrl="https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/@@project.version@@/maven-wrapper-@@project.version@@.jar";
     fi
-    while IFS="=" read key value; do
+    while IFS="=" read -r key value; do
       case "$key" in (wrapperUrl) wrapperUrl="$value"; break ;;
       esac
     done < "$MAVEN_PROJECTBASEDIR/.mvn/wrapper/maven-wrapper.properties"
     log "Downloading from: $wrapperUrl"
 
     if $cygwin; then
-      wrapperJarPath=`cygpath --path --windows "$wrapperJarPath"`
+      wrapperJarPath=$(cygpath --path --windows "$wrapperJarPath")
     fi
 
     if command -v wget > /dev/null; then
         log "Found wget ... using wget"
+        wgetrc=0
         [ "$MVNW_VERBOSE" = true ] && QUIET="" || QUIET="--quiet"
         if [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ]; then
             wget $QUIET "$wrapperUrl" -O "$wrapperJarPath"
+            wgetrc=$?
         else
             wget $QUIET --http-user="$MVNW_USERNAME" 
--http-password="$MVNW_PASSWORD" "$wrapperUrl" -O "$wrapperJarPath"
+            wgetrc=$?
         fi
-        [ $? -eq 0 ] || rm -f "$wrapperJarPath"
+        [ $wgetrc -eq 0 ] || rm -f "$wrapperJarPath"
     elif command -v curl > /dev/null; then
         log "Found curl ... using curl"
+        curlrc=0
         [ "$MVNW_VERBOSE" = true ] && QUIET="" || QUIET="--silent"
         if [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ]; then
             curl $QUIET -o "$wrapperJarPath" "$wrapperUrl" -f -L
+            curlrc=$?
         else
             curl $QUIET --user "$MVNW_USERNAME:$MVNW_PASSWORD" -o 
"$wrapperJarPath" "$wrapperUrl" -f -L
+            curlrc=$?
         fi
-        [ $? -eq 0 ] || rm -f "$wrapperJarPath"
+        [ $curlrc -eq 0 ] || rm -f "$wrapperJarPath"

Review Comment:
   What about:
   ```
   if command -v wget > /dev/null; then
       log "Found wget ... using wget"
       [ "$MVNW_VERBOSE" = true ] && QUIET="" || QUIET="--quiet"
       { [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ] ;} \
           && USEROPT="" \
           || USEROPT="--http-user=\"$MVNW_USERNAME\" 
--http-password=\"$MVNW_PASSWORD\""
       wget $QUIET $USEROPT "$wrapperUrl" -O "$wrapperJarPath" \
           || rm -f "$wrapperJarPath"
   elif command -v curl > /dev/null; then
       log "Found curl ... using curl"
       [ "$MVNW_VERBOSE" = true ] && QUIET="" || QUIET="--silent"
       { [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ] ;} \
           && USEROPT="" \
           || USEROPT="--user \"$MVNW_USERNAME:$MVNW_PASSWORD\""
       curl $QUIET $USEROPT -o "$wrapperJarPath" "$wrapperUrl" -f -L \
           || rm -f "$wrapperJarPath"
   else
   ```





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

Reply via email to