Refactor unix domain socket store config (#11109)

Following what is outlined in #10766 refactor the uds-remote-store such that the member variables (state) don't live in the store itself but in the config object. Additionally, the config object includes a new necessary constructor that takes a scheme & authority. Tests are commented out because of linking errors with the current config system. When there is a new config system we can reenable them. Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
NixOS · Jul 18, 2024 · 57399bf · 57399bf
1 parent 17051ca
commit 57399bf
Show file tree

Hide file tree

Showing 26 changed files with 574 additions and 218 deletions.
diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc
@@ -6,6 +6,13 @@ namespace nix {
 struct DummyStoreConfig : virtual StoreConfig {
     using StoreConfig::StoreConfig;
 
+    DummyStoreConfig(std::string_view scheme, std::string_view authority, const Params & params)
+        : StoreConfig(params)
+    {
+        if (!authority.empty())
+            throw UsageError("`%s` store URIs must not contain an authority part %s", scheme, authority);
+    }
+
     const std::string name() override { return "Dummy Store"; }
 
     std::string doc() override
@@ -19,18 +26,15 @@ struct DummyStoreConfig : virtual StoreConfig {
 struct DummyStore : public virtual DummyStoreConfig, public virtual Store
 {
     DummyStore(std::string_view scheme, std::string_view authority, const Params & params)
-        : DummyStore(params)
-    {
-        if (!authority.empty())
-            throw UsageError("`%s` store URIs must not contain an authority part %s", scheme, authority);
-    }
-
-    DummyStore(const Params & params)
         : StoreConfig(params)
-        , DummyStoreConfig(params)
+        , DummyStoreConfig(scheme, authority, params)
         , Store(params)
     { }
 
+    DummyStore(const Params & params)
+        : DummyStore("dummy", "", params)
+    { }
+
     std::string getUri() override
     {
         return *uriSchemes().begin();

diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc
@@ -1,4 +1,4 @@
-#include "binary-cache-store.hh"
+#include "http-binary-cache-store.hh"
 #include "filetransfer.hh"
 #include "globals.hh"
 #include "nar-info-disk-cache.hh"
@@ -8,26 +8,37 @@ namespace nix {
 
 MakeError(UploadToHTTP, Error);
 
-struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
+
+HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig(
+    std::string_view scheme,
+    std::string_view _cacheUri,
+    const Params & params)
+    : StoreConfig(params)
+    , BinaryCacheStoreConfig(params)
+    , cacheUri(
+        std::string { scheme }
+        + "://"
+        + (!_cacheUri.empty()
+            ? _cacheUri
+            : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme)))
 {
-    using BinaryCacheStoreConfig::BinaryCacheStoreConfig;
+    while (!cacheUri.empty() && cacheUri.back() == '/')
+        cacheUri.pop_back();
+}
 
-    const std::string name() override { return "HTTP Binary Cache Store"; }
 
-    std::string doc() override
-    {
-        return
-          #include "http-binary-cache-store.md"
-          ;
-    }
-};
+std::string HttpBinaryCacheStoreConfig::doc()
+{
+    return
+      #include "http-binary-cache-store.md"
+      ;
+}
+
 
 class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public virtual BinaryCacheStore
 {
 private:
 
-    Path cacheUri;
-
     struct State
     {
         bool enabled = true;
@@ -40,23 +51,14 @@ class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public v
 
     HttpBinaryCacheStore(
         std::string_view scheme,
-        PathView _cacheUri,
+        PathView cacheUri,
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
-        , HttpBinaryCacheStoreConfig(params)
+        , HttpBinaryCacheStoreConfig(scheme, cacheUri, params)
         , Store(params)
         , BinaryCacheStore(params)
-        , cacheUri(
-            std::string { scheme }
-            + "://"
-            + (!_cacheUri.empty()
-                ? _cacheUri
-                : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme)))
     {
-        while (!cacheUri.empty() && cacheUri.back() == '/')
-            cacheUri.pop_back();
-
         diskCache = getNarInfoDiskCache();
     }
 

diff --git a/src/libstore/http-binary-cache-store.hh b/src/libstore/http-binary-cache-store.hh
@@ -0,0 +1,21 @@
+#include "binary-cache-store.hh"
+
+namespace nix {
+
+struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
+{
+    using BinaryCacheStoreConfig::BinaryCacheStoreConfig;
+
+    HttpBinaryCacheStoreConfig(std::string_view scheme, std::string_view _cacheUri, const Params & params);
+
+    Path cacheUri;
+
+    const std::string name() override
+    {
+        return "HTTP Binary Cache Store";
+    }
+
+    std::string doc() override;
+};
+
+}
diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc
@@ -1,4 +1,4 @@
-#include "binary-cache-store.hh"
+#include "local-binary-cache-store.hh"
 #include "globals.hh"
 #include "nar-info-disk-cache.hh"
 #include "signals.hh"
@@ -7,28 +7,27 @@
 
 namespace nix {
 
-struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
+LocalBinaryCacheStoreConfig::LocalBinaryCacheStoreConfig(
+    std::string_view scheme,
+    PathView binaryCacheDir,
+    const Params & params)
+    : StoreConfig(params)
+    , BinaryCacheStoreConfig(params)
+    , binaryCacheDir(binaryCacheDir)
 {
-    using BinaryCacheStoreConfig::BinaryCacheStoreConfig;
-
-    const std::string name() override { return "Local Binary Cache Store"; }
+}
 
-    std::string doc() override
-    {
-        return
-          #include "local-binary-cache-store.md"
-          ;
-    }
-};
 
-class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public virtual BinaryCacheStore
+std::string LocalBinaryCacheStoreConfig::doc()
 {
-private:
-
-    Path binaryCacheDir;
+    return
+      #include "local-binary-cache-store.md"
+      ;
+}
 
-public:
 
+struct LocalBinaryCacheStore : virtual LocalBinaryCacheStoreConfig, virtual BinaryCacheStore
+{
     /**
      * @param binaryCacheDir `file://` is a short-hand for `file:///`
      * for now.
@@ -39,10 +38,9 @@ class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
-        , LocalBinaryCacheStoreConfig(params)
+        , LocalBinaryCacheStoreConfig(scheme, binaryCacheDir, params)
         , Store(params)
         , BinaryCacheStore(params)
-        , binaryCacheDir(binaryCacheDir)
     {
     }
 

diff --git a/src/libstore/local-binary-cache-store.hh b/src/libstore/local-binary-cache-store.hh
@@ -0,0 +1,21 @@
+#include "binary-cache-store.hh"
+
+namespace nix {
+
+struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
+{
+    using BinaryCacheStoreConfig::BinaryCacheStoreConfig;
+
+    LocalBinaryCacheStoreConfig(std::string_view scheme, PathView binaryCacheDir, const Params & params);
+
+    Path binaryCacheDir;
+
+    const std::string name() override
+    {
+        return "Local Binary Cache Store";
+    }
+
+    std::string doc() override;
+};
+
+}
diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc
@@ -8,6 +8,20 @@
 
 namespace nix {
 
+LocalFSStoreConfig::LocalFSStoreConfig(PathView rootDir, const Params & params)
+    : StoreConfig(params)
+    // Default `?root` from `rootDir` if non set
+    // FIXME don't duplicate description once we don't have root setting
+    , rootDir{
+        this,
+        !rootDir.empty() && params.count("root") == 0
+            ? (std::optional<Path>{rootDir})
+            : std::nullopt,
+        "root",
+        "Directory prefixed to all other paths."}
+{
+}
+
 LocalFSStore::LocalFSStore(const Params & params)
     : Store(params)
 {

diff --git a/src/libstore/local-fs-store.hh b/src/libstore/local-fs-store.hh
@@ -11,6 +11,15 @@ struct LocalFSStoreConfig : virtual StoreConfig
 {
     using StoreConfig::StoreConfig;
 
+    /**
+     * Used to override the `root` settings. Can't be done via modifying
+     * `params` reliably because this parameter is unused except for
+     * passing to base class constructors.
+     *
+     * @todo Make this less error-prone with new store settings system.
+     */
+    LocalFSStoreConfig(PathView path, const Params & params);
+
     const OptionalPathSetting rootDir{this, std::nullopt,
         "root",
         "Directory prefixed to all other paths."};

diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc
@@ -18,11 +18,11 @@ Path LocalOverlayStoreConfig::toUpperPath(const StorePath & path) {
     return upperLayer + "/" + path.to_string();
 }
 
-LocalOverlayStore::LocalOverlayStore(const Params & params)
+LocalOverlayStore::LocalOverlayStore(std::string_view scheme, PathView path, const Params & params)
     : StoreConfig(params)
-    , LocalFSStoreConfig(params)
+    , LocalFSStoreConfig(path, params)
     , LocalStoreConfig(params)
-    , LocalOverlayStoreConfig(params)
+    , LocalOverlayStoreConfig(scheme, path, params)
     , Store(params)
     , LocalFSStore(params)
     , LocalStore(params)

diff --git a/src/libstore/local-overlay-store.hh b/src/libstore/local-overlay-store.hh
@@ -8,11 +8,16 @@ namespace nix {
 struct LocalOverlayStoreConfig : virtual LocalStoreConfig
 {
     LocalOverlayStoreConfig(const StringMap & params)
-        : StoreConfig(params)
-        , LocalFSStoreConfig(params)
-        , LocalStoreConfig(params)
+        : LocalOverlayStoreConfig("local-overlay", "", params)
     { }
 
+    LocalOverlayStoreConfig(std::string_view scheme, PathView path, const Params & params)
+        : StoreConfig(params)
+        , LocalFSStoreConfig(path, params)
+        , LocalStoreConfig(scheme, path, params)
+    {
+    }
+
     const Setting<std::string> lowerStoreUri{(StoreConfig*) this, "", "lower-store",
         R"(
           [Store URL](@docroot@/command-ref/new-cli/nix3-help-stores.md#store-url-format)
@@ -90,15 +95,13 @@ class LocalOverlayStore : public virtual LocalOverlayStoreConfig, public virtual
     ref<LocalFSStore> lowerStore;
 
 public:
-    LocalOverlayStore(const Params & params);
-
-    LocalOverlayStore(std::string_view scheme, PathView path, const Params & params)
-        : LocalOverlayStore(params)
+    LocalOverlayStore(const Params & params)
+        : LocalOverlayStore("local-overlay", "", params)
     {
-        if (!path.empty())
-            throw UsageError("local-overlay:// store url doesn't support path part, only scheme and query params");
     }
 
+    LocalOverlayStore(std::string_view scheme, PathView path, const Params & params);
+
     static std::set<std::string> uriSchemes()
     {
         return { "local-overlay" };

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
@@ -56,6 +56,15 @@
 
 namespace nix {
 
+LocalStoreConfig::LocalStoreConfig(
+    std::string_view scheme,
+    std::string_view authority,
+    const Params & params)
+    : StoreConfig(params)
+    , LocalFSStoreConfig(authority, params)
+{
+}
+
 std::string LocalStoreConfig::doc()
 {
     return
@@ -183,10 +192,13 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd)
     }
 }
 
-LocalStore::LocalStore(const Params & params)
+LocalStore::LocalStore(
+    std::string_view scheme,
+    PathView path,
+    const Params & params)
     : StoreConfig(params)
-    , LocalFSStoreConfig(params)
-    , LocalStoreConfig(params)
+    , LocalFSStoreConfig(path, params)
+    , LocalStoreConfig(scheme, path, params)
     , Store(params)
     , LocalFSStore(params)
     , dbDir(stateDir + "/db")
@@ -465,19 +477,8 @@ LocalStore::LocalStore(const Params & params)
 }
 
 
-LocalStore::LocalStore(
-    std::string_view scheme,
-    PathView path,
-    const Params & _params)
-    : LocalStore([&]{
-        // Default `?root` from `path` if non set
-        if (!path.empty() && _params.count("root") == 0) {
-            auto params = _params;
-            params.insert_or_assign("root", std::string { path });
-            return params;
-        }
-        return _params;
-    }())
+LocalStore::LocalStore(const Params & params)
+    : LocalStore("local", "", params)
 {
 }
 

diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
@@ -38,6 +38,11 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig
 {
     using LocalFSStoreConfig::LocalFSStoreConfig;
 
+    LocalStoreConfig(
+        std::string_view scheme,
+        std::string_view authority,
+        const Params & params);
+
     Setting<bool> requireSigs{this,
         settings.requireSigs,
         "require-sigs",

diff --git a/src/libstore/meson.build b/src/libstore/meson.build
@@ -243,10 +243,12 @@ headers = [config_h] + files(
   'filetransfer.hh',
   'gc-store.hh',
   'globals.hh',
+  'http-binary-cache-store.hh',
   'indirect-root-store.hh',
   'keys.hh',
   'legacy-ssh-store.hh',
   'length-prefixed-protocol-helper.hh',
+  'local-binary-cache-store.hh',
   'local-fs-store.hh',
   'local-overlay-store.hh',
   'local-store.hh',