diff options
| -rw-r--r-- | src/libstore/globals.cc | 27 | ||||
| -rw-r--r-- | src/libstore/globals.hh | 4 | ||||
| -rw-r--r-- | src/libutil/config-impl.hh | 71 | ||||
| -rw-r--r-- | src/libutil/config.cc | 94 | ||||
| -rw-r--r-- | src/libutil/config.hh | 46 | ||||
| -rw-r--r-- | src/libutil/tests/config.cc | 2 | ||||
| -rw-r--r-- | tests/experimental-features.sh | 60 |
7 files changed, 238 insertions, 66 deletions
diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 1b38e32fb..4c66d08ee 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -22,6 +22,9 @@ #include <dlfcn.h> #endif +#include "config-impl.hh" + + namespace nix { @@ -192,18 +195,18 @@ NLOHMANN_JSON_SERIALIZE_ENUM(SandboxMode, { {SandboxMode::smDisabled, false}, }); -template<> void BaseSetting<SandboxMode>::set(const std::string & str, bool append) +template<> SandboxMode BaseSetting<SandboxMode>::parse(const std::string & str) const { - if (str == "true") value = smEnabled; - else if (str == "relaxed") value = smRelaxed; - else if (str == "false") value = smDisabled; + if (str == "true") return smEnabled; + else if (str == "relaxed") return smRelaxed; + else if (str == "false") return smDisabled; else throw UsageError("option '%s' has invalid value '%s'", name, str); } -template<> bool BaseSetting<SandboxMode>::isAppendable() +template<> struct BaseSetting<SandboxMode>::trait { - return false; -} + static constexpr bool appendable = false; +}; template<> std::string BaseSetting<SandboxMode>::to_string() const { @@ -235,23 +238,23 @@ template<> void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::s }); } -void MaxBuildJobsSetting::set(const std::string & str, bool append) +unsigned int MaxBuildJobsSetting::parse(const std::string & str) const { - if (str == "auto") value = std::max(1U, std::thread::hardware_concurrency()); + if (str == "auto") return std::max(1U, std::thread::hardware_concurrency()); else { if (auto n = string2Int<decltype(value)>(str)) - value = *n; + return *n; else throw UsageError("configuration setting '%s' should be 'auto' or an integer", name); } } -void PluginFilesSetting::set(const std::string & str, bool append) +Paths PluginFilesSetting::parse(const std::string & str) const { if (pluginsLoaded) throw UsageError("plugin-files set after plugins were loaded, you may need to move the flag before the subcommand"); - BaseSetting<Paths>::set(str, append); + return BaseSetting<Paths>::parse(str); } diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index d6c5d437a..609cf53b8 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -26,7 +26,7 @@ struct MaxBuildJobsSetting : public BaseSetting<unsigned int> options->addSetting(this); } - void set(const std::string & str, bool append = false) override; + unsigned int parse(const std::string & str) const override; }; struct PluginFilesSetting : public BaseSetting<Paths> @@ -43,7 +43,7 @@ struct PluginFilesSetting : public BaseSetting<Paths> options->addSetting(this); } - void set(const std::string & str, bool append = false) override; + Paths parse(const std::string & str) const override; }; const uint32_t maxIdsPerBuild = diff --git a/src/libutil/config-impl.hh b/src/libutil/config-impl.hh new file mode 100644 index 000000000..b6cae5ec3 --- /dev/null +++ b/src/libutil/config-impl.hh @@ -0,0 +1,71 @@ +#pragma once +/** + * @file + * + * Template implementations (as opposed to mere declarations). + * + * One only needs to include this when one is declaring a + * `BaseClass<CustomType>` setting, or as derived class of such an + * instantiation. + */ + +#include "config.hh" + +namespace nix { + +template<> struct BaseSetting<Strings>::trait +{ + static constexpr bool appendable = true; +}; +template<> struct BaseSetting<StringSet>::trait +{ + static constexpr bool appendable = true; +}; +template<> struct BaseSetting<StringMap>::trait +{ + static constexpr bool appendable = true; +}; +template<> struct BaseSetting<std::set<ExperimentalFeature>>::trait +{ + static constexpr bool appendable = true; +}; + +template<typename T> +struct BaseSetting<T>::trait +{ + static constexpr bool appendable = false; +}; + +template<typename T> +bool BaseSetting<T>::isAppendable() +{ + return trait::appendable; +} + +template<> void BaseSetting<Strings>::appendOrSet(Strings && newValue, bool append); +template<> void BaseSetting<StringSet>::appendOrSet(StringSet && newValue, bool append); +template<> void BaseSetting<StringMap>::appendOrSet(StringMap && newValue, bool append); +template<> void BaseSetting<std::set<ExperimentalFeature>>::appendOrSet(std::set<ExperimentalFeature> && newValue, bool append); + +template<typename T> +void BaseSetting<T>::appendOrSet(T && newValue, bool append) +{ + static_assert(!trait::appendable, "using default `appendOrSet` implementation with an appendable type"); + assert(!append); + value = std::move(newValue); +} + +template<typename T> +void BaseSetting<T>::set(const std::string & str, bool append) +{ + if (experimentalFeatureSettings.isEnabled(experimentalFeature)) + appendOrSet(parse(str), append); + else { + assert(experimentalFeature); + warn("Ignoring setting '%s' because experimental feature '%s' is not enabled", + name, + showExperimentalFeature(*experimentalFeature)); + } +} + +} diff --git a/src/libutil/config.cc b/src/libutil/config.cc index a42f3a849..085a884dc 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -3,6 +3,8 @@ #include "abstract-setting-to-json.hh" #include "experimental-features.hh" +#include "config-impl.hh" + #include <nlohmann/json.hpp> namespace nix { @@ -80,6 +82,8 @@ void Config::getSettings(std::map<std::string, SettingInfo> & res, bool overridd void AbstractConfig::applyConfig(const std::string & contents, const std::string & path) { unsigned int pos = 0; + std::vector<std::pair<std::string, std::string>> parsedContents; + while (pos < contents.size()) { std::string line; while (pos < contents.size() && contents[pos] != '\n') @@ -125,8 +129,21 @@ void AbstractConfig::applyConfig(const std::string & contents, const std::string auto i = tokens.begin(); advance(i, 2); - set(name, concatStringsSep(" ", Strings(i, tokens.end()))); // FIXME: slow + parsedContents.push_back({ + name, + concatStringsSep(" ", Strings(i, tokens.end())), + }); }; + + // First apply experimental-feature related settings + for (auto & [name, value] : parsedContents) + if (name == "experimental-features" || name == "extra-experimental-features") + set(name, value); + + // Then apply other settings + for (auto & [name, value] : parsedContents) + if (name != "experimental-features" && name != "extra-experimental-features") + set(name, value); } void AbstractConfig::applyConfigFile(const Path & path) @@ -203,12 +220,6 @@ 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) { args.addFlag({ @@ -231,9 +242,9 @@ void BaseSetting<T>::convertToArg(Args & args, const std::string & category) }); } -template<> void BaseSetting<std::string>::set(const std::string & str, bool append) +template<> std::string BaseSetting<std::string>::parse(const std::string & str) const { - value = str; + return str; } template<> std::string BaseSetting<std::string>::to_string() const @@ -242,11 +253,11 @@ template<> std::string BaseSetting<std::string>::to_string() const } template<typename T> -void BaseSetting<T>::set(const std::string & str, bool append) +T BaseSetting<T>::parse(const std::string & str) const { static_assert(std::is_integral<T>::value, "Integer required."); if (auto n = string2Int<T>(str)) - value = *n; + return *n; else throw UsageError("setting '%s' has invalid value '%s'", name, str); } @@ -258,12 +269,12 @@ std::string BaseSetting<T>::to_string() const return std::to_string(value); } -template<> void BaseSetting<bool>::set(const std::string & str, bool append) +template<> bool BaseSetting<bool>::parse(const std::string & str) const { if (str == "true" || str == "yes" || str == "1") - value = true; + return true; else if (str == "false" || str == "no" || str == "0") - value = false; + return false; else throw UsageError("Boolean setting '%s' has invalid value '%s'", name, str); } @@ -291,16 +302,15 @@ template<> void BaseSetting<bool>::convertToArg(Args & args, const std::string & }); } -template<> void BaseSetting<Strings>::set(const std::string & str, bool append) +template<> Strings BaseSetting<Strings>::parse(const std::string & str) const { - auto ss = tokenizeString<Strings>(str); - if (!append) value.clear(); - for (auto & s : ss) value.push_back(std::move(s)); + return tokenizeString<Strings>(str); } -template<> bool BaseSetting<Strings>::isAppendable() +template<> void BaseSetting<Strings>::appendOrSet(Strings && newValue, bool append) { - return true; + if (!append) value.clear(); + for (auto && s : std::move(newValue)) value.push_back(std::move(s)); } template<> std::string BaseSetting<Strings>::to_string() const @@ -308,16 +318,16 @@ template<> std::string BaseSetting<Strings>::to_string() const return concatStringsSep(" ", value); } -template<> void BaseSetting<StringSet>::set(const std::string & str, bool append) +template<> StringSet BaseSetting<StringSet>::parse(const std::string & str) const { - if (!append) value.clear(); - for (auto & s : tokenizeString<StringSet>(str)) - value.insert(s); + return tokenizeString<StringSet>(str); } -template<> bool BaseSetting<StringSet>::isAppendable() +template<> void BaseSetting<StringSet>::appendOrSet(StringSet && newValue, bool append) { - return true; + if (!append) value.clear(); + for (auto && s : std::move(newValue)) + value.insert(s); } template<> std::string BaseSetting<StringSet>::to_string() const @@ -325,21 +335,24 @@ template<> std::string BaseSetting<StringSet>::to_string() const return concatStringsSep(" ", value); } -template<> void BaseSetting<std::set<ExperimentalFeature>>::set(const std::string & str, bool append) +template<> std::set<ExperimentalFeature> BaseSetting<std::set<ExperimentalFeature>>::parse(const std::string & str) const { - if (!append) value.clear(); + std::set<ExperimentalFeature> res; for (auto & s : tokenizeString<StringSet>(str)) { auto thisXpFeature = parseExperimentalFeature(s); if (thisXpFeature) - value.insert(thisXpFeature.value()); + res.insert(thisXpFeature.value()); else warn("unknown experimental feature '%s'", s); } + return res; } -template<> bool BaseSetting<std::set<ExperimentalFeature>>::isAppendable() +template<> void BaseSetting<std::set<ExperimentalFeature>>::appendOrSet(std::set<ExperimentalFeature> && newValue, bool append) { - return true; + if (!append) value.clear(); + for (auto && s : std::move(newValue)) + value.insert(s); } template<> std::string BaseSetting<std::set<ExperimentalFeature>>::to_string() const @@ -350,20 +363,23 @@ template<> std::string BaseSetting<std::set<ExperimentalFeature>>::to_string() c return concatStringsSep(" ", stringifiedXpFeatures); } -template<> void BaseSetting<StringMap>::set(const std::string & str, bool append) +template<> StringMap BaseSetting<StringMap>::parse(const std::string & str) const { - if (!append) value.clear(); + StringMap res; for (auto & s : tokenizeString<Strings>(str)) { auto eq = s.find_first_of('='); if (std::string::npos != eq) - value.emplace(std::string(s, 0, eq), std::string(s, eq + 1)); + res.emplace(std::string(s, 0, eq), std::string(s, eq + 1)); // else ignored } + return res; } -template<> bool BaseSetting<StringMap>::isAppendable() +template<> void BaseSetting<StringMap>::appendOrSet(StringMap && newValue, bool append) { - return true; + if (!append) value.clear(); + for (auto && [k, v] : std::move(newValue)) + value.emplace(std::move(k), std::move(v)); } template<> std::string BaseSetting<StringMap>::to_string() const @@ -387,15 +403,15 @@ template class BaseSetting<StringSet>; template class BaseSetting<StringMap>; template class BaseSetting<std::set<ExperimentalFeature>>; -void PathSetting::set(const std::string & str, bool append) +Path PathSetting::parse(const std::string & str) const { if (str == "") { if (allowEmpty) - value = ""; + return ""; else throw UsageError("setting '%s' cannot be empty", name); } else - value = canonPath(str); + return canonPath(str); } bool GlobalConfig::set(const std::string & name, const std::string & value) diff --git a/src/libutil/config.hh b/src/libutil/config.hh index 162626791..2675baed7 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -215,8 +215,11 @@ protected: virtual void set(const std::string & value, bool append = false) = 0; - virtual bool isAppendable() - { return false; } + /** + * Whether the type is appendable; i.e. whether the `append` + * parameter to `set()` is allowed to be `true`. + */ + virtual bool isAppendable() = 0; virtual std::string to_string() const = 0; @@ -241,6 +244,23 @@ protected: const T defaultValue; const bool documentDefault; + /** + * Parse the string into a `T`. + * + * Used by `set()`. + */ + virtual T parse(const std::string & str) const; + + /** + * Append or overwrite `value` with `newValue`. + * + * Some types to do not support appending in which case `append` + * should never be passed. The default handles this case. + * + * @param append Whether to append or overwrite. + */ + virtual void appendOrSet(T && newValue, bool append); + public: BaseSetting(const T & def, @@ -268,9 +288,25 @@ public: template<typename U> void setDefault(const U & v) { if (!overridden) value = v; } - void set(const std::string & str, bool append = false) override; + /** + * Require any experimental feature the setting depends on + * + * Uses `parse()` to get the value from `str`, and `appendOrSet()` + * to set it. + */ + void set(const std::string & str, bool append = false) override final; - bool isAppendable() override; + /** + * C++ trick; This is template-specialized to compile-time indicate whether + * the type is appendable. + */ + struct trait; + + /** + * Always defined based on the C++ magic + * with `trait` above. + */ + bool isAppendable() override final; virtual void override(const T & v) { @@ -336,7 +372,7 @@ public: options->addSetting(this); } - void set(const std::string & str, bool append = false) override; + Path parse(const std::string & str) const override; Path operator +(const char * p) const { return value + p; } diff --git a/src/libutil/tests/config.cc b/src/libutil/tests/config.cc index f250e934e..886e70da5 100644 --- a/src/libutil/tests/config.cc +++ b/src/libutil/tests/config.cc @@ -82,6 +82,7 @@ namespace nix { TestSetting() : AbstractSetting("test", "test", {}) {} void set(const std::string & value, bool append) override {} std::string to_string() const override { return {}; } + bool isAppendable() override { return false; } }; Config config; @@ -90,6 +91,7 @@ namespace nix { ASSERT_FALSE(config.set("test", "value")); config.addSetting(&setting); ASSERT_TRUE(config.set("test", "value")); + ASSERT_FALSE(config.set("extra-test", "value")); } TEST(Config, withInitialValue) { diff --git a/tests/experimental-features.sh b/tests/experimental-features.sh index 73554da8c..607bf0a8e 100644 --- a/tests/experimental-features.sh +++ b/tests/experimental-features.sh @@ -23,20 +23,64 @@ source common.sh # # Medium case, the configuration effects --help # grep_both_ways store gc --help -expect 1 nix --experimental-features 'nix-command' show-config --flake-registry 'https://no' -nix --experimental-features 'nix-command flakes' show-config --flake-registry 'https://no' +# Test settings that are gated on experimental features; the setting is ignored +# with a warning if the experimental feature is not enabled. The order of the +# `setting = value` lines in the configuration should not matter. + +# 'flakes' experimental-feature is disabled before, ignore and warn +NIX_CONFIG=' + experimental-features = nix-command + accept-flake-config = true +' nix show-config accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr +grepQuiet "false" $TEST_ROOT/stdout +grepQuiet "Ignoring setting 'accept-flake-config' because experimental feature 'flakes' is not enabled" $TEST_ROOT/stderr + +# 'flakes' experimental-feature is disabled after, ignore and warn +NIX_CONFIG=' + accept-flake-config = true + experimental-features = nix-command +' nix show-config accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr +grepQuiet "false" $TEST_ROOT/stdout +grepQuiet "Ignoring setting 'accept-flake-config' because experimental feature 'flakes' is not enabled" $TEST_ROOT/stderr + +# 'flakes' experimental-feature is enabled before, process +NIX_CONFIG=' + experimental-features = nix-command flakes + accept-flake-config = true +' nix show-config accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr +grepQuiet "true" $TEST_ROOT/stdout +grepQuietInverse "Ignoring setting 'accept-flake-config'" $TEST_ROOT/stderr + +# 'flakes' experimental-feature is enabled after, process +NIX_CONFIG=' + accept-flake-config = true + experimental-features = nix-command flakes +' nix show-config accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr +grepQuiet "true" $TEST_ROOT/stdout +grepQuietInverse "Ignoring setting 'accept-flake-config'" $TEST_ROOT/stderr + +function exit_code_both_ways { + expect 1 nix --experimental-features 'nix-command' "$@" 1>/dev/null + nix --experimental-features 'nix-command flakes' "$@" 1>/dev/null + + # Also, the order should not matter + expect 1 nix "$@" --experimental-features 'nix-command' 1>/dev/null + nix "$@" --experimental-features 'nix-command flakes' 1>/dev/null +} + +exit_code_both_ways show-config --flake-registry 'https://no' # Double check these are stable -nix --experimental-features '' --help -nix --experimental-features '' doctor --help -nix --experimental-features '' repl --help -nix --experimental-features '' upgrade-nix --help +nix --experimental-features '' --help 1>/dev/null +nix --experimental-features '' doctor --help 1>/dev/null +nix --experimental-features '' repl --help 1>/dev/null +nix --experimental-features '' upgrade-nix --help 1>/dev/null # These 3 arguments are currently given to all commands, which is wrong (as not # all care). To deal with fixing later, we simply make them require the # nix-command experimental features --- it so happens that the commands we wish # stabilizing to do not need them anyways. for arg in '--print-build-logs' '--offline' '--refresh'; do - nix --experimental-features 'nix-command' "$arg" --help - ! nix --experimental-features '' "$arg" --help + nix --experimental-features 'nix-command' "$arg" --help 1>/dev/null + expect 1 nix --experimental-features '' "$arg" --help 1>/dev/null done |