Merge pull request #7609 from obsidiansystems/hide-experimental-settings

Hide experimental settings

src/libutil/args.cc

@@ -52,7 +52,7 @@ std::shared_ptr<Completions> completions;
 
 std::string completionMarker = "___COMPLETE___";
 
-std::optional<std::string> needsCompletion(std::string_view s)
+static std::optional<std::string> needsCompletion(std::string_view s)
 {
     if (!completions) return {};
     auto i = s.find(completionMarker);
@@ -120,6 +120,12 @@ void Args::parseCmdline(const Strings & _cmdline)
 
     if (!argsSeen)
         initialFlagsProcessed();
+
+    /* Now that we are done parsing, make sure that any experimental
+     * feature required by the flags is enabled */
+    for (auto & f : flagExperimentalFeatures)
+        experimentalFeatureSettings.require(f);
+
 }
 
 bool Args::processFlag(Strings::iterator & pos, Strings::iterator end)
@@ -128,12 +134,18 @@ bool Args::processFlag(Strings::iterator & pos, Strings::iterator end)
 
     auto process = [&](const std::string & name, const Flag & flag) -> bool {
         ++pos;
+
+        if (auto & f = flag.experimentalFeature)
+            flagExperimentalFeatures.insert(*f);
+
         std::vector<std::string> args;
         bool anyCompleted = false;
         for (size_t n = 0 ; n < flag.handler.arity; ++n) {
             if (pos == end) {
                 if (flag.handler.arity == ArityAny || anyCompleted) break;
-                throw UsageError("flag '%s' requires %d argument(s)", name, flag.handler.arity);
+                throw UsageError(
+                    "flag '%s' requires %d argument(s), but only %d were given",
+                    name, flag.handler.arity, n);
             }
             if (auto prefix = needsCompletion(*pos)) {
                 anyCompleted = true;
@@ -152,7 +164,11 @@ bool Args::processFlag(Strings::iterator & pos, Strings::iterator end)
             for (auto & [name, flag] : longFlags) {
                 if (!hiddenCategories.count(flag->category)
                     && hasPrefix(name, std::string(*prefix, 2)))
+                {
+                    if (auto & f = flag->experimentalFeature)
+                        flagExperimentalFeatures.insert(*f);
                     completions->add("--" + name, flag->description);
+                }
             }
             return false;
         }
@@ -172,7 +188,8 @@ bool Args::processFlag(Strings::iterator & pos, Strings::iterator end)
         if (prefix == "-") {
             completions->add("--");
             for (auto & [flagName, flag] : shortFlags)
-                completions->add(std::string("-") + flagName, flag->description);
+                if (experimentalFeatureSettings.isEnabled(flag->experimentalFeature))
+                    completions->add(std::string("-") + flagName, flag->description);
         }
     }
 
@@ -219,6 +236,8 @@ nlohmann::json Args::toJSON()
     auto flags = nlohmann::json::object();
 
     for (auto & [name, flag] : longFlags) {
+        /* Skip experimental flags when listing flags. */
+        if (!experimentalFeatureSettings.isEnabled(flag->experimentalFeature)) continue;
         auto j = nlohmann::json::object();
         if (flag->aliases.count(name)) continue;
         if (flag->shortName)

src/libutil/args.hh

@@ -117,6 +117,8 @@ protected:
         Handler handler;
         std::function<void(size_t, std::string_view)> completer;
 
+        std::optional<ExperimentalFeature> experimentalFeature;
+
         static Flag mkHashTypeFlag(std::string && longName, HashType * ht);
         static Flag mkHashTypeOptFlag(std::string && longName, std::optional<HashType> * oht);
     };
@@ -188,6 +190,16 @@ public:
     friend class MultiCommand;
 
     MultiCommand * parent = nullptr;
+
+private:
+
+    /**
+     * Experimental features needed when parsing args. These are checked
+     * after flag parsing is completed in order to support enabling
+     * experimental features coming after the flag that needs the
+     * experimental feature.
+     */
+    std::set<ExperimentalFeature> flagExperimentalFeatures;
 };
 
 /* A command is an argument parser that can be executed by calling its
@@ -253,8 +265,6 @@ enum CompletionType {
 };
 extern CompletionType completionType;
 
-std::optional<std::string> needsCompletion(std::string_view s);
-
 void completePath(size_t, std::string_view prefix);
 
 void completeDir(size_t, std::string_view prefix);

src/libutil/config.cc

@@ -70,10 +70,17 @@ void AbstractConfig::reapplyUnknownSettings()
         set(s.first, s.second);
 }
 
+// Whether we should process the option. Excludes aliases, which are handled elsewhere, and disabled features.
+static bool applicable(const Config::SettingData & sd)
+{
+    return !sd.isAlias
+        && experimentalFeatureSettings.isEnabled(sd.setting->experimentalFeature);
+}
+
 void Config::getSettings(std::map<std::string, SettingInfo> & res, bool overriddenOnly)
 {
     for (auto & opt : _settings)
-        if (!opt.second.isAlias && (!overriddenOnly || opt.second.setting->overridden))
+        if (applicable(opt.second) && (!overriddenOnly || opt.second.setting->overridden))
             res.emplace(opt.first, SettingInfo{opt.second.setting->to_string(), opt.second.setting->description});
 }
 
@@ -147,9 +154,8 @@ nlohmann::json Config::toJSON()
 {
     auto res = nlohmann::json::object();
     for (auto & s : _settings)
-        if (!s.second.isAlias) {
+        if (applicable(s.second))
             res.emplace(s.first, s.second.setting->toJSON());
-        }
     return res;
 }
 
@@ -157,24 +163,31 @@ std::string Config::toKeyValue()
 {
     auto res = std::string();
     for (auto & s : _settings)
-        if (!s.second.isAlias) {
+        if (applicable(s.second))
             res += fmt("%s = %s\n", s.first, s.second.setting->to_string());
-        }
     return res;
 }
 
 void Config::convertToArgs(Args & args, const std::string & category)
 {
-    for (auto & s : _settings)
+    for (auto & s : _settings) {
+        /* We do include args for settings gated on disabled
+           experimental-features. The args themselves however will also be
+           gated on any experimental feature the underlying setting is. */
         if (!s.second.isAlias)
             s.second.setting->convertToArg(args, category);
+    }
 }
 
 AbstractSetting::AbstractSetting(
     const std::string & name,
     const std::string & description,
-    const std::set<std::string> & aliases)
-    : name(name), description(stripIndentation(description)), aliases(aliases)
+    const std::set<std::string> & aliases,
+    std::optional<ExperimentalFeature> experimentalFeature)
+    : name(name)
+    , description(stripIndentation(description))
+    , aliases(aliases)
+    , experimentalFeature(experimentalFeature)
 {
 }
 
@@ -210,6 +223,7 @@ void BaseSetting<T>::convertToArg(Args & args, const std::string & category)
         .category = category,
         .labels = {"value"},
         .handler = {[this](std::string s) { overridden = true; set(s); }},
+        .experimentalFeature = experimentalFeature,
     });
 
     if (isAppendable())
@@ -219,6 +233,7 @@ void BaseSetting<T>::convertToArg(Args & args, const std::string & category)
             .category = category,
             .labels = {"value"},
             .handler = {[this](std::string s) { overridden = true; set(s, true); }},
+            .experimentalFeature = experimentalFeature,
         });
 }
 
@@ -270,13 +285,15 @@ template<> void BaseSetting<bool>::convertToArg(Args & args, const std::string &
         .longName = name,
         .description = fmt("Enable the `%s` setting.", name),
         .category = category,
-        .handler = {[this]() { override(true); }}
+        .handler = {[this]() { override(true); }},
+        .experimentalFeature = experimentalFeature,
     });
     args.addFlag({
         .longName = "no-" + name,
         .description = fmt("Disable the `%s` setting.", name),
         .category = category,
-        .handler = {[this]() { override(false); }}
+        .handler = {[this]() { override(false); }},
+        .experimentalFeature = experimentalFeature,
     });
 }
 
@@ -444,4 +461,30 @@ GlobalConfig::Register::Register(Config * config)
     configRegistrations->emplace_back(config);
 }
 
+ExperimentalFeatureSettings experimentalFeatureSettings;
+
+static GlobalConfig::Register rSettings(&experimentalFeatureSettings);
+
+bool ExperimentalFeatureSettings::isEnabled(const ExperimentalFeature & feature) const
+{
+    auto & f = experimentalFeatures.get();
+    return std::find(f.begin(), f.end(), feature) != f.end();
+}
+
+void ExperimentalFeatureSettings::require(const ExperimentalFeature & feature) const
+{
+    if (!isEnabled(feature))
+        throw MissingExperimentalFeature(feature);
+}
+
+bool ExperimentalFeatureSettings::isEnabled(const std::optional<ExperimentalFeature> & feature) const
+{
+    return !feature || isEnabled(*feature);
+}
+
+void ExperimentalFeatureSettings::require(const std::optional<ExperimentalFeature> & feature) const
+{
+    if (feature) require(*feature);
+}
+
 }

src/libutil/config.hh

@@ -1,12 +1,13 @@
+#pragma once
+
 #include <cassert>
 #include <map>
 #include <set>
 
-#include "types.hh"
-
 #include <nlohmann/json_fwd.hpp>
 
-#pragma once
+#include "types.hh"
+#include "experimental-features.hh"
 
 namespace nix {
 
@@ -194,12 +195,15 @@ public:
 
     bool overridden = false;
 
+    std::optional<ExperimentalFeature> experimentalFeature;
+
 protected:
 
     AbstractSetting(
         const std::string & name,
         const std::string & description,
-        const std::set<std::string> & aliases);
+        const std::set<std::string> & aliases,
+        std::optional<ExperimentalFeature> experimentalFeature = std::nullopt);
 
     virtual ~AbstractSetting()
     {
@@ -240,8 +244,9 @@ public:
         const bool documentDefault,
         const std::string & name,
         const std::string & description,
-        const std::set<std::string> & aliases = {})
-        : AbstractSetting(name, description, aliases)
+        const std::set<std::string> & aliases = {},
+        std::optional<ExperimentalFeature> experimentalFeature = std::nullopt)
+        : AbstractSetting(name, description, aliases, experimentalFeature)
         , value(def)
         , defaultValue(def)
         , documentDefault(documentDefault)
@@ -296,8 +301,9 @@ public:
         const std::string & name,
         const std::string & description,
         const std::set<std::string> & aliases = {},
-        const bool documentDefault = true)
-        : BaseSetting<T>(def, documentDefault, name, description, aliases)
+        const bool documentDefault = true,
+        std::optional<ExperimentalFeature> experimentalFeature = std::nullopt)
+        : BaseSetting<T>(def, documentDefault, name, description, aliases, experimentalFeature)
     {
         options->addSetting(this);
     }
@@ -357,4 +363,37 @@ struct GlobalConfig : public AbstractConfig
 
 extern GlobalConfig globalConfig;
 
+
+struct ExperimentalFeatureSettings : Config {
+
+    Setting<std::set<ExperimentalFeature>> experimentalFeatures{this, {}, "experimental-features",
+        "Experimental Nix features to enable."};
+
+    /**
+     * Check whether the given experimental feature is enabled.
+     */
+    bool isEnabled(const ExperimentalFeature &) const;
+
+    /**
+     * Require an experimental feature be enabled, throwing an error if it is
+     * not.
+     */
+    void require(const ExperimentalFeature &) const;
+
+    /**
+     * `std::nullopt` pointer means no feature, which means there is nothing that could be
+     * disabled, and so the function returns true in that case.
+     */
+    bool isEnabled(const std::optional<ExperimentalFeature> &) const;
+
+    /**
+     * `std::nullopt` pointer means no feature, which means there is nothing that could be
+     * disabled, and so the function does nothing in that case.
+     */
+    void require(const std::optional<ExperimentalFeature> &) const;
+};
+
+// FIXME: don't use a global variable.
+extern ExperimentalFeatureSettings experimentalFeatureSettings;
+
 }

src/libutil/util.hh

@@ -450,7 +450,6 @@ template<class C> Strings quoteStrings(const C & c)
     return res;
 }
 
-
 /* Remove trailing whitespace from a string. FIXME: return
    std::string_view. */
 std::string chomp(std::string_view s);