Generalize extra-* settings

This removes the extra-substituters and extra-sandbox-paths settings
and instead makes every array setting extensible by setting
"extra-<name> = <value>" in the configuration file or passing
"--<name> <value>" on the command line.

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:
 

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] == '?') {

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) {

src/libstore/globals.cc

@@ -160,7 +160,7 @@ NLOHMANN_JSON_SERIALIZE_ENUM(SandboxMode, {
     {SandboxMode::smDisabled, false},
 });
 
-template<> void BaseSetting<SandboxMode>::set(const std::string & str)
+template<> void BaseSetting<SandboxMode>::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<SandboxMode>::set(const std::string & str)
     else throw UsageError("option '%s' has invalid value '%s'", name, str);
 }
 
+template<> bool BaseSetting<SandboxMode>::isAppendable()
+{
+    return false;
+}
+
 template<> std::string BaseSetting<SandboxMode>::to_string() const
 {
     if (value == smEnabled) return "true";
@@ -198,7 +203,7 @@ template<> void BaseSetting<SandboxMode>::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))

src/libstore/globals.hh

@@ -25,7 +25,7 @@ struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
         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<bool> sandboxFallback{this, true, "sandbox-fallback",
         "Whether to disable sandboxing when the kernel doesn't allow it."};
 
-    Setting<PathSet> 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<size_t> buildRepeat{
         this, 0, "repeat",
         R"(
@@ -599,17 +591,6 @@ public:
         )",
         {"binary-caches"}};
 
-    // FIXME: provide a way to add to option values.
-    Setting<Strings> 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<StringSet> trustedSubstituters{
         this, {}, "trusted-substituters",
         R"(

src/libstore/store-api.cc

@@ -1114,9 +1114,6 @@ std::list<ref<Store>> getDefaultSubstituters()
         for (auto uri : settings.substituters.get())
             addStore(uri);
 
-        for (auto uri : settings.extraSubstituters.get())
-            addStore(uri);
-
         stores.sort([](ref<Store> & a, ref<Store> & b) {
             return a->priority < b->priority;
         });

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<typename T>
+bool BaseSetting<T>::isAppendable()
+{
+    return false;
+}
+
 template<typename T>
 void BaseSetting<T>::convertToArg(Args & args, const std::string & category)
 {
@@ -190,9 +205,18 @@ void BaseSetting<T>::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<std::string>::set(const std::string & str)
+template<> void BaseSetting<std::string>::set(const std::string & str, bool append)
 {
     value = str;
 }
@@ -203,7 +227,7 @@ template<> std::string BaseSetting<std::string>::to_string() const
 }
 
 template<typename T>
-void BaseSetting<T>::set(const std::string & str)
+void BaseSetting<T>::set(const std::string & str, bool append)
 {
     static_assert(std::is_integral<T>::value, "Integer required.");
     if (!string2Int(str, value))
@@ -217,7 +241,7 @@ std::string BaseSetting<T>::to_string() const
     return std::to_string(value);
 }
 
-template<> void BaseSetting<bool>::set(const std::string & str)
+template<> void BaseSetting<bool>::set(const std::string & str, bool append)
 {
     if (str == "true" || str == "yes" || str == "1")
         value = true;
@@ -248,9 +272,16 @@ template<> void BaseSetting<bool>::convertToArg(Args & args, const std::string &
     });
 }
 
-template<> void BaseSetting<Strings>::set(const std::string & str)
+template<> void BaseSetting<Strings>::set(const std::string & str, bool append)
 {
-    value = tokenizeString<Strings>(str);
+    auto ss = tokenizeString<Strings>(str);
+    if (!append) value.clear();
+    for (auto & s : ss) value.push_back(std::move(s));
+}
+
+template<> bool BaseSetting<Strings>::isAppendable()
+{
+    return true;
 }
 
 template<> std::string BaseSetting<Strings>::to_string() const
@@ -258,7 +289,7 @@ template<> std::string BaseSetting<Strings>::to_string() const
     return concatStringsSep(" ", value);
 }
 
-template<> void BaseSetting<StringSet>::set(const std::string & str)
+template<> void BaseSetting<StringSet>::set(const std::string & str, bool append)
 {
     value = tokenizeString<StringSet>(str);
 }
@@ -268,9 +299,9 @@ template<> std::string BaseSetting<StringSet>::to_string() const
     return concatStringsSep(" ", value);
 }
 
-template<> void BaseSetting<StringMap>::set(const std::string & str)
+template<> void BaseSetting<StringMap>::set(const std::string & str, bool append)
 {
-    value.clear();
+    if (!append) value.clear();
     auto kvpairs = tokenizeString<Strings>(str);
     for (auto & s : kvpairs)
     {
@@ -281,6 +312,11 @@ template<> void BaseSetting<StringMap>::set(const std::string & str)
     }
 }
 
+template<> bool BaseSetting<StringMap>::isAppendable()
+{
+    return true;
+}
+
 template<> std::string BaseSetting<StringMap>::to_string() const
 {
     Strings kvstrs;
@@ -301,7 +337,7 @@ template class BaseSetting<Strings>;
 template class BaseSetting<StringSet>;
 template class BaseSetting<StringMap>;
 
-void PathSetting::set(const std::string & str)
+void PathSetting::set(const std::string & str, bool append)
 {
     if (str == "") {
         if (allowEmpty)

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; }