Fix most shellcheck warnings in git-arc.sh

Mostly adding quotes and replacing egrep/fgrep with grep -E/grep -F

Reviewed By:	markj
Differential Revision: https://reviews.freebsd.org/D29373

1 files changed, 75 insertions, 74 deletions

diff --git a/tools/tools/git/git-arc.sh b/tools/tools/git/git-arc.sh
index c3541e438c9f..53bf462eff18 100644
--- a/tools/tools/git/git-arc.sh
+++ b/tools/tools/git/git-arc.sh
@@ -34,7 +34,7 @@
 
 warn()
 {
-    echo "$(basename $0): $1" >&2
+    echo "$(basename "$0"): $1" >&2
 }
 
 err()
@@ -151,7 +151,7 @@ diff2phid()
         err "invalid diff ID $diff"
     fi
 
-    echo '{"names":["'$diff'"]}' |
+    echo '{"names":["'"$diff"'"]}' |
         arc call-conduit -- phid.lookup |
         jq -r "select(.response != []) | .response.${diff}.phid"
 }
@@ -166,11 +166,11 @@ diff2status()
     fi
 
     tmp=$(mktemp)
-    echo '{"names":["'$diff'"]}' |
-        arc call-conduit -- phid.lookup > $tmp
-    status=$(jq -r "select(.response != []) | .response.${diff}.status" < $tmp)
+    echo '{"names":["'"$diff"'"]}' |
+        arc call-conduit -- phid.lookup > "$tmp"
+    status=$(jq -r "select(.response != []) | .response.${diff}.status" < "$tmp")
     summary=$(jq -r "select(.response != []) |
-         .response.${diff}.fullName" < $tmp)
+         .response.${diff}.fullName" < "$tmp")
     printf "%-14s %s\n" "${status}" "${summary}"
 }
 
@@ -178,10 +178,10 @@ log2diff()
 {
     local diff
 
-    diff=$(git show -s --format=%B $commit |
+    diff=$(git show -s --format=%B "$commit" |
         sed -nE '/^Differential Revision:[[:space:]]+(https:\/\/reviews.freebsd.org\/)?(D[0-9]+)$/{s//\2/;p;}')
-    if [ -n "$diff" ] && [ $(echo "$diff" | wc -l) -eq 1 ]; then
-        echo $diff
+    if [ -n "$diff" ] && [ "$(echo "$diff" | wc -l)" -eq 1 ]; then
+        echo "$diff"
     else
         echo
     fi
@@ -195,23 +195,23 @@ commit2diff()
 
     # First, look for a valid differential reference in the commit
     # log.
-    diff=$(log2diff $commit)
+    diff=$(log2diff "$commit")
     if [ -n "$diff" ]; then
-        echo $diff
+        echo "$diff"
         return
     fi
 
     # Second, search the open reviews returned by 'arc list' looking
     # for a subject match.
-    title=$(git show -s --format=%s $commit)
-    diff=$(arc list | fgrep "$title" | egrep -o 'D[1-9][0-9]*:' | tr -d ':')
+    title=$(git show -s --format=%s "$commit")
+    diff=$(arc list | grep -F "$title" | grep -E -o 'D[1-9][0-9]*:' | tr -d ':')
     if [ -z "$diff" ]; then
         err "could not find review for '${title}'"
-    elif [ $(echo "$diff" | wc -l) -ne 1 ]; then
+    elif [ "$(echo "$diff" | wc -l)" -ne 1 ]; then
         err "found multiple reviews with the same title"
     fi
 
-    echo $diff
+    echo "$diff"
 }
 
 create_one_review()
@@ -225,61 +225,61 @@ create_one_review()
     parent=$4
     doprompt=$5
 
-    if [ "$doprompt" ] && ! show_and_prompt $commit; then
+    if [ "$doprompt" ] && ! show_and_prompt "$commit"; then
         return 1
     fi
 
-    git checkout -q $commit
+    git checkout -q "$commit"
 
     msg=$(mktemp)
-    git show -s --format='%B' $commit > $msg
-    printf "\nTest Plan:\n" >> $msg
-    printf "\nReviewers:\n" >> $msg
-    printf "${reviewers}\n" >> $msg
-    printf "\nSubscribers:\n" >> $msg
-    printf "${subscribers}\n" >> $msg
+    git show -s --format='%B' "$commit" > "$msg"
+    printf "\nTest Plan:\n" >> "$msg"
+    printf "\nReviewers:\n" >> "$msg"
+    printf "%s\n" "${reviewers}" >> "$msg"
+    printf "\nSubscribers:\n" >> "$msg"
+    printf "%s\n" "${subscribers}" >> "$msg"
 
     yes | env EDITOR=true \
-        arc diff --message-file $msg --never-apply-patches --create --allow-untracked $BROWSE HEAD~
+        arc diff --message-file "$msg" --never-apply-patches --create --allow-untracked $BROWSE HEAD~
     [ $? -eq 0 ] || err "could not create Phabricator diff"
 
     if [ -n "$parent" ]; then
-        diff=$(commit2diff $commit)
+        diff=$(commit2diff "$commit")
         [ -n "$diff" ] || err "failed to look up review ID for $commit"
 
-        childphid=$(diff2phid $diff)
-        parentphid=$(diff2phid $parent)
+        childphid=$(diff2phid "$diff")
+        parentphid=$(diff2phid "$parent")
         echo '{
-            "objectIdentifier": "'${childphid}'",
+            "objectIdentifier": "'"${childphid}"'",
             "transactions": [
                 {
                     "type": "parents.add",
-                    "value": ["'${parentphid}'"]
+                    "value": ["'"${parentphid}"'"]
                 }
              ]}' |
             arc call-conduit -- differential.revision.edit >&3
     fi
-    rm -f $msg
+    rm -f "$msg"
     return 0
 }
 
 # Get a list of reviewers who accepted the specified diff.
 diff2reviewers()
 {
-    local diff phid reviewid userids
+    local diff reviewid userids
 
     diff=$1
-    reviewid=$(diff2phid $diff)
+    reviewid=$(diff2phid "$diff")
     userids=$( \
         echo '{
-                  "constraints": {"phids": ["'$reviewid'"]},
+                  "constraints": {"phids": ["'"$reviewid"'"]},
                   "attachments": {"reviewers": true}
               }' |
         arc call-conduit -- differential.revision.search |
         jq '.response.data[0].attachments.reviewers.reviewers[] | select(.status == "accepted").reviewerPHID')
     if [ -n "$userids" ]; then
         echo '{
-                  "constraints": {"phids": ['$(echo -n $userids | tr '[:space:]' ',')']}
+                  "constraints": {"phids": ['"$(echo -n "$userids" | tr '[:space:]' ',')"']}
               }' |
             arc call-conduit -- user.search |
             jq -r '.response.data[].fields.username'
@@ -295,7 +295,7 @@ prompt()
     fi
 
     printf "\nDoes this look OK? [y/N] "
-    read resp
+    read -r resp
 
     case $resp in
     [Yy])
@@ -313,7 +313,7 @@ show_and_prompt()
 
     commit=$1
 
-    git show $commit
+    git show "$commit"
     prompt
 }
 
@@ -330,7 +330,7 @@ save_head()
 restore_head()
 {
     if [ -n "$SAVED_HEAD" ]; then
-        git checkout -q $SAVED_HEAD
+        git checkout -q "$SAVED_HEAD"
         SAVED_HEAD=
     fi
 }
@@ -339,15 +339,15 @@ build_commit_list()
 {
     local chash _commits commits
 
-    for chash in $@; do
+    for chash in "$@"; do
         _commits=$(git rev-parse "${chash}")
         if ! git cat-file -e "${chash}"'^{commit}' >/dev/null 2>&1; then
-            _commits=$(git rev-list $_commits | tail -r)
+            _commits=$(git rev-list "$_commits" | tail -r)
         fi
         [ -n "$_commits" ] || err "invalid commit ID ${chash}"
         commits="$commits $_commits"
     done
-    echo $commits
+    echo "$commits"
 }
 
 gitarc::create()
@@ -355,7 +355,7 @@ gitarc::create()
     local commit commits doprompt list o prev reviewers subscribers
 
     list=
-    if eval $(git config --bool --default false --get arc.list); then
+    if [ "$(git config --bool --default false --get arc.list)" != "false" ]; then
         list=1
     fi
     doprompt=1
@@ -377,11 +377,11 @@ gitarc::create()
     done
     shift $((OPTIND-1))
 
-    commits=$(build_commit_list $@)
+    commits=$(build_commit_list "$@")
 
     if [ "$list" ]; then
         for commit in ${commits}; do
-            git --no-pager show --oneline --no-patch $commit
+            git --no-pager show --oneline --no-patch "$commit"
         done | git_pager
         if ! prompt; then
             return
@@ -406,28 +406,28 @@ gitarc::list()
 {
     local chash commit commits diff title
 
-    commits=$(build_commit_list $@)
+    commits=$(build_commit_list "$@")
 
     for commit in $commits; do
-        chash=$(git show -s --format='%C(auto)%h' $commit)
+        chash=$(git show -s --format='%C(auto)%h' "$commit")
         echo -n "${chash} "
 
-        diff=$(log2diff $commit)
+        diff=$(log2diff "$commit")
         if [ -n "$diff" ]; then
-                diff2status $diff
+                diff2status "$diff"
                 continue
         fi
 
         # This does not use commit2diff as it needs to handle errors
         # differently and keep the entire status.  The extra 'cat'
         # after 'fgrep' avoids erroring due to -e.
-        title=$(git show -s --format=%s $commit)
-        diff=$(arc list | fgrep "$title" | cat)
+        title=$(git show -s --format=%s "$commit")
+        diff=$(arc list | grep -F "$title" | cat)
         if [ -z "$diff" ]; then
             echo "No Review      : $title"
-        elif [ $(echo "$diff" | wc -l) -ne 1 ]; then
+        elif [ "$(echo "$diff" | wc -l)" -ne 1 ]; then
             echo -n "Ambiguous Reviews: "
-            echo "$diff" | egrep -o 'D[1-9][0-9]*:' | tr -d ':' \
+            echo "$diff" | grep -E -o 'D[1-9][0-9]*:' | tr -d ':' \
                 | paste -sd ',' - | sed 's/,/, /g'
         else
             echo "$diff" | sed -e 's/^[^ ]* *//'
@@ -443,8 +443,8 @@ gitarc::patch()
         err_usage
     fi
 
-    for rev in $@; do
-        arc patch --skip-dependencies --nocommit --nobranch --force $rev
+    for rev in "$@"; do
+        arc patch --skip-dependencies --nocommit --nobranch --force "$rev"
         echo "Applying ${rev}..."
         [ $? -eq 0 ] || break
     done
@@ -467,34 +467,34 @@ gitarc::stage()
     done
     shift $((OPTIND-1))
 
-    commits=$(build_commit_list $@)
+    commits=$(build_commit_list "$@")
 
     if [ "$branch" = "main" ]; then
         git checkout -q main
     else
-        git checkout -q -b ${branch} main
+        git checkout -q -b "${branch}" main
     fi
 
     tmp=$(mktemp)
     for commit in $commits; do
-        git show -s --format=%B $commit > $tmp
-        diff=$(arc list | fgrep "$(git show -s --format=%s $commit)" |
-            egrep -o 'D[1-9][0-9]*:' | tr -d ':')
+        git show -s --format=%B "$commit" > "$tmp"
+        diff=$(arc list | grep -F "$(git show -s --format=%s "$commit")" |
+            grep -E -o 'D[1-9][0-9]*:' | tr -d ':')
         if [ -n "$diff" ]; then
             # XXX this leaves an extra newline in some cases.
-            reviewers=$(diff2reviewers $diff | sed '/^$/d' | paste -sd ',' - | sed 's/,/, /g')
+            reviewers=$(diff2reviewers "$diff" | sed '/^$/d' | paste -sd ',' - | sed 's/,/, /g')
             if [ -n "$reviewers" ]; then
-                printf "Reviewed by:\t${reviewers}\n" >> $tmp
+                printf "Reviewed by:\t%s\n" "${reviewers}" >> "$tmp"
             fi
-            printf "Differential Revision:\thttps://reviews.freebsd.org/${diff}" >> $tmp
+            printf "Differential Revision:\thttps://reviews.freebsd.org/%s" "${diff}" >> "$tmp"
         fi
-        author=$(git show -s --format='%an <%ae>' ${commit})
-        if ! git cherry-pick --no-commit ${commit}; then
-            warn "Failed to apply $(git rev-parse --short ${commit}).  Are you staging patches in the wrong order?"
+        author=$(git show -s --format='%an <%ae>' "${commit}")
+        if ! git cherry-pick --no-commit "${commit}"; then
+            warn "Failed to apply $(git rev-parse --short "${commit}").  Are you staging patches in the wrong order?"
             git checkout -f
             break
         fi
-        git commit --edit --file $tmp --author "${author}"
+        git commit --edit --file "$tmp" --author "${author}"
     done
 }
 
@@ -502,21 +502,21 @@ gitarc::update()
 {
     local commit commits diff
 
-    commits=$(build_commit_list $@)
+    commits=$(build_commit_list "$@")
     save_head
     for commit in ${commits}; do
-        diff=$(commit2diff $commit)
+        diff=$(commit2diff "$commit")
 
-        if ! show_and_prompt $commit; then
+        if ! show_and_prompt "$commit"; then
             break
         fi
 
-        git checkout -q $commit
+        git checkout -q "$commit"
 
         # The linter is stupid and applies patches to the working copy.
         # This would be tolerable if it didn't try to correct "misspelled" variable
         # names.
-        arc diff --allow-untracked --never-apply-patches --update $diff HEAD~
+        arc diff --allow-untracked --never-apply-patches --update "$diff" HEAD~
     done
     restore_head
 }
@@ -524,7 +524,7 @@ gitarc::update()
 set -e
 
 ASSUME_YES=
-if eval $(git config --bool --default false --get arc.assume-yes); then
+if [ "$(git config --bool --default false --get arc.assume-yes)" != "false" ]; then
     ASSUME_YES=1
 fi
 
@@ -575,6 +575,7 @@ git_sh_setup=$(git --exec-path)/git-sh-setup
 [ -f "$git_sh_setup" ] || err "cannot find git-sh-setup"
 SUBDIRECTORY_OK=y
 USAGE=
+# shellcheck disable=SC1090
 . "$git_sh_setup"
 
 # Bail if the working tree is unclean, except for "list" and "patch"
@@ -583,14 +584,14 @@ case $verb in
 list|patch)
     ;;
 *)
-    require_clean_work_tree $verb
+    require_clean_work_tree "$verb"
     ;;
 esac
 
-if eval $(git config --bool --default false --get arc.browse); then
+if [ "$(git config --bool --default false --get arc.browse)" != "false" ]; then
     BROWSE=--browse
 fi
 
 trap restore_head EXIT INT
 
-gitarc::${verb} $@
+gitarc::"${verb}" "$@"