lix-project/lix-releng-staging
0
0
Fork 0
Code Packages Activity

Merge pull request #8194 from obsidiansystems/redisplay-feature-gated-settings

Browse source 
Fix some issues with experimental config settings
This commit is contained in:
Robert Hensing 2023-04-17 19:25:06 +02:00 committed by GitHub
parent 3f9589f17e 2c8475600d
commit c56705c025
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 238 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
src/libstore/globals.cc
View file

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

4
src/libstore/globals.hh
View file

@ -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 =

71
src/libutil/config-impl.hh Normal file
View file

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

94
src/libutil/config.cc
View file

@ -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)
@ -202,12 +219,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)
{
@ -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)

46
src/libutil/config.hh
View file

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

2
src/libutil/tests/config.cc
View file

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

60
tests/experimental-features.sh
View file

@ -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