From 8497f0fe197cb1c8de2967cdae20e42c1ac25830 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 26 Sep 2024 14:12:18 -0700 Subject: [PATCH 1/2] tests: move nix-shell related tests to subdir This change feels kind of gross and reveals a fair bit about the disorganization of our tests, but I think it makes parts of it a bit better. Change-Id: Idb8d9a00cbd75d5c156678c6b408b42b59d5e4d7 --- tests/functional/ca/nix-shell.sh | 4 +-- tests/functional/flakes/develop-r8854.sh | 2 +- tests/functional/flakes/develop.sh | 2 +- tests/functional/meson.build | 5 +++- .../{nix-shell.sh => nix-shell/basic.sh} | 2 +- tests/functional/{ => nix-shell}/ca-shell.nix | 0 tests/functional/nix-shell/config.nix.in | 1 + tests/functional/nix-shell/meson.build | 6 ++++ tests/functional/{ => nix-shell}/shell.nix | 0 .../{ => nix-shell}/shell.shebang.nix | 0 .../{ => nix-shell}/shell.shebang.rb | 0 .../{ => nix-shell}/shell.shebang.sh | 0 .../structured-attrs-shell.nix | 0 .../functional/nix-shell/structured-attrs.sh | 28 +++++++++++++++++++ tests/functional/structured-attrs.sh | 21 -------------- 15 files changed, 44 insertions(+), 27 deletions(-) rename tests/functional/{nix-shell.sh => nix-shell/basic.sh} (99%) rename tests/functional/{ => nix-shell}/ca-shell.nix (100%) create mode 120000 tests/functional/nix-shell/config.nix.in create mode 100644 tests/functional/nix-shell/meson.build rename tests/functional/{ => nix-shell}/shell.nix (100%) rename tests/functional/{ => nix-shell}/shell.shebang.nix (100%) rename tests/functional/{ => nix-shell}/shell.shebang.rb (100%) rename tests/functional/{ => nix-shell}/shell.shebang.sh (100%) rename tests/functional/{ => nix-shell}/structured-attrs-shell.nix (100%) create mode 100644 tests/functional/nix-shell/structured-attrs.sh diff --git a/tests/functional/ca/nix-shell.sh b/tests/functional/ca/nix-shell.sh index d1fbe54d1..a9b09069e 100755 --- a/tests/functional/ca/nix-shell.sh +++ b/tests/functional/ca/nix-shell.sh @@ -3,5 +3,5 @@ source common.sh CONTENT_ADDRESSED=true -cd .. -source ./nix-shell.sh +cd ../nix-shell +source ./basic.sh diff --git a/tests/functional/flakes/develop-r8854.sh b/tests/functional/flakes/develop-r8854.sh index efe129bf4..9cf4e7cf3 100644 --- a/tests/functional/flakes/develop-r8854.sh +++ b/tests/functional/flakes/develop-r8854.sh @@ -24,7 +24,7 @@ EOF # Create fake nixpkgs flake. mkdir -p $TEST_HOME/nixpkgs -cp ../config.nix ../shell.nix $TEST_HOME/nixpkgs +cp ../config.nix ../nix-shell/shell.nix $TEST_HOME/nixpkgs cat <$TEST_HOME/nixpkgs/flake.nix { outputs = {self}: { diff --git a/tests/functional/flakes/develop.sh b/tests/functional/flakes/develop.sh index e1e53d364..71d448a78 100644 --- a/tests/functional/flakes/develop.sh +++ b/tests/functional/flakes/develop.sh @@ -21,7 +21,7 @@ EOF # Create fake nixpkgs flake. mkdir -p $TEST_HOME/nixpkgs -cp ../config.nix ../shell.nix $TEST_HOME/nixpkgs +cp ../config.nix ../nix-shell/shell.nix $TEST_HOME/nixpkgs cat <$TEST_HOME/nixpkgs/flake.nix { outputs = {self}: { diff --git a/tests/functional/meson.build b/tests/functional/meson.build index f56ced48d..3b610bcdc 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -25,6 +25,8 @@ config_nix_in = configure_file( subdir('ca') # Just configures `dyn-drv/config.nix.in`. Same as above. subdir('dyn-drv') +# Just configures `nix-shell/config.nix.in`. Same as above. +subdir('nix-shell') subdir('plugins') subdir('test-libstoreconsumer') @@ -111,7 +113,8 @@ functional_tests_scripts = [ 'hash.sh', 'gc-non-blocking.sh', 'check.sh', - 'nix-shell.sh', + 'nix-shell/basic.sh', + 'nix-shell/structured-attrs.sh', 'check-refs.sh', 'build-remote-input-addressed.sh', 'secure-drv-outputs.sh', diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell/basic.sh similarity index 99% rename from tests/functional/nix-shell.sh rename to tests/functional/nix-shell/basic.sh index 04c83138e..5c6e81ec2 100644 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell/basic.sh @@ -1,4 +1,4 @@ -source common.sh +source ../common.sh clearStore diff --git a/tests/functional/ca-shell.nix b/tests/functional/nix-shell/ca-shell.nix similarity index 100% rename from tests/functional/ca-shell.nix rename to tests/functional/nix-shell/ca-shell.nix diff --git a/tests/functional/nix-shell/config.nix.in b/tests/functional/nix-shell/config.nix.in new file mode 120000 index 000000000..af24ddb30 --- /dev/null +++ b/tests/functional/nix-shell/config.nix.in @@ -0,0 +1 @@ +../config.nix.in \ No newline at end of file diff --git a/tests/functional/nix-shell/meson.build b/tests/functional/nix-shell/meson.build new file mode 100644 index 000000000..f3f4a3b2b --- /dev/null +++ b/tests/functional/nix-shell/meson.build @@ -0,0 +1,6 @@ +# test_confdata set from tests/functional/meson.build +configure_file( + input : 'config.nix.in', + output : 'config.nix', + configuration : test_confdata, +) diff --git a/tests/functional/shell.nix b/tests/functional/nix-shell/shell.nix similarity index 100% rename from tests/functional/shell.nix rename to tests/functional/nix-shell/shell.nix diff --git a/tests/functional/shell.shebang.nix b/tests/functional/nix-shell/shell.shebang.nix similarity index 100% rename from tests/functional/shell.shebang.nix rename to tests/functional/nix-shell/shell.shebang.nix diff --git a/tests/functional/shell.shebang.rb b/tests/functional/nix-shell/shell.shebang.rb similarity index 100% rename from tests/functional/shell.shebang.rb rename to tests/functional/nix-shell/shell.shebang.rb diff --git a/tests/functional/shell.shebang.sh b/tests/functional/nix-shell/shell.shebang.sh similarity index 100% rename from tests/functional/shell.shebang.sh rename to tests/functional/nix-shell/shell.shebang.sh diff --git a/tests/functional/structured-attrs-shell.nix b/tests/functional/nix-shell/structured-attrs-shell.nix similarity index 100% rename from tests/functional/structured-attrs-shell.nix rename to tests/functional/nix-shell/structured-attrs-shell.nix diff --git a/tests/functional/nix-shell/structured-attrs.sh b/tests/functional/nix-shell/structured-attrs.sh new file mode 100644 index 000000000..1d2fa8fed --- /dev/null +++ b/tests/functional/nix-shell/structured-attrs.sh @@ -0,0 +1,28 @@ +source ../common.sh + +# 27ce722638 required some incompatible changes to the nix file, so skip this +# tests for the older versions +requireDaemonNewerThan "2.4pre20210712" + +clearStore + +export NIX_BUILD_SHELL=$SHELL +env NIX_PATH=nixpkgs=shell.nix nix-shell structured-attrs-shell.nix \ + --run 'test "3" = "$(jq ".my.list|length" < $NIX_ATTRS_JSON_FILE)"' + +nix develop -f structured-attrs-shell.nix -c bash -c 'test "3" = "$(jq ".my.list|length" < $NIX_ATTRS_JSON_FILE)"' + +# `nix develop` is a slightly special way of dealing with environment vars, it parses +# these from a shell-file exported from a derivation. This is to test especially `outputs` +# (which is an associative array in thsi case) being fine. +nix develop -f structured-attrs-shell.nix -c bash -c 'test -n "$out"' + +nix print-dev-env -f structured-attrs-shell.nix | grepQuiet 'NIX_ATTRS_JSON_FILE=' +nix print-dev-env -f structured-attrs-shell.nix | grepQuiet 'NIX_ATTRS_SH_FILE=' +nix print-dev-env -f shell.nix shellDrv | grepQuietInverse 'NIX_ATTRS_SH_FILE' + +jsonOut="$(nix print-dev-env -f structured-attrs-shell.nix --json)" + +test "$(<<<"$jsonOut" jq '.structuredAttrs|keys|.[]' -r)" = "$(printf ".attrs.json\n.attrs.sh")" + +test "$(<<<"$jsonOut" jq '.variables.out.value' -r)" = "$(<<<"$jsonOut" jq '.structuredAttrs.".attrs.json"' -r | jq -r '.outputs.out')" diff --git a/tests/functional/structured-attrs.sh b/tests/functional/structured-attrs.sh index f11992dcd..86d8b76c3 100644 --- a/tests/functional/structured-attrs.sh +++ b/tests/functional/structured-attrs.sh @@ -12,24 +12,3 @@ nix-build structured-attrs.nix -A all -o $TEST_ROOT/result [[ $(cat $TEST_ROOT/result/foo) = bar ]] [[ $(cat $TEST_ROOT/result-dev/foo) = foo ]] - -export NIX_BUILD_SHELL=$SHELL -env NIX_PATH=nixpkgs=shell.nix nix-shell structured-attrs-shell.nix \ - --run 'test "3" = "$(jq ".my.list|length" < $NIX_ATTRS_JSON_FILE)"' - -nix develop -f structured-attrs-shell.nix -c bash -c 'test "3" = "$(jq ".my.list|length" < $NIX_ATTRS_JSON_FILE)"' - -# `nix develop` is a slightly special way of dealing with environment vars, it parses -# these from a shell-file exported from a derivation. This is to test especially `outputs` -# (which is an associative array in thsi case) being fine. -nix develop -f structured-attrs-shell.nix -c bash -c 'test -n "$out"' - -nix print-dev-env -f structured-attrs-shell.nix | grepQuiet 'NIX_ATTRS_JSON_FILE=' -nix print-dev-env -f structured-attrs-shell.nix | grepQuiet 'NIX_ATTRS_SH_FILE=' -nix print-dev-env -f shell.nix shellDrv | grepQuietInverse 'NIX_ATTRS_SH_FILE' - -jsonOut="$(nix print-dev-env -f structured-attrs-shell.nix --json)" - -test "$(<<<"$jsonOut" jq '.structuredAttrs|keys|.[]' -r)" = "$(printf ".attrs.json\n.attrs.sh")" - -test "$(<<<"$jsonOut" jq '.variables.out.value' -r)" = "$(<<<"$jsonOut" jq '.structuredAttrs.".attrs.json"' -r | jq -r '.outputs.out')" From c1f4c60bc23fd4e4a6fa64d22c17f1d348e16247 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 26 Sep 2024 14:12:18 -0700 Subject: [PATCH 2/2] nix-shell: stop using dynamic format strings!! This was always a terrible idea independently of whether it crashes. Stop doing it! This commit was verified by running nix-shell on a trivial derivation with --debug --verbose to get the vomit-level output of the shell rc file and then diffing it before/after this change. I have reasonable confidence it did not regress anything, though this code is genuinely really hard to follow (which is a second reason that I split it into two fmt calls). Fixes: https://git.lix.systems/lix-project/lix/issues/533 Change-Id: I8e11ddbece2b12749fda13efe0b587a71b00bfe5 --- src/legacy/nix-build.cc | 79 +++++++++++--------- tests/functional/meson.build | 1 + tests/functional/nix-shell/regression-533.sh | 18 +++++ 3 files changed, 63 insertions(+), 35 deletions(-) create mode 100644 tests/functional/nix-shell/regression-533.sh diff --git a/src/legacy/nix-build.cc b/src/legacy/nix-build.cc index eb9b6cd8d..1107bc443 100644 --- a/src/legacy/nix-build.cc +++ b/src/legacy/nix-build.cc @@ -490,42 +490,51 @@ static void main_nix_build(int argc, char * * argv) environment variables and shell functions. Also don't lose the current $PATH directories. */ auto rcfile = (Path) tmpDir + "/rc"; + auto tz = getEnv("TZ"); std::string rc = fmt( - R"(_nix_shell_clean_tmpdir() { command rm -rf %1%; }; )"s + - (keepTmp ? - "trap _nix_shell_clean_tmpdir EXIT; " - "exitHooks+=(_nix_shell_clean_tmpdir); " - "failureHooks+=(_nix_shell_clean_tmpdir); ": - "_nix_shell_clean_tmpdir; ") + - (pure ? "" : "[ -n \"$PS1\" ] && [ -e ~/.bashrc ] && source ~/.bashrc;") + - "%2%" - // always clear PATH. - // when nix-shell is run impure, we rehydrate it with the `p=$PATH` above - "unset PATH;" - "dontAddDisableDepTrack=1;\n" - + structuredAttrsRC + - "\n[ -e $stdenv/setup ] && source $stdenv/setup; " - "%3%" - "PATH=%4%:\"$PATH\"; " - "SHELL=%5%; " - "BASH=%5%; " - "set +e; " - R"s([ -n "$PS1" -a -z "$NIX_SHELL_PRESERVE_PROMPT" ] && )s" + - (getuid() == 0 ? R"s(PS1='\n\[\033[1;31m\][nix-shell:\w]\$\[\033[0m\] '; )s" - : R"s(PS1='\n\[\033[1;32m\][nix-shell:\w]\$\[\033[0m\] '; )s") + - "if [ \"$(type -t runHook)\" = function ]; then runHook shellHook; fi; " - "unset NIX_ENFORCE_PURITY; " - "shopt -u nullglob; " - "unset TZ; %6%" - "shopt -s execfail;" - "%7%", - shellEscape(tmpDir), - (pure ? "" : "p=$PATH; "), - (pure ? "" : "PATH=$PATH:$p; unset p; "), - shellEscape(dirOf(*shell)), - shellEscape(*shell), - (getenv("TZ") ? (std::string("export TZ=") + shellEscape(getenv("TZ")) + "; ") : ""), - envCommand); + R"(_nix_shell_clean_tmpdir() { command rm -rf %1%; }; )" + "%2%" + "%3%" + // always clear PATH. + // when nix-shell is run impure, we rehydrate it with the `p=$PATH` above + "unset PATH;" + "dontAddDisableDepTrack=1;\n", + shellEscape(tmpDir), + (keepTmp + ? "trap _nix_shell_clean_tmpdir EXIT; " + "exitHooks+=(_nix_shell_clean_tmpdir); " + "failureHooks+=(_nix_shell_clean_tmpdir); " + : "_nix_shell_clean_tmpdir; "), + (pure + ? "" + : "[ -n \"$PS1\" ] && [ -e ~/.bashrc ] && source ~/.bashrc; p=$PATH; ") + ); + rc += structuredAttrsRC; + rc += fmt( + "\n[ -e $stdenv/setup ] && source $stdenv/setup; " + "%1%" + "PATH=%2%:\"$PATH\"; " + "SHELL=%3%; " + "BASH=%3%; " + "set +e; " + R"s([ -n "$PS1" -a -z "$NIX_SHELL_PRESERVE_PROMPT" ] && )s" + "%4%" + "if [ \"$(type -t runHook)\" = function ]; then runHook shellHook; fi; " + "unset NIX_ENFORCE_PURITY; " + "shopt -u nullglob; " + "unset TZ; %5%" + "shopt -s execfail;" + "%6%", + (pure ? "" : "PATH=$PATH:$p; unset p; "), + shellEscape(dirOf(*shell)), + shellEscape(*shell), + (getuid() == 0 ? R"s(PS1='\n\[\033[1;31m\][nix-shell:\w]\$\[\033[0m\] '; )s" + : R"s(PS1='\n\[\033[1;32m\][nix-shell:\w]\$\[\033[0m\] '; )s"), + (tz.has_value() + ? (std::string("export TZ=") + shellEscape(*tz) + "; ") + : ""), + envCommand + ); vomit("Sourcing nix-shell with file %s and contents:\n%s", rcfile, rc); writeFile(rcfile, rc); diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 3b610bcdc..8315a4652 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -115,6 +115,7 @@ functional_tests_scripts = [ 'check.sh', 'nix-shell/basic.sh', 'nix-shell/structured-attrs.sh', + 'nix-shell/regression-533.sh', 'check-refs.sh', 'build-remote-input-addressed.sh', 'secure-drv-outputs.sh', diff --git a/tests/functional/nix-shell/regression-533.sh b/tests/functional/nix-shell/regression-533.sh new file mode 100644 index 000000000..422f809fb --- /dev/null +++ b/tests/functional/nix-shell/regression-533.sh @@ -0,0 +1,18 @@ +source ../common.sh + +clearStore + +evil=$(cat <<-'EOF' +builtins.derivation { + name = "evil-kbity"; + system = "x86_64-darwin"; + builder = "/bin/sh"; + args = [ "-c" "> $out" ]; + __structuredAttrs = true; + env.oops = "lol %s"; +} +EOF +) + +# This should not crash +nix-shell --expr "$evil" --run 'echo yay' | grepQuiet yay