From 567265ae67d0de0d4273376dffd33ea8fb141c5c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 27 May 2024 09:40:49 -0400 Subject: [PATCH] Start getting all shell scripts passing shellcheck Like with the formatter, we are blacklisting most files by default. Do a few files to get us started, and get a sense of what this looks like. --- .shellcheckrc | 2 + maintainers/flake-module.nix | 235 +++++++++++++++++++++++- mk/common-test.sh | 9 +- mk/run-test.sh | 5 +- scripts/bigsur-nixbld-user-migration.sh | 16 +- tests/functional/repl.sh | 33 ++-- 6 files changed, 274 insertions(+), 26 deletions(-) create mode 100644 .shellcheckrc diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 000000000..662e2045c --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1,2 @@ +external-sources=true +source-path=SCRIPTDIR diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 5dbafabaa..dd986e686 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -11,13 +11,13 @@ pre-commit.settings = { hooks = { clang-format.enable = true; + shellcheck.enable = true; # TODO: nixfmt, https://github.com/NixOS/nixfmt/issues/153 }; excludes = [ # We don't want to format test data # ''tests/(?!nixos/).*\.nix'' - ''^tests/functional/.*$'' ''^tests/unit/[^/]*/data/.*$'' # Don't format vendored code @@ -25,7 +25,7 @@ ''^doc/manual/redirects\.js$'' ''^doc/manual/theme/highlight\.js$'' - # We haven't applied formatting to these files yet + # We haven't applied formatting to these files yet (C++) ''^doc/manual/redirects\.js$'' ''^doc/manual/theme/highlight\.js$'' ''^precompiled-headers\.h$'' @@ -428,6 +428,8 @@ ''^src/nix/verify\.cc$'' ''^src/nix/why-depends\.cc$'' + ''^tests/functional/plugins/plugintest\.cc'' + ''^tests/functional/test-libstoreconsumer/main\.cc'' ''^tests/nixos/ca-fd-leak/sender\.c'' ''^tests/nixos/ca-fd-leak/smuggler\.c'' ''^tests/unit/libexpr-support/tests/libexpr\.hh'' @@ -490,6 +492,235 @@ ''^tests/unit/libutil/tests\.cc'' ''^tests/unit/libutil/url\.cc'' ''^tests/unit/libutil/xml-writer\.cc'' + + # We haven't lintted these files yet (shell) + ''^config/install-sh$'' + ''^misc/systemv/nix-daemon$'' + ''^misc/bash/completion\.sh$'' + ''^misc/fish/completion\.fish$'' + ''^misc/zsh/completion\.zsh$'' + ''^scripts/check-hydra-status\.sh$'' + ''^scripts/create-darwin-volume\.sh$'' + ''^scripts/install-darwin-multi-user\.sh$'' + ''^scripts/install-multi-user\.sh$'' + ''^scripts/install-nix-from-closure\.sh$'' + ''^scripts/install-systemd-multi-user\.sh$'' + ''^src/nix/get-env\.sh$'' + ''^tests/functional/add\.sh$'' + ''^tests/functional/bash-profile\.sh$'' + ''^tests/functional/binary-cache-build-remote\.sh$'' + ''^tests/functional/binary-cache\.sh$'' + ''^tests/functional/brotli\.sh$'' + ''^tests/functional/build-delete\.sh$'' + ''^tests/functional/build-dry\.sh$'' + ''^tests/functional/build-remote-content-addressed-fixed\.sh$'' + ''^tests/functional/build-remote-content-addressed-floating\.sh$'' + ''^tests/functional/build-remote-input-addressed\.sh$'' + ''^tests/functional/build-remote-trustless-after\.sh$'' + ''^tests/functional/build-remote-trustless-should-fail-0\.sh$'' + ''^tests/functional/build-remote-trustless-should-pass-0\.sh$'' + ''^tests/functional/build-remote-trustless-should-pass-1\.sh$'' + ''^tests/functional/build-remote-trustless-should-pass-2\.sh$'' + ''^tests/functional/build-remote-trustless-should-pass-3\.sh$'' + ''^tests/functional/build-remote-trustless\.sh$'' + ''^tests/functional/build-remote-with-mounted-ssh-ng\.sh$'' + ''^tests/functional/build-remote\.sh$'' + ''^tests/functional/build\.sh$'' + ''^tests/functional/ca/build-cache\.sh$'' + ''^tests/functional/ca/build-dry\.sh$'' + ''^tests/functional/ca/build-with-garbage-path\.sh$'' + ''^tests/functional/ca/build\.sh$'' + ''^tests/functional/ca/common\.sh$'' + ''^tests/functional/ca/concurrent-builds\.sh$'' + ''^tests/functional/ca/derivation-json\.sh$'' + ''^tests/functional/ca/duplicate-realisation-in-closure\.sh$'' + ''^tests/functional/ca/eval-store\.sh$'' + ''^tests/functional/ca/gc\.sh$'' + ''^tests/functional/ca/import-derivation\.sh$'' + ''^tests/functional/ca/new-build-cmd\.sh$'' + ''^tests/functional/ca/nix-copy\.sh$'' + ''^tests/functional/ca/nix-run\.sh$'' + ''^tests/functional/ca/nix-shell\.sh$'' + ''^tests/functional/ca/post-hook\.sh$'' + ''^tests/functional/ca/recursive\.sh$'' + ''^tests/functional/ca/repl\.sh$'' + ''^tests/functional/ca/selfref-gc\.sh$'' + ''^tests/functional/ca/signatures\.sh$'' + ''^tests/functional/ca/substitute\.sh$'' + ''^tests/functional/ca/why-depends\.sh$'' + ''^tests/functional/case-hack\.sh$'' + ''^tests/functional/check-refs\.sh$'' + ''^tests/functional/check-reqs\.sh$'' + ''^tests/functional/check\.sh$'' + ''^tests/functional/chroot-store\.sh$'' + ''^tests/functional/common\.sh$'' + ''^tests/functional/common/init\.sh$'' + ''^tests/functional/completions\.sh$'' + ''^tests/functional/compression-levels\.sh$'' + ''^tests/functional/compute-levels\.sh$'' + ''^tests/functional/config\.sh$'' + ''^tests/functional/db-migration\.sh$'' + ''^tests/functional/debugger\.sh$'' + ''^tests/functional/dependencies\.builder0\.sh$'' + ''^tests/functional/dependencies\.sh$'' + ''^tests/functional/derivation-json\.sh$'' + ''^tests/functional/dump-db\.sh$'' + ''^tests/functional/dyn-drv/build-built-drv\.sh$'' + ''^tests/functional/dyn-drv/common\.sh$'' + ''^tests/functional/dyn-drv/dep-built-drv\.sh$'' + ''^tests/functional/dyn-drv/eval-outputOf\.sh$'' + ''^tests/functional/dyn-drv/old-daemon-error-hack\.sh$'' + ''^tests/functional/dyn-drv/recursive-mod-json\.sh$'' + ''^tests/functional/dyn-drv/text-hashed-output\.sh$'' + ''^tests/functional/eval-store\.sh$'' + ''^tests/functional/eval\.sh$'' + ''^tests/functional/experimental-features\.sh$'' + ''^tests/functional/export-graph\.sh$'' + ''^tests/functional/export\.sh$'' + ''^tests/functional/extra-sandbox-profile\.sh$'' + ''^tests/functional/fetchClosure\.sh$'' + ''^tests/functional/fetchGit\.sh$'' + ''^tests/functional/fetchGitRefs\.sh$'' + ''^tests/functional/fetchGitSubmodules\.sh$'' + ''^tests/functional/fetchGitVerification\.sh$'' + ''^tests/functional/fetchMercurial\.sh$'' + ''^tests/functional/fetchPath\.sh$'' + ''^tests/functional/fetchTree-file\.sh$'' + ''^tests/functional/fetchurl\.sh$'' + ''^tests/functional/filter-source\.sh$'' + ''^tests/functional/fixed\.builder1\.sh$'' + ''^tests/functional/fixed\.builder2\.sh$'' + ''^tests/functional/fixed\.sh$'' + ''^tests/functional/flakes/absolute-attr-paths\.sh$'' + ''^tests/functional/flakes/absolute-paths\.sh$'' + ''^tests/functional/flakes/build-paths\.sh$'' + ''^tests/functional/flakes/bundle\.sh$'' + ''^tests/functional/flakes/check\.sh$'' + ''^tests/functional/flakes/circular\.sh$'' + ''^tests/functional/flakes/common\.sh$'' + ''^tests/functional/flakes/config\.sh$'' + ''^tests/functional/flakes/develop\.sh$'' + ''^tests/functional/flakes/flake-in-submodule\.sh$'' + ''^tests/functional/flakes/flakes\.sh$'' + ''^tests/functional/flakes/follow-paths\.sh$'' + ''^tests/functional/flakes/init\.sh$'' + ''^tests/functional/flakes/inputs\.sh$'' + ''^tests/functional/flakes/mercurial\.sh$'' + ''^tests/functional/flakes/prefetch\.sh$'' + ''^tests/functional/flakes/run\.sh$'' + ''^tests/functional/flakes/search-root\.sh$'' + ''^tests/functional/flakes/show\.sh$'' + ''^tests/functional/flakes/unlocked-override\.sh$'' + ''^tests/functional/fmt\.sh$'' + ''^tests/functional/fmt\.simple\.sh$'' + ''^tests/functional/function-trace\.sh$'' + ''^tests/functional/gc-auto\.sh$'' + ''^tests/functional/gc-concurrent\.builder\.sh$'' + ''^tests/functional/gc-concurrent\.sh$'' + ''^tests/functional/gc-concurrent2\.builder\.sh$'' + ''^tests/functional/gc-non-blocking\.sh$'' + ''^tests/functional/gc-runtime\.sh$'' + ''^tests/functional/gc\.sh$'' + ''^tests/functional/git-hashing/common\.sh$'' + ''^tests/functional/git-hashing/simple\.sh$'' + ''^tests/functional/hash-convert\.sh$'' + ''^tests/functional/hash-path\.sh$'' + ''^tests/functional/help\.sh$'' + ''^tests/functional/import-derivation\.sh$'' + ''^tests/functional/impure-derivations\.sh$'' + ''^tests/functional/impure-env\.sh$'' + ''^tests/functional/impure-eval\.sh$'' + ''^tests/functional/install-darwin\.sh$'' + ''^tests/functional/lang-test-infra\.sh$'' + ''^tests/functional/lang\.sh$'' + ''^tests/functional/lang/framework\.sh$'' + ''^tests/functional/legacy-ssh-store\.sh$'' + ''^tests/functional/linux-sandbox\.sh$'' + ''^tests/functional/local-overlay-store/add-lower-inner\.sh$'' + ''^tests/functional/local-overlay-store/add-lower\.sh$'' + ''^tests/functional/local-overlay-store/bad-uris\.sh$'' + ''^tests/functional/local-overlay-store/build-inner\.sh$'' + ''^tests/functional/local-overlay-store/build\.sh$'' + ''^tests/functional/local-overlay-store/check-post-init-inner\.sh$'' + ''^tests/functional/local-overlay-store/check-post-init\.sh$'' + ''^tests/functional/local-overlay-store/common\.sh$'' + ''^tests/functional/local-overlay-store/delete-duplicate-inner\.sh$'' + ''^tests/functional/local-overlay-store/delete-duplicate\.sh$'' + ''^tests/functional/local-overlay-store/delete-refs-inner\.sh$'' + ''^tests/functional/local-overlay-store/delete-refs\.sh$'' + ''^tests/functional/local-overlay-store/gc-inner\.sh$'' + ''^tests/functional/local-overlay-store/gc\.sh$'' + ''^tests/functional/local-overlay-store/optimise-inner\.sh$'' + ''^tests/functional/local-overlay-store/optimise\.sh$'' + ''^tests/functional/local-overlay-store/redundant-add-inner\.sh$'' + ''^tests/functional/local-overlay-store/redundant-add\.sh$'' + ''^tests/functional/local-overlay-store/remount\.sh$'' + ''^tests/functional/local-overlay-store/stale-file-handle-inner\.sh$'' + ''^tests/functional/local-overlay-store/stale-file-handle\.sh$'' + ''^tests/functional/local-overlay-store/verify-inner\.sh$'' + ''^tests/functional/local-overlay-store/verify\.sh$'' + ''^tests/functional/logging\.sh$'' + ''^tests/functional/misc\.sh$'' + ''^tests/functional/multiple-outputs\.sh$'' + ''^tests/functional/nar-access\.sh$'' + ''^tests/functional/nested-sandboxing\.sh$'' + ''^tests/functional/nested-sandboxing/command\.sh$'' + ''^tests/functional/nix-build\.sh$'' + ''^tests/functional/nix-channel\.sh$'' + ''^tests/functional/nix-collect-garbage-d\.sh$'' + ''^tests/functional/nix-copy-ssh-common\.sh$'' + ''^tests/functional/nix-copy-ssh-ng\.sh$'' + ''^tests/functional/nix-copy-ssh\.sh$'' + ''^tests/functional/nix-daemon-untrusting\.sh$'' + ''^tests/functional/nix-profile\.sh$'' + ''^tests/functional/nix-shell\.sh$'' + ''^tests/functional/nix_path\.sh$'' + ''^tests/functional/optimise-store\.sh$'' + ''^tests/functional/output-normalization\.sh$'' + ''^tests/functional/parallel\.builder\.sh$'' + ''^tests/functional/parallel\.sh$'' + ''^tests/functional/pass-as-file\.sh$'' + ''^tests/functional/path-from-hash-part\.sh$'' + ''^tests/functional/path-info\.sh$'' + ''^tests/functional/placeholders\.sh$'' + ''^tests/functional/plugins\.sh$'' + ''^tests/functional/post-hook\.sh$'' + ''^tests/functional/pure-eval\.sh$'' + ''^tests/functional/push-to-store-old\.sh$'' + ''^tests/functional/push-to-store\.sh$'' + ''^tests/functional/read-only-store\.sh$'' + ''^tests/functional/readfile-context\.sh$'' + ''^tests/functional/recursive\.sh$'' + ''^tests/functional/referrers\.sh$'' + ''^tests/functional/remote-store\.sh$'' + ''^tests/functional/repair\.sh$'' + ''^tests/functional/restricted\.sh$'' + ''^tests/functional/search\.sh$'' + ''^tests/functional/secure-drv-outputs\.sh$'' + ''^tests/functional/selfref-gc\.sh$'' + ''^tests/functional/shell\.sh$'' + ''^tests/functional/shell\.shebang\.sh$'' + ''^tests/functional/signing\.sh$'' + ''^tests/functional/simple\.builder\.sh$'' + ''^tests/functional/simple\.sh$'' + ''^tests/functional/ssh-relay\.sh$'' + ''^tests/functional/store-info\.sh$'' + ''^tests/functional/structured-attrs\.sh$'' + ''^tests/functional/substitute-with-invalid-ca\.sh$'' + ''^tests/functional/suggestions\.sh$'' + ''^tests/functional/supplementary-groups\.sh$'' + ''^tests/functional/tarball\.sh$'' + ''^tests/functional/test-infra\.sh$'' + ''^tests/functional/test-libstoreconsumer\.sh$'' + ''^tests/functional/timeout\.sh$'' + ''^tests/functional/toString-path\.sh$'' + ''^tests/functional/user-envs-migration\.sh$'' + ''^tests/functional/user-envs-test-case\.sh$'' + ''^tests/functional/user-envs\.builder\.sh$'' + ''^tests/functional/user-envs\.sh$'' + ''^tests/functional/why-depends\.sh$'' + ''^tests/functional/zstd\.sh$'' + ''^tests/unit/libutil/data/git/check-data\.sh$'' ]; }; diff --git a/mk/common-test.sh b/mk/common-test.sh index 2abea7887..de24b6fcc 100644 --- a/mk/common-test.sh +++ b/mk/common-test.sh @@ -1,19 +1,24 @@ +# shellcheck shell=bash + # Remove overall test dir (at most one of the two should match) and # remove file extension. + +# shellcheck disable=SC2154 test_name=$(echo -n "$test" | sed \ -e "s|^tests/unit/[^/]*/data/||" \ -e "s|^tests/functional/||" \ -e "s|\.sh$||" \ ) +# shellcheck disable=SC2016 TESTS_ENVIRONMENT=( "TEST_NAME=$test_name" 'NIX_REMOTE=' 'PS4=+(${BASH_SOURCE[0]-$0}:$LINENO) ' ) -: ${BASH:=/usr/bin/env bash} +read -r -a bash <<< "${BASH:-/usr/bin/env bash}" run () { - cd "$(dirname $1)" && env "${TESTS_ENVIRONMENT[@]}" $BASH -x -e -u -o pipefail $(basename $1) + cd "$(dirname "$1")" && env "${TESTS_ENVIRONMENT[@]}" "${bash[@]}" -x -e -u -o pipefail "$(basename "$1")" } diff --git a/mk/run-test.sh b/mk/run-test.sh index 1256bfcf7..543c845e1 100755 --- a/mk/run-test.sh +++ b/mk/run-test.sh @@ -26,12 +26,13 @@ run_test () { run_test -if [ $status -eq 0 ]; then +if [[ "$status" = 0 ]]; then echo "$post_run_msg [${green}PASS$normal]" -elif [ $status -eq 99 ]; then +elif [[ "$status" = 99 ]]; then echo "$post_run_msg [${yellow}SKIP$normal]" else echo "$post_run_msg [${red}FAIL$normal]" + # shellcheck disable=SC2001 echo "$log" | sed 's/^/ /' exit "$status" fi diff --git a/scripts/bigsur-nixbld-user-migration.sh b/scripts/bigsur-nixbld-user-migration.sh index f1619fd56..0eb312e07 100755 --- a/scripts/bigsur-nixbld-user-migration.sh +++ b/scripts/bigsur-nixbld-user-migration.sh @@ -3,7 +3,7 @@ ((NEW_NIX_FIRST_BUILD_UID=301)) id_available(){ - dscl . list /Users UniqueID | grep -E '\b'$1'\b' >/dev/null + dscl . list /Users UniqueID | grep -E '\b'"$1"'\b' >/dev/null } change_nixbld_names_and_ids(){ @@ -26,18 +26,18 @@ change_nixbld_names_and_ids(){ fi done - if [[ $name == _* ]]; then + if [[ "$name" == _* ]]; then echo " It looks like $name has already been renamed--skipping." else # first 3 are cleanup, it's OK if they aren't here - sudo dscl . delete /Users/$name dsAttrTypeNative:_writers_passwd &>/dev/null || true - sudo dscl . change /Users/$name NFSHomeDirectory "/private/var/empty 1" "/var/empty" &>/dev/null || true + sudo dscl . delete "/Users/$name" dsAttrTypeNative:_writers_passwd &>/dev/null || true + sudo dscl . change "/Users/$name" NFSHomeDirectory "/private/var/empty 1" "/var/empty" &>/dev/null || true # remove existing user from group - sudo dseditgroup -o edit -t user -d $name nixbld || true - sudo dscl . change /Users/$name UniqueID $uid $next_id - sudo dscl . change /Users/$name RecordName $name _$name + sudo dseditgroup -o edit -t user -d "$name" nixbld || true + sudo dscl . change "/Users/$name" UniqueID "$uid" "$next_id" + sudo dscl . change "/Users/$name" RecordName "$name" "_$name" # add renamed user to group - sudo dseditgroup -o edit -t user -a _$name nixbld + sudo dseditgroup -o edit -t user -a "_$name" nixbld echo " $name migrated to _$name (uid: $next_id)" fi done < <(dscl . list /Users UniqueID | grep nixbld | sort -n -k2) diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index f11fa7140..222145a7a 100644 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -1,3 +1,4 @@ +#!/usr/bin/env bash source common.sh testDir="$PWD" @@ -21,11 +22,14 @@ import $testDir/undefined-variable.nix " testRepl () { - local nixArgs=("$@") + local nixArgs + nixArgs=("$@") rm -rf repl-result-out || true # cleanup from other runs backed by a foreign nix store - local replOutput="$(nix repl "${nixArgs[@]}" <<< "$replCmds")" + local replOutput + replOutput="$(nix repl "${nixArgs[@]}" <<< "$replCmds")" echo "$replOutput" - local outPath=$(echo "$replOutput" |& + local outPath + outPath=$(echo "$replOutput" |& grep -o -E "$NIX_STORE_DIR/\w*-simple") nix path-info "${nixArgs[@]}" "$outPath" [ "$(realpath ./repl-result-out)" == "$outPath" ] || fail "nix repl :bl doesn't make a symlink" @@ -34,11 +38,11 @@ testRepl () { # simple.nix prints a PATH during build echo "$replOutput" | grepQuiet -s 'PATH=' || fail "nix repl :log doesn't output logs" - local replOutput="$(nix repl "${nixArgs[@]}" <<< "$replFailingCmds" 2>&1)" + replOutput="$(nix repl "${nixArgs[@]}" <<< "$replFailingCmds" 2>&1)" echo "$replOutput" echo "$replOutput" | grepQuiet -s 'This should fail' \ || fail "nix repl :log doesn't output logs for a failed derivation" - local replOutput="$(nix repl --show-trace "${nixArgs[@]}" <<< "$replUndefinedVariable" 2>&1)" + replOutput="$(nix repl --show-trace "${nixArgs[@]}" <<< "$replUndefinedVariable" 2>&1)" echo "$replOutput" echo "$replOutput" | grepQuiet -s "while evaluating the file" \ || fail "nix repl --show-trace doesn't show the trace" @@ -48,7 +52,7 @@ testRepl () { nix repl "${nixArgs[@]}" 2>&1 <<< "builtins.currentSystem" \ | grep "$(nix-instantiate --eval -E 'builtins.currentSystem')" - expectStderr 1 nix repl ${testDir}/simple.nix \ + expectStderr 1 nix repl "${testDir}/simple.nix" \ | grepQuiet -s "error: path '$testDir/simple.nix' is not a flake" } @@ -63,10 +67,11 @@ stripColors () { } testReplResponseGeneral () { - local grepMode="$1"; shift - local commands="$1"; shift - local expectedResponse="$1"; shift - local response="$(nix repl "$@" <<< "$commands" | stripColors)" + local grepMode commands expectedResponse response + grepMode="$1"; shift + commands="$1"; shift + expectedResponse="$1"; shift + response="$(nix repl "$@" <<< "$commands" | stripColors)" echo "$response" | grepQuiet "$grepMode" -s "$expectedResponse" \ || fail "repl command set: @@ -91,6 +96,8 @@ testReplResponseNoRegex () { } # :a uses the newest version of a symbol +# +# shellcheck disable=SC2016 testReplResponse ' :a { a = "1"; } :a { a = "2"; } @@ -101,6 +108,8 @@ testReplResponse ' # note the escaped \, # \\ # because the second argument is a regex +# +# shellcheck disable=SC2016 testReplResponseNoRegex ' "$" + "{hi}" ' '"\${hi}"' @@ -108,12 +117,12 @@ testReplResponseNoRegex ' testReplResponse ' drvPath ' '".*-simple.drv"' \ ---file $testDir/simple.nix +--file "$testDir/simple.nix" testReplResponse ' drvPath ' '".*-simple.drv"' \ ---file $testDir/simple.nix --experimental-features 'ca-derivations' +--file "$testDir/simple.nix" --experimental-features 'ca-derivations' mkdir -p flake && cat < flake/flake.nix {