nix-shell: Fix $PATH handling in the impure case

We were passing "p=$PATH" rather than "p=$PATH;", resulting in some
invalid shell code.

Also, construct a separate environment for the child rather than
overwriting the parent's.

1 files changed, 35 insertions, 29 deletions

diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc
index a31f7c96c3..eb5719e47f 100755
--- a/src/nix-build/nix-build.cc
+++ b/src/nix-build/nix-build.cc
@@ -16,8 +16,6 @@
 
 using namespace nix;
 
-extern char ** environ;
-
 /* Recreate the effect of the perl shellwords function, breaking up a
  * string into arguments like a shell word, including escapes
  */
@@ -375,32 +373,26 @@ int main(int argc, char ** argv)
                 runProgram(settings.nixBinDir + "/nix-store", false, nixStoreArgs);
 
                 // Set the environment.
+                auto env = getEnv();
+
                 auto tmp = getEnv("TMPDIR", getEnv("XDG_RUNTIME_DIR", "/tmp"));
+
                 if (pure) {
-                    std::vector<string> skippedEnv{"HOME", "USER", "LOGNAME", "DISPLAY", "PATH", "TERM", "IN_NIX_SHELL", "TZ", "PAGER", "NIX_BUILD_SHELL"};
-                    std::vector<string> removed;
-                    for (auto i = size_t{0}; environ[i]; ++i) {
-                        auto eq = strchr(environ[i], '=');
-                        if (!eq)
-                            // invalid env, just keep going
-                            continue;
-                        std::string name(environ[i], eq);
-                        if (find(skippedEnv.begin(), skippedEnv.end(), name) == skippedEnv.end())
-                            removed.emplace_back(std::move(name));
-                    }
-                    for (const auto & name : removed)
-                        unsetenv(name.c_str());
+                    std::set<string> keepVars{"HOME", "USER", "LOGNAME", "DISPLAY", "PATH", "TERM", "IN_NIX_SHELL", "TZ", "PAGER", "NIX_BUILD_SHELL"};
+                    decltype(env) newEnv;
+                    for (auto & i : env)
+                        if (keepVars.count(i.first))
+                            newEnv.emplace(i);
+                    env = newEnv;
                     // NixOS hack: prevent /etc/bashrc from sourcing /etc/profile.
-                    setenv("__ETC_PROFILE_SOURCED", "1", 1);
+                    env["__ETC_PROFILE_SOURCED"] = "1";
                 }
-                setenv("NIX_BUILD_TOP", tmp.c_str(), 1);
-                setenv("TMPDIR", tmp.c_str(), 1);
-                setenv("TEMPDIR", tmp.c_str(), 1);
-                setenv("TMP", tmp.c_str(), 1);
-                setenv("TEMP", tmp.c_str(), 1);
-                setenv("NIX_STORE", store->storeDir.c_str(), 1);
-                for (const auto & env : drv.env)
-                    setenv(env.first.c_str(), env.second.c_str(), 1);
+
+                env["NIX_BUILD_TOP"] = env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] = tmp;
+                env["NIX_STORE"] = store->storeDir;
+
+                for (auto & var : drv.env)
+                    env.emplace(var);
 
                 restoreAffinity();
 
@@ -424,11 +416,25 @@ int main(int argc, char ** argv)
                         "shopt -u nullglob; "
                         "unset TZ; %4%"
                         "%5%"
-                        ) % (Path) tmpDir % (pure ? "" : "p=$PATH") % (pure ? "" : "PATH=$PATH:$p; unset p; ") % (getenv("TZ") ? (string("export TZ='") + getenv("TZ") + "'; ") : "") % envCommand).str());
-                if (interactive)
-                    execlp(getEnv("NIX_BUILD_SHELL", "bash").c_str(), "bash", "--rcfile", rcfile.c_str(), NULL);
-                else
-                    execlp(getEnv("NIX_BUILD_SHELL", "bash").c_str(), "bash", rcfile.c_str(), NULL);
+                        )
+                        % (Path) tmpDir
+                        % (pure ? "" : "p=$PATH; ")
+                        % (pure ? "" : "PATH=$PATH:$p; unset p; ")
+                        % (getenv("TZ") ? (string("export TZ='") + getenv("TZ") + "'; ") : "")
+                        % envCommand).str());
+
+                Strings envStrs;
+                for (auto & i : env)
+                    envStrs.push_back(i.first + "=" + i.second);
+
+                auto args = interactive
+                    ? Strings{"bash", "--rcfile", rcfile}
+                    : Strings{"bash", rcfile};
+
+                execvpe(getEnv("NIX_BUILD_SHELL", "bash").c_str(),
+                        stringsToCharPtrs(args).data(),
+                        stringsToCharPtrs(envStrs).data());
+
                 throw SysError("executing shell");
             }