From ff4dea63c9403880500f82ce273713ecf793d2d9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 29 Oct 2020 18:17:39 +0100 Subject: [PATCH] Generalize extra-* settings This removes the extra-substituters and extra-sandbox-paths settings and instead makes every array setting extensible by setting "extra- = " in the configuration file or passing "-- " on the command line. --- .../src/command-ref/conf-file-prefix.md | 27 ++++++--- src/libstore/build/derivation-goal.cc | 6 +- src/libstore/daemon.cc | 2 - src/libstore/globals.cc | 9 ++- src/libstore/globals.hh | 21 +------ src/libstore/store-api.cc | 3 - src/libutil/config.cc | 58 +++++++++++++++---- src/libutil/config.hh | 11 +++- 8 files changed, 83 insertions(+), 54 deletions(-) diff --git a/doc/manual/src/command-ref/conf-file-prefix.md b/doc/manual/src/command-ref/conf-file-prefix.md index e53a3ddf4..3140170ab 100644 --- a/doc/manual/src/command-ref/conf-file-prefix.md +++ b/doc/manual/src/command-ref/conf-file-prefix.md @@ -22,19 +22,30 @@ By default Nix reads settings from the following places: - If `NIX_CONFIG` is set, its contents is treated as the contents of a configuration file. -The configuration files consist of `name = -value` pairs, one per line. Other files can be included with a line like -`include -path`, where *path* is interpreted relative to the current conf file and -a missing file is an error unless `!include` is used instead. Comments +The configuration files consist of `name = value` pairs, one per +line. Other files can be included with a line like `include path`, +where *path* is interpreted relative to the current conf file and a +missing file is an error unless `!include` is used instead. Comments start with a `#` character. Here is an example configuration file: keep-outputs = true # Nice for developers keep-derivations = true # Idem -You can override settings on the command line using the `--option` flag, -e.g. `--option keep-outputs -false`. +You can override settings on the command line using the `--option` +flag, e.g. `--option keep-outputs false`. Every configuration setting +also has a corresponding command line flag, e.g. `--max-jobs 16`; for +Boolean settings, there are two flags to enable or disable the setting +(e.g. `--keep-failed` and `--no-keep-failed`). + +A configuration setting usually overrides any previous value. However, +you can prefix the name of the setting by `extra-` to *append* to the +previous value. For instance, + + substituters = a b + extra-substituters = c d + +defines the `substituters` setting to be `a b c d`. This is also +available as a command line flag (e.g. `--extra-substituters`). The following settings are currently available: diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 7db83c8be..3dacb218c 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1332,13 +1332,9 @@ void DerivationGoal::startBuilder() /* Allow a user-configurable set of directories from the host file system. */ - PathSet dirs = settings.sandboxPaths; - PathSet dirs2 = settings.extraSandboxPaths; - dirs.insert(dirs2.begin(), dirs2.end()); - dirsInChroot.clear(); - for (auto i : dirs) { + for (auto i : settings.sandboxPaths.get()) { if (i.empty()) continue; bool optional = false; if (i[i.size() - 1] == '?') { diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 99d8add92..4dbc7ba38 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -231,8 +231,6 @@ struct ClientSettings settings.set(name, value); else if (setSubstituters(settings.substituters)) ; - else if (setSubstituters(settings.extraSubstituters)) - ; else debug("ignoring the client-specified setting '%s', because it is a restricted setting and you are not a trusted user", name); } catch (UsageError & e) { diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 4df68d0c9..f38601d6d 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -160,7 +160,7 @@ NLOHMANN_JSON_SERIALIZE_ENUM(SandboxMode, { {SandboxMode::smDisabled, false}, }); -template<> void BaseSetting::set(const std::string & str) +template<> void BaseSetting::set(const std::string & str, bool append) { if (str == "true") value = smEnabled; else if (str == "relaxed") value = smRelaxed; @@ -168,6 +168,11 @@ template<> void BaseSetting::set(const std::string & str) else throw UsageError("option '%s' has invalid value '%s'", name, str); } +template<> bool BaseSetting::isAppendable() +{ + return false; +} + template<> std::string BaseSetting::to_string() const { if (value == smEnabled) return "true"; @@ -198,7 +203,7 @@ template<> void BaseSetting::convertToArg(Args & args, const std::s }); } -void MaxBuildJobsSetting::set(const std::string & str) +void MaxBuildJobsSetting::set(const std::string & str, bool append) { if (str == "auto") value = std::max(1U, std::thread::hardware_concurrency()); else if (!string2Int(str, value)) diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 8c63c5b34..eabd83e3f 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -25,7 +25,7 @@ struct MaxBuildJobsSetting : public BaseSetting options->addSetting(this); } - void set(const std::string & str) override; + void set(const std::string & str, bool append = false) override; }; class Settings : public Config { @@ -413,14 +413,6 @@ public: Setting sandboxFallback{this, true, "sandbox-fallback", "Whether to disable sandboxing when the kernel doesn't allow it."}; - Setting extraSandboxPaths{ - this, {}, "extra-sandbox-paths", - R"( - A list of additional paths appended to `sandbox-paths`. Useful if - you want to extend its default value. - )", - {"build-extra-chroot-dirs", "build-extra-sandbox-paths"}}; - Setting buildRepeat{ this, 0, "repeat", R"( @@ -599,17 +591,6 @@ public: )", {"binary-caches"}}; - // FIXME: provide a way to add to option values. - Setting extraSubstituters{ - this, {}, "extra-substituters", - R"( - Additional binary caches appended to those specified in - `substituters`. When used by unprivileged users, untrusted - substituters (i.e. those not listed in `trusted-substituters`) are - silently ignored. - )", - {"extra-binary-caches"}}; - Setting trustedSubstituters{ this, {}, "trusted-substituters", R"( diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 9f21f0434..83d3a1fa1 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1114,9 +1114,6 @@ std::list> getDefaultSubstituters() for (auto uri : settings.substituters.get()) addStore(uri); - for (auto uri : settings.extraSubstituters.get()) - addStore(uri); - stores.sort([](ref & a, ref & b) { return a->priority < b->priority; }); diff --git a/src/libutil/config.cc b/src/libutil/config.cc index eef01bde2..116dd6bfe 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -8,9 +8,18 @@ namespace nix { bool Config::set(const std::string & name, const std::string & value) { + bool append = false; auto i = _settings.find(name); - if (i == _settings.end()) return false; - i->second.setting->set(value); + if (i == _settings.end()) { + if (hasPrefix(name, "extra-")) { + i = _settings.find(std::string(name, 6)); + if (i == _settings.end() || !i->second.setting->isAppendable()) + return false; + append = true; + } else + return false; + } + i->second.setting->set(value, append); i->second.setting->overriden = true; return true; } @@ -180,6 +189,12 @@ void AbstractSetting::convertToArg(Args & args, const std::string & category) { } +template +bool BaseSetting::isAppendable() +{ + return false; +} + template void BaseSetting::convertToArg(Args & args, const std::string & category) { @@ -190,9 +205,18 @@ void BaseSetting::convertToArg(Args & args, const std::string & category) .labels = {"value"}, .handler = {[=](std::string s) { overriden = true; set(s); }}, }); + + if (isAppendable()) + args.addFlag({ + .longName = "extra-" + name, + .description = description, + .category = category, + .labels = {"value"}, + .handler = {[=](std::string s) { overriden = true; set(s, true); }}, + }); } -template<> void BaseSetting::set(const std::string & str) +template<> void BaseSetting::set(const std::string & str, bool append) { value = str; } @@ -203,7 +227,7 @@ template<> std::string BaseSetting::to_string() const } template -void BaseSetting::set(const std::string & str) +void BaseSetting::set(const std::string & str, bool append) { static_assert(std::is_integral::value, "Integer required."); if (!string2Int(str, value)) @@ -217,7 +241,7 @@ std::string BaseSetting::to_string() const return std::to_string(value); } -template<> void BaseSetting::set(const std::string & str) +template<> void BaseSetting::set(const std::string & str, bool append) { if (str == "true" || str == "yes" || str == "1") value = true; @@ -248,9 +272,16 @@ template<> void BaseSetting::convertToArg(Args & args, const std::string & }); } -template<> void BaseSetting::set(const std::string & str) +template<> void BaseSetting::set(const std::string & str, bool append) { - value = tokenizeString(str); + auto ss = tokenizeString(str); + if (!append) value.clear(); + for (auto & s : ss) value.push_back(std::move(s)); +} + +template<> bool BaseSetting::isAppendable() +{ + return true; } template<> std::string BaseSetting::to_string() const @@ -258,7 +289,7 @@ template<> std::string BaseSetting::to_string() const return concatStringsSep(" ", value); } -template<> void BaseSetting::set(const std::string & str) +template<> void BaseSetting::set(const std::string & str, bool append) { value = tokenizeString(str); } @@ -268,9 +299,9 @@ template<> std::string BaseSetting::to_string() const return concatStringsSep(" ", value); } -template<> void BaseSetting::set(const std::string & str) +template<> void BaseSetting::set(const std::string & str, bool append) { - value.clear(); + if (!append) value.clear(); auto kvpairs = tokenizeString(str); for (auto & s : kvpairs) { @@ -281,6 +312,11 @@ template<> void BaseSetting::set(const std::string & str) } } +template<> bool BaseSetting::isAppendable() +{ + return true; +} + template<> std::string BaseSetting::to_string() const { Strings kvstrs; @@ -301,7 +337,7 @@ template class BaseSetting; template class BaseSetting; template class BaseSetting; -void PathSetting::set(const std::string & str) +void PathSetting::set(const std::string & str, bool append) { if (str == "") { if (allowEmpty) diff --git a/src/libutil/config.hh b/src/libutil/config.hh index 1f5f4e7b9..71e31656d 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -202,7 +202,10 @@ protected: assert(created == 123); } - virtual void set(const std::string & value) = 0; + virtual void set(const std::string & value, bool append = false) = 0; + + virtual bool isAppendable() + { return false; } virtual std::string to_string() const = 0; @@ -243,7 +246,9 @@ public: void operator =(const T & v) { assign(v); } virtual void assign(const T & v) { value = v; } - void set(const std::string & str) override; + void set(const std::string & str, bool append = false) override; + + bool isAppendable() override; virtual void override(const T & v) { @@ -305,7 +310,7 @@ public: options->addSetting(this); } - void set(const std::string & str) override; + void set(const std::string & str, bool append = false) override; Path operator +(const char * p) const { return value + p; }