From 4dbbd721eb9db75d4968a624b8cb9e75e979a144 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Mon, 2 Sep 2024 20:09:35 +0200 Subject: [PATCH 1/3] treewide: consistently mark overridden settings as such Only overridden settings are sent to the daemon, and we're going to do the same for the build hook to. It needs to be ensured that overridden settings are in fact consistently marked as such, so that they actually get sent. Change-Id: I7cd58d925702f86cf2c35ad121eb191ceb62a355 --- src/legacy/nix-store.cc | 14 +++++------ src/libcmd/common-eval-args.cc | 2 +- src/libcmd/installables.cc | 4 +-- src/libcmd/repl.cc | 4 +-- src/libexpr/eval-settings.cc | 2 +- src/libmain/shared.cc | 8 +++--- src/libstore/builtins/fetchurl.cc | 4 +-- src/libstore/daemon.cc | 19 +++++++------- src/libstore/globals.cc | 18 ++++++------- src/libstore/remote-store.cc | 1 + src/libutil/config-impl.hh | 12 +++++++-- src/libutil/config.cc | 5 +--- src/libutil/config.hh | 17 +++---------- src/nix/flake.cc | 4 +-- src/nix/main.cc | 34 +++++++++++-------------- src/nix/repl.cc | 2 +- src/nix/upgrade-nix.cc | 2 +- tests/unit/libstore/machines.cc | 42 +++++++++++++++---------------- tests/unit/libutil/config.cc | 8 +++--- 19 files changed, 97 insertions(+), 105 deletions(-) diff --git a/src/legacy/nix-store.cc b/src/legacy/nix-store.cc index e42aa4065..d4742f61f 100644 --- a/src/legacy/nix-store.cc +++ b/src/legacy/nix-store.cc @@ -831,12 +831,12 @@ static void opServe(Strings opFlags, Strings opArgs) // FIXME: changing options here doesn't work if we're // building through the daemon. verbosity = lvlError; - settings.keepLog = false; - settings.useSubstitutes = false; - settings.maxSilentTime = readInt(in); - settings.buildTimeout = readInt(in); + settings.keepLog.override(false); + settings.useSubstitutes.override(false); + settings.maxSilentTime.override(readInt(in)); + settings.buildTimeout.override(readInt(in)); if (GET_PROTOCOL_MINOR(clientVersion) >= 2) - settings.maxLogSize = readNum(in); + settings.maxLogSize.override(readNum(in)); if (GET_PROTOCOL_MINOR(clientVersion) >= 3) { auto nrRepeats = readInt(in); if (nrRepeats != 0) { @@ -850,10 +850,10 @@ static void opServe(Strings opFlags, Strings opArgs) // asked for. readInt(in); - settings.runDiffHook = true; + settings.runDiffHook.override(true); } if (GET_PROTOCOL_MINOR(clientVersion) >= 7) { - settings.keepFailed = (bool) readInt(in); + settings.keepFailed.override((bool) readInt(in)); } }; diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index cbb7edbdd..089ad31d9 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -137,7 +137,7 @@ MixEvalArgs::MixEvalArgs() .description = "Allow access to mutable paths and repositories.", .category = category, .handler = {[&]() { - evalSettings.pureEval = false; + evalSettings.pureEval.override(false); }}, }); diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 711cf1b07..b81d4e0fd 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -212,7 +212,7 @@ void SourceExprCommand::completeInstallable(AddCompletions & completions, std::s if (file) { completions.setType(AddCompletions::Type::Attrs); - evalSettings.pureEval = false; + evalSettings.pureEval.override(false); auto state = getEvalState(); Expr & e = state->parseExprFromFile( resolveExprPath(state->checkSourcePath(lookupFileArg(*state, *file))) @@ -435,7 +435,7 @@ Installables SourceExprCommand::parseInstallables( throw UsageError("'--file' and '--expr' are exclusive"); // FIXME: backward compatibility hack - if (file) evalSettings.pureEval = false; + if (file) evalSettings.pureEval.override(false); auto state = getEvalState(); auto vFile = state->allocValue(); diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index ce19bc1eb..9d7fd47dc 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -817,10 +817,10 @@ ProcessLineResult NixRepl::processLine(std::string line) else if (command == ":te" || command == ":trace-enable") { if (arg == "false" || (arg == "" && loggerSettings.showTrace)) { std::cout << "not showing error traces\n"; - loggerSettings.showTrace = false; + loggerSettings.showTrace.override(false); } else if (arg == "true" || (arg == "" && !loggerSettings.showTrace)) { std::cout << "showing error traces\n"; - loggerSettings.showTrace = true; + loggerSettings.showTrace.override(true); } else { throw Error("unexpected argument '%s' to %s", arg, command); }; diff --git a/src/libexpr/eval-settings.cc b/src/libexpr/eval-settings.cc index 0bdf1b9a5..a31618929 100644 --- a/src/libexpr/eval-settings.cc +++ b/src/libexpr/eval-settings.cc @@ -47,7 +47,7 @@ static Strings parseNixPath(const std::string & s) EvalSettings::EvalSettings() { auto var = getEnv("NIX_PATH"); - if (var) nixPath = parseNixPath(*var); + if (var) nixPath.setDefault(parseNixPath(*var)); } Strings EvalSettings::getDefaultNixPath() diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 029b457b1..77c497237 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -196,20 +196,20 @@ LegacyArgs::LegacyArgs(const std::string & programName, .longName = "keep-failed", .shortName ='K', .description = "Keep temporary directories of failed builds.", - .handler = {&(bool&) settings.keepFailed, true}, + .handler = {[&]() { settings.keepFailed.override(true); }}, }); addFlag({ .longName = "keep-going", .shortName ='k', .description = "Keep going after a build fails.", - .handler = {&(bool&) settings.keepGoing, true}, + .handler = {[&]() { settings.keepGoing.override(true); }}, }); addFlag({ .longName = "fallback", .description = "Build from source if substitution fails.", - .handler = {&(bool&) settings.tryFallback, true}, + .handler = {[&]() { settings.tryFallback.override(true); }}, }); auto intSettingAlias = [&](char shortName, const std::string & longName, @@ -247,7 +247,7 @@ LegacyArgs::LegacyArgs(const std::string & programName, .longName = "store", .description = "The URL of the Nix store to use.", .labels = {"store-uri"}, - .handler = {&(std::string&) settings.storeUri}, + .handler = {[&](std::string storeUri) { settings.storeUri.override(storeUri); }}, }); } diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index b28eb01d0..69a9f993f 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -13,11 +13,11 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData, this to be stored in a file. It would be nice if we could just pass a pointer to the data. */ if (netrcData != "") { - settings.netrcFile = "netrc"; + settings.netrcFile.override("netrc"); writeFile(settings.netrcFile, netrcData, 0600); } - settings.caFile = "ca-certificates.crt"; + settings.caFile.override("ca-certificates.crt"); writeFile(settings.caFile, caFileData, 0600); auto getAttr = [&](const std::string & name) { diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index a9239197b..93b405c01 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -195,15 +195,15 @@ struct ClientSettings void apply(TrustedFlag trusted) { - settings.keepFailed = keepFailed; - settings.keepGoing = keepGoing; - settings.tryFallback = tryFallback; + settings.keepFailed.override(keepFailed); + settings.keepGoing.override(keepGoing); + settings.tryFallback.override(tryFallback); nix::verbosity = verbosity; - settings.maxBuildJobs.assign(maxBuildJobs); - settings.maxSilentTime = maxSilentTime; + settings.maxBuildJobs.override(maxBuildJobs); + settings.maxSilentTime.override(maxSilentTime); settings.verboseBuild = verboseBuild; - settings.buildCores = buildCores; - settings.useSubstitutes = useSubstitutes; + settings.buildCores.override(buildCores); + settings.useSubstitutes.override(useSubstitutes); for (auto & i : overrides) { auto & name(i.first); @@ -225,12 +225,13 @@ struct ClientSettings else warn("ignoring untrusted substituter '%s', you are not a trusted user.\n" "Run `man nix.conf` for more information on the `substituters` configuration option.", s); - res = subs; + res.override(subs); return true; }; try { - if (name == "ssh-auth-sock") // obsolete + if (name == "ssh-auth-sock" // obsolete + || name == "store") // the daemon *is* the store ; else if (name == experimentalFeatureSettings.experimentalFeatures.name) { // We don’t want to forward the experimental features to diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index f43b759d2..9377ac936 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -69,12 +69,12 @@ Settings::Settings() , nixManDir(canonPath(NIX_MAN_DIR)) , nixDaemonSocketFile(canonPath(getEnvNonEmpty("NIX_DAEMON_SOCKET_PATH").value_or(nixStateDir + DEFAULT_SOCKET_PATH))) { - buildUsersGroup = getuid() == 0 ? "nixbld" : ""; - allowSymlinkedStore = getEnv("NIX_IGNORE_SYMLINK_STORE") == "1"; + buildUsersGroup.setDefault(getuid() == 0 ? "nixbld" : ""); + allowSymlinkedStore.setDefault(getEnv("NIX_IGNORE_SYMLINK_STORE") == "1"); auto sslOverride = getEnv("NIX_SSL_CERT_FILE").value_or(getEnv("SSL_CERT_FILE").value_or("")); if (sslOverride != "") - caFile = sslOverride; + caFile.setDefault(sslOverride); /* Backwards compatibility. */ auto s = getEnv("NIX_REMOTE_SYSTEMS"); @@ -82,17 +82,17 @@ Settings::Settings() Strings ss; for (auto & p : tokenizeString(*s, ":")) ss.push_back("@" + p); - builders = concatStringsSep(" ", ss); + builders.setDefault(concatStringsSep(" ", ss)); } #if defined(__linux__) && defined(SANDBOX_SHELL) - sandboxPaths = tokenizeString("/bin/sh=" SANDBOX_SHELL); + sandboxPaths.setDefault(tokenizeString("/bin/sh=" SANDBOX_SHELL)); #endif /* chroot-like behavior from Apple's sandbox */ #if __APPLE__ - sandboxPaths = tokenizeString("/System/Library/Frameworks /System/Library/PrivateFrameworks /bin/sh /bin/bash /private/tmp /private/var/tmp /usr/lib"); - allowedImpureHostPrefixes = tokenizeString("/System/Library /usr/lib /dev /bin/sh"); + sandboxPaths.setDefault(tokenizeString("/System/Library/Frameworks /System/Library/PrivateFrameworks /bin/sh /bin/bash /private/tmp /private/var/tmp /usr/lib")); + allowedImpureHostPrefixes.setDefault(tokenizeString("/System/Library /usr/lib /dev /bin/sh")); #endif /* Set the build hook location @@ -118,10 +118,10 @@ Settings::Settings() if (!pathExists(nixExePath)) { nixExePath = getSelfExe().value_or("nix"); } - buildHook = { + buildHook.setDefault(Strings { nixExePath, "__build-remote", - }; + }); } void loadConfFile() diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index a9f9818be..ff3722085 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -139,6 +139,7 @@ void RemoteStore::setOptions(Connection & conn) overrides.erase(loggerSettings.showTrace.name); overrides.erase(experimentalFeatureSettings.experimentalFeatures.name); overrides.erase(settings.pluginFiles.name); + overrides.erase(settings.storeUri.name); // the daemon *is* the store conn.to << overrides.size(); for (auto & i : overrides) conn.to << i.first << i.second.value; diff --git a/src/libutil/config-impl.hh b/src/libutil/config-impl.hh index 748107b6e..2342c9a03 100644 --- a/src/libutil/config-impl.hh +++ b/src/libutil/config-impl.hh @@ -65,6 +65,7 @@ void BaseSetting::appendOrSet(T newValue, bool append, const ApplyConfigOptio "using default `appendOrSet` implementation with an appendable type"); assert(!append); + overridden = true; value = std::move(newValue); } @@ -85,6 +86,13 @@ void BaseSetting::set(const std::string & str, bool append, const ApplyConfig } } +template +void BaseSetting::override(const T & v) +{ + overridden = true; + value = v; +} + template<> void BaseSetting::convertToArg(Args & args, const std::string & category); template @@ -95,7 +103,7 @@ void BaseSetting::convertToArg(Args & args, const std::string & category) .description = fmt("Set the `%s` setting.", name), .category = category, .labels = {"value"}, - .handler = {[this](std::string s) { overridden = true; set(s); }}, + .handler = {[this](std::string s) { set(s); }}, .experimentalFeature = experimentalFeature, }); @@ -105,7 +113,7 @@ void BaseSetting::convertToArg(Args & args, const std::string & category) .description = fmt("Append to the `%s` setting.", name), .category = category, .labels = {"value"}, - .handler = {[this](std::string s) { overridden = true; set(s, true); }}, + .handler = {[this](std::string s) { set(s, true); }}, .experimentalFeature = experimentalFeature, }); } diff --git a/src/libutil/config.cc b/src/libutil/config.cc index 778da1413..f600a10ca 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -32,7 +32,6 @@ bool Config::set(const std::string & name, const std::string & value, const Appl return false; } i->second.setting->set(value, append, options); - i->second.setting->overridden = true; return true; } @@ -46,7 +45,6 @@ void Config::addSetting(AbstractSetting * setting) if (auto i = unknownSettings.find(setting->name); i != unknownSettings.end()) { setting->set(std::move(i->second)); - setting->overridden = true; unknownSettings.erase(i); set = true; } @@ -58,7 +56,6 @@ void Config::addSetting(AbstractSetting * setting) alias, setting->name); else { setting->set(std::move(i->second)); - setting->overridden = true; unknownSettings.erase(i); set = true; } @@ -80,7 +77,7 @@ void AbstractConfig::reapplyUnknownSettings() { auto unknownSettings2 = std::move(unknownSettings); unknownSettings = {}; - for (auto & s : unknownSettings2) + for (auto & s : unknownSettings2) set(s.first, s.second); } diff --git a/src/libutil/config.hh b/src/libutil/config.hh index 59cc281c5..b3dcc122f 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -218,6 +218,7 @@ protected: virtual void convertToArg(Args & args, const std::string & category); bool isOverridden() const; + }; /** @@ -267,16 +268,12 @@ public: { } operator const T &() const { return value; } - operator T &() { return value; } const T & get() const { return value; } template bool operator ==(const U & v2) const { return value == v2; } template bool operator !=(const U & v2) const { return value != v2; } template - void operator =(const U & v) { assign(v); } - virtual void assign(const T & v) { value = v; } - template void setDefault(const U & v) { if (!overridden) value = v; } /** @@ -287,6 +284,8 @@ public: */ void set(const std::string & str, bool append = false, const ApplyConfigOptions & options = {}) override final; + void override(const T & v); + /** * C++ trick; This is template-specialized to compile-time indicate whether * the type is appendable. @@ -299,12 +298,6 @@ public: */ bool isAppendable() override final; - virtual void override(const T & v) - { - overridden = true; - value = v; - } - std::string to_string() const override; void convertToArg(Args & args, const std::string & category) override; @@ -349,8 +342,6 @@ public: : Setting(options, def, name, description, aliases, documentDefault, std::move(experimentalFeature), true) { } - - void operator =(const T & v) { this->assign(v); } }; /** @@ -375,8 +366,6 @@ public: } T parse(const std::string & str, const ApplyConfigOptions & options) const override; - - void operator =(const T & v) { this->assign(v); } }; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 0c704a995..1dc13fb3f 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -116,7 +116,7 @@ public: void run(nix::ref store) override { - settings.tarballTtl = 0; + settings.tarballTtl.override(0); auto updateAll = lockFlags.inputUpdates.empty(); lockFlags.recreateLockFile = updateAll; @@ -158,7 +158,7 @@ struct CmdFlakeLock : FlakeCommand void run(nix::ref store) override { - settings.tarballTtl = 0; + settings.tarballTtl.override(0); lockFlags.writeLockFile = true; lockFlags.applyNixConfig = true; diff --git a/src/nix/main.cc b/src/nix/main.cc index fdd3ac2ae..cf1f876dd 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -247,8 +247,8 @@ static void showHelp(std::vector subcommand, NixArgs & toplevel) { auto mdName = subcommand.empty() ? "nix" : fmt("nix3-%s", concatStringsSep("-", subcommand)); - evalSettings.restrictEval = false; - evalSettings.pureEval = false; + evalSettings.restrictEval.override(false); + evalSettings.pureEval.override(false); EvalState state({}, openStore("dummy://")); auto vGenerateManpage = state.allocValue(); @@ -389,7 +389,7 @@ void mainWrapped(int argc, char * * argv) if (legacy) return legacy(argc, argv); } - evalSettings.pureEval = true; + evalSettings.pureEval.setDefault(true); setLogFormat(LogFormat::bar); settings.verboseBuild = false; @@ -408,11 +408,11 @@ void mainWrapped(int argc, char * * argv) } if (argc == 2 && std::string(argv[1]) == "__dump-language") { - experimentalFeatureSettings.experimentalFeatures = ExperimentalFeatures{} + experimentalFeatureSettings.experimentalFeatures.override(ExperimentalFeatures{} | Xp::Flakes | Xp::FetchClosure - | Xp::DynamicDerivations; - evalSettings.pureEval = false; + | Xp::DynamicDerivations); + evalSettings.pureEval.override(false); EvalState state({}, openStore("dummy://")); auto res = nlohmann::json::object(); res["builtins"] = ({ @@ -513,24 +513,20 @@ void mainWrapped(int argc, char * * argv) if (!args.useNet) { // FIXME: should check for command line overrides only. - if (!settings.useSubstitutes.overridden) - settings.useSubstitutes = false; - if (!settings.tarballTtl.overridden) - settings.tarballTtl = std::numeric_limits::max(); - if (!fileTransferSettings.tries.overridden) - fileTransferSettings.tries = 0; - if (!fileTransferSettings.connectTimeout.overridden) - fileTransferSettings.connectTimeout = 1; + settings.useSubstitutes.setDefault(false); + settings.tarballTtl.setDefault(std::numeric_limits::max()); + fileTransferSettings.tries.setDefault(0); + fileTransferSettings.connectTimeout.setDefault(1); } if (args.refresh) { - settings.tarballTtl = 0; - settings.ttlNegativeNarInfoCache = 0; - settings.ttlPositiveNarInfoCache = 0; + settings.tarballTtl.override(0); + settings.ttlNegativeNarInfoCache.override(0); + settings.ttlPositiveNarInfoCache.override(0); } - if (args.command->second->forceImpureByDefault() && !evalSettings.pureEval.overridden) { - evalSettings.pureEval = false; + if (args.command->second->forceImpureByDefault()) { + evalSettings.pureEval.setDefault(false); } args.command->second->run(); } diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 88aeed859..13186277e 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -10,7 +10,7 @@ namespace nix { struct CmdRepl : RawInstallablesCommand { CmdRepl() { - evalSettings.pureEval = false; + evalSettings.pureEval.override(false); } /** diff --git a/src/nix/upgrade-nix.cc b/src/nix/upgrade-nix.cc index 15c5960d4..19f598874 100644 --- a/src/nix/upgrade-nix.cc +++ b/src/nix/upgrade-nix.cc @@ -72,7 +72,7 @@ struct CmdUpgradeNix : MixDryRun, EvalCommand void run(ref store) override { - evalSettings.pureEval = true; + evalSettings.pureEval.override(true); if (profileDir == "") { profileDir = getProfileDir(store); diff --git a/tests/unit/libstore/machines.cc b/tests/unit/libstore/machines.cc index ba27d85b7..8a1aeb56c 100644 --- a/tests/unit/libstore/machines.cc +++ b/tests/unit/libstore/machines.cc @@ -24,20 +24,20 @@ using nix::settings; class Environment : public ::testing::Environment { public: - void SetUp() override { settings.thisSystem = "TEST_ARCH-TEST_OS"; } + void SetUp() override { settings.thisSystem.override("TEST_ARCH-TEST_OS"); } }; testing::Environment* const foo_env = testing::AddGlobalTestEnvironment(new Environment); TEST(machines, getMachinesWithEmptyBuilders) { - settings.builders = ""; + settings.builders.override(""); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(0)); } TEST(machines, getMachinesUriOnly) { - settings.builders = "nix@scratchy.labs.cs.uu.nl"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl"); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(1)); EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq("ssh://nix@scratchy.labs.cs.uu.nl"))); @@ -51,7 +51,7 @@ TEST(machines, getMachinesUriOnly) { } TEST(machines, getMachinesDefaults) { - settings.builders = "nix@scratchy.labs.cs.uu.nl - - - - - - -"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl - - - - - - -"); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(1)); EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq("ssh://nix@scratchy.labs.cs.uu.nl"))); @@ -65,7 +65,7 @@ TEST(machines, getMachinesDefaults) { } TEST(machines, getMachinesWithNewLineSeparator) { - settings.builders = "nix@scratchy.labs.cs.uu.nl\nnix@itchy.labs.cs.uu.nl"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl\nnix@itchy.labs.cs.uu.nl"); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(2)); EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl")))); @@ -73,7 +73,7 @@ TEST(machines, getMachinesWithNewLineSeparator) { } TEST(machines, getMachinesWithSemicolonSeparator) { - settings.builders = "nix@scratchy.labs.cs.uu.nl ; nix@itchy.labs.cs.uu.nl"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl ; nix@itchy.labs.cs.uu.nl"); Machines actual = getMachines(); EXPECT_THAT(actual, SizeIs(2)); EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl")))); @@ -81,9 +81,9 @@ TEST(machines, getMachinesWithSemicolonSeparator) { } TEST(machines, getMachinesWithCorrectCompleteSingleBuilder) { - settings.builders = "nix@scratchy.labs.cs.uu.nl i686-linux " + settings.builders.override("nix@scratchy.labs.cs.uu.nl i686-linux " "/home/nix/.ssh/id_scratchy_auto 8 3 kvm " - "benchmark SSH+HOST+PUBLIC+KEY+BASE64+ENCODED=="; + "benchmark SSH+HOST+PUBLIC+KEY+BASE64+ENCODED=="); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(1)); EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl"))); @@ -98,10 +98,10 @@ TEST(machines, getMachinesWithCorrectCompleteSingleBuilder) { TEST(machines, getMachinesWithCorrectCompleteSingleBuilderWithTabColumnDelimiter) { - settings.builders = + settings.builders.override( "nix@scratchy.labs.cs.uu.nl\ti686-linux\t/home/nix/.ssh/" "id_scratchy_auto\t8\t3\tkvm\tbenchmark\tSSH+HOST+PUBLIC+" - "KEY+BASE64+ENCODED=="; + "KEY+BASE64+ENCODED=="); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(1)); EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl"))); @@ -115,9 +115,9 @@ TEST(machines, } TEST(machines, getMachinesWithMultiOptions) { - settings.builders = "nix@scratchy.labs.cs.uu.nl Arch1,Arch2 - - - " + settings.builders.override("nix@scratchy.labs.cs.uu.nl Arch1,Arch2 - - - " "SupportedFeature1,SupportedFeature2 " - "MandatoryFeature1,MandatoryFeature2"; + "MandatoryFeature1,MandatoryFeature2"); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(1)); EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl"))); @@ -127,15 +127,15 @@ TEST(machines, getMachinesWithMultiOptions) { } TEST(machines, getMachinesWithIncorrectFormat) { - settings.builders = "nix@scratchy.labs.cs.uu.nl - - eight"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl - - eight"); EXPECT_THROW(getMachines(), FormatError); - settings.builders = "nix@scratchy.labs.cs.uu.nl - - -1"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl - - -1"); EXPECT_THROW(getMachines(), FormatError); - settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 three"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl - - 8 three"); EXPECT_THROW(getMachines(), FormatError); - settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 -3"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl - - 8 -3"); EXPECT_THROW(getMachines(), UsageError); - settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 3 - - BAD_BASE64"; + settings.builders.override("nix@scratchy.labs.cs.uu.nl - - 8 3 - - BAD_BASE64"); EXPECT_THROW(getMachines(), FormatError); } @@ -143,7 +143,7 @@ TEST(machines, getMachinesWithCorrectFileReference) { auto path = nix::getUnitTestDataPath("machines.valid"); ASSERT_TRUE(pathExists(path)); - settings.builders = std::string("@") + path; + settings.builders.override(std::string("@") + path); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(3)); EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl")))); @@ -155,18 +155,18 @@ TEST(machines, getMachinesWithCorrectFileReferenceToEmptyFile) { auto path = "/dev/null"; ASSERT_TRUE(pathExists(path)); - settings.builders = std::string("@") + path; + settings.builders.override(std::string("@") + path); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(0)); } TEST(machines, getMachinesWithIncorrectFileReference) { - settings.builders = std::string("@") + absPath("/not/a/file"); + settings.builders.override(std::string("@") + absPath("/not/a/file")); Machines actual = getMachines(); ASSERT_THAT(actual, SizeIs(0)); } TEST(machines, getMachinesWithCorrectFileReferenceToIncorrectFile) { - settings.builders = std::string("@") + nix::getUnitTestDataPath("machines.bad_format"); + settings.builders.override(std::string("@") + nix::getUnitTestDataPath("machines.bad_format")); EXPECT_THROW(getMachines(), FormatError); } diff --git a/tests/unit/libutil/config.cc b/tests/unit/libutil/config.cc index 1600f4ff0..b10b288c1 100644 --- a/tests/unit/libutil/config.cc +++ b/tests/unit/libutil/config.cc @@ -57,7 +57,7 @@ namespace nix { std::map settings; Setting setting{&config, value, "name-of-the-setting", "description"}; - setting.assign("value"); + setting.override("value"); config.getSettings(settings, /* overriddenOnly = */ false); const auto iter = settings.find("name-of-the-setting"); @@ -131,7 +131,7 @@ namespace nix { { std::map settings; - setting.set("foo"); + setting.setDefault("foo"); ASSERT_EQ(setting.get(), "foo"); config.getSettings(settings, /* overriddenOnly = */ true); ASSERT_TRUE(settings.empty()); @@ -170,7 +170,7 @@ namespace nix { "name-of-the-setting", "description", }; - setting.assign("value"); + setting.override("value"); ASSERT_EQ(config.toJSON(), R"#({ @@ -197,7 +197,7 @@ namespace nix { true, Xp::Flakes, }; - setting.assign("value"); + setting.override("value"); ASSERT_EQ(config.toJSON(), R"#({ From ece99fee23d53185436a91f4cdd5cf5ad9652384 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Mon, 2 Sep 2024 20:17:34 +0200 Subject: [PATCH 2/3] libstore/build: only send overridden settings to the build hook The build hook is still running locally, so it will run with the same default settings. Hence, just as with the daemon, it is enough to send it only the overridden settings. This will prevent warnings like warning: Ignoring setting 'auto-allocate-uids' because experimental feature 'auto-allocate-uids' is not enabled when the user didn't actually set those settings. This is inspired by and an alternative to [0]. [0] https://github.com/NixOS/nix/pull/10049 Change-Id: I77ea62cd017614b16b55979dd30e75f09f860d21 --- src/libstore/build/hook-instance.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/hook-instance.cc b/src/libstore/build/hook-instance.cc index 521f34917..9f76eca4d 100644 --- a/src/libstore/build/hook-instance.cc +++ b/src/libstore/build/hook-instance.cc @@ -74,7 +74,7 @@ HookInstance::HookInstance() sink = FdSink(toHook.get()); std::map settings; - globalConfig.getSettings(settings); + globalConfig.getSettings(settings, true); for (auto & setting : settings) sink << 1 << setting.first << setting.second.value; sink << 0; From 689eb45630a183f0fbbd8864cb7a3c7cb1704451 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Tue, 20 Aug 2024 19:18:53 +0200 Subject: [PATCH 3/3] treewide: make more settings conditionally available Some settings only make sense on particular platforms, or only when a certain experimental feature is enabled. Several of those were already conditionally available. Do the same for a bunch more instead of silently ignoring them. Exceptionally, the use-case-hack setting is not made conditional because it is included in the test suite. Change-Id: I29e66ad8ee6178a7c0eff9efb55c3410fae32514 --- src/libstore/globals.hh | 20 ++++++++++++++------ src/nix/develop.cc | 9 ++++++--- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index bfba6ab01..dfb90cbe6 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -380,7 +380,8 @@ public: users in `build-users-group`. UIDs are allocated starting at 872415232 (0x34000000) on Linux and 56930 on macOS. - )"}; + )", + {}, true, Xp::AutoAllocateUids}; Setting startId{this, #if __linux__ @@ -389,7 +390,10 @@ public: 56930, #endif "start-id", - "The first UID and GID to use for dynamic ID allocation."}; + "The first UID and GID to use for dynamic ID allocation.", + {}, + true, + Xp::AutoAllocateUids}; Setting uidCount{this, #if __linux__ @@ -398,7 +402,10 @@ public: 128, #endif "id-count", - "The number of UIDs/GIDs to use for dynamic ID allocation."}; + "The number of UIDs/GIDs to use for dynamic ID allocation.", + {}, + true, + Xp::AutoAllocateUids}; #if __linux__ Setting useCgroups{ @@ -409,12 +416,13 @@ public: Cgroups are required and enabled automatically for derivations that require the `uid-range` system feature. - )"}; - #endif + )", + {}, true, Xp::Cgroups}; Setting impersonateLinux26{this, false, "impersonate-linux-26", "Whether to impersonate a Linux 2.6 machine on newer kernels.", {"build-impersonate-linux-26"}}; + #endif Setting keepLog{ this, true, "keep-build-log", @@ -567,6 +575,7 @@ public: Setting sandboxFallback{this, true, "sandbox-fallback", "Whether to disable sandboxing when the kernel doesn't allow it."}; +#if __linux__ Setting requireDropSupplementaryGroups{this, getuid() == 0, "require-drop-supplementary-groups", R"( Following the principle of least privilege, @@ -585,7 +594,6 @@ public: and `false` otherwise. )"}; -#if __linux__ Setting sandboxShmSize{ this, "50%", "sandbox-dev-shm-size", R"( diff --git a/src/nix/develop.cc b/src/nix/develop.cc index d1615ecdc..81bc73e12 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -19,13 +19,16 @@ using namespace nix; struct DevelopSettings : Config { Setting bashPrompt{this, "", "bash-prompt", - "The bash prompt (`PS1`) in `nix develop` shells."}; + "The bash prompt (`PS1`) in `nix develop` shells.", + {}, true, Xp::NixCommand}; Setting bashPromptPrefix{this, "", "bash-prompt-prefix", - "Prefix prepended to the `PS1` environment variable in `nix develop` shells."}; + "Prefix prepended to the `PS1` environment variable in `nix develop` shells.", + {}, true, Xp::NixCommand}; Setting bashPromptSuffix{this, "", "bash-prompt-suffix", - "Suffix appended to the `PS1` environment variable in `nix develop` shells."}; + "Suffix appended to the `PS1` environment variable in `nix develop` shells.", + {}, true, Xp::NixCommand}; }; static DevelopSettings developSettings;