Make experimental-features a proper type

Rather than having them plain strings scattered through the whole codebase, create an enum containing all the known experimental features. This means that - Nix can now `warn` when an unkwown experimental feature is passed (making it much nicer to spot typos and spot deprecated features) - It’s now easy to remove a feature altogether (once the feature isn’t experimental anymore or is dropped) by just removing the field for the enum and letting the compiler point us to all the now invalid usages of it.
2025-11-13 05:56:03 +01:00 · 2021-10-25 15:53:01 +02:00 · 2021-10-25 15:53:01 +02:00 · af99941279
commit af99941279
parent 4a2b7cc68c
28 changed files with 188 additions and 61 deletions
--- a/src/libstore/build/derivation-goal.cc
+++ b/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 = {
--- a/src/libstore/build/entry-points.cc
+++ b/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(
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/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,
--- a/src/libstore/daemon.cc
+++ b/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
--- a/src/libstore/derivations.cc
+++ b/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 {
--- a/src/libstore/derived-path.cc
+++ b/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
--- a/src/libstore/globals.cc
+++ b/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()
--- a/src/libstore/globals.hh
+++ b/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."};
--- a/src/libstore/local-store.cc
+++ b/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);
--- a/src/libstore/store-api.cc
+++ b/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;