From 8c54a01df5ee59e4acf151dba8077a9842e8bdc5 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Mon, 13 Mar 2023 21:14:19 +0100 Subject: [PATCH 1/5] nix: develop: always force SHELL to chosen shell SHELL was inherited from the system environment. This resulted in a new shell being started, but with SHELL still referring to the system shell and not the one used by nix-develop. Applications like make, use SHELL to run commands, which meant that top-level commands are run inside the nix-develop-shell, but sub-commands are ran inside the system shell. This setenv forces SHELL to always be set to the shell used by nix-develop. --- src/nix/develop.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 38482ed42..4a561e52b 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -293,7 +293,6 @@ struct Common : InstallableCommand, MixProfile "NIX_LOG_FD", "NIX_REMOTE", "PPID", - "SHELL", "SHELLOPTS", "SSL_CERT_FILE", // FIXME: only want to ignore /no-cert-file.crt "TEMP", @@ -643,6 +642,10 @@ struct CmdDevelop : Common, MixEnvironment ignoreException(); } + // Override SHELL with the one chosen for this environment. + // This is to make sure the system shell doesn't leak into the build environment. + setenv("SHELL", shell.data(), 1); + // If running a phase or single command, don't want an interactive shell running after // Ctrl-C, so don't pass --rcfile auto args = phase || !command.empty() ? Strings{std::string(baseNameOf(shell)), rcFilePath} From ceab20d056a119317fb29eb0e06dfd0eb0b9d8ad Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Mon, 13 Nov 2023 22:04:34 +0100 Subject: [PATCH 2/5] nix: develop: add tests for interactive shell --- tests/functional/flakes/develop.sh | 75 ++++++++++++++++++++++++++++++ tests/functional/local.mk | 1 + 2 files changed, 76 insertions(+) create mode 100644 tests/functional/flakes/develop.sh diff --git a/tests/functional/flakes/develop.sh b/tests/functional/flakes/develop.sh new file mode 100644 index 000000000..59f731239 --- /dev/null +++ b/tests/functional/flakes/develop.sh @@ -0,0 +1,75 @@ +source ../common.sh + +clearStore +rm -rf $TEST_HOME/.cache $TEST_HOME/.config $TEST_HOME/.local + +# Create flake under test. +cp ../shell-hello.nix ../config.nix $TEST_HOME/ +cat <$TEST_HOME/flake.nix +{ + inputs.nixpkgs.url = "$TEST_HOME/nixpkgs"; + outputs = {self, nixpkgs}: { + packages.$system.hello = (import ./config.nix).mkDerivation { + name = "hello"; + outputs = [ "out" "dev" ]; + meta.outputsToInstall = [ "out" ]; + buildCommand = ""; + }; + }; +} +EOF + +# Create fake nixpkgs flake. +mkdir -p $TEST_HOME/nixpkgs +cp ../config.nix ../shell.nix $TEST_HOME/nixpkgs +cat <$TEST_HOME/nixpkgs/flake.nix +{ + outputs = {self}: { + legacyPackages.$system.bashInteractive = (import ./shell.nix {}).bashInteractive; + }; +} +EOF + +cd $TEST_HOME + +# Test whether `nix develop` passes through environment variables. +[[ "$( + ENVVAR=a nix develop --no-write-lock-file .#hello < Date: Thu, 16 Nov 2023 15:12:31 +0100 Subject: [PATCH 3/5] fixup! nix: develop: add tests for interactive shell --- tests/functional/common/vars-and-functions.sh.in | 1 + tests/functional/flakes/develop.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/functional/common/vars-and-functions.sh.in b/tests/functional/common/vars-and-functions.sh.in index 848988af9..02773bf60 100644 --- a/tests/functional/common/vars-and-functions.sh.in +++ b/tests/functional/common/vars-and-functions.sh.in @@ -45,6 +45,7 @@ if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then DAEMON_PATH="${NIX_DAEMON_PACKAGE}/bin:$DAEMON_PATH" fi coreutils=@coreutils@ +lsof=@lsof@ export dot=@dot@ export SHELL="@bash@" diff --git a/tests/functional/flakes/develop.sh b/tests/functional/flakes/develop.sh index 59f731239..db23ca0c0 100644 --- a/tests/functional/flakes/develop.sh +++ b/tests/functional/flakes/develop.sh @@ -54,7 +54,7 @@ BASH_INTERACTIVE_EXECUTABLE="$PWD/bash-interactive/bin/bash" [[ "$( nix develop --no-write-lock-file .#hello <&1 | grep -o '/.*/bash' EOF )" -ef "$BASH_INTERACTIVE_EXECUTABLE" ]] From 06a745120bc8fe7625954e970c61028f8a42c31e Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Sun, 26 Nov 2023 21:27:46 +0100 Subject: [PATCH 4/5] nix: develop: remove test for interactive shell executable --- tests/functional/flakes/develop.sh | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/functional/flakes/develop.sh b/tests/functional/flakes/develop.sh index db23ca0c0..e1e53d364 100644 --- a/tests/functional/flakes/develop.sh +++ b/tests/functional/flakes/develop.sh @@ -50,14 +50,6 @@ EOF nix build --no-write-lock-file './nixpkgs#bashInteractive' --out-link ./bash-interactive BASH_INTERACTIVE_EXECUTABLE="$PWD/bash-interactive/bin/bash" -# Test whether `nix develop` uses nixpkgs#bashInteractive shell. -[[ "$( - nix develop --no-write-lock-file .#hello <&1 | grep -o '/.*/bash' -EOF -)" -ef "$BASH_INTERACTIVE_EXECUTABLE" ]] - # Test whether `nix develop` sets `SHELL` to nixpkgs#bashInteractive shell. [[ "$( SHELL=custom nix develop --no-write-lock-file .#hello < Date: Fri, 12 Jan 2024 13:00:53 +0100 Subject: [PATCH 5/5] .data() -> .c_str() to be on the safe side --- src/nix/develop.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 5e25833eb..1f2891378 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -603,7 +603,7 @@ struct CmdDevelop : Common, MixEnvironment setEnviron(); // prevent garbage collection until shell exits - setenv("NIX_GCROOT", gcroot.data(), 1); + setenv("NIX_GCROOT", gcroot.c_str(), 1); Path shell = "bash"; @@ -648,7 +648,7 @@ struct CmdDevelop : Common, MixEnvironment // Override SHELL with the one chosen for this environment. // This is to make sure the system shell doesn't leak into the build environment. - setenv("SHELL", shell.data(), 1); + setenv("SHELL", shell.c_str(), 1); // If running a phase or single command, don't want an interactive shell running after // Ctrl-C, so don't pass --rcfile