Merge pull request #8043 from bobvanderlinden/pr-shell-env

nix: develop: use SHELL from rc script
(cherry picked from commit 0d55d660d5ea081630aa9606b047eb755cff1528)
Change-Id: I83be6c63b282d7f01a0defa78d9e787c77f1f02d

src/nix/develop.cc

@@ -262,7 +262,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",
@@ -527,7 +526,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";
 
@@ -570,6 +569,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.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
         auto args = phase || !command.empty() ? Strings{std::string(baseNameOf(shell)), rcFilePath}

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@"

tests/functional/flakes/develop.sh

@@ -0,0 +1,67 @@
+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 <<EOF >$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 <<EOF >$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 <<EOF
+echo "\$ENVVAR"
+EOF
+)" = "a" ]]
+
+# Test whether `nix develop --ignore-environment` does _not_ pass through environment variables.
+[[ -z "$(
+    ENVVAR=a nix develop --ignore-environment --no-write-lock-file .#hello <<EOF
+echo "\$ENVVAR"
+EOF
+)" ]]
+
+# Determine the bashInteractive executable.
+nix build --no-write-lock-file './nixpkgs#bashInteractive' --out-link ./bash-interactive
+BASH_INTERACTIVE_EXECUTABLE="$PWD/bash-interactive/bin/bash"
+
+# Test whether `nix develop` sets `SHELL` to nixpkgs#bashInteractive shell.
+[[ "$(
+    SHELL=custom nix develop --no-write-lock-file .#hello <<EOF
+echo "\$SHELL"
+EOF
+)" -ef "$BASH_INTERACTIVE_EXECUTABLE" ]]
+
+# Test whether `nix develop` with ignore environment sets `SHELL` to nixpkgs#bashInteractive shell.
+[[ "$(
+    SHELL=custom nix develop --ignore-environment --no-write-lock-file .#hello <<EOF
+echo "\$SHELL"
+EOF
+)" -ef "$BASH_INTERACTIVE_EXECUTABLE" ]]
+
+clearStore

tests/functional/local.mk

@@ -2,6 +2,7 @@ nix_tests = \
   test-infra.sh \
   init.sh \
   flakes/flakes.sh \
+  flakes/develop.sh \
   flakes/run.sh \
   flakes/mercurial.sh \
   flakes/circular.sh \