From 6e2c11e296a6b52c33c8d276796c568460889ef8 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Wed, 24 Sep 2025 19:15:53 -0700 Subject: [PATCH 01/24] shellcheck fix functional/dump-db.sh Add back the path variable --- tests/functional/dump-db.sh | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/functional/dump-db.sh b/tests/functional/dump-db.sh index 14181b4b6..70d79e9fb 100755 --- a/tests/functional/dump-db.sh +++ b/tests/functional/dump-db.sh @@ -8,19 +8,18 @@ needLocalStore "--dump-db requires a local store" clearStore -path=$(nix-build dependencies.nix -o $TEST_ROOT/result) +nix-build dependencies.nix -o "$TEST_ROOT"/result +deps="$(nix-store -qR "$TEST_ROOT"/result)" -deps="$(nix-store -qR $TEST_ROOT/result)" +nix-store --dump-db > "$TEST_ROOT"/dump -nix-store --dump-db > $TEST_ROOT/dump +rm -rf "$NIX_STATE_DIR"/db -rm -rf $NIX_STATE_DIR/db +nix-store --load-db < "$TEST_ROOT"/dump -nix-store --load-db < $TEST_ROOT/dump - -deps2="$(nix-store -qR $TEST_ROOT/result)" +deps2="$(nix-store -qR "$TEST_ROOT"/result)" [ "$deps" = "$deps2" ]; -nix-store --dump-db > $TEST_ROOT/dump2 -cmp $TEST_ROOT/dump $TEST_ROOT/dump2 +nix-store --dump-db > "$TEST_ROOT"/dump2 +cmp "$TEST_ROOT"/dump "$TEST_ROOT"/dump2 From 412e51215f262916ecfeed871d501ebfc2911d12 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:29:27 -0700 Subject: [PATCH 02/24] shellcheck fix: functional/dyn-drv/eval-outputOf.sh --- maintainers/flake-module.nix | 1 - 1 file changed, 1 deletion(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 5f8a80a23..54f53fb6a 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.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/eval-store\.sh$'' From 119489f2535cab51b65c8a7908b013487e5fe3cb Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:30:03 -0700 Subject: [PATCH 03/24] shellcheck fix: tests/functional/dyn-drv/old-daemon-error-hack.sh --- maintainers/flake-module.nix | 1 - tests/functional/dyn-drv/old-daemon-error-hack.sh | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 54f53fb6a..671591ab4 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' - ''^tests/functional/dyn-drv/old-daemon-error-hack\.sh$'' ''^tests/functional/dyn-drv/recursive-mod-json\.sh$'' ''^tests/functional/eval-store\.sh$'' ''^tests/functional/export-graph\.sh$'' diff --git a/tests/functional/dyn-drv/old-daemon-error-hack.sh b/tests/functional/dyn-drv/old-daemon-error-hack.sh index 43b049973..02129bd73 100644 --- a/tests/functional/dyn-drv/old-daemon-error-hack.sh +++ b/tests/functional/dyn-drv/old-daemon-error-hack.sh @@ -1,3 +1,4 @@ +# shellcheck shell=bash # Purposely bypassing our usual common for this subgroup source ../common.sh From dc69e2e5205ee062e67795f37740ab1d179a2c95 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:31:06 -0700 Subject: [PATCH 04/24] shellcheck fix: tests/functional/dyn-drv/recursive-mod-json.sh --- maintainers/flake-module.nix | 1 - tests/functional/dyn-drv/recursive-mod-json.sh | 11 ++++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 671591ab4..505424633 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' - ''^tests/functional/dyn-drv/recursive-mod-json\.sh$'' ''^tests/functional/eval-store\.sh$'' ''^tests/functional/export-graph\.sh$'' ''^tests/functional/export\.sh$'' diff --git a/tests/functional/dyn-drv/recursive-mod-json.sh b/tests/functional/dyn-drv/recursive-mod-json.sh index 0698b81bd..01e8f16e9 100644 --- a/tests/functional/dyn-drv/recursive-mod-json.sh +++ b/tests/functional/dyn-drv/recursive-mod-json.sh @@ -1,3 +1,4 @@ +# shellcheck shell=bash source common.sh # FIXME @@ -10,18 +11,18 @@ restartDaemon clearStore -rm -f $TEST_ROOT/result +rm -f "$TEST_ROOT"/result -EXTRA_PATH=$(dirname $(type -p nix)):$(dirname $(type -p jq)) +EXTRA_PATH=$(dirname "$(type -p nix)"):$(dirname "$(type -p jq)") export EXTRA_PATH # Will produce a drv metaDrv=$(nix-instantiate ./recursive-mod-json.nix) # computed "dynamic" derivation -drv=$(nix-store -r $metaDrv) +drv=$(nix-store -r "$metaDrv") # build that dyn drv -res=$(nix-store -r $drv) +res=$(nix-store -r "$drv") -grep 'I am alive!' $res/hello +grep 'I am alive!' "$res"/hello From b8c24cdaef3a7439b381eae2cc050ffff9dcb68a Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:33:40 -0700 Subject: [PATCH 05/24] shellcheck fix: tests/functional/eval-store.sh --- maintainers/flake-module.nix | 1 - tests/functional/eval-store.sh | 21 +++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 505424633..85a4d90f1 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' - ''^tests/functional/eval-store\.sh$'' ''^tests/functional/export-graph\.sh$'' ''^tests/functional/export\.sh$'' ''^tests/functional/extra-sandbox-profile\.sh$'' diff --git a/tests/functional/eval-store.sh b/tests/functional/eval-store.sh index 92faa4005..9f4b3b036 100755 --- a/tests/functional/eval-store.sh +++ b/tests/functional/eval-store.sh @@ -6,6 +6,7 @@ TODO_NixOS # Using `--eval-store` with the daemon will eventually copy everything # to the build store, invalidating most of the tests here +# shellcheck disable=SC1111 needLocalStore "“--eval-store” doesn't achieve much with the daemon" eval_store=$TEST_ROOT/eval-store @@ -15,7 +16,7 @@ rm -rf "$eval_store" nix build -f dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result" [[ -e $TEST_ROOT/result/foobar ]] -if [[ ! -n "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then +if [[ -z "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then # Resolved CA derivations are written to store for building # # TODO when we something more systematic @@ -23,35 +24,35 @@ if [[ ! -n "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then # between scratch storage for building and the final destination # store, we'll be able to make this unconditional again -- resolved # derivations should only appear in the scratch store. - (! ls $NIX_STORE_DIR/*.drv) + (! ls "$NIX_STORE_DIR"/*.drv) fi -ls $eval_store/nix/store/*.drv +ls "$eval_store"/nix/store/*.drv clearStore rm -rf "$eval_store" nix-instantiate dependencies.nix --eval-store "$eval_store" -(! ls $NIX_STORE_DIR/*.drv) -ls $eval_store/nix/store/*.drv +(! ls "$NIX_STORE_DIR"/*.drv) +ls "$eval_store"/nix/store/*.drv clearStore rm -rf "$eval_store" nix-build dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result" [[ -e $TEST_ROOT/result/foobar ]] -if [[ ! -n "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then +if [[ -z "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then # See above - (! ls $NIX_STORE_DIR/*.drv) + (! ls "$NIX_STORE_DIR"/*.drv) fi -ls $eval_store/nix/store/*.drv +ls "$eval_store"/nix/store/*.drv clearStore rm -rf "$eval_store" # Confirm that import-from-derivation builds on the build store [[ $(nix eval --eval-store "$eval_store?require-sigs=false" --impure --raw --file ./ifd.nix) = hi ]] -ls $NIX_STORE_DIR/*dependencies-top/foobar -(! ls $eval_store/nix/store/*dependencies-top/foobar) +ls "$NIX_STORE_DIR"/*dependencies-top/foobar +(! ls "$eval_store"/nix/store/*dependencies-top/foobar) # Can't write .drv by default (! nix-instantiate dependencies.nix --eval-store "dummy://") From a209748ec04cc7142bf3c01edffc058fa4846661 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:35:01 -0700 Subject: [PATCH 06/24] shellcheck fix: tests/functional/export-graph.sh --- maintainers/flake-module.nix | 1 - tests/functional/export-graph.sh | 14 ++++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 85a4d90f1..8277788c6 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' - ''^tests/functional/export-graph\.sh$'' ''^tests/functional/export\.sh$'' ''^tests/functional/extra-sandbox-profile\.sh$'' ''^tests/functional/fetchClosure\.sh$'' diff --git a/tests/functional/export-graph.sh b/tests/functional/export-graph.sh index b507b6d3a..0490b580d 100755 --- a/tests/functional/export-graph.sh +++ b/tests/functional/export-graph.sh @@ -8,27 +8,29 @@ clearStore clearProfiles checkRef() { - nix-store -q --references $TEST_ROOT/result | grepQuiet "$1"'$' || fail "missing reference $1" + nix-store -q --references "$TEST_ROOT"/result | grepQuiet "$1"'$' || fail "missing reference $1" } # Test the export of the runtime dependency graph. -outPath=$(nix-build ./export-graph.nix -A 'foo."bar.runtimeGraph"' -o $TEST_ROOT/result) +outPath=$(nix-build ./export-graph.nix -A 'foo."bar.runtimeGraph"' -o "$TEST_ROOT"/result) -test $(nix-store -q --references $TEST_ROOT/result | wc -l) = 3 || fail "bad nr of references" +test "$(nix-store -q --references "$TEST_ROOT"/result | wc -l)" = 3 || fail "bad nr of references" checkRef input-2 -for i in $(cat $outPath); do checkRef $i; done +# shellcheck disable=SC2013 +for i in $(cat "$outPath"); do checkRef "$i"; done # Test the export of the build-time dependency graph. nix-store --gc # should force rebuild of input-1 -outPath=$(nix-build ./export-graph.nix -A 'foo."bar.buildGraph"' -o $TEST_ROOT/result) +outPath=$(nix-build ./export-graph.nix -A 'foo."bar.buildGraph"' -o "$TEST_ROOT"/result) checkRef input-1 checkRef input-1.drv checkRef input-2 checkRef input-2.drv -for i in $(cat $outPath); do checkRef $i; done +# shellcheck disable=SC2013 +for i in $(cat "$outPath"); do checkRef "$i"; done From 9e3c5025218ede84c26ac1c86fcf2711b9b83567 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:35:26 -0700 Subject: [PATCH 07/24] shellcheck fix: tests/functional/extra-sandbox-profile.sh --- maintainers/flake-module.nix | 1 - 1 file changed, 1 deletion(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 8277788c6..7d593d26e 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -108,7 +108,6 @@ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' ''^tests/functional/export\.sh$'' - ''^tests/functional/extra-sandbox-profile\.sh$'' ''^tests/functional/fetchClosure\.sh$'' ''^tests/functional/fetchGit\.sh$'' ''^tests/functional/fetchGitRefs\.sh$'' From 230da1cbe73800524d3fd3373b3eecb988d9b435 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:36:12 -0700 Subject: [PATCH 08/24] shellcheck fix: tests/functional/export.sh --- maintainers/flake-module.nix | 1 - tests/functional/export.sh | 16 +++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 7d593d26e..0a46dc57f 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' - ''^tests/functional/export\.sh$'' ''^tests/functional/fetchClosure\.sh$'' ''^tests/functional/fetchGit\.sh$'' ''^tests/functional/fetchGitRefs\.sh$'' diff --git a/tests/functional/export.sh b/tests/functional/export.sh index 3e895a540..53bbdd9ac 100755 --- a/tests/functional/export.sh +++ b/tests/functional/export.sh @@ -8,11 +8,12 @@ clearStore outPath=$(nix-build dependencies.nix --no-out-link) -nix-store --export $outPath > $TEST_ROOT/exp +nix-store --export "$outPath" > "$TEST_ROOT"/exp -nix-store --export $(nix-store -qR $outPath) > $TEST_ROOT/exp_all +# shellcheck disable=SC2046 +nix-store --export $(nix-store -qR "$outPath") > "$TEST_ROOT"/exp_all -if nix-store --export $outPath >/dev/full ; then +if nix-store --export "$outPath" >/dev/full ; then echo "exporting to a bad file descriptor should fail" exit 1 fi @@ -20,7 +21,7 @@ fi clearStore -if nix-store --import < $TEST_ROOT/exp; then +if nix-store --import < "$TEST_ROOT"/exp; then echo "importing a non-closure should fail" exit 1 fi @@ -28,13 +29,14 @@ fi clearStore -nix-store --import < $TEST_ROOT/exp_all +nix-store --import < "$TEST_ROOT"/exp_all -nix-store --export $(nix-store -qR $outPath) > $TEST_ROOT/exp_all2 +# shellcheck disable=SC2046 +nix-store --export $(nix-store -qR "$outPath") > "$TEST_ROOT"/exp_all2 clearStore # Regression test: the derivers in exp_all2 are empty, which shouldn't # cause a failure. -nix-store --import < $TEST_ROOT/exp_all2 +nix-store --import < "$TEST_ROOT"/exp_all2 From d07dd92db30d9e560d01a6cf9013a07d530dce24 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:38:51 -0700 Subject: [PATCH 09/24] shellcheck fix: tests/functional/fetchClosure.sh --- maintainers/flake-module.nix | 1 - tests/functional/fetchClosure.sh | 48 +++++++++++++++++--------------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 0a46dc57f..5ad6e05eb 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' - ''^tests/functional/fetchClosure\.sh$'' ''^tests/functional/fetchGit\.sh$'' ''^tests/functional/fetchGitRefs\.sh$'' ''^tests/functional/fetchGitSubmodules\.sh$'' diff --git a/tests/functional/fetchClosure.sh b/tests/functional/fetchClosure.sh index 7ef635d36..9b79ab396 100755 --- a/tests/functional/fetchClosure.sh +++ b/tests/functional/fetchClosure.sh @@ -17,14 +17,14 @@ requireDaemonNewerThan "2.16.0pre20230524" # Initialize binary cache. nonCaPath=$(nix build --json --file ./dependencies.nix --no-link | jq -r .[].outputs.out) -caPath=$(nix store make-content-addressed --json $nonCaPath | jq -r '.rewrites | map(.) | .[]') -nix copy --to file://$cacheDir $nonCaPath +caPath=$(nix store make-content-addressed --json "$nonCaPath" | jq -r '.rewrites | map(.) | .[]') +nix copy --to file://"$cacheDir" "$nonCaPath" # Test basic fetchClosure rewriting from non-CA to CA. clearStore -[ ! -e $nonCaPath ] -[ ! -e $caPath ] +[ ! -e "$nonCaPath" ] +[ ! -e "$caPath" ] [[ $(nix eval -v --raw --expr " builtins.fetchClosure { @@ -32,10 +32,10 @@ clearStore fromPath = $nonCaPath; toPath = $caPath; } -") = $caPath ]] +") = "$caPath" ]] -[ ! -e $nonCaPath ] -[ -e $caPath ] +[ ! -e "$nonCaPath" ] +[ -e "$caPath" ] clearStore @@ -55,7 +55,7 @@ if [[ "$NIX_REMOTE" != "daemon" ]]; then # TODO: Should the closure be rejected, despite single user mode? # [ ! -e $nonCaPath ] - [ ! -e $caPath ] + [ ! -e "$caPath" ] # We can use non-CA paths when we ask explicitly. [[ $(nix eval --raw --no-require-sigs --expr " @@ -64,15 +64,15 @@ if [[ "$NIX_REMOTE" != "daemon" ]]; then fromPath = $nonCaPath; inputAddressed = true; } - ") = $nonCaPath ]] + ") = "$nonCaPath" ]] - [ -e $nonCaPath ] - [ ! -e $caPath ] + [ -e "$nonCaPath" ] + [ ! -e "$caPath" ] fi -[ ! -e $caPath ] +[ ! -e "$caPath" ] # 'toPath' set to empty string should fail but print the expected path. expectStderr 1 nix eval -v --json --expr " @@ -84,39 +84,41 @@ expectStderr 1 nix eval -v --json --expr " " | grep "error: rewriting.*$nonCaPath.*yielded.*$caPath" # If fromPath is CA, then toPath isn't needed. -nix copy --to file://$cacheDir $caPath +nix copy --to file://"$cacheDir" "$caPath" clearStore -[ ! -e $caPath ] +[ ! -e "$caPath" ] [[ $(nix eval -v --raw --expr " builtins.fetchClosure { fromStore = \"file://$cacheDir\"; fromPath = $caPath; } -") = $caPath ]] +") = "$caPath" ]] -[ -e $caPath ] +[ -e "$caPath" ] # Check that URL query parameters aren't allowed. clearStore narCache=$TEST_ROOT/nar-cache -rm -rf $narCache +rm -rf "$narCache" (! nix eval -v --raw --expr " builtins.fetchClosure { fromStore = \"file://$cacheDir?local-nar-cache=$narCache\"; fromPath = $caPath; } ") -(! [ -e $narCache ]) +# shellcheck disable=SC2235 +(! [ -e "$narCache" ]) # If toPath is specified but wrong, we check it (only) when the path is missing. clearStore -badPath=$(echo $caPath | sed -e 's!/store/................................-!/store/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx-!') +# shellcheck disable=SC2001 +badPath=$(echo "$caPath" | sed -e 's!/store/................................-!/store/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx-!') -[ ! -e $badPath ] +[ ! -e "$badPath" ] expectStderr 1 nix eval -v --raw --expr " builtins.fetchClosure { @@ -126,11 +128,11 @@ expectStderr 1 nix eval -v --raw --expr " } " | grep "error: rewriting.*$nonCaPath.*yielded.*$caPath.*while.*$badPath.*was expected" -[ ! -e $badPath ] +[ ! -e "$badPath" ] # We only check it when missing, as a performance optimization similar to what we do for fixed output derivations. So if it's already there, we don't check it. # It would be nice for this to fail, but checking it would be too(?) slow. -[ -e $caPath ] +[ -e "$caPath" ] [[ $(nix eval -v --raw --expr " builtins.fetchClosure { @@ -138,7 +140,7 @@ expectStderr 1 nix eval -v --raw --expr " fromPath = $badPath; toPath = $caPath; } -") = $caPath ]] +") = "$caPath" ]] # However, if the output address is unexpected, we can report it From 32e1b5209bc0d6925fb33e593b61569c7b06b86d Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:44:29 -0700 Subject: [PATCH 10/24] shellcheck fix: tests/functional/fetchGit.sh --- maintainers/flake-module.nix | 1 - tests/functional/fetchGit.sh | 192 ++++++++++++++++++----------------- 2 files changed, 98 insertions(+), 95 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 5ad6e05eb..54d02d9f1 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' - ''^tests/functional/fetchGit\.sh$'' ''^tests/functional/fetchGitRefs\.sh$'' ''^tests/functional/fetchGitSubmodules\.sh$'' ''^tests/functional/fetchGitVerification\.sh$'' diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh index e7c9c77a5..be8b5cb34 100755 --- a/tests/functional/fetchGit.sh +++ b/tests/functional/fetchGit.sh @@ -12,25 +12,25 @@ repo=$TEST_ROOT/./git export _NIX_FORCE_HTTP=1 -rm -rf $repo ${repo}-tmp $TEST_HOME/.cache/nix $TEST_ROOT/worktree $TEST_ROOT/minimal +rm -rf "$repo" "${repo}"-tmp "$TEST_HOME"/.cache/nix "$TEST_ROOT"/worktree "$TEST_ROOT"/minimal -git init $repo -git -C $repo config user.email "foobar@example.com" -git -C $repo config user.name "Foobar" +git init "$repo" +git -C "$repo" config user.email "foobar@example.com" +git -C "$repo" config user.name "Foobar" -echo utrecht > $repo/hello -touch $repo/.gitignore -git -C $repo add hello .gitignore -git -C $repo commit -m 'Bla1' -rev1=$(git -C $repo rev-parse HEAD) -git -C $repo tag -a tag1 -m tag1 +echo utrecht > "$repo"/hello +touch "$repo"/.gitignore +git -C "$repo" add hello .gitignore +git -C "$repo" commit -m 'Bla1' +rev1=$(git -C "$repo" rev-parse HEAD) +git -C "$repo" tag -a tag1 -m tag1 -echo world > $repo/hello -git -C $repo commit -m 'Bla2' -a -git -C $repo worktree add $TEST_ROOT/worktree -echo hello >> $TEST_ROOT/worktree/hello -rev2=$(git -C $repo rev-parse HEAD) -git -C $repo tag -a tag2 -m tag2 +echo world > "$repo"/hello +git -C "$repo" commit -m 'Bla2' -a +git -C "$repo" worktree add "$TEST_ROOT"/worktree +echo hello >> "$TEST_ROOT"/worktree/hello +rev2=$(git -C "$repo" rev-parse HEAD) +git -C "$repo" tag -a tag2 -m tag2 # Check whether fetching in read-only mode works. nix-instantiate --eval -E "builtins.readFile ((builtins.fetchGit file://$TEST_ROOT/worktree) + \"/hello\") == \"utrecht\\n\"" @@ -40,52 +40,52 @@ unset _NIX_FORCE_HTTP expectStderr 0 nix eval -vvvv --impure --raw --expr "(builtins.fetchGit file://$TEST_ROOT/worktree).outPath" | grepQuiet "copying '$TEST_ROOT/worktree/' to the store" path0=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$TEST_ROOT/worktree).outPath") path0_=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; url = file://$TEST_ROOT/worktree; }).outPath") -[[ $path0 = $path0_ ]] +[[ $path0 = "$path0_" ]] path0_=$(nix eval --impure --raw --expr "(builtins.fetchTree git+file://$TEST_ROOT/worktree).outPath") -[[ $path0 = $path0_ ]] +[[ $path0 = "$path0_" ]] export _NIX_FORCE_HTTP=1 -[[ $(tail -n 1 $path0/hello) = "hello" ]] +[[ $(tail -n 1 "$path0"/hello) = "hello" ]] # Nuke the cache -rm -rf $TEST_HOME/.cache/nix +rm -rf "$TEST_HOME"/.cache/nix # Fetch the default branch. path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") -[[ $(cat $path/hello) = world ]] +[[ $(cat "$path"/hello) = world ]] # Fetch again. This should be cached. # NOTE: This has to be done before the test case below which tries to pack-refs # the reason being that the lookup on the cache uses the ref-file `/refs/heads/master` # which does not exist after packing. -mv $repo ${repo}-tmp +mv "$repo" "${repo}"-tmp path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") -[[ $path = $path2 ]] +[[ $path = "$path2" ]] [[ $(nix eval --impure --expr "(builtins.fetchGit file://$repo).revCount") = 2 ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).rev") = $rev2 ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).shortRev") = ${rev2:0:7} ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).rev") = "$rev2" ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).shortRev") = "${rev2:0:7}" ]] # Fetching with a explicit hash should succeed. path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev2\"; }).outPath") -[[ $path = $path2 ]] +[[ $path = "$path2" ]] path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev1\"; }).outPath") -[[ $(cat $path2/hello) = utrecht ]] +[[ $(cat "$path2"/hello) = utrecht ]] -mv ${repo}-tmp $repo +mv "${repo}"-tmp "$repo" # Fetch when the cache has packed-refs # Regression test of #8822 -git -C $TEST_HOME/.cache/nix/gitv3/*/ pack-refs --all +git -C "$TEST_HOME"/.cache/nix/gitv3/*/ pack-refs --all path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") # Fetch a rev from another branch -git -C $repo checkout -b devtest -echo "different file" >> $TEST_ROOT/git/differentbranch -git -C $repo add differentbranch -git -C $repo commit -m 'Test2' -git -C $repo checkout master -devrev=$(git -C $repo rev-parse devtest) +git -C "$repo" checkout -b devtest +echo "different file" >> "$TEST_ROOT"/git/differentbranch +git -C "$repo" add differentbranch +git -C "$repo" commit -m 'Test2' +git -C "$repo" checkout master +devrev=$(git -C "$repo" rev-parse devtest) nix eval --raw --expr "builtins.fetchGit { url = file://$repo; rev = \"$devrev\"; }" [[ $(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = file://$repo; rev = \"$devrev\"; allRefs = true; } + \"/differentbranch\")") = 'different file' ]] @@ -96,7 +96,7 @@ nix eval --raw --expr "builtins.fetchGit { url = file://$repo; rev = \"$devrev\" # Fetch using an explicit revision hash. path2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev2\"; }).outPath") -[[ $path = $path2 ]] +[[ $path = "$path2" ]] # In pure eval mode, fetchGit with a revision should succeed. [[ $(nix eval --raw --expr "builtins.readFile (fetchGit { url = file://$repo; rev = \"$rev2\"; } + \"/hello\")") = world ]] @@ -106,23 +106,23 @@ expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "'f # Using a clean working tree should produce the same result. path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") -[[ $path = $path2 ]] +[[ $path = "$path2" ]] # Using an unclean tree should yield the tracked but uncommitted changes. -mkdir $repo/dir1 $repo/dir2 -echo foo > $repo/dir1/foo -echo bar > $repo/bar -echo bar > $repo/dir2/bar -git -C $repo add dir1/foo -git -C $repo rm hello +mkdir "$repo"/dir1 "$repo"/dir2 +echo foo > "$repo"/dir1/foo +echo bar > "$repo"/bar +echo bar > "$repo"/dir2/bar +git -C "$repo" add dir1/foo +git -C "$repo" rm hello unset _NIX_FORCE_HTTP path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") -[ ! -e $path2/hello ] -[ ! -e $path2/bar ] -[ ! -e $path2/dir2/bar ] -[ ! -e $path2/.git ] -[[ $(cat $path2/dir1/foo) = foo ]] +[ ! -e "$path2"/hello ] +[ ! -e "$path2"/bar ] +[ ! -e "$path2"/dir2/bar ] +[ ! -e "$path2"/.git ] +[[ $(cat "$path2"/dir1/foo) = foo ]] [[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] [[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).dirtyRev") = "${rev2}-dirty" ]] @@ -130,16 +130,16 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") # ... unless we're using an explicit ref or rev. path3=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"master\"; }).outPath") -[[ $path = $path3 ]] +[[ $path = "$path3" ]] path3=$(nix eval --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; }).outPath") -[[ $path = $path3 ]] +[[ $path = "$path3" ]] # Committing should not affect the store path. -git -C $repo commit -m 'Bla3' -a +git -C "$repo" commit -m 'Bla3' -a path4=$(nix eval --impure --refresh --raw --expr "(builtins.fetchGit file://$repo).outPath") -[[ $path2 = $path4 ]] +[[ $path2 = "$path4" ]] [[ $(nix eval --impure --expr "builtins.hasAttr \"rev\" (builtins.fetchGit $repo)") == "true" ]] [[ $(nix eval --impure --expr "builtins.hasAttr \"dirtyRev\" (builtins.fetchGit $repo)") == "false" ]] @@ -148,7 +148,7 @@ path4=$(nix eval --impure --refresh --raw --expr "(builtins.fetchGit file://$rep expect 102 nix eval --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-B5yIPHhEm0eysJKEsO7nqxprh9vcblFxpJG11gXJus1=\"; }).outPath" path5=$(nix eval --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-Hr8g6AqANb3xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath") -[[ $path = $path5 ]] +[[ $path = "$path5" ]] # Ensure that NAR hashes are checked. expectStderr 102 nix eval --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-Hr8g6AqANb4xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath" | grepQuiet "error: NAR hash mismatch" @@ -157,22 +157,22 @@ expectStderr 102 nix eval --raw --expr "(builtins.fetchGit { url = $repo; rev = expectStderr 0 nix eval --raw --expr "(builtins.fetchGit { url = $repo; ref = \"tag2\"; narHash = \"sha256-Hr8g6AqANb3xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath" | grepQuiet "warning: Input .* is unlocked" # tarball-ttl should be ignored if we specify a rev -echo delft > $repo/hello -git -C $repo add hello -git -C $repo commit -m 'Bla4' -rev3=$(git -C $repo rev-parse HEAD) +echo delft > "$repo"/hello +git -C "$repo" add hello +git -C "$repo" commit -m 'Bla4' +rev3=$(git -C "$repo" rev-parse HEAD) nix eval --tarball-ttl 3600 --expr "builtins.fetchGit { url = $repo; rev = \"$rev3\"; }" >/dev/null # Update 'path' to reflect latest master path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") # Check behavior when non-master branch is used -git -C $repo checkout $rev2 -b dev -echo dev > $repo/hello +git -C "$repo" checkout "$rev2" -b dev +echo dev > "$repo"/hello # File URI uses dirty tree unless specified otherwise path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") -[ $(cat $path2/hello) = dev ] +[ "$(cat "$path2"/hello)" = dev ] # Using local path with branch other than 'master' should work when clean or dirty path3=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") @@ -181,53 +181,53 @@ path3=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).shortRev") = 0000000 ]] # Making a dirty tree clean again and fetching it should # record correct revision information. See: #4140 -echo world > $repo/hello -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = $rev2 ]] +echo world > "$repo"/hello +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = "$rev2" ]] # Committing shouldn't change store path, or switch to using 'master' -echo dev > $repo/hello -git -C $repo commit -m 'Bla5' -a +echo dev > "$repo"/hello +git -C "$repo" commit -m 'Bla5' -a path4=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") -[[ $(cat $path4/hello) = dev ]] -[[ $path3 = $path4 ]] +[[ $(cat "$path4"/hello) = dev ]] +[[ $path3 = "$path4" ]] # Using remote path with branch other than 'master' should fetch the HEAD revision. # (--tarball-ttl 0 to prevent using the cached repo above) export _NIX_FORCE_HTTP=1 path4=$(nix eval --tarball-ttl 0 --impure --raw --expr "(builtins.fetchGit $repo).outPath") -[[ $(cat $path4/hello) = dev ]] -[[ $path3 = $path4 ]] +[[ $(cat "$path4"/hello) = dev ]] +[[ $path3 = "$path4" ]] unset _NIX_FORCE_HTTP # Confirm same as 'dev' branch path5=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"dev\"; }).outPath") -[[ $path3 = $path5 ]] +[[ $path3 = "$path5" ]] # Nuke the cache -rm -rf $TEST_HOME/.cache/nix +rm -rf "$TEST_HOME"/.cache/nix # Try again. This should work. path5=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"dev\"; }).outPath") -[[ $path3 = $path5 ]] +[[ $path3 = "$path5" ]] # Fetching from a repo with only a specific revision and no branches should # not fall back to copying files and record correct revision information. See: #5302 -mkdir $TEST_ROOT/minimal -git -C $TEST_ROOT/minimal init -git -C $TEST_ROOT/minimal fetch $repo $rev2 -git -C $TEST_ROOT/minimal checkout $rev2 -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit { url = $TEST_ROOT/minimal; }).rev") = $rev2 ]] +mkdir "$TEST_ROOT"/minimal +git -C "$TEST_ROOT"/minimal init +git -C "$TEST_ROOT"/minimal fetch "$repo" "$rev2" +git -C "$TEST_ROOT"/minimal checkout "$rev2" +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit { url = $TEST_ROOT/minimal; }).rev") = "$rev2" ]] # Explicit ref = "HEAD" should work, and produce the same outPath as without ref path7=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"HEAD\"; }).outPath") path8=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; }).outPath") -[[ $path7 = $path8 ]] +[[ $path7 = "$path8" ]] # ref = "HEAD" should fetch the HEAD revision -rev4=$(git -C $repo rev-parse HEAD) +rev4=$(git -C "$repo" rev-parse HEAD) rev4_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"HEAD\"; }).rev") -[[ $rev4 = $rev4_nix ]] +[[ $rev4 = "$rev4_nix" ]] # The name argument should be handled path9=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"HEAD\"; name = \"foo\"; }).outPath") @@ -236,33 +236,36 @@ path9=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$rep # Specifying a ref without a rev shouldn't pick a cached rev for a different ref export _NIX_FORCE_HTTP=1 rev_tag1_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/tag1\"; }).rev") -rev_tag1=$(git -C $repo rev-parse refs/tags/tag1^{commit}) -[[ $rev_tag1_nix = $rev_tag1 ]] +# shellcheck disable=SC1083 +rev_tag1=$(git -C "$repo" rev-parse refs/tags/tag1^{commit}) +[[ $rev_tag1_nix = "$rev_tag1" ]] rev_tag2_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/tag2\"; }).rev") -rev_tag2=$(git -C $repo rev-parse refs/tags/tag2^{commit}) -[[ $rev_tag2_nix = $rev_tag2 ]] +# shellcheck disable=SC1083 +rev_tag2=$(git -C "$repo" rev-parse refs/tags/tag2^{commit}) +[[ $rev_tag2_nix = "$rev_tag2" ]] unset _NIX_FORCE_HTTP # Ensure .gitattributes is respected -touch $repo/not-exported-file -touch $repo/exported-wonky -echo "/not-exported-file export-ignore" >> $repo/.gitattributes -echo "/exported-wonky export-ignore=wonk" >> $repo/.gitattributes -git -C $repo add not-exported-file exported-wonky .gitattributes -git -C $repo commit -m 'Bla6' -rev5=$(git -C $repo rev-parse HEAD) +touch "$repo"/not-exported-file +touch "$repo"/exported-wonky +echo "/not-exported-file export-ignore" >> "$repo"/.gitattributes +echo "/exported-wonky export-ignore=wonk" >> "$repo"/.gitattributes +git -C "$repo" add not-exported-file exported-wonky .gitattributes +git -C "$repo" commit -m 'Bla6' +rev5=$(git -C "$repo" rev-parse HEAD) path12=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev5\"; }).outPath") [[ ! -e $path12/not-exported-file ]] [[ -e $path12/exported-wonky ]] # should fail if there is no repo -rm -rf $repo/.git -rm -rf $TEST_HOME/.cache/nix +rm -rf "$repo"/.git +rm -rf "$TEST_HOME"/.cache/nix (! nix eval --impure --raw --expr "(builtins.fetchGit \"file://$repo\").outPath") # should succeed for a repo without commits -git init $repo -git -C $repo add hello # need to add at least one file to cause the root of the repo to be visible +git init "$repo" +git -C "$repo" add hello # need to add at least one file to cause the root of the repo to be visible +# shellcheck disable=SC2034 path10=$(nix eval --impure --raw --expr "(builtins.fetchGit \"file://$repo\").outPath") # should succeed for a path with a space @@ -277,6 +280,7 @@ touch "$repo/.gitignore" git -C "$repo" add hello .gitignore git -C "$repo" commit -m 'Bla1' cd "$repo" +# shellcheck disable=SC2034 path11=$(nix eval --impure --raw --expr "(builtins.fetchGit ./.).outPath") # Test a workdir with no commits. From 1619409bf2f655248128cdb8bd1effd0f646dbae Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 10:46:34 -0700 Subject: [PATCH 11/24] shellcheck fix: tests/functional/fetchGitRefs.sh --- maintainers/flake-module.nix | 1 - tests/functional/fetchGitRefs.sh | 7 ++++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 54d02d9f1..50574c129 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^tests/functional/dump-db\.sh$'' - ''^tests/functional/fetchGitRefs\.sh$'' ''^tests/functional/fetchGitSubmodules\.sh$'' ''^tests/functional/fetchGitVerification\.sh$'' ''^tests/functional/fetchMercurial\.sh$'' diff --git a/tests/functional/fetchGitRefs.sh b/tests/functional/fetchGitRefs.sh index 258a65525..288b26591 100755 --- a/tests/functional/fetchGitRefs.sh +++ b/tests/functional/fetchGitRefs.sh @@ -38,16 +38,16 @@ path=$(nix eval --raw --impure --expr "(builtins.fetchGit { url = $repo; ref = \ # 10. They cannot contain a \. valid_ref() { - { set +x; printf >&2 '\n>>>>>>>>>> valid_ref %s\b <<<<<<<<<<\n' $(printf %s "$1" | sed -n -e l); set -x; } + { set +x; printf >&2 '\n>>>>>>>>>> valid_ref %s\b <<<<<<<<<<\n' "$(printf %s "$1" | sed -n -e l)"; set -x; } git check-ref-format --branch "$1" >/dev/null git -C "$repo" branch "$1" master >/dev/null path1=$(nix eval --raw --impure --expr "(builtins.fetchGit { url = $repo; ref = ''$1''; }).outPath") - [[ $path1 = $path ]] + [[ $path1 = "$path" ]] git -C "$repo" branch -D "$1" >/dev/null } invalid_ref() { - { set +x; printf >&2 '\n>>>>>>>>>> invalid_ref %s\b <<<<<<<<<<\n' $(printf %s "$1" | sed -n -e l); set -x; } + { set +x; printf >&2 '\n>>>>>>>>>> invalid_ref %s\b <<<<<<<<<<\n' "$(printf %s "$1" | sed -n -e l)"; set -x; } # special case for a sole @: # --branch @ will try to interpret @ as a branch reference and not fail. Thus we need --allow-onelevel if [ "$1" = "@" ]; then @@ -68,6 +68,7 @@ valid_ref 'heads/foo@bar' valid_ref "$(printf 'heads/fu\303\237')" valid_ref 'foo-bar-baz' valid_ref 'branch#' +# shellcheck disable=SC2016 valid_ref '$1' valid_ref 'foo.locke' From ea035ae16599861b3cd05c9f86c694ddf527d279 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:07:41 -0700 Subject: [PATCH 12/24] shellcheck fix: tests/tests/functional/dump-db.sh --- maintainers/flake-module.nix | 1 - 1 file changed, 1 deletion(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 50574c129..b2acc36d9 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/dump-db\.sh$'' ''^tests/functional/fetchGitSubmodules\.sh$'' ''^tests/functional/fetchGitVerification\.sh$'' ''^tests/functional/fetchMercurial\.sh$'' From 53ad2433b4263bd844c43d67fc3681983acc6fd6 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:09:36 -0700 Subject: [PATCH 13/24] shellcheck fix: tests/functional/fetchGitSubmodules.sh --- maintainers/flake-module.nix | 1 - tests/functional/fetchGitSubmodules.sh | 153 +++++++++++++------------ 2 files changed, 77 insertions(+), 77 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index b2acc36d9..ba38633bc 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/fetchGitSubmodules\.sh$'' ''^tests/functional/fetchGitVerification\.sh$'' ''^tests/functional/fetchMercurial\.sh$'' ''^tests/functional/fixed\.builder1\.sh$'' diff --git a/tests/functional/fetchGitSubmodules.sh b/tests/functional/fetchGitSubmodules.sh index cd3b51674..2a25245be 100755 --- a/tests/functional/fetchGitSubmodules.sh +++ b/tests/functional/fetchGitSubmodules.sh @@ -11,7 +11,7 @@ clearStoreIfPossible rootRepo=$TEST_ROOT/gitSubmodulesRoot subRepo=$TEST_ROOT/gitSubmodulesSub -rm -rf ${rootRepo} ${subRepo} $TEST_HOME/.cache/nix +rm -rf "${rootRepo}" "${subRepo}" "$TEST_HOME"/.cache/nix # Submodules can't be fetched locally by default, which can cause # information leakage vulnerabilities, but for these tests our @@ -23,35 +23,35 @@ export XDG_CONFIG_HOME=$TEST_HOME/.config git config --global protocol.file.allow always initGitRepo() { - git init $1 - git -C $1 config user.email "foobar@example.com" - git -C $1 config user.name "Foobar" + git init "$1" + git -C "$1" config user.email "foobar@example.com" + git -C "$1" config user.name "Foobar" } addGitContent() { - echo "lorem ipsum" > $1/content - git -C $1 add content - git -C $1 commit -m "Initial commit" + echo "lorem ipsum" > "$1"/content + git -C "$1" add content + git -C "$1" commit -m "Initial commit" } -initGitRepo $subRepo -addGitContent $subRepo +initGitRepo "$subRepo" +addGitContent "$subRepo" -initGitRepo $rootRepo +initGitRepo "$rootRepo" -git -C $rootRepo submodule init -git -C $rootRepo submodule add $subRepo sub -git -C $rootRepo add sub -git -C $rootRepo commit -m "Add submodule" +git -C "$rootRepo" submodule init +git -C "$rootRepo" submodule add "$subRepo" sub +git -C "$rootRepo" add sub +git -C "$rootRepo" commit -m "Add submodule" -rev=$(git -C $rootRepo rev-parse HEAD) +rev=$(git -C "$rootRepo" rev-parse HEAD) r1=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; }).outPath") r2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = false; }).outPath") r3=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }).outPath") -[[ $r1 == $r2 ]] -[[ $r2 != $r3 ]] +[[ $r1 == "$r2" ]] +[[ $r2 != "$r3" ]] r4=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; }).outPath") r5=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = false; }).outPath") @@ -59,11 +59,11 @@ r6=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \ r7=$(nix eval --raw --expr "(builtins.fetchGit { url = $rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = true; }).outPath") r8=$(nix eval --raw --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; submodules = true; }).outPath") -[[ $r1 == $r4 ]] -[[ $r4 == $r5 ]] -[[ $r3 == $r6 ]] -[[ $r6 == $r7 ]] -[[ $r7 == $r8 ]] +[[ $r1 == "$r4" ]] +[[ $r4 == "$r5" ]] +[[ $r3 == "$r6" ]] +[[ $r6 == "$r7" ]] +[[ $r7 == "$r8" ]] have_submodules=$(nix eval --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; }).submodules") [[ $have_submodules == false ]] @@ -80,13 +80,13 @@ pathWithSubmodulesAgain=$(nix eval --raw --expr "(builtins.fetchGit { url = file pathWithSubmodulesAgainWithRef=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = true; }).outPath") # The resulting store path cannot be the same. -[[ $pathWithoutSubmodules != $pathWithSubmodules ]] +[[ $pathWithoutSubmodules != "$pathWithSubmodules" ]] # Checking out the same repo with submodules returns in the same store path. -[[ $pathWithSubmodules == $pathWithSubmodulesAgain ]] +[[ $pathWithSubmodules == "$pathWithSubmodulesAgain" ]] # Checking out the same repo with submodules returns in the same store path. -[[ $pathWithSubmodulesAgain == $pathWithSubmodulesAgainWithRef ]] +[[ $pathWithSubmodulesAgain == "$pathWithSubmodulesAgainWithRef" ]] # The submodules flag is actually honored. [[ ! -e $pathWithoutSubmodules/sub/content ]] @@ -98,14 +98,14 @@ pathWithSubmodulesAgainWithRef=$(nix eval --raw --expr "(builtins.fetchGit { url test "$(find "$pathWithSubmodules" -name .git)" = "" # Git repos without submodules can be fetched with submodules = true. -subRev=$(git -C $subRepo rev-parse HEAD) +subRev=$(git -C "$subRepo" rev-parse HEAD) noSubmoduleRepoBaseline=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subRepo; rev = \"$subRev\"; }).outPath") noSubmoduleRepo=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subRepo; rev = \"$subRev\"; submodules = true; }).outPath") -[[ $noSubmoduleRepoBaseline == $noSubmoduleRepo ]] +[[ $noSubmoduleRepoBaseline == "$noSubmoduleRepo" ]] # Test .gitmodules with entries that refer to non-existent objects or objects that are not submodules. -cat >> $rootRepo/.gitmodules <> "$rootRepo"/.gitmodules <> $rootRepo/.gitmodules < $rootRepo/file -git -C $rootRepo add file -git -C $rootRepo commit -a -m "Add bad submodules" +echo foo > "$rootRepo"/file +git -C "$rootRepo" add file +git -C "$rootRepo" commit -a -m "Add bad submodules" -rev=$(git -C $rootRepo rev-parse HEAD) +rev=$(git -C "$rootRepo" rev-parse HEAD) r=$(nix eval --raw --expr "builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }") @@ -126,44 +126,44 @@ r=$(nix eval --raw --expr "builtins.fetchGit { url = file://$rootRepo; rev = \"$ [[ ! -e $r/missing ]] # Test relative submodule URLs. -rm $TEST_HOME/.cache/nix/fetcher-cache* -rm -rf $rootRepo/.git $rootRepo/.gitmodules $rootRepo/sub -initGitRepo $rootRepo -git -C $rootRepo submodule add ../gitSubmodulesSub sub -git -C $rootRepo commit -m "Add submodule" -rev2=$(git -C $rootRepo rev-parse HEAD) +rm "$TEST_HOME"/.cache/nix/fetcher-cache* +rm -rf "$rootRepo"/.git "$rootRepo"/.gitmodules "$rootRepo"/sub +initGitRepo "$rootRepo" +git -C "$rootRepo" submodule add ../gitSubmodulesSub sub +git -C "$rootRepo" commit -m "Add submodule" +rev2=$(git -C "$rootRepo" rev-parse HEAD) pathWithRelative=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev2\"; submodules = true; }).outPath") -diff -r -x .gitmodules $pathWithSubmodules $pathWithRelative +diff -r -x .gitmodules "$pathWithSubmodules" "$pathWithRelative" # Test clones that have an upstream with relative submodule URLs. -rm $TEST_HOME/.cache/nix/fetcher-cache* +rm "$TEST_HOME"/.cache/nix/fetcher-cache* cloneRepo=$TEST_ROOT/a/b/gitSubmodulesClone # NB /a/b to make the relative path not work relative to $cloneRepo -git clone $rootRepo $cloneRepo +git clone "$rootRepo" "$cloneRepo" pathIndirect=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$cloneRepo; rev = \"$rev2\"; submodules = true; }).outPath") -[[ $pathIndirect = $pathWithRelative ]] +[[ $pathIndirect = "$pathWithRelative" ]] # Test submodule export-ignore interaction -git -C $rootRepo/sub config user.email "foobar@example.com" -git -C $rootRepo/sub config user.name "Foobar" +git -C "$rootRepo"/sub config user.email "foobar@example.com" +git -C "$rootRepo"/sub config user.name "Foobar" -echo "/exclude-from-root export-ignore" >> $rootRepo/.gitattributes +echo "/exclude-from-root export-ignore" >> "$rootRepo"/.gitattributes # TBD possible semantics for submodules + exportIgnore # echo "/sub/exclude-deep export-ignore" >> $rootRepo/.gitattributes -echo nope > $rootRepo/exclude-from-root -git -C $rootRepo add .gitattributes exclude-from-root -git -C $rootRepo commit -m "Add export-ignore" +echo nope > "$rootRepo"/exclude-from-root +git -C "$rootRepo" add .gitattributes exclude-from-root +git -C "$rootRepo" commit -m "Add export-ignore" -echo "/exclude-from-sub export-ignore" >> $rootRepo/sub/.gitattributes -echo nope > $rootRepo/sub/exclude-from-sub +echo "/exclude-from-sub export-ignore" >> "$rootRepo"/sub/.gitattributes +echo nope > "$rootRepo"/sub/exclude-from-sub # TBD possible semantics for submodules + exportIgnore # echo aye > $rootRepo/sub/exclude-from-root -git -C $rootRepo/sub add .gitattributes exclude-from-sub -git -C $rootRepo/sub commit -m "Add export-ignore (sub)" +git -C "$rootRepo"/sub add .gitattributes exclude-from-sub +git -C "$rootRepo"/sub commit -m "Add export-ignore (sub)" -git -C $rootRepo add sub -git -C $rootRepo commit -m "Update submodule" +git -C "$rootRepo" add sub +git -C "$rootRepo" commit -m "Update submodule" -git -C $rootRepo status +git -C "$rootRepo" status # # TBD: not supported yet, because semantics are undecided and current implementation leaks rules from the root to submodules # # exportIgnore can be used with submodules @@ -199,39 +199,40 @@ test_submodule_nested() { local repoB=$TEST_ROOT/submodule_nested/b local repoC=$TEST_ROOT/submodule_nested/c - rm -rf $repoA $repoB $repoC $TEST_HOME/.cache/nix + rm -rf "$repoA" "$repoB" "$repoC" "$TEST_HOME"/.cache/nix - initGitRepo $repoC - touch $repoC/inside-c - git -C $repoC add inside-c - addGitContent $repoC + initGitRepo "$repoC" + touch "$repoC"/inside-c + git -C "$repoC" add inside-c + addGitContent "$repoC" - initGitRepo $repoB - git -C $repoB submodule add $repoC c - git -C $repoB add c - addGitContent $repoB + initGitRepo "$repoB" + git -C "$repoB" submodule add "$repoC" c + git -C "$repoB" add c + addGitContent "$repoB" - initGitRepo $repoA - git -C $repoA submodule add $repoB b - git -C $repoA add b - addGitContent $repoA + initGitRepo "$repoA" + git -C "$repoA" submodule add "$repoB" b + git -C "$repoA" add b + addGitContent "$repoA" # Check non-worktree fetch - local rev=$(git -C $repoA rev-parse HEAD) + local rev + rev=$(git -C "$repoA" rev-parse HEAD) out=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repoA\"; rev = \"$rev\"; submodules = true; }).outPath") - test -e $out/b/c/inside-c - test -e $out/content - test -e $out/b/content - test -e $out/b/c/content + test -e "$out"/b/c/inside-c + test -e "$out"/content + test -e "$out"/b/content + test -e "$out"/b/c/content local nonWorktree=$out # Check worktree based fetch # TODO: make it work without git submodule update - git -C $repoA submodule update --init --recursive + git -C "$repoA" submodule update --init --recursive out=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repoA\"; submodules = true; }).outPath") - find $out - [[ $out == $nonWorktree ]] || { find $out; false; } + find "$out" + [[ $out == "$nonWorktree" ]] || { find "$out"; false; } } test_submodule_nested From 7b3c193bd3586eb8ab9f8d7a26c1fdc5e213f514 Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Mon, 22 Sep 2025 14:38:55 -0400 Subject: [PATCH 14/24] libexpr: move eval memory allocation to own struct Co-authored-by: eldritch horrors Co-authored-by: Sergei Zimmerman See original commit on lix: https://git.lix.systems/lix-project/lix/commit/f5754dc90ae9b1207656d0e29ad2704d3ef1e554 --- src/libcmd/repl.cc | 2 +- src/libexpr-c/nix_api_value.cc | 2 +- src/libexpr/attr-set.cc | 10 +-- src/libexpr/eval.cc | 52 ++++++----- src/libexpr/include/nix/expr/attr-set.hh | 14 +-- src/libexpr/include/nix/expr/eval-inline.hh | 10 +-- src/libexpr/include/nix/expr/eval.hh | 98 +++++++++++++++------ src/libexpr/include/nix/expr/value.hh | 2 +- src/libexpr/primops.cc | 4 +- 9 files changed, 122 insertions(+), 72 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 01d786deb..5c6dd7ffb 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -760,7 +760,7 @@ void NixRepl::loadFlake(const std::string & flakeRefS) void NixRepl::initEnv() { - env = &state->allocEnv(envSize); + env = &state->mem.allocEnv(envSize); env->up = &state->baseEnv; displ = 0; staticEnv->vars.clear(); diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index c58d4fe89..3b8c7dd04 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -679,7 +679,7 @@ nix_err nix_bindings_builder_insert(nix_c_context * context, BindingsBuilder * b context->last_err_code = NIX_OK; try { auto & v = check_value_not_null(value); - nix::Symbol s = bb->builder.state.get().symbols.create(name); + nix::Symbol s = bb->builder.symbols.get().create(name); bb->builder.insert(s, &v); } NIXC_CATCH_ERRS diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc index a1b646120..92b67f6ad 100644 --- a/src/libexpr/attr-set.cc +++ b/src/libexpr/attr-set.cc @@ -10,27 +10,27 @@ Bindings Bindings::emptyBindings; /* Allocate a new array of attributes for an attribute set with a specific capacity. The space is implicitly reserved after the Bindings structure. */ -Bindings * EvalState::allocBindings(size_t capacity) +Bindings * EvalMemory::allocBindings(size_t capacity) { if (capacity == 0) return &Bindings::emptyBindings; if (capacity > std::numeric_limits::max()) throw Error("attribute set of size %d is too big", capacity); - nrAttrsets++; - nrAttrsInAttrsets += capacity; + stats.nrAttrsets++; + stats.nrAttrsInAttrsets += capacity; return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings(); } Value & BindingsBuilder::alloc(Symbol name, PosIdx pos) { - auto value = state.get().allocValue(); + auto value = mem.get().allocValue(); bindings->push_back(Attr(name, value, pos)); return *value; } Value & BindingsBuilder::alloc(std::string_view name, PosIdx pos) { - return alloc(state.get().symbols.create(name), pos); + return alloc(symbols.get().create(name), pos); } void Bindings::sort() diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f82fd93b5..6cf902e35 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -194,6 +194,15 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env) static constexpr size_t BASE_ENV_SIZE = 128; +EvalMemory::EvalMemory() +#if NIX_USE_BOEHMGC + : valueAllocCache(std::allocate_shared(traceable_allocator(), nullptr)) + , env1AllocCache(std::allocate_shared(traceable_allocator(), nullptr)) +#endif +{ + assertGCInitialized(); +} + EvalState::EvalState( const LookupPath & lookupPathFromArguments, ref store, @@ -274,12 +283,10 @@ EvalState::EvalState( , fileEvalCache(make_ref()) , regexCache(makeRegexCache()) #if NIX_USE_BOEHMGC - , valueAllocCache(std::allocate_shared(traceable_allocator(), nullptr)) - , env1AllocCache(std::allocate_shared(traceable_allocator(), nullptr)) - , baseEnvP(std::allocate_shared(traceable_allocator(), &allocEnv(BASE_ENV_SIZE))) + , baseEnvP(std::allocate_shared(traceable_allocator(), &mem.allocEnv(BASE_ENV_SIZE))) , baseEnv(**baseEnvP) #else - , baseEnv(allocEnv(BASE_ENV_SIZE)) + , baseEnv(mem.allocEnv(BASE_ENV_SIZE)) #endif , staticBaseEnv{std::make_shared(nullptr, nullptr)} { @@ -288,8 +295,6 @@ EvalState::EvalState( countCalls = getEnv("NIX_COUNT_CALLS").value_or("0") != "0"; - assertGCInitialized(); - static_assert(sizeof(Env) <= 16, "environment must be <= 16 bytes"); static_assert(sizeof(Counter) == 64, "counters must be 64 bytes"); @@ -885,11 +890,10 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) } } -ListBuilder::ListBuilder(EvalState & state, size_t size) +ListBuilder::ListBuilder(size_t size) : size(size) , elems(size <= 2 ? inlineElems : (Value **) allocBytes(size * sizeof(Value *))) { - state.nrListElems += size; } Value * EvalState::getBool(bool b) @@ -1183,7 +1187,7 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v) Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up) { - Env & inheritEnv = state.allocEnv(inheritFromExprs->size()); + Env & inheritEnv = state.mem.allocEnv(inheritFromExprs->size()); inheritEnv.up = &up; Displacement displ = 0; @@ -1202,7 +1206,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) if (recursive) { /* Create a new environment that contains the attributes in this `rec'. */ - Env & env2(state.allocEnv(attrs.size())); + Env & env2(state.mem.allocEnv(attrs.size())); env2.up = &env; dynamicEnv = &env2; Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr; @@ -1294,7 +1298,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v) { /* Create a new environment that contains the attributes in this `let'. */ - Env & env2(state.allocEnv(attrs->attrs.size())); + Env & env2(state.mem.allocEnv(attrs->attrs.size())); env2.up = &env; Env * inheritEnv = attrs->inheritFromExprs ? attrs->buildInheritFromEnv(state, env2) : nullptr; @@ -1500,7 +1504,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, ExprLambda & lambda(*vCur.lambda().fun); auto size = (!lambda.arg ? 0 : 1) + (lambda.hasFormals() ? lambda.formals->formals.size() : 0); - Env & env2(allocEnv(size)); + Env & env2(mem.allocEnv(size)); env2.up = vCur.lambda().env; Displacement displ = 0; @@ -1789,7 +1793,7 @@ https://nix.dev/manual/nix/stable/language/syntax.html#functions.)", void ExprWith::eval(EvalState & state, Env & env, Value & v) { - Env & env2(state.allocEnv(1)); + Env & env2(state.mem.allocEnv(1)); env2.up = &env; env2.values[0] = attrs->maybeThunk(state, env); @@ -2916,10 +2920,12 @@ void EvalState::printStatistics() std::chrono::microseconds cpuTimeDuration = getCpuUserTime(); float cpuTime = std::chrono::duration_cast>(cpuTimeDuration).count(); - uint64_t bEnvs = nrEnvs * sizeof(Env) + nrValuesInEnvs * sizeof(Value *); - uint64_t bLists = nrListElems * sizeof(Value *); - uint64_t bValues = nrValues * sizeof(Value); - uint64_t bAttrsets = nrAttrsets * sizeof(Bindings) + nrAttrsInAttrsets * sizeof(Attr); + auto & memstats = mem.getStats(); + + uint64_t bEnvs = memstats.nrEnvs * sizeof(Env) + memstats.nrValuesInEnvs * sizeof(Value *); + uint64_t bLists = memstats.nrListElems * sizeof(Value *); + uint64_t bValues = memstats.nrValues * sizeof(Value); + uint64_t bAttrsets = memstats.nrAttrsets * sizeof(Bindings) + memstats.nrAttrsInAttrsets * sizeof(Attr); #if NIX_USE_BOEHMGC GC_word heapSize, totalBytes; @@ -2945,18 +2951,18 @@ void EvalState::printStatistics() #endif }; topObj["envs"] = { - {"number", nrEnvs.load()}, - {"elements", nrValuesInEnvs.load()}, + {"number", memstats.nrEnvs.load()}, + {"elements", memstats.nrValuesInEnvs.load()}, {"bytes", bEnvs}, }; topObj["nrExprs"] = Expr::nrExprs.load(); topObj["list"] = { - {"elements", nrListElems.load()}, + {"elements", memstats.nrListElems.load()}, {"bytes", bLists}, {"concats", nrListConcats.load()}, }; topObj["values"] = { - {"number", nrValues.load()}, + {"number", memstats.nrValues.load()}, {"bytes", bValues}, }; topObj["symbols"] = { @@ -2964,9 +2970,9 @@ void EvalState::printStatistics() {"bytes", symbols.totalSize()}, }; topObj["sets"] = { - {"number", nrAttrsets.load()}, + {"number", memstats.nrAttrsets.load()}, {"bytes", bAttrsets}, - {"elements", nrAttrsInAttrsets.load()}, + {"elements", memstats.nrAttrsInAttrsets.load()}, }; topObj["sizes"] = { {"Env", sizeof(Env)}, diff --git a/src/libexpr/include/nix/expr/attr-set.hh b/src/libexpr/include/nix/expr/attr-set.hh index 52ce958ce..46eecd9bd 100644 --- a/src/libexpr/include/nix/expr/attr-set.hh +++ b/src/libexpr/include/nix/expr/attr-set.hh @@ -13,7 +13,7 @@ namespace nix { -class EvalState; +class EvalMemory; struct Value; /** @@ -426,7 +426,7 @@ public: return res; } - friend class EvalState; + friend class EvalMemory; }; static_assert(std::forward_iterator); @@ -448,12 +448,13 @@ private: Bindings * bindings; Bindings::size_type capacity_; - friend class EvalState; + friend class EvalMemory; - BindingsBuilder(EvalState & state, Bindings * bindings, size_type capacity) + BindingsBuilder(EvalMemory & mem, SymbolTable & symbols, Bindings * bindings, size_type capacity) : bindings(bindings) , capacity_(capacity) - , state(state) + , mem(mem) + , symbols(symbols) { } @@ -471,7 +472,8 @@ private: } public: - std::reference_wrapper state; + std::reference_wrapper mem; + std::reference_wrapper symbols; void insert(Symbol name, Value * value, PosIdx pos = noPos) { diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index 749e51537..1320da914 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/src/libexpr/include/nix/expr/eval-inline.hh @@ -26,7 +26,7 @@ inline void * allocBytes(size_t n) } [[gnu::always_inline]] -Value * EvalState::allocValue() +Value * EvalMemory::allocValue() { #if NIX_USE_BOEHMGC /* We use the boehm batch allocator to speed up allocations of Values (of which there are many). @@ -48,15 +48,15 @@ Value * EvalState::allocValue() void * p = allocBytes(sizeof(Value)); #endif - nrValues++; + stats.nrValues++; return (Value *) p; } [[gnu::always_inline]] -Env & EvalState::allocEnv(size_t size) +Env & EvalMemory::allocEnv(size_t size) { - nrEnvs++; - nrValuesInEnvs += size; + stats.nrEnvs++; + stats.nrValuesInEnvs += size; Env * env; diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index e5b87cc97..f61dab3a8 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -302,6 +302,63 @@ struct StaticEvalSymbols } }; +class EvalMemory +{ +#if NIX_USE_BOEHMGC + /** + * Allocation cache for GC'd Value objects. + */ + std::shared_ptr valueAllocCache; + + /** + * Allocation cache for size-1 Env objects. + */ + std::shared_ptr env1AllocCache; +#endif + +public: + struct Statistics + { + Counter nrEnvs; + Counter nrValuesInEnvs; + Counter nrValues; + Counter nrAttrsets; + Counter nrAttrsInAttrsets; + Counter nrListElems; + }; + + EvalMemory(); + + EvalMemory(const EvalMemory &) = delete; + EvalMemory(EvalMemory &&) = delete; + EvalMemory & operator=(const EvalMemory &) = delete; + EvalMemory & operator=(EvalMemory &&) = delete; + + inline Value * allocValue(); + inline Env & allocEnv(size_t size); + + Bindings * allocBindings(size_t capacity); + + BindingsBuilder buildBindings(SymbolTable & symbols, size_t capacity) + { + return BindingsBuilder(*this, symbols, allocBindings(capacity), capacity); + } + + ListBuilder buildList(size_t size) + { + stats.nrListElems += size; + return ListBuilder(size); + } + + const Statistics & getStats() const & + { + return stats; + } + +private: + Statistics stats; +}; + class EvalState : public std::enable_shared_from_this { public: @@ -312,6 +369,8 @@ public: SymbolTable symbols; PosTable positions; + EvalMemory mem; + /** * If set, force copying files to the Nix store even if they * already exist there. @@ -441,18 +500,6 @@ private: */ std::shared_ptr regexCache; -#if NIX_USE_BOEHMGC - /** - * Allocation cache for GC'd Value objects. - */ - std::shared_ptr valueAllocCache; - - /** - * Allocation cache for size-1 Env objects. - */ - std::shared_ptr env1AllocCache; -#endif - public: EvalState( @@ -463,6 +510,15 @@ public: std::shared_ptr buildStore = nullptr); ~EvalState(); + /** + * A wrapper around EvalMemory::allocValue() to avoid code churn when it + * was introduced. + */ + inline Value * allocValue() + { + return mem.allocValue(); + } + LookupPath getLookupPath() { return lookupPath; @@ -834,22 +890,14 @@ public: */ void autoCallFunction(const Bindings & args, Value & fun, Value & res); - /** - * Allocation primitives. - */ - inline Value * allocValue(); - inline Env & allocEnv(size_t size); - - Bindings * allocBindings(size_t capacity); - BindingsBuilder buildBindings(size_t capacity) { - return BindingsBuilder(*this, allocBindings(capacity), capacity); + return mem.buildBindings(symbols, capacity); } ListBuilder buildList(size_t size) { - return ListBuilder(*this, size); + return mem.buildList(size); } /** @@ -966,13 +1014,7 @@ private: */ std::string mkSingleDerivedPathStringRaw(const SingleDerivedPath & p); - Counter nrEnvs; - Counter nrValuesInEnvs; - Counter nrValues; - Counter nrListElems; Counter nrLookups; - Counter nrAttrsets; - Counter nrAttrsInAttrsets; Counter nrAvoided; Counter nrOpUpdates; Counter nrOpUpdateValuesCopied; diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index e526fcde0..22d85dc99 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -155,7 +155,7 @@ class ListBuilder Value * inlineElems[2] = {nullptr, nullptr}; public: Value ** elems; - ListBuilder(EvalState & state, size_t size); + ListBuilder(size_t size); // NOTE: Can be noexcept because we are just copying integral values and // raw pointers. diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index a046a2c28..a8ac8d159 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -262,7 +262,7 @@ static void scopedImport(EvalState & state, const PosIdx pos, SourcePath & path, { state.forceAttrs(*vScope, pos, "while evaluating the first argument passed to builtins.scopedImport"); - Env * env = &state.allocEnv(vScope->attrs()->size()); + Env * env = &state.mem.allocEnv(vScope->attrs()->size()); env->up = &state.baseEnv; auto staticEnv = std::make_shared(nullptr, state.staticBaseEnv, vScope->attrs()->size()); @@ -3161,7 +3161,7 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value ** args, // Step 1. Sort the name-value attrsets in place using the memory we allocate for the result auto listView = args[0]->listView(); size_t listSize = listView.size(); - auto & bindings = *state.allocBindings(listSize); + auto & bindings = *state.mem.allocBindings(listSize); using ElemPtr = decltype(&bindings[0].value); for (const auto & [n, v2] : enumerate(listView)) { From ff82de86da4308b3a79b1c1d1bcb5f33edef066d Mon Sep 17 00:00:00 2001 From: Seth Flynn Date: Fri, 26 Sep 2025 02:03:50 -0400 Subject: [PATCH 15/24] nix-cli: use pure/restricted eval for help pages This avoids any complications that can arise from the environment affecting evaluation of the help pages (which don't need to be calling out to anything external anyways) A recent example of one of these problems is https://github.com/NixOS/nix/issues/14085, which would break help pages by causing them to make invalid calls to the dummy store they're evaluated with Fixes: https://github.com/NixOS/nix/issues/14062 Co-authored-by: Sergei Zimmerman --- src/nix/main.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/main.cc b/src/nix/main.cc index a6077f5e9..ed889a189 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -226,8 +226,8 @@ static void showHelp(std::vector subcommand, NixArgs & toplevel) auto mdName = subcommand.empty() ? "nix" : fmt("nix3-%s", concatStringsSep("-", subcommand)); - evalSettings.restrictEval = false; - evalSettings.pureEval = false; + evalSettings.restrictEval = true; + evalSettings.pureEval = true; EvalState state({}, openStore("dummy://"), fetchSettings, evalSettings); auto vGenerateManpage = state.allocValue(); From c4c35243180c7ed55c115bb9258aeb412572cb61 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:11:07 -0700 Subject: [PATCH 16/24] shellcheck fix: tests/functional/fetchGitVerification.sh --- maintainers/flake-module.nix | 1 - tests/functional/fetchGitVerification.sh | 25 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index ba38633bc..d303cb213 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/fetchGitVerification\.sh$'' ''^tests/functional/fetchMercurial\.sh$'' ''^tests/functional/fixed\.builder1\.sh$'' ''^tests/functional/fixed\.builder2\.sh$'' diff --git a/tests/functional/fetchGitVerification.sh b/tests/functional/fetchGitVerification.sh index 4012d8229..79c78d0c9 100755 --- a/tests/functional/fetchGitVerification.sh +++ b/tests/functional/fetchGitVerification.sh @@ -21,29 +21,29 @@ ssh-keygen -f "$keysDir/testkey2" -t rsa -P "" -C "test key 2" key2File="$keysDir/testkey2.pub" publicKey2=$(awk '{print $2}' "$key2File") -git init $repo -git -C $repo config user.email "foobar@example.com" -git -C $repo config user.name "Foobar" -git -C $repo config gpg.format ssh +git init "$repo" +git -C "$repo" config user.email "foobar@example.com" +git -C "$repo" config user.name "Foobar" +git -C "$repo" config gpg.format ssh -echo 'hello' > $repo/text -git -C $repo add text -git -C $repo -c "user.signingkey=$key1File" commit -S -m 'initial commit' +echo 'hello' > "$repo"/text +git -C "$repo" add text +git -C "$repo" -c "user.signingkey=$key1File" commit -S -m 'initial commit' out=$(nix eval --impure --raw --expr "builtins.fetchGit { url = \"file://$repo\"; keytype = \"ssh-rsa\"; publicKey = \"$publicKey2\"; }" 2>&1) || status=$? [[ $status == 1 ]] -[[ $out =~ 'No principal matched.' ]] +[[ $out == *'No principal matched.'* ]] [[ $(nix eval --impure --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; publicKey = \"$publicKey1\"; } + \"/text\")") = 'hello' ]] -echo 'hello world' > $repo/text +echo 'hello world' > "$repo"/text # Verification on a dirty repo should fail. out=$(nix eval --impure --raw --expr "builtins.fetchGit { url = \"file://$repo\"; keytype = \"ssh-rsa\"; publicKey = \"$publicKey2\"; }" 2>&1) || status=$? [[ $status == 1 ]] [[ $out =~ 'dirty' ]] -git -C $repo add text -git -C $repo -c "user.signingkey=$key2File" commit -S -m 'second commit' +git -C "$repo" add text +git -C "$repo" -c "user.signingkey=$key2File" commit -S -m 'second commit' [[ $(nix eval --impure --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; publicKeys = [{key = \"$publicKey1\";} {type = \"ssh-rsa\"; key = \"$publicKey2\";}]; } + \"/text\")") = 'hello world' ]] @@ -80,5 +80,6 @@ cat > "$flakeDir/flake.nix" <&1) || status=$? + [[ $status == 1 ]] -[[ $out =~ 'No principal matched.' ]] +[[ $out == *'No principal matched.'* ]] From 4cec876319d84e6033a184ef6fb81d9d6fac265e Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:13:38 -0700 Subject: [PATCH 17/24] shellcheck fix: tests/functional/fetchMercurial.sh --- maintainers/flake-module.nix | 1 - tests/functional/fetchMercurial.sh | 85 +++++++++++++++--------------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index d303cb213..3470f853f 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/fetchMercurial\.sh$'' ''^tests/functional/fixed\.builder1\.sh$'' ''^tests/functional/fixed\.builder2\.sh$'' ''^tests/functional/fixed\.sh$'' diff --git a/tests/functional/fetchMercurial.sh b/tests/functional/fetchMercurial.sh index 6de192865..6293fb76a 100755 --- a/tests/functional/fetchMercurial.sh +++ b/tests/functional/fetchMercurial.sh @@ -12,34 +12,35 @@ clearStore # See https://github.com/NixOS/nix/issues/6195 repo=$TEST_ROOT/./hg -rm -rf $repo ${repo}-tmp $TEST_HOME/.cache/nix +rm -rf "$repo" "${repo}"-tmp "$TEST_HOME"/.cache/nix -hg init $repo -echo '[ui]' >> $repo/.hg/hgrc -echo 'username = Foobar ' >> $repo/.hg/hgrc +hg init "$repo" +{ + echo '[ui]' + echo 'username = Foobar ' + # Set ui.tweakdefaults to ensure HGPLAIN is being set. + echo 'tweakdefaults = True' +} >> "$repo"/.hg/hgrc -# Set ui.tweakdefaults to ensure HGPLAIN is being set. -echo 'tweakdefaults = True' >> $repo/.hg/hgrc +echo utrecht > "$repo"/hello +touch "$repo"/.hgignore +hg add --cwd "$repo" hello .hgignore +hg commit --cwd "$repo" -m 'Bla1' +rev1=$(hg log --cwd "$repo" -r tip --template '{node}') -echo utrecht > $repo/hello -touch $repo/.hgignore -hg add --cwd $repo hello .hgignore -hg commit --cwd $repo -m 'Bla1' -rev1=$(hg log --cwd $repo -r tip --template '{node}') - -echo world > $repo/hello -hg commit --cwd $repo -m 'Bla2' -rev2=$(hg log --cwd $repo -r tip --template '{node}') +echo world > "$repo"/hello +hg commit --cwd "$repo" -m 'Bla2' +rev2=$(hg log --cwd "$repo" -r tip --template '{node}') # Fetch an unclean branch. -echo unclean > $repo/hello +echo unclean > "$repo"/hello path=$(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).outPath") -[[ $(cat $path/hello) = unclean ]] -hg revert --cwd $repo --all +[[ $(cat "$path"/hello) = unclean ]] +hg revert --cwd "$repo" --all # Fetch the default branch. path=$(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).outPath") -[[ $(cat $path/hello) = world ]] +[[ $(cat "$path"/hello) = world ]] # In pure eval mode, fetchGit without a revision should fail. [[ $(nix eval --impure --raw --expr "(builtins.readFile (fetchMercurial file://$repo + \"/hello\"))") = world ]] @@ -47,64 +48,64 @@ path=$(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).ou # Fetch using an explicit revision hash. path2=$(nix eval --impure --raw --expr "(builtins.fetchMercurial { url = file://$repo; rev = \"$rev2\"; }).outPath") -[[ $path = $path2 ]] +[[ $path = "$path2" ]] # In pure eval mode, fetchGit with a revision should succeed. [[ $(nix eval --raw --expr "builtins.readFile (fetchMercurial { url = file://$repo; rev = \"$rev2\"; } + \"/hello\")") = world ]] # Fetch again. This should be cached. -mv $repo ${repo}-tmp +mv "$repo" "${repo}"-tmp path2=$(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).outPath") -[[ $path = $path2 ]] +[[ $path = "$path2" ]] [[ $(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).branch") = default ]] [[ $(nix eval --impure --expr "(builtins.fetchMercurial file://$repo).revCount") = 1 ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).rev") = $rev2 ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).rev") = "$rev2" ]] # But with TTL 0, it should fail. (! nix eval --impure --refresh --expr "builtins.fetchMercurial file://$repo") # Fetching with a explicit hash should succeed. path2=$(nix eval --refresh --raw --expr "(builtins.fetchMercurial { url = file://$repo; rev = \"$rev2\"; }).outPath") -[[ $path = $path2 ]] +[[ $path = "$path2" ]] path2=$(nix eval --refresh --raw --expr "(builtins.fetchMercurial { url = file://$repo; rev = \"$rev1\"; }).outPath") -[[ $(cat $path2/hello) = utrecht ]] +[[ $(cat "$path2"/hello) = utrecht ]] -mv ${repo}-tmp $repo +mv "${repo}"-tmp "$repo" # Using a clean working tree should produce the same result. path2=$(nix eval --impure --raw --expr "(builtins.fetchMercurial $repo).outPath") -[[ $path = $path2 ]] +[[ $path = "$path2" ]] # Using an unclean tree should yield the tracked but uncommitted changes. -mkdir $repo/dir1 $repo/dir2 -echo foo > $repo/dir1/foo -echo bar > $repo/bar -echo bar > $repo/dir2/bar -hg add --cwd $repo dir1/foo -hg rm --cwd $repo hello +mkdir "$repo"/dir1 "$repo"/dir2 +echo foo > "$repo"/dir1/foo +echo bar > "$repo"/bar +echo bar > "$repo"/dir2/bar +hg add --cwd "$repo" dir1/foo +hg rm --cwd "$repo" hello path2=$(nix eval --impure --raw --expr "(builtins.fetchMercurial $repo).outPath") -[ ! -e $path2/hello ] -[ ! -e $path2/bar ] -[ ! -e $path2/dir2/bar ] -[ ! -e $path2/.hg ] -[[ $(cat $path2/dir1/foo) = foo ]] +[ ! -e "$path2"/hello ] +[ ! -e "$path2"/bar ] +[ ! -e "$path2"/dir2/bar ] +[ ! -e "$path2"/.hg ] +[[ $(cat "$path2"/dir1/foo) = foo ]] [[ $(nix eval --impure --raw --expr "(builtins.fetchMercurial $repo).rev") = 0000000000000000000000000000000000000000 ]] # ... unless we're using an explicit ref. path3=$(nix eval --impure --raw --expr "(builtins.fetchMercurial { url = $repo; rev = \"default\"; }).outPath") -[[ $path = $path3 ]] +[[ $path = "$path3" ]] # Committing should not affect the store path. -hg commit --cwd $repo -m 'Bla3' +hg commit --cwd "$repo" -m 'Bla3' path4=$(nix eval --impure --refresh --raw --expr "(builtins.fetchMercurial file://$repo).outPath") -[[ $path2 = $path4 ]] +[[ $path2 = "$path4" ]] -echo paris > $repo/hello +echo paris > "$repo"/hello # Passing a `name` argument should be reflected in the output path path5=$(nix eval -vvvvv --impure --refresh --raw --expr "(builtins.fetchMercurial { url = \"file://$repo\"; name = \"foo\"; } ).outPath") From f8e351cd945f4ae444b57c3917b3648af3b0c709 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:16:47 -0700 Subject: [PATCH 18/24] shellcheck fix: tests/functional/fixed --- maintainers/flake-module.nix | 3 --- tests/functional/fixed.builder1.sh | 4 +++- tests/functional/fixed.builder2.sh | 13 ++++++++----- tests/functional/fixed.sh | 20 ++++++++++---------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 3470f853f..0742d2a6a 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,9 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/fixed\.builder1\.sh$'' - ''^tests/functional/fixed\.builder2\.sh$'' - ''^tests/functional/fixed\.sh$'' ''^tests/functional/flakes/absolute-paths\.sh$'' ''^tests/functional/flakes/check\.sh$'' ''^tests/functional/flakes/config\.sh$'' diff --git a/tests/functional/fixed.builder1.sh b/tests/functional/fixed.builder1.sh index c41bb2b9a..172f65e6b 100644 --- a/tests/functional/fixed.builder1.sh +++ b/tests/functional/fixed.builder1.sh @@ -1,3 +1,5 @@ +# shellcheck shell=bash if test "$IMPURE_VAR1" != "foo"; then exit 1; fi if test "$IMPURE_VAR2" != "bar"; then exit 1; fi -echo "Hello World!" > $out +# shellcheck disable=SC2154 +echo "Hello World!" > "$out" diff --git a/tests/functional/fixed.builder2.sh b/tests/functional/fixed.builder2.sh index 31ea1579a..9fbcf022e 100644 --- a/tests/functional/fixed.builder2.sh +++ b/tests/functional/fixed.builder2.sh @@ -1,6 +1,9 @@ -echo dummy: $dummy +# shellcheck shell=bash +# shellcheck disable=SC2154 +echo dummy: "$dummy" if test -n "$dummy"; then sleep 2; fi -mkdir $out -mkdir $out/bla -echo "Hello World!" > $out/foo -ln -s foo $out/bar +# shellcheck disable=SC2154 +mkdir "$out" +mkdir "$out"/bla +echo "Hello World!" > "$out"/foo +ln -s foo "$out"/bar diff --git a/tests/functional/fixed.sh b/tests/functional/fixed.sh index d98769e64..edf6f88d4 100755 --- a/tests/functional/fixed.sh +++ b/tests/functional/fixed.sh @@ -6,7 +6,7 @@ TODO_NixOS clearStore -path=$(nix-store -q $(nix-instantiate fixed.nix -A good.0)) +path=$(nix-store -q "$(nix-instantiate fixed.nix -A good.0)") echo 'testing bad...' nix-build fixed.nix -A bad --no-out-link && fail "should fail" @@ -14,7 +14,7 @@ nix-build fixed.nix -A bad --no-out-link && fail "should fail" # Building with the bad hash should produce the "good" output path as # a side-effect. [[ -e $path ]] -nix path-info --json $path | grep fixed:md5:2qk15sxzzjlnpjk9brn7j8ppcd +nix path-info --json "$path" | grep fixed:md5:2qk15sxzzjlnpjk9brn7j8ppcd echo 'testing good...' nix-build fixed.nix -A good --no-out-link @@ -37,7 +37,7 @@ fi # While we're at it, check attribute selection a bit more. echo 'testing attribute selection...' -test $(nix-instantiate fixed.nix -A good.1 | wc -l) = 1 +test "$(nix-instantiate fixed.nix -A good.1 | wc -l)" = 1 # Test parallel builds of derivations that produce the same output. # Only one should run at the same time. @@ -51,16 +51,16 @@ echo 'testing sameAsAdd...' out=$(nix-build fixed.nix -A sameAsAdd --no-out-link) # This is what fixed.builder2 produces... -rm -rf $TEST_ROOT/fixed -mkdir $TEST_ROOT/fixed -mkdir $TEST_ROOT/fixed/bla -echo "Hello World!" > $TEST_ROOT/fixed/foo -ln -s foo $TEST_ROOT/fixed/bar +rm -rf "$TEST_ROOT"/fixed +mkdir "$TEST_ROOT"/fixed +mkdir "$TEST_ROOT"/fixed/bla +echo "Hello World!" > "$TEST_ROOT"/fixed/foo +ln -s foo "$TEST_ROOT"/fixed/bar -out2=$(nix-store --add $TEST_ROOT/fixed) +out2=$(nix-store --add "$TEST_ROOT"/fixed) [ "$out" = "$out2" ] -out3=$(nix-store --add-fixed --recursive sha256 $TEST_ROOT/fixed) +out3=$(nix-store --add-fixed --recursive sha256 "$TEST_ROOT"/fixed) [ "$out" = "$out3" ] out4=$(nix-store --print-fixed-path --recursive sha256 "1ixr6yd3297ciyp9im522dfxpqbkhcw0pylkb2aab915278fqaik" fixed) From 8839bab84d97acc8531bf44399b5ca1269dd6f21 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:20:00 -0700 Subject: [PATCH 19/24] shellcheck fix: completion files --- maintainers/flake-module.nix | 4 ---- misc/fish/completion.fish | 1 + misc/zsh/completion.zsh | 1 + 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 0742d2a6a..3de348703 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -197,10 +197,6 @@ ''^tests/functional/user-envs\.sh$'' ''^tests/functional/why-depends\.sh$'' - # Shellcheck doesn't support fish or zsh shell syntax - ''^misc/fish/completion\.fish$'' - ''^misc/zsh/completion\.zsh$'' - # Content-addressed test files that use recursive-*looking* sourcing # (cd .. && source ), causing shellcheck to loop # They're small wrapper scripts with not a lot going on diff --git a/misc/fish/completion.fish b/misc/fish/completion.fish index c6b8ef16a..b6584963b 100644 --- a/misc/fish/completion.fish +++ b/misc/fish/completion.fish @@ -1,3 +1,4 @@ +# shellcheck disable=all function _nix_complete # Get the current command up to a cursor. # - Behaves correctly even with pipes and nested in commands like env. diff --git a/misc/zsh/completion.zsh b/misc/zsh/completion.zsh index f9b3dca74..eb26a16cb 100644 --- a/misc/zsh/completion.zsh +++ b/misc/zsh/completion.zsh @@ -1,3 +1,4 @@ +# shellcheck disable=all #compdef nix function _nix() { From 9bf8e7b73019615ffa14a73086205ab424a328a8 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:20:51 -0700 Subject: [PATCH 20/24] shellcheck fix: tests/functional/flakes/absolute-paths.sh --- maintainers/flake-module.nix | 1 - tests/functional/flakes/absolute-paths.sh | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 3de348703..84f444480 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/flakes/absolute-paths\.sh$'' ''^tests/functional/flakes/check\.sh$'' ''^tests/functional/flakes/config\.sh$'' ''^tests/functional/flakes/flakes\.sh$'' diff --git a/tests/functional/flakes/absolute-paths.sh b/tests/functional/flakes/absolute-paths.sh index a355a7a1c..6565857cb 100755 --- a/tests/functional/flakes/absolute-paths.sh +++ b/tests/functional/flakes/absolute-paths.sh @@ -7,13 +7,13 @@ requireGit flake1Dir=$TEST_ROOT/flake1 flake2Dir=$TEST_ROOT/flake2 -createGitRepo $flake1Dir -cat > $flake1Dir/flake.nix < "$flake1Dir"/flake.nix < Date: Thu, 25 Sep 2025 13:23:02 -0700 Subject: [PATCH 21/24] shellcheck fix: tests/functional/flakes/check.sh --- maintainers/flake-module.nix | 1 - tests/functional/flakes/check.sh | 61 +++++++++++++++++--------------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 84f444480..6f9985e0b 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/flakes/check\.sh$'' ''^tests/functional/flakes/config\.sh$'' ''^tests/functional/flakes/flakes\.sh$'' ''^tests/functional/flakes/follow-paths\.sh$'' diff --git a/tests/functional/flakes/check.sh b/tests/functional/flakes/check.sh index 9a356c2ed..55cd3805f 100755 --- a/tests/functional/flakes/check.sh +++ b/tests/functional/flakes/check.sh @@ -3,9 +3,9 @@ source common.sh flakeDir=$TEST_ROOT/flake3 -mkdir -p $flakeDir +mkdir -p "$flakeDir" -cat > $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix < $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix < $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix <&1 && fail "nix flake check --all-systems should have failed" || true) +# shellcheck disable=SC2015 +checkRes=$(nix flake check "$flakeDir" 2>&1 && fail "nix flake check --all-systems should have failed" || true) echo "$checkRes" | grepQuiet "error: overlay is not a function, but a set instead" -cat > $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix < $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix < $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix < $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix <&1 && fail "nix flake check --all-systems should have failed" || true) +# shellcheck disable=SC2015 +checkRes=$(nix flake check --all-systems --keep-going "$flakeDir" 2>&1 && fail "nix flake check --all-systems should have failed" || true) echo "$checkRes" | grepQuiet "packages.system-1.default" echo "$checkRes" | grepQuiet "packages.system-2.default" -cat > $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix < $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix <&1 && fail "nix flake check --all-systems should have failed" || true) +# shellcheck disable=SC2015 +checkRes=$(nix flake check --all-systems "$flakeDir" 2>&1 && fail "nix flake check --all-systems should have failed" || true) echo "$checkRes" | grepQuiet "unknown-attr" -cat > $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix <&1 && fail "nix flake check --all-systems should have failed" || true) +# shellcheck disable=SC2015 +checkRes=$(nix flake check --all-systems "$flakeDir" 2>&1 && fail "nix flake check --all-systems should have failed" || true) echo "$checkRes" | grepQuiet "formatter.system-1" # Test whether `nix flake check` builds checks. -cat > $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix < "$flakeDir"/flake.nix < $flakeDir/flake.nix < $flakeDir/flake.nix < "$flakeDir"/flake.nix <&1 && fail "nix flake check should have failed" || true) +# shellcheck disable=SC2015 +checkRes=$(nix flake check "$flakeDir" 2>&1 && fail "nix flake check should have failed" || true) echo "$checkRes" | grepQuiet -E "builder( for .*)? failed with exit code 1" From ac5615dd91e042711177f25e6e01d778697b55f1 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:24:30 -0700 Subject: [PATCH 22/24] shellcheck fix: tests/functional/flakes/config.sh --- maintainers/flake-module.nix | 1 - tests/functional/flakes/config.sh | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 6f9985e0b..024565116 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/flakes/config\.sh$'' ''^tests/functional/flakes/flakes\.sh$'' ''^tests/functional/flakes/follow-paths\.sh$'' ''^tests/functional/flakes/prefetch\.sh$'' diff --git a/tests/functional/flakes/config.sh b/tests/functional/flakes/config.sh index ab2d9f47c..87714b5db 100755 --- a/tests/functional/flakes/config.sh +++ b/tests/functional/flakes/config.sh @@ -2,9 +2,9 @@ source common.sh -cp ../simple.nix ../simple.builder.sh "${config_nix}" $TEST_HOME +cp ../simple.nix ../simple.builder.sh "${config_nix}" "$TEST_HOME" -cd $TEST_HOME +cd "$TEST_HOME" rm -f post-hook-ran cat < echoing-post-hook.sh @@ -37,6 +37,7 @@ if type -p script >/dev/null && script -q -c true /dev/null; then else echo "script is not available or not GNU-like, so we skip testing with an added tty" fi +# shellcheck disable=SC2235 (! [[ -f post-hook-ran ]]) TODO_NixOS clearStore From 6fc8f04ecb3241c39a10988d358cdcd226de5ac9 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Thu, 25 Sep 2025 13:26:53 -0700 Subject: [PATCH 23/24] shellcheck fix: tests/functional/flakes/flakes.sh --- maintainers/flake-module.nix | 1 - tests/functional/flakes/flakes.sh | 90 ++++++++++++++++--------------- 2 files changed, 46 insertions(+), 45 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 024565116..50601e06f 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/flakes/flakes\.sh$'' ''^tests/functional/flakes/follow-paths\.sh$'' ''^tests/functional/flakes/prefetch\.sh$'' ''^tests/functional/flakes/run\.sh$'' diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index 97d238654..5b1da0f02 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -7,7 +7,7 @@ TODO_NixOS requireGit clearStore -rm -rf $TEST_HOME/.cache $TEST_HOME/.config +rm -rf "$TEST_HOME"/.cache "$TEST_HOME"/.config createFlake1 createFlake2 @@ -59,7 +59,7 @@ nix flake metadata flake1 nix flake metadata flake1 | grepQuiet 'Locked URL:.*flake1.*' # Test 'nix flake metadata' on a chroot store. -nix flake metadata --store $TEST_ROOT/chroot-store flake1 +nix flake metadata --store "$TEST_ROOT"/chroot-store flake1 # Test 'nix flake metadata' on a local flake. (cd "$flake1Dir" && nix flake metadata) | grepQuiet 'URL:.*flake1.*' @@ -75,17 +75,18 @@ hash1=$(echo "$json" | jq -r .revision) [[ -n $(echo "$json" | jq -r .fingerprint) ]] echo foo > "$flake1Dir/foo" -git -C "$flake1Dir" add $flake1Dir/foo +git -C "$flake1Dir" add "$flake1Dir"/foo [[ $(nix flake metadata flake1 --json --refresh | jq -r .dirtyRevision) == "$hash1-dirty" ]] [[ "$(nix flake metadata flake1 --json | jq -r .fingerprint)" != null ]] echo -n '# foo' >> "$flake1Dir/flake.nix" flake1OriginalCommit=$(git -C "$flake1Dir" rev-parse HEAD) git -C "$flake1Dir" commit -a -m 'Foo' +# shellcheck disable=SC2034 flake1NewCommit=$(git -C "$flake1Dir" rev-parse HEAD) hash2=$(nix flake metadata flake1 --json --refresh | jq -r .revision) [[ $(nix flake metadata flake1 --json --refresh | jq -r .dirtyRevision) == "null" ]] -[[ $hash1 != $hash2 ]] +[[ $hash1 != "$hash2" ]] # Test 'nix build' on a flake. nix build -o "$TEST_ROOT/result" flake1#foo @@ -204,8 +205,8 @@ git -C "$flake3Dir" add flake.nix git -C "$flake3Dir" commit -m 'Update flake.nix' # Check whether `nix build` works with an incomplete lockfile -nix build -o $TEST_ROOT/result "$flake3Dir#sth sth" -nix build -o $TEST_ROOT/result "$flake3Dir#sth%20sth" +nix build -o "$TEST_ROOT"/result "$flake3Dir#sth sth" +nix build -o "$TEST_ROOT"/result "$flake3Dir#sth%20sth" # Check whether it saved the lockfile [[ -n $(git -C "$flake3Dir" diff master) ]] @@ -249,7 +250,7 @@ nix flake lock "$flake3Dir" [[ -z $(git -C "$flake3Dir" diff master || echo failed) ]] nix flake update --flake "$flake3Dir" --override-flake flake2 nixpkgs -[[ ! -z $(git -C "$flake3Dir" diff master || echo failed) ]] +[[ -n $(git -C "$flake3Dir" diff master || echo failed) ]] # Testing the nix CLI nix registry add flake1 flake3 @@ -262,7 +263,7 @@ nix registry remove flake1 [[ $(nix registry list | wc -l) == 4 ]] # Test 'nix registry list' with a disabled global registry. -nix registry add user-flake1 git+file://$flake1Dir +nix registry add user-flake1 git+file://"$flake1Dir" nix registry add user-flake2 "git+file://$percentEncodedFlake2Dir" [[ $(nix --flake-registry "" registry list | wc -l) == 2 ]] nix --flake-registry "" registry list | grepQuietInverse '^global' # nothing in global registry @@ -273,9 +274,9 @@ nix registry remove user-flake2 [[ $(nix registry list | wc -l) == 4 ]] # Test 'nix flake clone'. -rm -rf $TEST_ROOT/flake1-v2 -nix flake clone flake1 --dest $TEST_ROOT/flake1-v2 -[ -e $TEST_ROOT/flake1-v2/flake.nix ] +rm -rf "$TEST_ROOT"/flake1-v2 +nix flake clone flake1 --dest "$TEST_ROOT"/flake1-v2 +[ -e "$TEST_ROOT"/flake1-v2/flake.nix ] # Test 'follows' inputs. cat > "$flake3Dir/flake.nix" < "$flake3Dir/flake.nix" < "$flake3Dir/flake.nix" < $badFlakeDir/flake.nix -nix store delete $(nix store add-path $badFlakeDir) +rm -rf "$badFlakeDir" +mkdir "$badFlakeDir" +echo INVALID > "$badFlakeDir"/flake.nix +nix store delete "$(nix store add-path "$badFlakeDir")" -[[ $(nix path-info $(nix store add-path $flake1Dir)) =~ flake1 ]] -[[ $(nix path-info path:$(nix store add-path $flake1Dir)) =~ simple ]] +[[ $(nix path-info "$(nix store add-path "$flake1Dir")") =~ flake1 ]] +[[ $(nix path-info path:"$(nix store add-path "$flake1Dir")") =~ simple ]] # Test fetching flakerefs in the legacy CLI. [[ $(nix-instantiate --eval flake:flake3 -A x) = 123 ]] @@ -424,15 +426,15 @@ nix store delete $(nix store add-path $badFlakeDir) [[ $(NIX_PATH=flake3=flake:flake3 nix-instantiate --eval '' -A x) = 123 ]] # Test alternate lockfile paths. -nix flake lock "$flake2Dir" --output-lock-file $TEST_ROOT/flake2.lock -cmp "$flake2Dir/flake.lock" $TEST_ROOT/flake2.lock >/dev/null # lockfiles should be identical, since we're referencing flake2's original one +nix flake lock "$flake2Dir" --output-lock-file "$TEST_ROOT"/flake2.lock +cmp "$flake2Dir/flake.lock" "$TEST_ROOT"/flake2.lock >/dev/null # lockfiles should be identical, since we're referencing flake2's original one -nix flake lock "$flake2Dir" --output-lock-file $TEST_ROOT/flake2-overridden.lock --override-input flake1 git+file://$flake1Dir?rev=$flake1OriginalCommit -expectStderr 1 cmp "$flake2Dir/flake.lock" $TEST_ROOT/flake2-overridden.lock -nix flake metadata "$flake2Dir" --reference-lock-file $TEST_ROOT/flake2-overridden.lock | grepQuiet $flake1OriginalCommit +nix flake lock "$flake2Dir" --output-lock-file "$TEST_ROOT"/flake2-overridden.lock --override-input flake1 git+file://"$flake1Dir"?rev="$flake1OriginalCommit" +expectStderr 1 cmp "$flake2Dir/flake.lock" "$TEST_ROOT"/flake2-overridden.lock +nix flake metadata "$flake2Dir" --reference-lock-file "$TEST_ROOT"/flake2-overridden.lock | grepQuiet "$flake1OriginalCommit" # reference-lock-file can only be used if allow-dirty is set. -expectStderr 1 nix flake metadata "$flake2Dir" --no-allow-dirty --reference-lock-file $TEST_ROOT/flake2-overridden.lock +expectStderr 1 nix flake metadata "$flake2Dir" --no-allow-dirty --reference-lock-file "$TEST_ROOT"/flake2-overridden.lock # After changing an input (flake2 from newFlake2Rev to prevFlake2Rev), we should have the transitive inputs locked by revision $prevFlake2Rev of flake2. prevFlake1Rev=$(nix flake metadata --json "$flake1Dir" | jq -r .revision) @@ -459,7 +461,7 @@ git -C "$flake3Dir" commit flake.nix -m 'bla' rm "$flake3Dir/flake.lock" nix flake lock "$flake3Dir" -[[ "$(nix flake metadata --json "$flake3Dir" | jq -r .locks.nodes.flake1.locked.rev)" = $newFlake1Rev ]] +[[ "$(nix flake metadata --json "$flake3Dir" | jq -r .locks.nodes.flake1.locked.rev)" = "$newFlake1Rev" ]] cat > "$flake3Dir/flake.nix" < "$flake3Dir/flake.nix" < Date: Thu, 25 Sep 2025 13:27:34 -0700 Subject: [PATCH 24/24] shellcheck fix: tests/functional/flakes/follow-paths.sh --- maintainers/flake-module.nix | 1 - tests/functional/flakes/follow-paths.sh | 97 +++++++++++++------------ 2 files changed, 49 insertions(+), 49 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 50601e06f..0e9363408 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -106,7 +106,6 @@ enable = true; excludes = [ # We haven't linted these files yet - ''^tests/functional/flakes/follow-paths\.sh$'' ''^tests/functional/flakes/prefetch\.sh$'' ''^tests/functional/flakes/run\.sh$'' ''^tests/functional/flakes/show\.sh$'' diff --git a/tests/functional/flakes/follow-paths.sh b/tests/functional/flakes/follow-paths.sh index cf27681cb..1a6661be5 100755 --- a/tests/functional/flakes/follow-paths.sh +++ b/tests/functional/flakes/follow-paths.sh @@ -11,13 +11,13 @@ flakeFollowsD=$TEST_ROOT/follows/flakeA/flakeD flakeFollowsE=$TEST_ROOT/follows/flakeA/flakeE # Test following path flakerefs. -createGitRepo $flakeFollowsA -mkdir -p $flakeFollowsB -mkdir -p $flakeFollowsC -mkdir -p $flakeFollowsD -mkdir -p $flakeFollowsE +createGitRepo "$flakeFollowsA" +mkdir -p "$flakeFollowsB" +mkdir -p "$flakeFollowsC" +mkdir -p "$flakeFollowsD" +mkdir -p "$flakeFollowsE" -cat > $flakeFollowsA/flake.nix < "$flakeFollowsA"/flake.nix < $flakeFollowsA/flake.nix < $flakeFollowsB/flake.nix < "$flakeFollowsB"/flake.nix < $flakeFollowsB/flake.nix < $flakeFollowsC/flake.nix < "$flakeFollowsC"/flake.nix < $flakeFollowsC/flake.nix < $flakeFollowsD/flake.nix < "$flakeFollowsD"/flake.nix < $flakeFollowsD/flake.nix < $flakeFollowsE/flake.nix < "$flakeFollowsE"/flake.nix < $flakeFollowsE/flake.nix < $flakeFollowsA/flake.nix < "$flakeFollowsA"/flake.nix < $flakeFollowsA/flake.nix < $flakeFollowsA/flake.nix < "$flakeFollowsA"/flake.nix < $flakeFollowsA/flake.nix <&1 | grep '/flakeB.*is forbidden in pure evaluation mode' -expect 1 nix flake lock --impure $flakeFollowsA 2>&1 | grep '/flakeB.*does not exist' +expect 1 nix flake lock "$flakeFollowsA" 2>&1 | grep '/flakeB.*is forbidden in pure evaluation mode' +expect 1 nix flake lock --impure "$flakeFollowsA" 2>&1 | grep '/flakeB.*does not exist' # Test relative non-flake inputs. -cat > $flakeFollowsA/flake.nix < "$flakeFollowsA"/flake.nix < $flakeFollowsA/flake.nix < $flakeFollowsA/foo.nix +echo 123 > "$flakeFollowsA"/foo.nix -git -C $flakeFollowsA add flake.nix foo.nix +git -C "$flakeFollowsA" add flake.nix foo.nix -nix flake lock $flakeFollowsA +nix flake lock "$flakeFollowsA" -[[ $(nix eval --json $flakeFollowsA#e) = 123 ]] +[[ $(nix eval --json "$flakeFollowsA"#e) = 123 ]] # Non-existant follows should print a warning. -cat >$flakeFollowsA/flake.nix <"$flakeFollowsA"/flake.nix <$flakeFollowsA/flake.nix <&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid'" nix flake lock "$flakeFollowsA" 2>&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid2'" @@ -269,7 +269,7 @@ flakeFollowCycle="$TEST_ROOT/follows/followCycle" # Test following path flakerefs. mkdir -p "$flakeFollowCycle" -cat > $flakeFollowCycle/flake.nix < "$flakeFollowCycle"/flake.nix < $flakeFollowCycle/flake.nix <&1 && fail "nix flake lock should have failed." || true) -echo $checkRes | grep -F "error: follow cycle detected: [baz -> foo -> bar -> baz]" +echo "$checkRes" | grep -F "error: follow cycle detected: [baz -> foo -> bar -> baz]" # Test transitive input url locking @@ -362,22 +363,22 @@ echo "$json" | jq .locks.nodes.C.original # Test deep overrides, e.g. `inputs.B.inputs.C.inputs.D.follows = ...`. -cat < $flakeFollowsD/flake.nix +cat < "$flakeFollowsD"/flake.nix { outputs = _: {}; } EOF -cat < $flakeFollowsC/flake.nix +cat < "$flakeFollowsC"/flake.nix { inputs.D.url = "path:nosuchflake"; outputs = _: {}; } EOF -cat < $flakeFollowsB/flake.nix +cat < "$flakeFollowsB"/flake.nix { inputs.C.url = "path:$flakeFollowsC"; outputs = _: {}; } EOF -cat < $flakeFollowsA/flake.nix +cat < "$flakeFollowsA"/flake.nix { inputs.B.url = "path:$flakeFollowsB"; inputs.D.url = "path:$flakeFollowsD"; @@ -386,26 +387,26 @@ cat < $flakeFollowsA/flake.nix } EOF -nix flake lock $flakeFollowsA +nix flake lock "$flakeFollowsA" -[[ $(jq -c .nodes.C.inputs.D $flakeFollowsA/flake.lock) = '["D"]' ]] +[[ $(jq -c .nodes.C.inputs.D "$flakeFollowsA"/flake.lock) = '["D"]' ]] # Test overlapping flake follows: B has D follow C/D, while A has B/C follow C -cat < $flakeFollowsC/flake.nix +cat < "$flakeFollowsC"/flake.nix { inputs.D.url = "path:$flakeFollowsD"; outputs = _: {}; } EOF -cat < $flakeFollowsB/flake.nix +cat < "$flakeFollowsB"/flake.nix { inputs.C.url = "path:nosuchflake"; inputs.D.follows = "C/D"; outputs = _: {}; } EOF -cat < $flakeFollowsA/flake.nix +cat < "$flakeFollowsA"/flake.nix { inputs.B.url = "path:$flakeFollowsB"; inputs.C.url = "path:$flakeFollowsC"; @@ -415,12 +416,12 @@ cat < $flakeFollowsA/flake.nix EOF # bug was not triggered without recreating the lockfile -nix flake lock $flakeFollowsA --recreate-lock-file +nix flake lock "$flakeFollowsA" --recreate-lock-file -[[ $(jq -c .nodes.B.inputs.D $flakeFollowsA/flake.lock) = '["B","C","D"]' ]] +[[ $(jq -c .nodes.B.inputs.D "$flakeFollowsA"/flake.lock) = '["B","C","D"]' ]] # Check that you can't have both a flakeref and a follows attribute on an input. -cat < $flakeFollowsB/flake.nix +cat < "$flakeFollowsB"/flake.nix { inputs.C.url = "path:nosuchflake"; inputs.D.url = "path:nosuchflake"; @@ -429,4 +430,4 @@ cat < $flakeFollowsB/flake.nix } EOF -expectStderr 1 nix flake lock $flakeFollowsA --recreate-lock-file | grepQuiet "flake input has both a flake reference and a follows attribute" +expectStderr 1 nix flake lock "$flakeFollowsA" --recreate-lock-file | grepQuiet "flake input has both a flake reference and a follows attribute"