From ef2d10f7e71fbbeb2fe78c327ffe5469e8ae4d04 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 4 Apr 2024 12:48:54 -0400 Subject: [PATCH] Clean up env var logic in preparation for Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 4 ++-- src/libutil/environment-variables.cc | 13 +++++++++++++ src/libutil/environment-variables.hh | 8 ++++++++ src/libutil/unix/environment-variables.cc | 17 ++++------------- src/nix-build/nix-build.cc | 2 +- src/nix/develop.cc | 4 ++-- src/nix/unix/run.cc | 2 +- tests/unit/libexpr/primops.cc | 2 +- 8 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 7bced0aa4..f3dd73101 100644 --- a/src/libmain/shared.cc +++ b/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); diff --git a/src/libutil/environment-variables.cc b/src/libutil/environment-variables.cc index 7f4bb2d00..d43197aa0 100644 --- a/src/libutil/environment-variables.cc +++ b/src/libutil/environment-variables.cc @@ -32,4 +32,17 @@ std::map getEnv() return env; } +void clearEnv() +{ + for (auto & name : getEnv()) + unsetenv(name.first.c_str()); +} + +void replaceEnv(const std::map & newEnv) +{ + clearEnv(); + for (auto & newEnvVar : newEnv) + setEnv(newEnvVar.first.c_str(), newEnvVar.second.c_str()); +} + } diff --git a/src/libutil/environment-variables.hh b/src/libutil/environment-variables.hh index 21eb4619b..21c2356a4 100644 --- a/src/libutil/environment-variables.hh +++ b/src/libutil/environment-variables.hh @@ -28,6 +28,14 @@ std::optional getEnvNonEmpty(const std::string & key); */ std::map 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. */ diff --git a/src/libutil/unix/environment-variables.cc b/src/libutil/unix/environment-variables.cc index c72880896..9c6fd3b18 100644 --- a/src/libutil/unix/environment-variables.cc +++ b/src/libutil/unix/environment-variables.cc @@ -1,21 +1,12 @@ -#include "util.hh" -#include "environment-variables.hh" +#include -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()) - unsetenv(name.first.c_str()); -} - -void replaceEnv(const std::map & newEnv) -{ - clearEnv(); - for (auto & newEnvVar : newEnv) - setenv(newEnvVar.first.c_str(), newEnvVar.second.c_str(), 1); + return ::setenv(name, value, 1); } } diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 8276be8e8..60dea3a80 100644 --- a/src/nix-build/nix-build.cc +++ b/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; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index c1842f2d5..bb96f7786 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.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 diff --git a/src/nix/unix/run.cc b/src/nix/unix/run.cc index e86837679..02e809e5c 100644 --- a/src/nix/unix/run.cc +++ b/src/nix/unix/run.cc @@ -134,7 +134,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment auto unixPath = tokenizeString(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); diff --git a/tests/unit/libexpr/primops.cc b/tests/unit/libexpr/primops.cc index b1426edae..92319e0c3 100644 --- a/tests/unit/libexpr/primops.cc +++ b/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")); }