From a409338e879deb0b5d00e15baaf7d4afce96ce70 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 27 Mar 2018 18:41:31 +0200 Subject: [PATCH] Modularize config settings Allow global config settings to be defined in multiple Config classes. For example, this means that libutil can have settings and evaluator settings can be moved out of libstore. The Config classes are registered in a new GlobalConfig class to which config files etc. are applied. Relevant to https://github.com/NixOS/nix/issues/2009 in that it removes the need for ad hoc handling of useCaseHack, which was the underlying cause of that issue. --- perl/lib/Nix/Store.xs | 2 +- src/libmain/common-args.cc | 4 +- src/libmain/shared.cc | 2 +- src/libstore/build.cc | 6 ++- src/libstore/globals.cc | 38 +++++----------- src/libstore/globals.hh | 36 +-------------- src/libstore/remote-store.cc | 5 +- src/libstore/store-api.cc | 2 +- src/libutil/archive.cc | 26 +++++++---- src/libutil/archive.hh | 4 -- src/libutil/config.cc | 88 ++++++++++++++++++++++++++++-------- src/libutil/config.hh | 81 ++++++++++++++++++++++++++------- src/nix/main.cc | 7 +-- src/nix/show-config.cc | 8 ++-- tests/plugins/plugintest.cc | 13 ++++-- 15 files changed, 192 insertions(+), 130 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index bbfb2934315..ce553bb53eb 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -27,7 +27,7 @@ static ref store() static std::shared_ptr _store; if (!_store) { try { - settings.loadConfFile(); + loadConfFile(); settings.lockCPU = false; _store = openStore(); } catch (Error & e) { diff --git a/src/libmain/common-args.cc b/src/libmain/common-args.cc index bcc05c2cdad..4c35a419959 100644 --- a/src/libmain/common-args.cc +++ b/src/libmain/common-args.cc @@ -29,14 +29,14 @@ MixCommonArgs::MixCommonArgs(const string & programName) .arity(2) .handler([](std::vector ss) { try { - settings.set(ss[0], ss[1]); + globalConfig.set(ss[0], ss[1]); } catch (UsageError & e) { warn(e.what()); } }); std::string cat = "config"; - settings.convertToArgs(*this, cat); + globalConfig.convertToArgs(*this, cat); // Backward compatibility hack: nix-env already had a --system flag. if (programName == "nix-env") longFlags.erase("system"); diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 91a4eaf922a..4ed34e54dc5 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -109,7 +109,7 @@ void initNix() opensslLocks = std::vector(CRYPTO_num_locks()); CRYPTO_set_locking_callback(opensslLockCallback); - settings.loadConfFile(); + loadConfFile(); startSignalHandlerThread(); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index f70ab8108fd..07b53378393 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -672,8 +672,10 @@ HookInstance::HookInstance() toHook.readSide = -1; sink = FdSink(toHook.writeSide.get()); - for (auto & setting : settings.getSettings()) - sink << 1 << setting.first << setting.second; + std::map settings; + globalConfig.getSettings(settings); + for (auto & setting : settings) + sink << 1 << setting.first << setting.second.value; sink << 0; } diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 544566e0b57..d95db56726c 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -28,9 +28,10 @@ namespace nix { Settings settings; +static GlobalConfig::Register r1(&settings); + Settings::Settings() - : Config({}) - , nixPrefix(NIX_PREFIX) + : nixPrefix(NIX_PREFIX) , nixStore(canonPath(getEnv("NIX_STORE_DIR", getEnv("NIX_STORE", NIX_STORE_DIR)))) , nixDataDir(canonPath(getEnv("NIX_DATA_DIR", NIX_DATA_DIR))) , nixLogDir(canonPath(getEnv("NIX_LOG_DIR", NIX_LOG_DIR))) @@ -69,20 +70,15 @@ Settings::Settings() allowedImpureHostPrefixes = tokenizeString(DEFAULT_ALLOWED_IMPURE_PREFIXES); } -void Settings::loadConfFile() +void loadConfFile() { - applyConfigFile(nixConfDir + "/nix.conf"); + globalConfig.applyConfigFile(settings.nixConfDir + "/nix.conf"); /* We only want to send overrides to the daemon, i.e. stuff from ~/.nix/nix.conf or the command line. */ - resetOverriden(); + globalConfig.resetOverriden(); - applyConfigFile(getConfigDir() + "/nix/nix.conf"); -} - -void Settings::set(const string & name, const string & value) -{ - Config::set(name, value); + globalConfig.applyConfigFile(getConfigDir() + "/nix/nix.conf"); } unsigned int Settings::getDefaultCores() @@ -162,23 +158,11 @@ void initPlugins() throw Error("could not dynamically open plugin file '%s': %s", file, dlerror()); } } - /* We handle settings registrations here, since plugins can add settings */ - if (RegisterSetting::settingRegistrations) { - for (auto & registration : *RegisterSetting::settingRegistrations) - settings.addSetting(registration); - delete RegisterSetting::settingRegistrations; - } - settings.handleUnknownSettings(); -} - -RegisterSetting::SettingRegistrations * RegisterSetting::settingRegistrations; -RegisterSetting::RegisterSetting(AbstractSetting * s) -{ - if (!settingRegistrations) - settingRegistrations = new SettingRegistrations; - settingRegistrations->emplace_back(s); + /* Since plugins can add settings, try to re-apply previously + unknown settings. */ + globalConfig.reapplyUnknownSettings(); + globalConfig.warnUnknownSettings(); } - } diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 9360096aae8..e8d95aed605 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -13,26 +13,6 @@ namespace nix { typedef enum { smEnabled, smRelaxed, smDisabled } SandboxMode; -extern bool useCaseHack; // FIXME - -struct CaseHackSetting : public BaseSetting -{ - CaseHackSetting(Config * options, - const std::string & name, - const std::string & description, - const std::set & aliases = {}) - : BaseSetting(useCaseHack, name, description, aliases) - { - options->addSetting(this); - } - - void set(const std::string & str) override - { - BaseSetting::set(str); - nix::useCaseHack = value; - } -}; - struct MaxBuildJobsSetting : public BaseSetting { MaxBuildJobsSetting(Config * options, @@ -56,10 +36,6 @@ public: Settings(); - void loadConfFile(); - - void set(const string & name, const string & value); - Path nixPrefix; /* The directory where we store sources and derived files. */ @@ -353,9 +329,6 @@ public: Setting enableImportFromDerivation{this, true, "allow-import-from-derivation", "Whether the evaluator allows importing the result of a derivation."}; - CaseHackSetting useCaseHack{this, "use-case-hack", - "Whether to enable a Darwin-specific hack for dealing with file name collisions."}; - Setting connectTimeout{this, 0, "connect-timeout", "Timeout for connecting to servers during downloads. 0 means use curl's builtin default."}; @@ -398,15 +371,8 @@ extern Settings settings; anything else */ void initPlugins(); +void loadConfFile(); extern const string nixVersion; -struct RegisterSetting -{ - typedef std::vector SettingRegistrations; - static SettingRegistrations * settingRegistrations; - RegisterSetting(AbstractSetting * s); -}; - - } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index d5303755855..a157c6c4858 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -187,10 +187,11 @@ void RemoteStore::setOptions(Connection & conn) << settings.useSubstitutes; if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 12) { - auto overrides = settings.getSettings(true); + std::map overrides; + globalConfig.getSettings(overrides, true); conn.to << overrides.size(); for (auto & i : overrides) - conn.to << i.first << i.second; + conn.to << i.first << i.second.value; } conn.processStderr(); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index a8f3ae1ea6c..49b32d11584 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -849,7 +849,7 @@ ref openStore(const std::string & uri_, for (auto fun : *RegisterStoreImplementation::implementations) { auto store = fun(uri, params); if (store) { - store->handleUnknownSettings(); + store->warnUnknownSettings(); return ref(store); } } diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 154e2d20430..1be8934a2eb 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -13,17 +13,25 @@ #include "archive.hh" #include "util.hh" - +#include "config.hh" namespace nix { +struct ArchiveSettings : Config +{ + Setting useCaseHack{this, + #if __APPLE__ + true, + #else + false, + #endif + "use-case-hack", + "Whether to enable a Darwin-specific hack for dealing with file name collisions."}; +}; -bool useCaseHack = -#if __APPLE__ - true; -#else - false; -#endif +static ArchiveSettings archiveSettings; + +static GlobalConfig::Register r1(&archiveSettings); const std::string narVersionMagic1 = "nix-archive-1"; @@ -78,7 +86,7 @@ static void dump(const Path & path, Sink & sink, PathFilter & filter) the case hack applied by restorePath(). */ std::map unhacked; for (auto & i : readDirectory(path)) - if (useCaseHack) { + if (archiveSettings.useCaseHack) { string name(i.name); size_t pos = i.name.find(caseHackSuffix); if (pos != string::npos) { @@ -243,7 +251,7 @@ static void parse(ParseSink & sink, Source & source, const Path & path) if (name <= prevName) throw Error("NAR directory is not sorted"); prevName = name; - if (useCaseHack) { + if (archiveSettings.useCaseHack) { auto i = names.find(name); if (i != names.end()) { debug(format("case collision between '%1%' and '%2%'") % i->first % name); diff --git a/src/libutil/archive.hh b/src/libutil/archive.hh index 7a0e688e420..25be426c1a4 100644 --- a/src/libutil/archive.hh +++ b/src/libutil/archive.hh @@ -78,10 +78,6 @@ void restorePath(const Path & path, Source & source); void copyNAR(Source & source, Sink & sink); -// FIXME: global variables are bad m'kay. -extern bool useCaseHack; - - extern const std::string narVersionMagic1; diff --git a/src/libutil/config.cc b/src/libutil/config.cc index ce6858f0d65..9023cb1bb6d 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -4,15 +4,13 @@ namespace nix { -void Config::set(const std::string & name, const std::string & value) +bool Config::set(const std::string & name, const std::string & value) { auto i = _settings.find(name); - if (i == _settings.end()) { - extras.emplace(name, value); - } else { - i->second.setting->set(value); - i->second.setting->overriden = true; - } + if (i == _settings.end()) return false; + i->second.setting->set(value); + i->second.setting->overriden = true; + return true; } void Config::addSetting(AbstractSetting * setting) @@ -23,46 +21,51 @@ void Config::addSetting(AbstractSetting * setting) bool set = false; - auto i = extras.find(setting->name); - if (i != extras.end()) { + auto i = unknownSettings.find(setting->name); + if (i != unknownSettings.end()) { setting->set(i->second); setting->overriden = true; - extras.erase(i); + unknownSettings.erase(i); set = true; } for (auto & alias : setting->aliases) { - auto i = extras.find(alias); - if (i != extras.end()) { + auto i = unknownSettings.find(alias); + if (i != unknownSettings.end()) { if (set) warn("setting '%s' is set, but it's an alias of '%s' which is also set", alias, setting->name); else { setting->set(i->second); setting->overriden = true; - extras.erase(i); + unknownSettings.erase(i); set = true; } } } } -void Config::handleUnknownSettings() +void AbstractConfig::warnUnknownSettings() { - for (auto & s : extras) + for (auto & s : unknownSettings) warn("unknown setting '%s'", s.first); } -StringMap Config::getSettings(bool overridenOnly) +void AbstractConfig::reapplyUnknownSettings() +{ + auto unknownSettings2 = std::move(unknownSettings); + for (auto & s : unknownSettings2) + set(s.first, s.second); +} + +void Config::getSettings(std::map & res, bool overridenOnly) { - StringMap res; for (auto & opt : _settings) if (!opt.second.isAlias && (!overridenOnly || opt.second.setting->overriden)) - res.emplace(opt.first, opt.second.setting->to_string()); - return res; + res.emplace(opt.first, SettingInfo{opt.second.setting->to_string(), opt.second.setting->description}); } -void Config::applyConfigFile(const Path & path) +void AbstractConfig::applyConfigFile(const Path & path) { try { string contents = readFile(path); @@ -287,4 +290,49 @@ void PathSetting::set(const std::string & str) value = canonPath(str); } +bool GlobalConfig::set(const std::string & name, const std::string & value) +{ + for (auto & config : *configRegistrations) + if (config->set(name, value)) return true; + + unknownSettings.emplace(name, value); + + return false; +} + +void GlobalConfig::getSettings(std::map & res, bool overridenOnly) +{ + for (auto & config : *configRegistrations) + config->getSettings(res, overridenOnly); +} + +void GlobalConfig::resetOverriden() +{ + for (auto & config : *configRegistrations) + config->resetOverriden(); +} + +void GlobalConfig::toJSON(JSONObject & out) +{ + for (auto & config : *configRegistrations) + config->toJSON(out); +} + +void GlobalConfig::convertToArgs(Args & args, const std::string & category) +{ + for (auto & config : *configRegistrations) + config->convertToArgs(args, category); +} + +GlobalConfig globalConfig; + +GlobalConfig::ConfigRegistrations * GlobalConfig::configRegistrations; + +GlobalConfig::Register::Register(Config * config) +{ + if (!configRegistrations) + configRegistrations = new ConfigRegistrations; + configRegistrations->emplace_back(config); +} + } diff --git a/src/libutil/config.hh b/src/libutil/config.hh index d2e7faf1743..d86c65ff033 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -12,6 +12,40 @@ class AbstractSetting; class JSONPlaceholder; class JSONObject; +class AbstractConfig +{ +protected: + StringMap unknownSettings; + + AbstractConfig(const StringMap & initials = {}) + : unknownSettings(initials) + { } + +public: + + virtual bool set(const std::string & name, const std::string & value) = 0; + + struct SettingInfo + { + std::string value; + std::string description; + }; + + virtual void getSettings(std::map & res, bool overridenOnly = false) = 0; + + void applyConfigFile(const Path & path); + + virtual void resetOverriden() = 0; + + virtual void toJSON(JSONObject & out) = 0; + + virtual void convertToArgs(Args & args, const std::string & category) = 0; + + void warnUnknownSettings(); + + void reapplyUnknownSettings(); +}; + /* A class to simplify providing configuration settings. The typical use is to inherit Config and add Setting members: @@ -27,7 +61,7 @@ class JSONObject; }; */ -class Config +class Config : public AbstractConfig { friend class AbstractSetting; @@ -48,31 +82,23 @@ private: Settings _settings; - StringMap extras; - public: - Config(const StringMap & initials) - : extras(initials) + Config(const StringMap & initials = {}) + : AbstractConfig(initials) { } - void set(const std::string & name, const std::string & value); + bool set(const std::string & name, const std::string & value) override; void addSetting(AbstractSetting * setting); - void handleUnknownSettings(); - - StringMap getSettings(bool overridenOnly = false); + void getSettings(std::map & res, bool overridenOnly = false) override; - const Settings & _getSettings() { return _settings; } - - void applyConfigFile(const Path & path); + void resetOverriden() override; - void resetOverriden(); + void toJSON(JSONObject & out) override; - void toJSON(JSONObject & out); - - void convertToArgs(Args & args, const std::string & category); + void convertToArgs(Args & args, const std::string & category) override; }; class AbstractSetting @@ -209,4 +235,27 @@ public: void operator =(const Path & v) { this->assign(v); } }; +struct GlobalConfig : public AbstractConfig +{ + typedef std::vector ConfigRegistrations; + static ConfigRegistrations * configRegistrations; + + bool set(const std::string & name, const std::string & value) override; + + void getSettings(std::map & res, bool overridenOnly = false) override; + + void resetOverriden() override; + + void toJSON(JSONObject & out) override; + + void convertToArgs(Args & args, const std::string & category) override; + + struct Register + { + Register(Config * config); + }; +}; + +extern GlobalConfig globalConfig; + } diff --git a/src/nix/main.cc b/src/nix/main.cc index bb107ec7d3f..9cd5d21c84b 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -34,9 +34,10 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs .handler([&]() { std::cout << "The following configuration options are available:\n\n"; Table2 tbl; - for (const auto & s : settings._getSettings()) - if (!s.second.isAlias) - tbl.emplace_back(s.first, s.second.setting->description); + std::map settings; + globalConfig.getSettings(settings); + for (const auto & s : settings) + tbl.emplace_back(s.first, s.second.description); printTable(std::cout, tbl); throw Exit(); }); diff --git a/src/nix/show-config.cc b/src/nix/show-config.cc index c64b12c8dd6..86638b50d2c 100644 --- a/src/nix/show-config.cc +++ b/src/nix/show-config.cc @@ -27,10 +27,12 @@ struct CmdShowConfig : Command, MixJSON if (json) { // FIXME: use appropriate JSON types (bool, ints, etc). JSONObject jsonObj(std::cout); - settings.toJSON(jsonObj); + globalConfig.toJSON(jsonObj); } else { - for (auto & s : settings.getSettings()) - std::cout << s.first + " = " + s.second + "\n"; + std::map settings; + globalConfig.getSettings(settings); + for (auto & s : settings) + std::cout << s.first + " = " + s.second.value + "\n"; } } }; diff --git a/tests/plugins/plugintest.cc b/tests/plugins/plugintest.cc index 8da15ebabd7..c085d33295b 100644 --- a/tests/plugins/plugintest.cc +++ b/tests/plugins/plugintest.cc @@ -1,16 +1,21 @@ -#include "globals.hh" +#include "config.hh" #include "primops.hh" using namespace nix; -static BaseSetting settingSet{false, "setting-set", +struct MySettings : Config +{ + Setting settingSet{this, false, "setting-set", "Whether the plugin-defined setting was set"}; +}; + +MySettings mySettings; -static RegisterSetting rs(&settingSet); +static GlobalConfig::Register rs(&mySettings); static void prim_anotherNull (EvalState & state, const Pos & pos, Value ** args, Value & v) { - if (settingSet) + if (mySettings.settingSet) mkNull(v); else mkBool(v, false);