libutil-tests: Fix crash on Windows
libutil tests were crashing on Windows due to issues finding `environ`. Replace process creation of `getEnv` with a new `getEnvOs` function that uses native windows APIs. Also convert a bunch of `RunOptions` fields to use `OsString` to better reflect the underlying interfaces. Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
This commit is contained in:
committed by
John Ericson
parent
6cbf80a0b0
commit
c9abefbc30
@@ -1,5 +1,6 @@
|
||||
#include "nix/fetchers/fetchers.hh"
|
||||
#include "nix/util/processes.hh"
|
||||
#include "nix/util/environment-variables.hh"
|
||||
#include "nix/util/users.hh"
|
||||
#include "nix/fetchers/cache.hh"
|
||||
#include "nix/store/globals.hh"
|
||||
@@ -16,9 +17,9 @@ namespace nix::fetchers {
|
||||
|
||||
static RunOptions hgOptions(const Strings & args)
|
||||
{
|
||||
auto env = getEnv();
|
||||
auto env = getEnvOs();
|
||||
// Set HGPLAIN: this means we get consistent output from hg and avoids leakage from a user or system .hgrc.
|
||||
env["HGPLAIN"] = "";
|
||||
env[OS_STR("HGPLAIN")] = OS_STR("");
|
||||
|
||||
return {.program = "hg", .lookupPath = true, .args = args, .environment = env};
|
||||
}
|
||||
|
||||
@@ -17,9 +17,12 @@
|
||||
#include <cstdlib>
|
||||
#include <sys/time.h>
|
||||
#include <sys/stat.h>
|
||||
#include <sys/resource.h>
|
||||
#include <unistd.h>
|
||||
#include <signal.h>
|
||||
|
||||
#ifndef _WIN32
|
||||
# include <sys/resource.h>
|
||||
#endif
|
||||
#ifdef __linux__
|
||||
# include <features.h>
|
||||
#endif
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
# include "nix/store/build/derivation-builder.hh"
|
||||
#endif
|
||||
#include "nix/util/processes.hh"
|
||||
#include "nix/util/environment-variables.hh"
|
||||
#include "nix/util/config-global.hh"
|
||||
#include "nix/store/build/worker.hh"
|
||||
#include "nix/util/util.hh"
|
||||
@@ -888,11 +889,12 @@ static void runPostBuildHook(
|
||||
fmt("running post-build-hook '%s'", workerSettings.postBuildHook),
|
||||
Logger::Fields{store.printStorePath(drvPath)});
|
||||
PushActivity pact(act.id);
|
||||
StringMap hookEnvironment = getEnv();
|
||||
OsStringMap hookEnvironment = getEnvOs();
|
||||
|
||||
hookEnvironment.emplace("DRV_PATH", store.printStorePath(drvPath));
|
||||
hookEnvironment.emplace("OUT_PATHS", chomp(concatStringsSep(" ", store.printStorePathSet(outputPaths))));
|
||||
hookEnvironment.emplace("NIX_CONFIG", globalConfig.toKeyValue());
|
||||
hookEnvironment.emplace(OS_STR("DRV_PATH"), string_to_os_string(store.printStorePath(drvPath)));
|
||||
hookEnvironment.emplace(
|
||||
OS_STR("OUT_PATHS"), string_to_os_string(chomp(concatStringsSep(" ", store.printStorePathSet(outputPaths)))));
|
||||
hookEnvironment.emplace(OS_STR("NIX_CONFIG"), string_to_os_string(globalConfig.toKeyValue()));
|
||||
|
||||
struct LogSink : Sink
|
||||
{
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
#include "nix/util/util.hh"
|
||||
#include "nix/util/environment-variables.hh"
|
||||
|
||||
extern char ** environ __attribute__((weak));
|
||||
|
||||
namespace nix {
|
||||
|
||||
std::optional<std::string> getEnv(const std::string & key)
|
||||
@@ -29,24 +27,10 @@ std::optional<OsString> getEnvOsNonEmpty(const OsString & key)
|
||||
return value;
|
||||
}
|
||||
|
||||
StringMap getEnv()
|
||||
{
|
||||
StringMap env;
|
||||
for (size_t i = 0; environ[i]; ++i) {
|
||||
auto s = environ[i];
|
||||
auto eq = strchr(s, '=');
|
||||
if (!eq)
|
||||
// invalid env, just keep going
|
||||
continue;
|
||||
env.emplace(std::string(s, eq), std::string(eq + 1));
|
||||
}
|
||||
return env;
|
||||
}
|
||||
|
||||
void clearEnv()
|
||||
{
|
||||
for (auto & name : getEnv())
|
||||
unsetenv(name.first.c_str());
|
||||
for (auto & [name, value] : getEnvOs())
|
||||
unsetEnvOs(name.c_str());
|
||||
}
|
||||
|
||||
void replaceEnv(const StringMap & newEnv)
|
||||
|
||||
@@ -23,6 +23,11 @@ std::optional<std::string> getEnv(const std::string & key);
|
||||
*/
|
||||
std::optional<OsString> getEnvOs(const OsString & key);
|
||||
|
||||
/**
|
||||
* Like `getEnv`, but using `OsString` to avoid coercions.
|
||||
*/
|
||||
OsStringMap getEnvOs();
|
||||
|
||||
/**
|
||||
* @return a non empty environment variable. Returns nullopt if the env
|
||||
* variable is set to ""
|
||||
@@ -60,6 +65,11 @@ int setEnv(const char * name, const char * value);
|
||||
*/
|
||||
int setEnvOs(const OsString & name, const OsString & value);
|
||||
|
||||
/**
|
||||
* Like `unsetenv`, but using `OsChar` to avoid coercions.
|
||||
*/
|
||||
int unsetEnvOs(const OsChar * name);
|
||||
|
||||
/**
|
||||
* Clear the environment.
|
||||
*/
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
#pragma once
|
||||
///@file
|
||||
|
||||
#include <map>
|
||||
#include <optional>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
@@ -36,6 +37,11 @@ using OsString = std::basic_string<OsChar>;
|
||||
*/
|
||||
using OsStringView = std::basic_string_view<OsChar>;
|
||||
|
||||
/**
|
||||
* `nix::StringMap` counterpart for `OsString`
|
||||
*/
|
||||
using OsStringMap = std::map<OsString, OsString, std::less<>>;
|
||||
|
||||
std::string os_string_to_string(OsStringView path);
|
||||
|
||||
OsString string_to_os_string(std::string_view s);
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
#include "nix/util/types.hh"
|
||||
#include "nix/util/error.hh"
|
||||
#include "nix/util/file-descriptor.hh"
|
||||
#include "nix/util/file-path.hh"
|
||||
#include "nix/util/logging.hh"
|
||||
#include "nix/util/ansicolor.hh"
|
||||
|
||||
@@ -119,8 +120,8 @@ struct RunOptions
|
||||
std::optional<uid_t> uid;
|
||||
std::optional<uid_t> gid;
|
||||
#endif
|
||||
std::optional<Path> chdir;
|
||||
std::optional<StringMap> environment;
|
||||
std::optional<std::filesystem::path> chdir;
|
||||
std::optional<OsStringMap> environment;
|
||||
std::optional<std::string> input;
|
||||
Source * standardIn = nullptr;
|
||||
Sink * standardOut = nullptr;
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
#include <cstdlib>
|
||||
#include <cstring>
|
||||
|
||||
#include "nix/util/environment-variables.hh"
|
||||
|
||||
extern char ** environ __attribute__((weak));
|
||||
|
||||
namespace nix {
|
||||
|
||||
int setEnv(const char * name, const char * value)
|
||||
@@ -14,9 +17,33 @@ std::optional<std::string> getEnvOs(const std::string & key)
|
||||
return getEnv(key);
|
||||
}
|
||||
|
||||
OsStringMap getEnvOs()
|
||||
{
|
||||
OsStringMap env;
|
||||
for (size_t i = 0; environ[i]; ++i) {
|
||||
auto s = environ[i];
|
||||
auto eq = strchr(s, '=');
|
||||
if (!eq)
|
||||
// invalid env, just keep going
|
||||
continue;
|
||||
env.emplace(std::string(s, eq), std::string(eq + 1));
|
||||
}
|
||||
return env;
|
||||
}
|
||||
|
||||
StringMap getEnv()
|
||||
{
|
||||
return getEnvOs();
|
||||
}
|
||||
|
||||
int setEnvOs(const OsString & name, const OsString & value)
|
||||
{
|
||||
return setEnv(name.c_str(), value.c_str());
|
||||
}
|
||||
|
||||
int unsetEnvOs(const OsChar * name)
|
||||
{
|
||||
return unsetenv(name);
|
||||
}
|
||||
|
||||
} // namespace nix
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
#ifdef _WIN32
|
||||
# include "processenv.h"
|
||||
# include "shlwapi.h"
|
||||
|
||||
namespace nix {
|
||||
|
||||
@@ -30,11 +31,50 @@ std::optional<OsString> getEnvOs(const OsString & key)
|
||||
return value;
|
||||
}
|
||||
|
||||
OsStringMap getEnvOs()
|
||||
{
|
||||
OsStringMap env;
|
||||
|
||||
auto freeStrings = [](wchar_t * strings) { FreeEnvironmentStringsW(strings); };
|
||||
auto envStrings = std::unique_ptr<wchar_t, decltype(freeStrings)>(GetEnvironmentStringsW(), freeStrings);
|
||||
auto s = envStrings.get();
|
||||
|
||||
while (true) {
|
||||
auto eq = StrChrW(s, L'=');
|
||||
// Object ends with an empty string, which naturally won't have an =
|
||||
if (eq == nullptr)
|
||||
break;
|
||||
|
||||
auto value_len = lstrlenW(eq + 1);
|
||||
|
||||
env[OsString(s, eq - s)] = OsString(eq + 1, value_len);
|
||||
|
||||
// 1 to skip L'=', then value, then 1 to skip L'\0'
|
||||
s = eq + 1 + value_len + 1;
|
||||
}
|
||||
|
||||
return env;
|
||||
}
|
||||
|
||||
StringMap getEnv()
|
||||
{
|
||||
StringMap env;
|
||||
for (auto & [name, value] : getEnvOs()) {
|
||||
env.emplace(os_string_to_string(name), os_string_to_string(value));
|
||||
}
|
||||
return env;
|
||||
}
|
||||
|
||||
int unsetenv(const char * name)
|
||||
{
|
||||
return -SetEnvironmentVariableA(name, nullptr);
|
||||
}
|
||||
|
||||
int unsetEnvOs(const OsChar * name)
|
||||
{
|
||||
return -SetEnvironmentVariableW(name, nullptr);
|
||||
}
|
||||
|
||||
int setEnv(const char * name, const char * value)
|
||||
{
|
||||
return -SetEnvironmentVariableA(name, value);
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
#include "nix/util/executable-path.hh"
|
||||
#include "nix/util/file-descriptor.hh"
|
||||
#include "nix/util/file-path.hh"
|
||||
#include "nix/util/os-string.hh"
|
||||
#include "nix/util/signals.hh"
|
||||
#include "nix/util/processes.hh"
|
||||
#include "nix/util/finally.hh"
|
||||
@@ -216,18 +217,20 @@ Pid spawnProcess(const Path & realProgram, const RunOptions & options, Pipe & ou
|
||||
startInfo.hStdOutput = out.writeSide.get();
|
||||
startInfo.hStdError = out.writeSide.get();
|
||||
|
||||
std::string envline;
|
||||
// Retain the current processes' environment variables.
|
||||
for (const auto & envVar : getEnv()) {
|
||||
envline += (envVar.first + '=' + envVar.second + '\0');
|
||||
}
|
||||
// Also add new ones specified in options.
|
||||
auto env = getEnvOs();
|
||||
|
||||
if (options.environment) {
|
||||
for (const auto & envVar : *options.environment) {
|
||||
envline += (envVar.first + '=' + envVar.second + '\0');
|
||||
env[envVar.first] = envVar.second;
|
||||
}
|
||||
}
|
||||
|
||||
OsString envline;
|
||||
|
||||
for (const auto & envVar : env) {
|
||||
envline += (envVar.first + L'=' + envVar.second + L'\0');
|
||||
}
|
||||
|
||||
std::string cmdline = windowsEscape(realProgram, false);
|
||||
for (const auto & arg : options.args) {
|
||||
// TODO: This isn't the right way to escape windows command
|
||||
@@ -244,8 +247,8 @@ Pid spawnProcess(const Path & realProgram, const RunOptions & options, Pipe & ou
|
||||
NULL,
|
||||
TRUE,
|
||||
CREATE_UNICODE_ENVIRONMENT | CREATE_SUSPENDED,
|
||||
string_to_os_string(envline).data(),
|
||||
options.chdir.has_value() ? string_to_os_string(*options.chdir).data() : NULL,
|
||||
envline.data(),
|
||||
options.chdir.has_value() ? options.chdir->c_str() : NULL,
|
||||
&startInfo,
|
||||
&procInfo)
|
||||
== 0) {
|
||||
|
||||
@@ -7,14 +7,15 @@
|
||||
#include "nix/cmd/installable-value.hh"
|
||||
#include "nix/cmd/repl.hh"
|
||||
#include "nix/util/processes.hh"
|
||||
#include "nix/util/environment-variables.hh"
|
||||
#include "self-exe.hh"
|
||||
|
||||
namespace nix {
|
||||
|
||||
void runNix(const std::string & program, const Strings & args, const std::optional<std::string> & input = {})
|
||||
{
|
||||
auto subprocessEnv = getEnv();
|
||||
subprocessEnv["NIX_CONFIG"] = globalConfig.toKeyValue();
|
||||
auto subprocessEnv = getEnvOs();
|
||||
subprocessEnv[OS_STR("NIX_CONFIG")] = string_to_os_string(globalConfig.toKeyValue());
|
||||
// isInteractive avoid grabling interactive commands
|
||||
runProgram2(
|
||||
RunOptions{
|
||||
|
||||
Reference in New Issue
Block a user