diff options
author | W. J. van der Laan <laanwj@protonmail.com> | 2021-11-15 16:19:58 +0100 |
---|---|---|
committer | W. J. van der Laan <laanwj@protonmail.com> | 2021-11-15 16:22:52 +0100 |
commit | aec631bccc2f469f5dbdf345edb38860904b1dd9 (patch) | |
tree | 19147bd49404cc760c399401abc9c983708e03e2 /test | |
parent | 2efc8c0999a4b99cfe3076f7312806e83e778261 (diff) | |
parent | fe0ff569ea6c353f88609c0f5f9b6fa80ff74f15 (diff) | |
download | bitcoin-aec631bccc2f469f5dbdf345edb38860904b1dd9.tar.xz |
Merge bitcoin/bitcoin#23462: test: Enable SC2046 and SC2086 shellcheck rules
fe0ff569ea6c353f88609c0f5f9b6fa80ff74f15 test: Enable SC2046 shellcheck rule (Hennadii Stepanov)
9a1ad7bc0dd8a0769738ca4dffbeb8d55438b0dc test: Enable SC2086 shellcheck rule (Hennadii Stepanov)
Pull request description:
Closes #20879.
Replaces #22695.
**Note for reviewers**. Some touched shell scripts are not being run in CI, therefore they require more thorough reviewing:
- `contrib/devtools/gen-manpages.sh`
- `contrib/macdeploy/detached-sig-apply.sh`
- `contrib/windeploy/detached-sig-create.sh`
- `src/qt/res/animation/makespinner.sh`
ACKs for top commit:
laanwj:
Code review re-ACK fe0ff569ea6c353f88609c0f5f9b6fa80ff74f15
Tree-SHA512: 73619b9a7bcb6cf0dfc4189a753ef550d40c82a3432bb9d8d8a994310d42594576038daac7e0c2fc004d716976bb1413b9a77848ecf088b25b69ed0773b77e8e
Diffstat (limited to 'test')
-rwxr-xr-x | test/lint/commit-script-check.sh | 16 | ||||
-rwxr-xr-x | test/lint/git-subtree-check.sh | 10 | ||||
-rwxr-xr-x | test/lint/lint-files.sh | 2 | ||||
-rwxr-xr-x | test/lint/lint-git-commit-check.sh | 2 | ||||
-rwxr-xr-x | test/lint/lint-includes.sh | 2 | ||||
-rwxr-xr-x | test/lint/lint-python-dead-code.sh | 5 | ||||
-rwxr-xr-x | test/lint/lint-python.sh | 4 | ||||
-rwxr-xr-x | test/lint/lint-shell.sh | 8 | ||||
-rwxr-xr-x | test/lint/lint-spelling.sh | 3 |
9 files changed, 28 insertions, 24 deletions
diff --git a/test/lint/commit-script-check.sh b/test/lint/commit-script-check.sh index 9fca1129b6..912bb24f9d 100755 --- a/test/lint/commit-script-check.sh +++ b/test/lint/commit-script-check.sh @@ -12,7 +12,7 @@ # one. Any remaining diff signals an error. export LC_ALL=C -if test -z $1; then +if test -z "$1"; then echo "Usage: $0 <commit>..." exit 1 fi @@ -20,10 +20,10 @@ fi RET=0 PREV_BRANCH=$(git name-rev --name-only HEAD) PREV_HEAD=$(git rev-parse HEAD) -for commit in $(git rev-list --reverse $1); do - if git rev-list -n 1 --pretty="%s" $commit | grep -q "^scripted-diff:"; then - git checkout --quiet $commit^ || exit - SCRIPT="$(git rev-list --format=%b -n1 $commit | sed '/^-BEGIN VERIFY SCRIPT-$/,/^-END VERIFY SCRIPT-$/{//!b};d')" +for commit in $(git rev-list --reverse "$1"); do + if git rev-list -n 1 --pretty="%s" "$commit" | grep -q "^scripted-diff:"; then + git checkout --quiet "$commit"^ || exit + SCRIPT="$(git rev-list --format=%b -n1 "$commit" | sed '/^-BEGIN VERIFY SCRIPT-$/,/^-END VERIFY SCRIPT-$/{//!b};d')" if test -z "$SCRIPT"; then echo "Error: missing script for: $commit" echo "Failed" @@ -32,16 +32,16 @@ for commit in $(git rev-list --reverse $1); do echo "Running script for: $commit" echo "$SCRIPT" (eval "$SCRIPT") - git --no-pager diff --exit-code $commit && echo "OK" || (echo "Failed"; false) || RET=1 + git --no-pager diff --exit-code "$commit" && echo "OK" || (echo "Failed"; false) || RET=1 fi git reset --quiet --hard HEAD else - if git rev-list "--format=%b" -n1 $commit | grep -q '^-\(BEGIN\|END\)[ a-zA-Z]*-$'; then + if git rev-list "--format=%b" -n1 "$commit" | grep -q '^-\(BEGIN\|END\)[ a-zA-Z]*-$'; then echo "Error: script block marker but no scripted-diff in title of commit $commit" echo "Failed" RET=1 fi fi done -git checkout --quiet $PREV_BRANCH 2>/dev/null || git checkout --quiet $PREV_HEAD +git checkout --quiet "$PREV_BRANCH" 2>/dev/null || git checkout --quiet "$PREV_HEAD" exit $RET diff --git a/test/lint/git-subtree-check.sh b/test/lint/git-subtree-check.sh index 3556e49e08..cdaa5752ac 100755 --- a/test/lint/git-subtree-check.sh +++ b/test/lint/git-subtree-check.sh @@ -82,6 +82,7 @@ if [ -z "$latest_squash" ]; then echo "ERROR: $DIR is not a subtree" >&2 exit 2 fi +# shellcheck disable=SC2086 set $latest_squash old=$1 rev=$2 @@ -92,6 +93,7 @@ if [ -z "$tree_actual" ]; then echo "FAIL: subtree directory $DIR not found in $COMMIT" >&2 exit 1 fi +# shellcheck disable=SC2086 set $tree_actual tree_actual_type=$2 tree_actual_tree=$3 @@ -102,23 +104,23 @@ if [ "d$tree_actual_type" != "dtree" ]; then fi # get the tree at the time of the last subtree update -tree_commit=$(git show -s --format="%T" $old) +tree_commit=$(git show -s --format="%T" "$old") echo "$DIR in $COMMIT was last updated in commit $old (tree $tree_commit)" # ... and compare the actual tree with it if [ "$tree_actual_tree" != "$tree_commit" ]; then - git diff $tree_commit $tree_actual_tree >&2 + git diff "$tree_commit" "$tree_actual_tree" >&2 echo "FAIL: subtree directory was touched without subtree merge" >&2 exit 1 fi if [ "$check_remote" != "0" ]; then # get the tree in the subtree commit referred to - if [ "d$(git cat-file -t $rev 2>/dev/null)" != dcommit ]; then + if [ "d$(git cat-file -t "$rev" 2>/dev/null)" != dcommit ]; then echo "subtree commit $rev unavailable: cannot compare. Did you add and fetch the remote?" >&2 exit 1 fi - tree_subtree=$(git show -s --format="%T" $rev) + tree_subtree=$(git show -s --format="%T" "$rev") echo "$DIR in $COMMIT was last updated to upstream commit $rev (tree $tree_subtree)" # ... and compare the actual tree with it diff --git a/test/lint/lint-files.sh b/test/lint/lint-files.sh index 1e115778bd..f9ede4bc68 100755 --- a/test/lint/lint-files.sh +++ b/test/lint/lint-files.sh @@ -3,5 +3,5 @@ export LC_ALL=C set -e -cd "$(dirname $0)/../.." +cd "$(dirname "$0")/../.." test/lint/lint-files.py diff --git a/test/lint/lint-git-commit-check.sh b/test/lint/lint-git-commit-check.sh index 2b3a9b87c2..d1ab72658b 100755 --- a/test/lint/lint-git-commit-check.sh +++ b/test/lint/lint-git-commit-check.sh @@ -38,7 +38,7 @@ while IFS= read -r commit_hash || [[ -n "$commit_hash" ]]; do while IFS= read -r line || [[ -n "$line" ]]; do n_line=$((n_line+1)) length=${#line} - if [ $n_line -eq 2 ] && [ $length -ne 0 ]; then + if [ $n_line -eq 2 ] && [ "$length" -ne 0 ]; then echo "The subject line of commit hash ${commit_hash} is followed by a non-empty line. Subject lines should always be followed by a blank line." EXIT_CODE=1 fi diff --git a/test/lint/lint-includes.sh b/test/lint/lint-includes.sh index e9893558e4..9b19699dab 100755 --- a/test/lint/lint-includes.sh +++ b/test/lint/lint-includes.sh @@ -12,7 +12,7 @@ export LC_ALL=C IGNORE_REGEXP="/(leveldb|secp256k1|minisketch|univalue|crc32c)/" # cd to root folder of git repo for git ls-files to work properly -cd "$(dirname $0)/../.." || exit 1 +cd "$(dirname "$0")/../.." || exit 1 filter_suffix() { git ls-files | grep -E "^src/.*\.${1}"'$' | grep -Ev "${IGNORE_REGEXP}" diff --git a/test/lint/lint-python-dead-code.sh b/test/lint/lint-python-dead-code.sh index c3b6ff3c98..247bfb310a 100755 --- a/test/lint/lint-python-dead-code.sh +++ b/test/lint/lint-python-dead-code.sh @@ -15,9 +15,8 @@ fi # --min-confidence 100 will only report code that is guaranteed to be unused within the analyzed files. # Any value below 100 introduces the risk of false positives, which would create an unacceptable maintenance burden. -if ! vulture \ - --min-confidence 100 \ - $(git ls-files -- "*.py"); then +mapfile -t FILES < <(git ls-files -- "*.py") +if ! vulture --min-confidence 100 "${FILES[@]}"; then echo "Python dead code detection found some issues" exit 1 fi diff --git a/test/lint/lint-python.sh b/test/lint/lint-python.sh index 3d22407fd1..96b34d715e 100755 --- a/test/lint/lint-python.sh +++ b/test/lint/lint-python.sh @@ -92,6 +92,7 @@ fi EXIT_CODE=0 +# shellcheck disable=SC2046 if ! PYTHONWARNINGS="ignore" flake8 --ignore=B,C,E,F,I,N,W --select=$(IFS=","; echo "${enabled[*]}") $( if [[ $# == 0 ]]; then git ls-files "*.py" @@ -102,7 +103,8 @@ if ! PYTHONWARNINGS="ignore" flake8 --ignore=B,C,E,F,I,N,W --select=$(IFS=","; e EXIT_CODE=1 fi -if ! mypy --show-error-codes $(git ls-files "test/functional/*.py" "contrib/devtools/*.py"); then +mapfile -t FILES < <(git ls-files "test/functional/*.py" "contrib/devtools/*.py") +if ! mypy --show-error-codes "${FILES[@]}"; then EXIT_CODE=1 fi diff --git a/test/lint/lint-shell.sh b/test/lint/lint-shell.sh index 0e18c73013..d697dfcbea 100755 --- a/test/lint/lint-shell.sh +++ b/test/lint/lint-shell.sh @@ -10,8 +10,6 @@ export LC_ALL=C # Disabled warnings: disabled=( - SC2046 # Quote this to prevent word splitting. - SC2086 # Double quote to prevent globbing and word splitting. SC2162 # read without -r will mangle backslashes. ) @@ -24,8 +22,10 @@ fi SHELLCHECK_CMD=(shellcheck --external-sources --check-sourced) EXCLUDE="--exclude=$(IFS=','; echo "${disabled[*]}")" -SOURCED_FILES=$(git ls-files | xargs gawk '/^# shellcheck shell=/ {print FILENAME} {nextfile}') # Check shellcheck directive used for sourced files -if ! "${SHELLCHECK_CMD[@]}" "$EXCLUDE" $SOURCED_FILES $(git ls-files -- '*.sh' | grep -vE 'src/(leveldb|secp256k1|minisketch|univalue)/'); then +# Check shellcheck directive used for sourced files +mapfile -t SOURCED_FILES < <(git ls-files | xargs gawk '/^# shellcheck shell=/ {print FILENAME} {nextfile}') +mapfile -t FILES < <(git ls-files -- '*.sh' | grep -vE 'src/(leveldb|secp256k1|minisketch|univalue)/') +if ! "${SHELLCHECK_CMD[@]}" "$EXCLUDE" "${SOURCED_FILES[@]}" "${FILES[@]}"; then EXIT_CODE=1 fi diff --git a/test/lint/lint-spelling.sh b/test/lint/lint-spelling.sh index 8eefa8f2ce..c98003de8d 100755 --- a/test/lint/lint-spelling.sh +++ b/test/lint/lint-spelling.sh @@ -15,6 +15,7 @@ if ! command -v codespell > /dev/null; then fi IGNORE_WORDS_FILE=test/lint/lint-spelling.ignore-words.txt -if ! codespell --check-filenames --disable-colors --quiet-level=7 --ignore-words=${IGNORE_WORDS_FILE} $(git ls-files -- ":(exclude)build-aux/m4/" ":(exclude)contrib/seeds/*.txt" ":(exclude)depends/" ":(exclude)doc/release-notes/" ":(exclude)src/leveldb/" ":(exclude)src/crc32c/" ":(exclude)src/qt/locale/" ":(exclude)src/qt/*.qrc" ":(exclude)src/secp256k1/" ":(exclude)src/minisketch/" ":(exclude)src/univalue/" ":(exclude)contrib/builder-keys/keys.txt" ":(exclude)contrib/guix/patches"); then +mapfile -t FILES < <(git ls-files -- ":(exclude)build-aux/m4/" ":(exclude)contrib/seeds/*.txt" ":(exclude)depends/" ":(exclude)doc/release-notes/" ":(exclude)src/leveldb/" ":(exclude)src/crc32c/" ":(exclude)src/qt/locale/" ":(exclude)src/qt/*.qrc" ":(exclude)src/secp256k1/" ":(exclude)src/minisketch/" ":(exclude)src/univalue/" ":(exclude)contrib/builder-keys/keys.txt" ":(exclude)contrib/guix/patches") +if ! codespell --check-filenames --disable-colors --quiet-level=7 --ignore-words=${IGNORE_WORDS_FILE} "${FILES[@]}"; then echo "^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in ${IGNORE_WORDS_FILE}" fi |