Clean up env var logic in preparation for Windows

It's a little weird we don't check the return status for these, but
changing that would introduce risk so I did not.

Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>

src/libmain/shared.cc

@@ -308,7 +308,7 @@ void printVersion(const std::string & programName)
 void showManPage(const std::string & name)
 {
     restoreProcessContext();
-    setenv("MANPATH", settings.nixManDir.c_str(), 1);
+    setEnv("MANPATH", settings.nixManDir.c_str());
     execlp("man", "man", name.c_str(), nullptr);
     throw SysError("command 'man %1%' failed", name.c_str());
 }
@@ -369,7 +369,7 @@ RunPager::RunPager()
         if (dup2(toPager.readSide.get(), STDIN_FILENO) == -1)
             throw SysError("dupping stdin");
         if (!getenv("LESS"))
-            setenv("LESS", "FRSXMK", 1);
+            setEnv("LESS", "FRSXMK");
         restoreProcessContext();
         if (pager)
             execl("/bin/sh", "sh", "-c", pager, nullptr);

src/libutil/environment-variables.cc

@@ -32,4 +32,17 @@ std::map<std::string, std::string> getEnv()
     return env;
 }
 
+void clearEnv()
+{
+    for (auto & name : getEnv())
+        unsetenv(name.first.c_str());
+}
+
+void replaceEnv(const std::map<std::string, std::string> & newEnv)
+{
+    clearEnv();
+    for (auto & newEnvVar : newEnv)
+        setEnv(newEnvVar.first.c_str(), newEnvVar.second.c_str());
+}
+
 }

src/libutil/environment-variables.hh

@@ -28,6 +28,14 @@ std::optional<std::string> getEnvNonEmpty(const std::string & key);
  */
 std::map<std::string, std::string> getEnv();
 
+/**
+ * Like POSIX `setenv`, but always overrides.
+ *
+ * We don't need the non-overriding version, and this is easier to
+ * reimplement on Windows.
+ */
+int setEnv(const char * name, const char * value);
+
 /**
  * Clear the environment.
  */

src/libutil/unix/environment-variables.cc

@@ -1,21 +1,12 @@
-#include "util.hh"
+#include <cstdlib>
-#include "environment-variables.hh"
 
-extern char * * environ __attribute__((weak));
+#include "environment-variables.hh"
 
 namespace nix {
 
-void clearEnv()
+int setEnv(const char * name, const char * value)
 {
-    for (auto & name : getEnv())
+    return ::setenv(name, value, 1);
-        unsetenv(name.first.c_str());
-}
-
-void replaceEnv(const std::map<std::string, std::string> & newEnv)
-{
-    clearEnv();
-    for (auto & newEnvVar : newEnv)
-        setenv(newEnvVar.first.c_str(), newEnvVar.second.c_str(), 1);
 }
 
 }

src/nix-build/nix-build.cc

@@ -288,7 +288,7 @@ static void main_nix_build(int argc, char * * argv)
     }
 
     if (runEnv)
-        setenv("IN_NIX_SHELL", pure ? "pure" : "impure", 1);
+        setEnv("IN_NIX_SHELL", pure ? "pure" : "impure");
 
     PackageInfos drvs;
 

src/nix/develop.cc

@@ -603,7 +603,7 @@ struct CmdDevelop : Common, MixEnvironment
 
         setEnviron();
         // prevent garbage collection until shell exits
-        setenv("NIX_GCROOT", gcroot.c_str(), 1);
+        setEnv("NIX_GCROOT", gcroot.c_str());
 
         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.c_str(), 1);
+        setEnv("SHELL", shell.c_str());
 
         // If running a phase or single command, don't want an interactive shell running after
         // Ctrl-C, so don't pass --rcfile

src/nix/unix/run.cc

@@ -134,7 +134,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment
         auto unixPath = tokenizeString<Strings>(getEnv("PATH").value_or(""), ":");
         unixPath.insert(unixPath.begin(), pathAdditions.begin(), pathAdditions.end());
         auto unixPathString = concatStringsSep(":", unixPath);
-        setenv("PATH", unixPathString.c_str(), 1);
+        setEnv("PATH", unixPathString.c_str());
 
         Strings args;
         for (auto & arg : command) args.push_back(arg);

tests/unit/libexpr/primops.cc

@@ -91,7 +91,7 @@ namespace nix {
     }
 
     TEST_F(PrimOpTest, getEnv) {
-        setenv("_NIX_UNIT_TEST_ENV_VALUE", "test value", 1);
+        setEnv("_NIX_UNIT_TEST_ENV_VALUE", "test value");
         auto v = eval("builtins.getEnv \"_NIX_UNIT_TEST_ENV_VALUE\"");
         ASSERT_THAT(v, IsStringEq("test value"));
     }