Merge remote-tracking branch 'origin/overhaul-xp-features'

src/build-remote/build-remote.cc

@@ -18,6 +18,7 @@
 #include "derivations.hh"
 #include "local-store.hh"
 #include "legacy.hh"
+#include "experimental-features.hh"
 
 using namespace nix;
 using std::cin;
@@ -295,7 +296,7 @@ connected:
 
         std::set<Realisation> missingRealisations;
         StorePathSet missingPaths;
-        if (settings.isExperimentalFeatureEnabled("ca-derivations") && !derivationHasKnownOutputPaths(drv.type())) {
+        if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && !derivationHasKnownOutputPaths(drv.type())) {
             for (auto & outputName : wantedOutputs) {
                 auto thisOutputHash = outputHashes.at(outputName);
                 auto thisOutputId = DrvOutput{ thisOutputHash, outputName };
@@ -327,7 +328,7 @@ connected:
         for (auto & realisation : missingRealisations) {
             // Should hold, because if the feature isn't enabled the set
             // of missing realisations should be empty
-            settings.requireExperimentalFeature("ca-derivations");
+            settings.requireExperimentalFeature(Xp::CaDerivations);
             store->registerDrvOutput(realisation);
         }
 

src/libcmd/installables.cc

@@ -714,7 +714,7 @@ BuiltPaths getBuiltPaths(ref<Store> evalStore, ref<Store> store, const DerivedPa
                                 "the derivation '%s' doesn't have an output named '%s'",
                                 store->printStorePath(bfd.drvPath), output);
                         if (settings.isExperimentalFeatureEnabled(
-                                "ca-derivations")) {
+                                Xp::CaDerivations)) {
                             auto outputId =
                                 DrvOutput{outputHashes.at(output), output};
                             auto realisation =

src/libexpr/eval.cc

@@ -466,7 +466,7 @@ EvalState::~EvalState()
 
 
 void EvalState::requireExperimentalFeatureOnEvaluation(
-    const std::string & feature,
+    const ExperimentalFeature & feature,
     const std::string_view fName,
     const Pos & pos)
 {

src/libexpr/eval.hh

@@ -5,6 +5,7 @@
 #include "nixexpr.hh"
 #include "symbol-table.hh"
 #include "config.hh"
+#include "experimental-features.hh"
 
 #include <map>
 #include <optional>
@@ -141,7 +142,7 @@ public:
     ~EvalState();
 
     void requireExperimentalFeatureOnEvaluation(
-        const std::string & feature,
+        const ExperimentalFeature &,
         const std::string_view fName,
         const Pos & pos
     );

src/libexpr/flake/flake.cc

@@ -297,7 +297,7 @@ LockedFlake lockFlake(
     const FlakeRef & topRef,
     const LockFlags & lockFlags)
 {
-    settings.requireExperimentalFeature("flakes");
+    settings.requireExperimentalFeature(Xp::Flakes);
 
     FlakeCache flakeCache;
 
@@ -687,7 +687,7 @@ void callFlake(EvalState & state,
 
 static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Value & v)
 {
-    state.requireExperimentalFeatureOnEvaluation("flakes", "builtins.getFlake", pos);
+    state.requireExperimentalFeatureOnEvaluation(Xp::Flakes, "builtins.getFlake", pos);
 
     auto flakeRefS = state.forceStringNoCtx(*args[0], pos);
     auto flakeRef = parseFlakeRef(flakeRefS, {}, true);

src/libexpr/parser.y

@@ -418,7 +418,7 @@ expr_simple
           new ExprString(data->symbols.create(path)));
   }
   | URI {
-      static bool noURLLiterals = settings.isExperimentalFeatureEnabled("no-url-literals");
+      static bool noURLLiterals = settings.isExperimentalFeatureEnabled(Xp::NoUrlLiterals);
       if (noURLLiterals)
           throw ParseError({
               .msg = hintfmt("URL literals are disabled"),

src/libexpr/primops.cc

@@ -985,7 +985,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
             }
 
             if (i->name == state.sContentAddressed) {
-                settings.requireExperimentalFeature("ca-derivations");
+                settings.requireExperimentalFeature(Xp::CaDerivations);
                 contentAddressed = state.forceBool(*i->value, pos);
             }
 

src/libexpr/primops/fetchTree.cc

@@ -176,7 +176,7 @@ static void fetchTree(
 
 static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v)
 {
-    settings.requireExperimentalFeature("flakes");
+    settings.requireExperimentalFeature(Xp::Flakes);
     fetchTree(state, pos, args, v, std::nullopt, FetchTreeParams { .allowNameArgument = false });
 }
 

src/libstore/build/derivation-goal.cc

@@ -204,7 +204,7 @@ void DerivationGoal::haveDerivation()
     trace("have derivation");
 
     if (drv->type() == DerivationType::CAFloating)
-        settings.requireExperimentalFeature("ca-derivations");
+        settings.requireExperimentalFeature(Xp::CaDerivations);
 
     retrySubstitution = false;
 
@@ -453,7 +453,7 @@ void DerivationGoal::inputsRealised()
     if (useDerivation) {
         auto & fullDrv = *dynamic_cast<Derivation *>(drv.get());
 
-        if (settings.isExperimentalFeatureEnabled("ca-derivations") &&
+        if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) &&
             ((!fullDrv.inputDrvs.empty() && derivationIsCA(fullDrv.type()))
             || fullDrv.type() == DerivationType::DeferredInputAddressed)) {
             /* We are be able to resolve this derivation based on the
@@ -1273,7 +1273,7 @@ void DerivationGoal::checkPathValidity()
                     : PathStatus::Corrupt,
             };
         }
-        if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
+        if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) {
             auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first};
             if (auto real = worker.store.queryRealisation(drvOutput)) {
                 info.known = {

src/libstore/build/entry-points.cc

@@ -74,7 +74,7 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat
                     outputId,
                     Realisation{ outputId, *staticOutput.second}
                     );
-        if (settings.isExperimentalFeatureEnabled("ca-derivations") && !derivationHasKnownOutputPaths(drv.type())) {
+        if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && !derivationHasKnownOutputPaths(drv.type())) {
             auto realisation = this->queryRealisation(outputId);
             if (realisation)
                 result.builtOutputs.insert_or_assign(

src/libstore/build/local-derivation-goal.cc

@@ -1259,7 +1259,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
             for (auto & [outputName, outputPath] : outputs)
                 if (wantOutput(outputName, bfd.outputs)) {
                     newPaths.insert(outputPath);
-                    if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
+                    if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) {
                         auto thisRealisation = next->queryRealisation(
                             DrvOutput{drvHashes.at(outputName), outputName}
                         );
@@ -1320,7 +1320,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
 
 void LocalDerivationGoal::startDaemon()
 {
-    settings.requireExperimentalFeature("recursive-nix");
+    settings.requireExperimentalFeature(Xp::RecursiveNix);
 
     Store::Params params;
     params["path-info-cache-size"] = "0";
@@ -2561,7 +2561,7 @@ void LocalDerivationGoal::registerOutputs()
        that for floating CA derivations, which otherwise couldn't be cached,
        but it's fine to do in all cases. */
 
-    if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
+    if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) {
         for (auto& [outputName, newInfo] : infos) {
             auto thisRealisation = Realisation{
                 .id = DrvOutput{initialOutputs.at(outputName).outputHash,

src/libstore/daemon.cc

@@ -230,7 +230,7 @@ struct ClientSettings
                 else if (name == settings.experimentalFeatures.name) {
                     // We don’t want to forward the experimental features to
                     // the daemon, as that could cause some pretty weird stuff
-                    if (tokenizeString<Strings>(value) != settings.experimentalFeatures.get())
+                    if (parseFeatures(tokenizeString<StringSet>(value)) != settings.experimentalFeatures.get())
                         debug("Ignoring the client-specified experimental features");
                 }
                 else if (trusted

src/libstore/derivations.cc

@@ -187,7 +187,7 @@ static DerivationOutput parseDerivationOutput(const Store & store,
                 },
             };
         } else {
-            settings.requireExperimentalFeature("ca-derivations");
+            settings.requireExperimentalFeature(Xp::CaDerivations);
             assert(pathS == "");
             return DerivationOutput {
                 .output = DerivationOutputCAFloating {

src/libstore/derived-path.cc

@@ -100,7 +100,7 @@ RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const
                     staticOutputHashes(store, store.readDerivation(p.drvPath));
                 for (auto& [outputName, outputPath] : p.outputs) {
                     if (settings.isExperimentalFeatureEnabled(
-                            "ca-derivations")) {
+                                Xp::CaDerivations)) {
                         auto thisRealisation = store.queryRealisation(
                             DrvOutput{drvHashes.at(outputName), outputName});
                         assert(thisRealisation);  // We’ve built it, so we must h

src/libstore/globals.cc

@@ -160,21 +160,16 @@ StringSet Settings::getDefaultExtraPlatforms()
     return extraPlatforms;
 }
 
-bool Settings::isExperimentalFeatureEnabled(const std::string & name)
+bool Settings::isExperimentalFeatureEnabled(const ExperimentalFeature & feature)
 {
     auto & f = experimentalFeatures.get();
-    return std::find(f.begin(), f.end(), name) != f.end();
+    return std::find(f.begin(), f.end(), feature) != f.end();
 }
 
-MissingExperimentalFeature::MissingExperimentalFeature(std::string feature)
-    : Error("experimental Nix feature '%1%' is disabled; use '--extra-experimental-features %1%' to override", feature)
-    , missingFeature(feature)
-    {}
-
-void Settings::requireExperimentalFeature(const std::string & name)
+void Settings::requireExperimentalFeature(const ExperimentalFeature & feature)
 {
-    if (!isExperimentalFeatureEnabled(name))
-        throw MissingExperimentalFeature(name);
+    if (!isExperimentalFeatureEnabled(feature))
+        throw MissingExperimentalFeature(feature);
 }
 
 bool Settings::isWSL1()

src/libstore/globals.hh

@@ -3,6 +3,7 @@
 #include "types.hh"
 #include "config.hh"
 #include "util.hh"
+#include "experimental-features.hh"
 
 #include <map>
 #include <limits>
@@ -45,15 +46,6 @@ struct PluginFilesSetting : public BaseSetting<Paths>
     void set(const std::string & str, bool append = false) override;
 };
 
-class MissingExperimentalFeature: public Error
-{
-public:
-    std::string missingFeature;
-
-    MissingExperimentalFeature(std::string feature);
-    virtual const char* sname() const override { return "MissingExperimentalFeature"; }
-};
-
 class Settings : public Config {
 
     unsigned int getDefaultCores();
@@ -925,12 +917,12 @@ public:
           value.
           )"};
 
-    Setting<Strings> experimentalFeatures{this, {}, "experimental-features",
+    Setting<std::set<ExperimentalFeature>> experimentalFeatures{this, {}, "experimental-features",
         "Experimental Nix features to enable."};
 
-    bool isExperimentalFeatureEnabled(const std::string & name);
+    bool isExperimentalFeatureEnabled(const ExperimentalFeature &);
 
-    void requireExperimentalFeature(const std::string & name);
+    void requireExperimentalFeature(const ExperimentalFeature &);
 
     Setting<bool> allowDirty{this, true, "allow-dirty",
         "Whether to allow dirty Git/Mercurial trees."};

src/libstore/local-store.cc

@@ -309,7 +309,7 @@ LocalStore::LocalStore(const Params & params)
 
     else openDB(*state, false);
 
-    if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
+    if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) {
         migrateCASchema(state->db, dbDir + "/ca-schema", globalLock);
     }
 
@@ -339,7 +339,7 @@ LocalStore::LocalStore(const Params & params)
     state->stmts->QueryPathFromHashPart.create(state->db,
         "select path from ValidPaths where path >= ? limit 1;");
     state->stmts->QueryValidPaths.create(state->db, "select path from ValidPaths");
-    if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
+    if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) {
         state->stmts->RegisterRealisedOutput.create(state->db,
             R"(
                 insert into Realisations (drvPath, outputName, outputPath, signatures)
@@ -708,7 +708,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat
 
 void LocalStore::registerDrvOutput(const Realisation & info, CheckSigsFlag checkSigs)
 {
-    settings.requireExperimentalFeature("ca-derivations");
+    settings.requireExperimentalFeature(Xp::CaDerivations);
     if (checkSigs == NoCheckSigs || !realisationIsUntrusted(info))
         registerDrvOutput(info);
     else
@@ -717,7 +717,7 @@ void LocalStore::registerDrvOutput(const Realisation & info, CheckSigsFlag check
 
 void LocalStore::registerDrvOutput(const Realisation & info)
 {
-    settings.requireExperimentalFeature("ca-derivations");
+    settings.requireExperimentalFeature(Xp::CaDerivations);
     retrySQLite<void>([&]() {
         auto state(_state.lock());
         if (auto oldR = queryRealisation_(*state, info.id)) {
@@ -1003,7 +1003,7 @@ LocalStore::queryPartialDerivationOutputMap(const StorePath & path_)
         return outputs;
     });
 
-    if (!settings.isExperimentalFeatureEnabled("ca-derivations"))
+    if (!settings.isExperimentalFeatureEnabled(Xp::CaDerivations))
         return outputs;
 
     auto drv = readInvalidDerivation(path);

src/libstore/store-api.cc

@@ -355,7 +355,7 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
 StringSet StoreConfig::getDefaultSystemFeatures()
 {
     auto res = settings.systemFeatures.get();
-    if (settings.isExperimentalFeatureEnabled("ca-derivations"))
+    if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations))
         res.insert("ca-derivations");
     return res;
 }
@@ -860,7 +860,7 @@ std::map<StorePath, StorePath> copyPaths(
     for (auto & path : paths) {
         storePaths.insert(path.path());
         if (auto realisation = std::get_if<Realisation>(&path.raw)) {
-            settings.requireExperimentalFeature("ca-derivations");
+            settings.requireExperimentalFeature(Xp::CaDerivations);
             toplevelRealisations.insert(*realisation);
         }
     }
@@ -892,7 +892,7 @@ std::map<StorePath, StorePath> copyPaths(
         // Don't fail if the remote doesn't support CA derivations is it might
         // not be within our control to change that, and we might still want
         // to at least copy the output paths.
-        if (e.missingFeature == "ca-derivations")
+        if (e.missingFeature == Xp::CaDerivations)
             ignoreException();
         else
             throw;

src/libutil/config.cc

@@ -1,6 +1,7 @@
 #include "config.hh"
 #include "args.hh"
 #include "abstract-setting-to-json.hh"
+#include "experimental-features.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -313,6 +314,31 @@ template<> std::string BaseSetting<StringSet>::to_string() const
     return concatStringsSep(" ", value);
 }
 
+template<> void BaseSetting<std::set<ExperimentalFeature>>::set(const std::string & str, bool append)
+{
+    if (!append) value.clear();
+    for (auto & s : tokenizeString<StringSet>(str)) {
+        auto thisXpFeature = parseExperimentalFeature(s);
+        if (thisXpFeature)
+            value.insert(thisXpFeature.value());
+        else
+            warn("Unknown experimental feature %s", s);
+    }
+}
+
+template<> bool BaseSetting<std::set<ExperimentalFeature>>::isAppendable()
+{
+    return true;
+}
+
+template<> std::string BaseSetting<std::set<ExperimentalFeature>>::to_string() const
+{
+    StringSet stringifiedXpFeatures;
+    for (auto & feature : value)
+        stringifiedXpFeatures.insert(std::string(showExperimentalFeature(feature)));
+    return concatStringsSep(" ", stringifiedXpFeatures);
+}
+
 template<> void BaseSetting<StringMap>::set(const std::string & str, bool append)
 {
     if (!append) value.clear();
@@ -348,6 +374,7 @@ template class BaseSetting<std::string>;
 template class BaseSetting<Strings>;
 template class BaseSetting<StringSet>;
 template class BaseSetting<StringMap>;
+template class BaseSetting<std::set<ExperimentalFeature>>;
 
 void PathSetting::set(const std::string & str, bool append)
 {

src/libutil/experimental-features.cc

@@ -0,0 +1,55 @@
+#include "experimental-features.hh"
+#include "nlohmann/json.hpp"
+
+namespace nix {
+
+std::map<ExperimentalFeature, std::string> stringifiedXpFeatures = {
+    { Xp::CaDerivations, "ca-derivations" },
+    { Xp::Flakes, "flakes" },
+    { Xp::NixCommand, "nix-command" },
+    { Xp::RecursiveNix, "recursive-nix" },
+    { Xp::NoUrlLiterals, "no-url-literals" },
+};
+
+const std::optional<ExperimentalFeature> parseExperimentalFeature(const std::string_view & name)
+{
+    using ReverseXpMap = std::map<std::string_view, ExperimentalFeature>;
+    static ReverseXpMap * reverseXpMap;
+    if (!reverseXpMap) {
+        reverseXpMap = new ReverseXpMap{};
+        for (auto & [feature, name] : stringifiedXpFeatures)
+            (*reverseXpMap)[name] = feature;
+    }
+
+    auto featureIter = reverseXpMap->find(name);
+    if (featureIter == reverseXpMap->end())
+        return std::nullopt;
+    return {featureIter->second};
+}
+
+std::string_view showExperimentalFeature(const ExperimentalFeature feature)
+{
+    return stringifiedXpFeatures.at(feature);
+}
+
+std::set<ExperimentalFeature> parseFeatures(const std::set<std::string> & rawFeatures)
+{
+    std::set<ExperimentalFeature> res;
+    for (auto & rawFeature : rawFeatures) {
+        if (auto feature = parseExperimentalFeature(rawFeature))
+            res.insert(*feature);
+    }
+    return res;
+}
+
+MissingExperimentalFeature::MissingExperimentalFeature(ExperimentalFeature feature)
+    : Error("experimental Nix feature '%1%' is disabled; use '--extra-experimental-features %1%' to override", showExperimentalFeature(feature))
+    , missingFeature(feature)
+    {}
+
+std::ostream & operator <<(std::ostream & str, const ExperimentalFeature & feature)
+{
+    return str << showExperimentalFeature(feature);
+}
+
+}

src/libutil/experimental-features.hh

@@ -0,0 +1,56 @@
+#pragma once
+
+#include "comparator.hh"
+#include "error.hh"
+#include "nlohmann/json_fwd.hpp"
+#include "types.hh"
+
+namespace nix {
+
+/**
+ * The list of available experimental features.
+ *
+ * If you update this, don’t forget to also change the map defining their
+ * string representation in the corresponding `.cc` file.
+ **/
+enum struct ExperimentalFeature
+{
+    CaDerivations,
+    Flakes,
+    NixCommand,
+    RecursiveNix,
+    NoUrlLiterals
+};
+
+/**
+ * Just because writing `ExperimentalFeature::CaDerivations` is way too long
+ */
+using Xp = ExperimentalFeature;
+
+const std::optional<ExperimentalFeature> parseExperimentalFeature(
+        const std::string_view & name);
+std::string_view showExperimentalFeature(const ExperimentalFeature);
+
+std::ostream & operator<<(
+        std::ostream & str,
+        const ExperimentalFeature & feature);
+
+/**
+ * Parse a set of strings to the corresponding set of experimental features,
+ * ignoring (but warning for) any unkwown feature.
+ */
+std::set<ExperimentalFeature> parseFeatures(const std::set<std::string> &);
+
+class MissingExperimentalFeature : public Error
+{
+public:
+    ExperimentalFeature missingFeature;
+
+    MissingExperimentalFeature(ExperimentalFeature);
+    virtual const char * sname() const override
+    {
+        return "MissingExperimentalFeature";
+    }
+};
+
+}

src/nix-build/nix-build.cc

@@ -401,7 +401,7 @@ static void main_nix_build(int argc, char * * argv)
 
         if (dryRun) return;
 
-        if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
+        if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) {
             auto resolvedDrv = drv.tryResolve(*store);
             assert(resolvedDrv && "Successfully resolved the derivation");
             drv = *resolvedDrv;

src/nix/develop.cc

@@ -195,7 +195,7 @@ static StorePath getDerivationEnvironment(ref<Store> store, ref<Store> evalStore
        'buildDerivation', but that's privileged. */
     drv.name += "-env";
     drv.inputSrcs.insert(std::move(getEnvShPath));
-    if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
+    if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) {
         for (auto & output : drv.outputs) {
             output.second = {
                 .output = DerivationOutputDeferred{},

src/nix/flake.cc

@@ -1138,7 +1138,7 @@ struct CmdFlake : NixMultiCommand
     {
         if (!command)
             throw UsageError("'nix flake' requires a sub-command.");
-        settings.requireExperimentalFeature("flakes");
+        settings.requireExperimentalFeature(Xp::Flakes);
         command->second->prepare();
         command->second->run();
     }

src/nix/main.cc

@@ -337,7 +337,7 @@ void mainWrapped(int argc, char * * argv)
     if (args.command->first != "repl"
         && args.command->first != "doctor"
         && args.command->first != "upgrade-nix")
-        settings.requireExperimentalFeature("nix-command");
+        settings.requireExperimentalFeature(Xp::NixCommand);
 
     if (args.useNet && !haveInternet()) {
         warn("you don't have Internet access; disabling some network-dependent features");

src/nix/realisation.cc

@@ -46,7 +46,7 @@ struct CmdRealisationInfo : BuiltPathsCommand, MixJSON
 
     void run(ref<Store> store, BuiltPaths && paths) override
     {
-        settings.requireExperimentalFeature("ca-derivations");
+        settings.requireExperimentalFeature(Xp::CaDerivations);
         RealisedPath::Set realisations;
 
         for (auto & builtPath : paths) {

src/nix/sigs.cc

@@ -219,7 +219,7 @@ struct CmdKey : NixMultiCommand
     {
         if (!command)
             throw UsageError("'nix flake' requires a sub-command.");
-        settings.requireExperimentalFeature("flakes");
+        settings.requireExperimentalFeature(Xp::Flakes);
         command->second->prepare();
         command->second->run();
     }

tests/config.sh

@@ -50,4 +50,4 @@ exp_cores=$(nix show-config | grep '^cores' | cut -d '=' -f 2 | xargs)
 exp_features=$(nix show-config | grep '^experimental-features' | cut -d '=' -f 2 | xargs)
 [[ $prev != $exp_cores ]]
 [[ $exp_cores == "4242" ]]
-[[ $exp_features == "nix-command flakes" ]]
+[[ $exp_features == "flakes nix-command" ]]