Date: Thu, 25 Mar 2021 12:07:52 GMT From: Alex Richardson <arichardson@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 4fd0c6ab1a9e - main - Fix most shellcheck warnings in git-arc.sh Message-ID: <202103251207.12PC7q1O018854@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by arichardson: URL: https://cgit.FreeBSD.org/src/commit/?id=4fd0c6ab1a9e3bfd7e52f64b9c301dfc9caa0627 commit 4fd0c6ab1a9e3bfd7e52f64b9c301dfc9caa0627 Author: Alex Richardson <arichardson@FreeBSD.org> AuthorDate: 2021-03-25 11:17:30 +0000 Commit: Alex Richardson <arichardson@FreeBSD.org> CommitDate: 2021-03-25 11:17:32 +0000 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 --- tools/tools/git/git-arc.sh | 149 +++++++++++++++++++++++---------------------- 1 file 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}" "$@"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202103251207.12PC7q1O018854>