From d2438f86d50399467b42284a0ba8f74e13b1defe Mon Sep 17 00:00:00 2001 From: matthew Date: Thu, 31 Oct 2019 20:46:49 -0500 Subject: [PATCH 1/5] environment fixes in run Move environment related code to a separate function. Create a new char** if ignoreEnvironment is set rather than calling clearEnv --- src/nix/run.cc | 66 ++++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/src/nix/run.cc b/src/nix/run.cc index ed15527b8..33e821162 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -24,7 +24,7 @@ struct RunCommon : virtual Command { void runProgram(ref store, const std::string & program, - const Strings & args) + const Strings & args, char** env = environ) { stopProgressBar(); @@ -46,12 +46,12 @@ struct RunCommon : virtual Command Strings helperArgs = { chrootHelperName, store->storeDir, store2->realStoreDir, program }; for (auto & arg : args) helperArgs.push_back(arg); - execv(readLink("/proc/self/exe").c_str(), stringsToCharPtrs(helperArgs).data()); + execve(readLink("/proc/self/exe").c_str(), stringsToCharPtrs(helperArgs).data(), env); throw SysError("could not execute chroot helper"); } - execvp(program.c_str(), stringsToCharPtrs(args).data()); + execvpe(program.c_str(), stringsToCharPtrs(args).data(), env); throw SysError("unable to execute '%s'", program); } @@ -61,6 +61,7 @@ struct CmdRun : InstallablesCommand, RunCommon { std::vector command = { "bash" }; StringSet keep, unset; + Strings stringEnv; bool ignoreEnvironment = false; CmdRun() @@ -126,42 +127,44 @@ struct CmdRun : InstallablesCommand, RunCommon }; } + char** newEnviron() { + if (ignoreEnvironment) { + + if (!unset.empty()) + throw UsageError("--unset does not make sense with --ignore-environment"); + + for (const auto & var : keep) { + auto val = getenv(var.c_str()); + if (val) stringEnv.emplace_back(fmt("%s=%s", var.c_str(), val)); + } + + return stringsToCharPtrs(stringEnv).data(); + + } else { + if (!keep.empty()) + throw UsageError("--keep does not make sense without --ignore-environment"); + + for (const auto & var : unset) + unsetenv(var.c_str()); + + return environ; + } + } + void run(ref store) override { auto outPaths = toStorePaths(store, Build, installables); auto accessor = store->getFSAccessor(); - if (ignoreEnvironment) { - - if (!unset.empty()) - throw UsageError("--unset does not make sense with --ignore-environment"); - - std::map kept; - for (auto & var : keep) { - auto s = getenv(var.c_str()); - if (s) kept[var] = s; - } - - clearEnv(); - - for (auto & var : kept) - setenv(var.first.c_str(), var.second.c_str(), 1); - - } else { - - if (!keep.empty()) - throw UsageError("--keep does not make sense without --ignore-environment"); - - for (auto & var : unset) - unsetenv(var.c_str()); - } std::unordered_set done; std::queue todo; for (auto & path : outPaths) todo.push(path); - auto unixPath = tokenizeString(getEnv("PATH"), ":"); + Strings unixPath; + if (!ignoreEnvironment || keep.find("PATH") != keep.end()) + unixPath = tokenizeString(getEnv("PATH"), ":"); while (!todo.empty()) { Path path = todo.front(); @@ -179,11 +182,16 @@ struct CmdRun : InstallablesCommand, RunCommon } setenv("PATH", concatStringsSep(":", unixPath).c_str(), 1); + if (ignoreEnvironment) { + keep.emplace("PATH"); + } else { + unset.erase("PATH"); + } Strings args; for (auto & arg : command) args.push_back(arg); - runProgram(store, *command.begin(), args); + runProgram(store, *command.begin(), args, newEnviron()); } }; From 693e8b1286dc3f39eb00871c91aaf96773e24a68 Mon Sep 17 00:00:00 2001 From: matthew Date: Mon, 4 Nov 2019 18:40:25 -0600 Subject: [PATCH 2/5] changes --- src/nix/run.cc | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/nix/run.cc b/src/nix/run.cc index 33e821162..d65a642c2 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -24,7 +24,7 @@ struct RunCommon : virtual Command { void runProgram(ref store, const std::string & program, - const Strings & args, char** env = environ) + const Strings & args) { stopProgressBar(); @@ -46,12 +46,12 @@ struct RunCommon : virtual Command Strings helperArgs = { chrootHelperName, store->storeDir, store2->realStoreDir, program }; for (auto & arg : args) helperArgs.push_back(arg); - execve(readLink("/proc/self/exe").c_str(), stringsToCharPtrs(helperArgs).data(), env); + execv(readLink("/proc/self/exe").c_str(), stringsToCharPtrs(helperArgs).data()); throw SysError("could not execute chroot helper"); } - execvpe(program.c_str(), stringsToCharPtrs(args).data(), env); + execvp(program.c_str(), stringsToCharPtrs(args).data()); throw SysError("unable to execute '%s'", program); } @@ -127,7 +127,7 @@ struct CmdRun : InstallablesCommand, RunCommon }; } - char** newEnviron() { + void setNewEnviron() { if (ignoreEnvironment) { if (!unset.empty()) @@ -138,7 +138,7 @@ struct CmdRun : InstallablesCommand, RunCommon if (val) stringEnv.emplace_back(fmt("%s=%s", var.c_str(), val)); } - return stringsToCharPtrs(stringEnv).data(); + environ = stringsToCharPtrs(stringEnv).data(); } else { if (!keep.empty()) @@ -146,8 +146,6 @@ struct CmdRun : InstallablesCommand, RunCommon for (const auto & var : unset) unsetenv(var.c_str()); - - return environ; } } @@ -162,9 +160,9 @@ struct CmdRun : InstallablesCommand, RunCommon std::queue todo; for (auto & path : outPaths) todo.push(path); - Strings unixPath; - if (!ignoreEnvironment || keep.find("PATH") != keep.end()) - unixPath = tokenizeString(getEnv("PATH"), ":"); + setNewEnviron(); + + auto unixPath = tokenizeString(getEnv("PATH"), ":"); while (!todo.empty()) { Path path = todo.front(); @@ -182,16 +180,11 @@ struct CmdRun : InstallablesCommand, RunCommon } setenv("PATH", concatStringsSep(":", unixPath).c_str(), 1); - if (ignoreEnvironment) { - keep.emplace("PATH"); - } else { - unset.erase("PATH"); - } Strings args; for (auto & arg : command) args.push_back(arg); - runProgram(store, *command.begin(), args, newEnviron()); + runProgram(store, *command.begin(), args); } }; From 75c897cf3d2be5cc156a87ec54c6726e8dc2a926 Mon Sep 17 00:00:00 2001 From: matthew Date: Thu, 7 Nov 2019 17:16:48 -0600 Subject: [PATCH 3/5] Factor out code to handle environment in run into MixEnvironment --- src/nix/command.cc | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/nix/command.hh | 15 ++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/nix/command.cc b/src/nix/command.cc index 1cb4cc92a..aa5c8cf1e 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -128,4 +128,48 @@ MixDefaultProfile::MixDefaultProfile() profile = getDefaultProfile(); } +MixEnvironment::MixEnvironment() : ignoreEnvironment(false) { + mkFlag() + .longName("ignore-environment") + .shortName('i') + .description("clear the entire environment (except those specified with --keep)") + .set(&ignoreEnvironment, true); + + mkFlag() + .longName("keep") + .shortName('k') + .description("keep specified environment variable") + .arity(1) + .labels({"name"}) + .handler([&](std::vector ss) { keep.insert(ss.front()); }); + + mkFlag() + .longName("unset") + .shortName('u') + .description("unset specified environment variable") + .arity(1) + .labels({"name"}) + .handler([&](std::vector ss) { unset.insert(ss.front()); }); +} + +void MixEnvironment::setEnviron() { + if (ignoreEnvironment) { + if (!unset.empty()) + throw UsageError("--unset does not make sense with --ignore-environment"); + + for (const auto & var : keep) { + auto val = getenv(var.c_str()); + if (val) stringEnv.emplace_back(fmt("%s=%s", var.c_str(), val)); + } + vectorEnv = stringsToCharPtrs(stringsEnv); + environ = vectorEnv.data(); + } else { + if (!keep.empty()) + throw UsageError("--keep does not make sense without --ignore-environment"); + + for (const auto & var : unset) + unsetenv(var.c_str()); + } +} + } diff --git a/src/nix/command.hh b/src/nix/command.hh index 13f3a0dc9..52fb4e1f5 100644 --- a/src/nix/command.hh +++ b/src/nix/command.hh @@ -194,4 +194,17 @@ struct MixDefaultProfile : MixProfile MixDefaultProfile(); }; -} +struct MixEnvironment : virtual Args { + + StringSet keep, unset; + Strings stringsEnv; + std::vector vectorEnv; + bool ignoreEnvironment; + + MixEnvironment(); + + /* Modify global environ based on ignoreEnvironment, keep, and unset. It's expected that exec will be called before this class goes out of scope, otherwise environ will become invalid. */ + void setEnviron(); +}; + +} \ No newline at end of file From 6419f5028b30e8e43222d71d9fd45fd674eed1b7 Mon Sep 17 00:00:00 2001 From: matthew Date: Thu, 7 Nov 2019 17:18:31 -0600 Subject: [PATCH 4/5] use MixEnvironment in run and shell --- src/nix/run.cc | 51 ++---------------------------------------------- src/nix/shell.cc | 4 +++- 2 files changed, 5 insertions(+), 50 deletions(-) diff --git a/src/nix/run.cc b/src/nix/run.cc index d65a642c2..0fbd0b8a8 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -57,12 +57,9 @@ struct RunCommon : virtual Command } }; -struct CmdRun : InstallablesCommand, RunCommon +struct CmdRun : InstallablesCommand, RunCommon, MixEnvironment { std::vector command = { "bash" }; - StringSet keep, unset; - Strings stringEnv; - bool ignoreEnvironment = false; CmdRun() { @@ -76,28 +73,6 @@ struct CmdRun : InstallablesCommand, RunCommon if (ss.empty()) throw UsageError("--command requires at least one argument"); command = ss; }); - - mkFlag() - .longName("ignore-environment") - .shortName('i') - .description("clear the entire environment (except those specified with --keep)") - .set(&ignoreEnvironment, true); - - mkFlag() - .longName("keep") - .shortName('k') - .description("keep specified environment variable") - .arity(1) - .labels({"name"}) - .handler([&](std::vector ss) { keep.insert(ss.front()); }); - - mkFlag() - .longName("unset") - .shortName('u') - .description("unset specified environment variable") - .arity(1) - .labels({"name"}) - .handler([&](std::vector ss) { unset.insert(ss.front()); }); } std::string description() override @@ -127,28 +102,6 @@ struct CmdRun : InstallablesCommand, RunCommon }; } - void setNewEnviron() { - if (ignoreEnvironment) { - - if (!unset.empty()) - throw UsageError("--unset does not make sense with --ignore-environment"); - - for (const auto & var : keep) { - auto val = getenv(var.c_str()); - if (val) stringEnv.emplace_back(fmt("%s=%s", var.c_str(), val)); - } - - environ = stringsToCharPtrs(stringEnv).data(); - - } else { - if (!keep.empty()) - throw UsageError("--keep does not make sense without --ignore-environment"); - - for (const auto & var : unset) - unsetenv(var.c_str()); - } - } - void run(ref store) override { auto outPaths = toStorePaths(store, Build, installables); @@ -160,7 +113,7 @@ struct CmdRun : InstallablesCommand, RunCommon std::queue todo; for (auto & path : outPaths) todo.push(path); - setNewEnviron(); + setEnviron(); auto unixPath = tokenizeString(getEnv("PATH"), ":"); diff --git a/src/nix/shell.cc b/src/nix/shell.cc index 50d0f9c88..5e13604bc 100644 --- a/src/nix/shell.cc +++ b/src/nix/shell.cc @@ -229,7 +229,7 @@ struct Common : InstallableCommand, MixProfile } }; -struct CmdDevShell : Common +struct CmdDevShell : Common, MixEnvironment { std::string description() override { @@ -275,6 +275,8 @@ struct CmdDevShell : Common auto shell = getEnv("SHELL", "bash"); + setEnviron(); + auto args = Strings{baseNameOf(shell), "--rcfile", rcFilePath}; restoreAffinity(); From 062012eee15810de522613f1ca3ca9df75fa39e9 Mon Sep 17 00:00:00 2001 From: matthew Date: Sun, 1 Dec 2019 18:34:59 -0700 Subject: [PATCH 5/5] typo --- src/nix/command.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/command.cc b/src/nix/command.cc index aa5c8cf1e..06655b8e2 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -159,7 +159,7 @@ void MixEnvironment::setEnviron() { for (const auto & var : keep) { auto val = getenv(var.c_str()); - if (val) stringEnv.emplace_back(fmt("%s=%s", var.c_str(), val)); + if (val) stringsEnv.emplace_back(fmt("%s=%s", var.c_str(), val)); } vectorEnv = stringsToCharPtrs(stringsEnv); environ = vectorEnv.data();