Merge "libutil/config: unify path setting types" into main

src/libcmd/repl.cc

@@ -926,7 +926,7 @@ void NixRepl::loadFiles()
 
 void NixRepl::loadReplOverlays()
 {
-    if (!evalSettings.replOverlays) {
+    if (evalSettings.replOverlays.get().empty()) {
         return;
     }
 

src/libexpr/eval-settings.hh

@@ -151,7 +151,7 @@ struct EvalSettings : Config
           This is useful for debugging warnings in third-party Nix code.
         )"};
 
-    PathsSetting replOverlays{this, Paths(), "repl-overlays",
+    PathsSetting<Paths> replOverlays{this, Paths(), "repl-overlays",
         R"(
           A list of files containing Nix expressions that can be used to add
           default bindings to [`nix

src/libstore/globals.hh

@@ -634,7 +634,7 @@ public:
           line.
         )"};
 
-    OptionalPathSetting diffHook{
+    PathsSetting<std::optional<Path>> diffHook{
         this, std::nullopt, "diff-hook",
         R"(
           Absolute path to an executable capable of diffing build

src/libstore/local-fs-store.hh

@@ -11,21 +11,21 @@ struct LocalFSStoreConfig : virtual StoreConfig
 {
     using StoreConfig::StoreConfig;
 
-    const OptionalPathSetting rootDir{this, std::nullopt,
+    const PathsSetting<std::optional<Path>>  rootDir{this, std::nullopt,
         "root",
         "Directory prefixed to all other paths."};
 
-    const PathSetting stateDir{this,
+    const PathsSetting<Path> stateDir{this,
         rootDir.get() ? *rootDir.get() + "/nix/var/nix" : settings.nixStateDir,
         "state",
         "Directory where Lix will store state."};
 
-    const PathSetting logDir{this,
+    const PathsSetting<Path> logDir{this,
         rootDir.get() ? *rootDir.get() + "/nix/var/log/nix" : settings.nixLogDir,
         "log",
         "directory where Lix will store log files."};
 
-    const PathSetting realStoreDir{this,
+    const PathsSetting<Path> realStoreDir{this,
         rootDir.get() ? *rootDir.get() + "/nix/store" : storeDir, "real",
         "Physical path of the Nix store."};
 };

src/libstore/store-api.hh

@@ -144,7 +144,7 @@ struct StoreConfig : public Config
         return std::nullopt;
     }
 
-    const PathSetting storeDir_{this, settings.nixStore,
+    const PathsSetting<Path> storeDir_{this, settings.nixStore,
         "store",
         R"(
           Logical location of the Nix store, usually

src/libutil/config.cc

@@ -434,34 +434,12 @@ static Path parsePath(const AbstractSetting & s, const std::string & str)
         return canonPath(str);
 }
 
-PathSetting::PathSetting(Config * options,
+template<> Path PathsSetting<Path>::parse(const std::string & str) const
-    const Path & def,
-    const std::string & name,
-    const std::string & description,
-    const std::set<std::string> & aliases)
-    : BaseSetting<Path>(def, true, name, description, aliases)
-{
-    options->addSetting(this);
-}
-
-Path PathSetting::parse(const std::string & str) const
 {
     return parsePath(*this, str);
 }
 
-
+template<> std::optional<Path> PathsSetting<std::optional<Path>>::parse(const std::string & str) const
-OptionalPathSetting::OptionalPathSetting(Config * options,
-    const std::optional<Path> & def,
-    const std::string & name,
-    const std::string & description,
-    const std::set<std::string> & aliases)
-    : BaseSetting<std::optional<Path>>(def, true, name, description, aliases)
-{
-    options->addSetting(this);
-}
-
-
-std::optional<Path> OptionalPathSetting::parse(const std::string & str) const
 {
     if (str == "")
         return std::nullopt;
@@ -469,23 +447,7 @@ std::optional<Path> OptionalPathSetting::parse(const std::string & str) const
         return parsePath(*this, str);
 }
 
-void OptionalPathSetting::operator =(const std::optional<Path> & v)
+template<> Paths PathsSetting<Paths>::parse(const std::string & str) const
-{
-    this->assign(v);
-}
-
-PathsSetting::PathsSetting(Config * options,
-    const Paths & def,
-    const std::string & name,
-    const std::string & description,
-    const std::set<std::string> & aliases)
-    : BaseSetting<Paths>(def, true, name, description, aliases)
-{
-    options->addSetting(this);
-}
-
-
-Paths PathsSetting::parse(const std::string & str) const
 {
     auto strings = tokenizeString<Strings>(str);
     Paths parsed;
@@ -497,10 +459,10 @@ Paths PathsSetting::parse(const std::string & str) const
     return parsed;
 }
 
-PathsSetting::operator bool() const noexcept
+template class PathsSetting<Path>;
-{
+template class PathsSetting<std::optional<Path>>;
-    return !get().empty();
+template class PathsSetting<Paths>;
-}
+
 
 bool GlobalConfig::set(const std::string & name, const std::string & value)
 {

src/libutil/config.hh

@@ -353,69 +353,32 @@ public:
 };
 
 /**
- * A special setting for Paths. These are automatically canonicalised
+ * A special setting for Paths.
- * (e.g. "/foo//bar/" becomes "/foo/bar").
+ * These are automatically canonicalised (e.g. "/foo//bar/" becomes "/foo/bar").
- *
+ * The empty string is not permitted when a path is required.
- * It is mandatory to specify a path; i.e. the empty string is not
- * permitted.
  */
-class PathSetting : public BaseSetting<Path>
+template<typename T>
+class PathsSetting : public BaseSetting<T>
 {
 public:
-
-    PathSetting(Config * options,
-        const Path & def,
-        const std::string & name,
-        const std::string & description,
-        const std::set<std::string> & aliases = {});
-
-    Path parse(const std::string & str) const override;
-
-    Path operator +(const char * p) const { return value + p; }
-
-    void operator =(const Path & v) { this->assign(v); }
-};
-
-/**
- * Like `PathSetting`, but the absence of a path is also allowed.
- *
- * `std::optional` is used instead of the empty string for clarity.
- */
-class OptionalPathSetting : public BaseSetting<std::optional<Path>>
-{
-public:
-
-    OptionalPathSetting(Config * options,
-        const std::optional<Path> & def,
-        const std::string & name,
-        const std::string & description,
-        const std::set<std::string> & aliases = {});
-
-    std::optional<Path> parse(const std::string & str) const override;
-
-    void operator =(const std::optional<Path> & v);
-};
-
-/**
- * Like `OptionalPathSetting`, but allows multiple paths.
- */
-class PathsSetting : public BaseSetting<Paths>
-{
-public:
-
     PathsSetting(Config * options,
-        const Paths & def,
+        const T & def,
         const std::string & name,
         const std::string & description,
-        const std::set<std::string> & aliases = {});
+        const std::set<std::string> & aliases = {},
+        const bool documentDefault = true,
+        std::optional<ExperimentalFeature> experimentalFeature = std::nullopt)
+        : BaseSetting<T>(def, documentDefault, name, description, aliases, std::move(experimentalFeature))
+    {
+        options->addSetting(this);
+    }
 
-    Paths parse(const std::string & str) const override;
+    T parse(const std::string & str) const override;
 
-    void operator =(const Paths & v);
+    void operator =(const T & v) { this->assign(v); }
-
-    operator bool() const noexcept;
 };
 
+
 struct GlobalConfig : public AbstractConfig
 {
     typedef std::vector<Config*> ConfigRegistrations;

tests/unit/libutil/paths-setting.cc

@@ -15,7 +15,7 @@ public:
         : Config()
     { }
 
-    PathsSetting paths{this, Paths(), "paths", "documentation"};
+    PathsSetting<Paths> paths{this, Paths(), "paths", "documentation"};
 };
 
 struct PathsSettingTest : public ::testing::Test {
@@ -55,20 +55,6 @@ TEST_F(PathsSettingTest, parse) {
     );
 }
 
-TEST_F(PathsSettingTest, bool) {
-    auto config = mkConfig();
-    // No paths:
-    ASSERT_FALSE(config.paths);
-    // Set a path:
-    config.set("paths", "/puppy.nix");
-    // Now there are paths:
-    ASSERT_TRUE(config.paths);
-
-    // Multiple paths count too:
-    config.set("paths", "/puppy.nix /doggy.nix");
-    ASSERT_TRUE(config.paths);
-}
-
 TEST_F(PathsSettingTest, append) {
     auto config = mkConfig();