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>

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,16 +26,13 @@ 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);
-    }
+        : StoreConfig(params)
+        , DummyStoreConfig(scheme, authority, params)
+        , Store(params)
+    { }
 
     DummyStore(const Params & params)
-        : StoreConfig(params)
-        , DummyStoreConfig(params)
-        , Store(params)
+        : DummyStore("dummy", "", params)
     { }
 
     std::string getUri() override

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
+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 @@ public:
 
     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();
     }
 

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

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
+std::string LocalBinaryCacheStoreConfig::doc()
 {
     return
       #include "local-binary-cache-store.md"
       ;
 }
-};
 
-class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public virtual BinaryCacheStore
+
+struct LocalBinaryCacheStore : virtual LocalBinaryCacheStoreConfig, virtual BinaryCacheStore
 {
-private:
-
-    Path binaryCacheDir;
-
-public:
-
     /**
      * @param binaryCacheDir `file://` is a short-hand for `file:///`
      * for now.
@@ -39,10 +38,9 @@ public:
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
-        , LocalBinaryCacheStoreConfig(params)
+        , LocalBinaryCacheStoreConfig(scheme, binaryCacheDir, params)
         , Store(params)
         , BinaryCacheStore(params)
-        , binaryCacheDir(binaryCacheDir)
     {
     }
 

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

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

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."};

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)

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

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

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",

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',

src/libstore/s3-binary-cache-store.cc

@@ -1,5 +1,7 @@
 #if ENABLE_S3
 
+#include <assert.h>
+
 #include "s3.hh"
 #include "s3-binary-cache-store.hh"
 #include "nar-info.hh"
@@ -190,76 +192,31 @@ S3BinaryCacheStore::S3BinaryCacheStore(const Params & params)
     , BinaryCacheStore(params)
 { }
 
-struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
+
+S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig(
+    std::string_view uriScheme,
+    std::string_view bucketName,
+    const Params & params)
+    : StoreConfig(params)
+    , BinaryCacheStoreConfig(params)
+    , bucketName(bucketName)
 {
-    using BinaryCacheStoreConfig::BinaryCacheStoreConfig;
+    // Don't want to use use AWS SDK in header, so we check the default
+    // here. TODO do this better after we overhaul the store settings
+    // system.
+    assert(std::string{defaultRegion} == std::string{Aws::Region::US_EAST_1});
 
-    const Setting<std::string> profile{this, "", "profile",
-        R"(
-          The name of the AWS configuration profile to use. By default
-          Nix will use the `default` profile.
-        )"};
+    if (bucketName.empty())
+        throw UsageError("`%s` store requires a bucket name in its Store URI", uriScheme);
+}
 
-    const Setting<std::string> region{this, Aws::Region::US_EAST_1, "region",
-        R"(
-          The region of the S3 bucket. If your bucket is not in
-          `us–east-1`, you should always explicitly specify the region
-          parameter.
-        )"};
-
-    const Setting<std::string> scheme{this, "", "scheme",
-        R"(
-          The scheme used for S3 requests, `https` (default) or `http`. This
-          option allows you to disable HTTPS for binary caches which don't
-          support it.
-
-          > **Note**
-          >
-          > HTTPS should be used if the cache might contain sensitive
-          > information.
-        )"};
-
-    const Setting<std::string> endpoint{this, "", "endpoint",
-        R"(
-          The URL of the endpoint of an S3-compatible service such as MinIO.
-          Do not specify this setting if you're using Amazon S3.
-
-          > **Note**
-          >
-          > This endpoint must support HTTPS and will use path-based
-          > addressing instead of virtual host based addressing.
-        )"};
-
-    const Setting<std::string> narinfoCompression{this, "", "narinfo-compression",
-        "Compression method for `.narinfo` files."};
-
-    const Setting<std::string> lsCompression{this, "", "ls-compression",
-        "Compression method for `.ls` files."};
-
-    const Setting<std::string> logCompression{this, "", "log-compression",
-        R"(
-          Compression method for `log/*` files. It is recommended to
-          use a compression method supported by most web browsers
-          (e.g. `brotli`).
-        )"};
-
-    const Setting<bool> multipartUpload{
-        this, false, "multipart-upload",
-        "Whether to use multi-part uploads."};
-
-    const Setting<uint64_t> bufferSize{
-        this, 5 * 1024 * 1024, "buffer-size",
-        "Size (in bytes) of each part in multi-part uploads."};
-
-    const std::string name() override { return "S3 Binary Cache Store"; }
-
-    std::string doc() override
+std::string S3BinaryCacheStoreConfig::doc()
 {
     return
       #include "s3-binary-cache-store.md"
       ;
 }
-};
+
 
 struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual S3BinaryCacheStore
 {
@@ -275,15 +232,12 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
-        , S3BinaryCacheStoreConfig(params)
+        , S3BinaryCacheStoreConfig(uriScheme, bucketName, params)
         , Store(params)
         , BinaryCacheStore(params)
         , S3BinaryCacheStore(params)
-        , bucketName(bucketName)
         , s3Helper(profile, region, scheme, endpoint)
     {
-        if (bucketName.empty())
-            throw UsageError("`%s` store requires a bucket name in its Store URI", uriScheme);
         diskCache = getNarInfoDiskCache();
     }
 

src/libstore/s3-binary-cache-store.hh

@@ -7,6 +7,96 @@
 
 namespace nix {
 
+struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
+{
+    std::string bucketName;
+
+    using BinaryCacheStoreConfig::BinaryCacheStoreConfig;
+
+    S3BinaryCacheStoreConfig(std::string_view uriScheme, std::string_view bucketName, const Params & params);
+
+    const Setting<std::string> profile{
+        this,
+        "",
+        "profile",
+        R"(
+          The name of the AWS configuration profile to use. By default
+          Nix will use the `default` profile.
+        )"};
+
+protected:
+
+    constexpr static const char * defaultRegion = "us-east-1";
+
+public:
+
+    const Setting<std::string> region{
+        this,
+        defaultRegion,
+        "region",
+        R"(
+          The region of the S3 bucket. If your bucket is not in
+          `us–east-1`, you should always explicitly specify the region
+          parameter.
+        )"};
+
+    const Setting<std::string> scheme{
+        this,
+        "",
+        "scheme",
+        R"(
+          The scheme used for S3 requests, `https` (default) or `http`. This
+          option allows you to disable HTTPS for binary caches which don't
+          support it.
+
+          > **Note**
+          >
+          > HTTPS should be used if the cache might contain sensitive
+          > information.
+        )"};
+
+    const Setting<std::string> endpoint{
+        this,
+        "",
+        "endpoint",
+        R"(
+          The URL of the endpoint of an S3-compatible service such as MinIO.
+          Do not specify this setting if you're using Amazon S3.
+
+          > **Note**
+          >
+          > This endpoint must support HTTPS and will use path-based
+          > addressing instead of virtual host based addressing.
+        )"};
+
+    const Setting<std::string> narinfoCompression{
+        this, "", "narinfo-compression", "Compression method for `.narinfo` files."};
+
+    const Setting<std::string> lsCompression{this, "", "ls-compression", "Compression method for `.ls` files."};
+
+    const Setting<std::string> logCompression{
+        this,
+        "",
+        "log-compression",
+        R"(
+          Compression method for `log/*` files. It is recommended to
+          use a compression method supported by most web browsers
+          (e.g. `brotli`).
+        )"};
+
+    const Setting<bool> multipartUpload{this, false, "multipart-upload", "Whether to use multi-part uploads."};
+
+    const Setting<uint64_t> bufferSize{
+        this, 5 * 1024 * 1024, "buffer-size", "Size (in bytes) of each part in multi-part uploads."};
+
+    const std::string name() override
+    {
+        return "S3 Binary Cache Store";
+    }
+
+    std::string doc() override;
+};
+
 class S3BinaryCacheStore : public virtual BinaryCacheStore
 {
 protected:

src/libstore/ssh-store.cc

@@ -89,12 +89,8 @@ protected:
     };
 };
 
-struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfig
-{
-    using SSHStoreConfig::SSHStoreConfig;
-    using LocalFSStoreConfig::LocalFSStoreConfig;
 
-    MountedSSHStoreConfig(StringMap params)
+MountedSSHStoreConfig::MountedSSHStoreConfig(StringMap params)
     : StoreConfig(params)
     , RemoteStoreConfig(params)
     , CommonSSHStoreConfig(params)
@@ -103,7 +99,7 @@ struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfi
 {
 }
 
-    MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params)
+MountedSSHStoreConfig::MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params)
     : StoreConfig(params)
     , RemoteStoreConfig(params)
     , CommonSSHStoreConfig(scheme, host, params)
@@ -112,20 +108,13 @@ struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfi
 {
 }
 
-    const std::string name() override { return "Experimental SSH Store with filesystem mounted"; }
-
-    std::string doc() override
+std::string MountedSSHStoreConfig::doc()
 {
     return
       #include "mounted-ssh-store.md"
       ;
 }
 
-    std::optional<ExperimentalFeature> experimentalFeature() const override
-    {
-        return ExperimentalFeature::MountedSSHStore;
-    }
-};
 
 /**
  * The mounted ssh store assumes that filesystems on the remote host are

src/libstore/ssh-store.hh

@@ -3,6 +3,7 @@
 
 #include "common-ssh-store-config.hh"
 #include "store-api.hh"
+#include "local-fs-store.hh"
 #include "remote-store.hh"
 
 namespace nix {
@@ -25,4 +26,26 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
     std::string doc() override;
 };
 
+struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfig
+{
+    using LocalFSStoreConfig::LocalFSStoreConfig;
+    using SSHStoreConfig::SSHStoreConfig;
+
+    MountedSSHStoreConfig(StringMap params);
+
+    MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params);
+
+    const std::string name() override
+    {
+        return "Experimental SSH Store with filesystem mounted";
+    }
+
+    std::string doc() override;
+
+    std::optional<ExperimentalFeature> experimentalFeature() const override
+    {
+        return ExperimentalFeature::MountedSSHStore;
+    }
+};
+
 }

src/libstore/uds-remote-store.cc

@@ -2,10 +2,8 @@
 #include "unix-domain-socket.hh"
 #include "worker-protocol.hh"
 
-#include <cstring>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
 
@@ -19,6 +17,21 @@
 
 namespace nix {
 
+UDSRemoteStoreConfig::UDSRemoteStoreConfig(
+    std::string_view scheme,
+    std::string_view authority,
+    const Params & params)
+    : StoreConfig(params)
+    , LocalFSStoreConfig(params)
+    , RemoteStoreConfig(params)
+    , path{authority.empty() ? settings.nixDaemonSocketFile : authority}
+{
+    if (scheme != UDSRemoteStoreConfig::scheme) {
+        throw UsageError("Scheme must be 'unix'");
+    }
+}
+
+
 std::string UDSRemoteStoreConfig::doc()
 {
     return
@@ -27,11 +40,20 @@ std::string UDSRemoteStoreConfig::doc()
 }
 
 
+// A bit gross that we now pass empty string but this is knowing that
+// empty string will later default to the same nixDaemonSocketFile. Why
+// don't we just wire it all through? I believe there are cases where it
+// will live reload so we want to continue to account for that.
 UDSRemoteStore::UDSRemoteStore(const Params & params)
+    : UDSRemoteStore(scheme, "", params)
+{}
+
+
+UDSRemoteStore::UDSRemoteStore(std::string_view scheme, std::string_view authority, const Params & params)
     : StoreConfig(params)
     , LocalFSStoreConfig(params)
     , RemoteStoreConfig(params)
-    , UDSRemoteStoreConfig(params)
+    , UDSRemoteStoreConfig(scheme, authority, params)
     , Store(params)
     , LocalFSStore(params)
     , RemoteStore(params)
@@ -39,25 +61,15 @@ UDSRemoteStore::UDSRemoteStore(const Params & params)
 }
 
 
-UDSRemoteStore::UDSRemoteStore(
-    std::string_view scheme,
-    PathView socket_path,
-    const Params & params)
-    : UDSRemoteStore(params)
-{
-    if (!socket_path.empty())
-        path.emplace(socket_path);
-}
-
-
 std::string UDSRemoteStore::getUri()
 {
-    if (path) {
-        return std::string("unix://") + *path;
-    } else {
+    return path == settings.nixDaemonSocketFile
+        ? // FIXME: Not clear why we return daemon here and not default
+          // to settings.nixDaemonSocketFile
+          //
           // unix:// with no path also works. Change what we return?
-        return "daemon";
-    }
+          "daemon"
+        : std::string(scheme) + "://" + path;
 }
 
 
@@ -74,7 +86,7 @@ ref<RemoteStore::Connection> UDSRemoteStore::openConnection()
     /* Connect to a daemon that does the privileged work for us. */
     conn->fd = createUnixDomainSocket();
 
-    nix::connect(toSocket(conn->fd.get()), path ? *path : settings.nixDaemonSocketFile);
+    nix::connect(toSocket(conn->fd.get()), path);
 
     conn->from.fd = conn->fd.get();
     conn->to.fd = conn->fd.get();

src/libstore/uds-remote-store.hh

@@ -9,16 +9,33 @@ namespace nix {
 
 struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreConfig
 {
-    UDSRemoteStoreConfig(const Params & params)
-        : StoreConfig(params)
-        , LocalFSStoreConfig(params)
-        , RemoteStoreConfig(params)
-    {
-    }
+    // TODO(fzakaria): Delete this constructor once moved over to the factory pattern
+    // outlined in https://github.com/NixOS/nix/issues/10766
+    using LocalFSStoreConfig::LocalFSStoreConfig;
+    using RemoteStoreConfig::RemoteStoreConfig;
+
+    /**
+     * @param authority is the socket path.
+     */
+    UDSRemoteStoreConfig(
+        std::string_view scheme,
+        std::string_view authority,
+        const Params & params);
 
     const std::string name() override { return "Local Daemon Store"; }
 
     std::string doc() override;
+
+    /**
+     * The path to the unix domain socket.
+     *
+     * The default is `settings.nixDaemonSocketFile`, but we don't write
+     * that below, instead putting in the constructor.
+     */
+    Path path;
+
+protected:
+    static constexpr char const * scheme = "unix";
 };
 
 class UDSRemoteStore : public virtual UDSRemoteStoreConfig
@@ -27,16 +44,23 @@ class UDSRemoteStore : public virtual UDSRemoteStoreConfig
 {
 public:
 
+    /**
+     * @deprecated This is the old API to construct the store.
+    */
     UDSRemoteStore(const Params & params);
+
+    /**
+     * @param authority is the socket path.
+     */
     UDSRemoteStore(
         std::string_view scheme,
-        PathView path,
+        std::string_view authority,
         const Params & params);
 
     std::string getUri() override;
 
     static std::set<std::string> uriSchemes()
-    { return {"unix"}; }
+    { return {scheme}; }
 
     ref<SourceAccessor> getFSAccessor(bool requireValidPath = true) override
     { return LocalFSStore::getFSAccessor(requireValidPath); }
@@ -63,7 +87,6 @@ private:
     };
 
     ref<RemoteStore::Connection> openConnection() override;
-    std::optional<std::string> path;
 };
 
 }

tests/unit/libstore/http-binary-cache-store.cc

@@ -0,0 +1,21 @@
+#include <gtest/gtest.h>
+
+#include "http-binary-cache-store.hh"
+
+namespace nix {
+
+TEST(HttpBinaryCacheStore, constructConfig)
+{
+    HttpBinaryCacheStoreConfig config{"http", "foo.bar.baz", {}};
+
+    EXPECT_EQ(config.cacheUri, "http://foo.bar.baz");
+}
+
+TEST(HttpBinaryCacheStore, constructConfigNoTrailingSlash)
+{
+    HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b/", {}};
+
+    EXPECT_EQ(config.cacheUri, "https://foo.bar.baz/a/b");
+}
+
+} // namespace nix

tests/unit/libstore/local-binary-cache-store.cc

@@ -0,0 +1,14 @@
+#include <gtest/gtest.h>
+
+#include "local-binary-cache-store.hh"
+
+namespace nix {
+
+TEST(LocalBinaryCacheStore, constructConfig)
+{
+    LocalBinaryCacheStoreConfig config{"local", "/foo/bar/baz", {}};
+
+    EXPECT_EQ(config.binaryCacheDir, "/foo/bar/baz");
+}
+
+} // namespace nix

tests/unit/libstore/local-overlay-store.cc

@@ -0,0 +1,34 @@
+// FIXME: Odd failures for templates that are causing the PR to break
+// for now with discussion with @Ericson2314 to comment out.
+#if 0
+#  include <gtest/gtest.h>
+
+#  include "local-overlay-store.hh"
+
+namespace nix {
+
+TEST(LocalOverlayStore, constructConfig_rootQueryParam)
+{
+    LocalOverlayStoreConfig config{
+        "local-overlay",
+        "",
+        {
+            {
+                "root",
+                "/foo/bar",
+            },
+        },
+    };
+
+    EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"});
+}
+
+TEST(LocalOverlayStore, constructConfig_rootPath)
+{
+    LocalOverlayStoreConfig config{"local-overlay", "/foo/bar", {}};
+
+    EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"});
+}
+
+} // namespace nix
+#endif

tests/unit/libstore/local-store.cc

@@ -0,0 +1,40 @@
+// FIXME: Odd failures for templates that are causing the PR to break
+// for now with discussion with @Ericson2314 to comment out.
+#if 0
+#  include <gtest/gtest.h>
+
+#  include "local-store.hh"
+
+// Needed for template specialisations. This is not good! When we
+// overhaul how store configs work, this should be fixed.
+#  include "args.hh"
+#  include "config-impl.hh"
+#  include "abstract-setting-to-json.hh"
+
+namespace nix {
+
+TEST(LocalStore, constructConfig_rootQueryParam)
+{
+    LocalStoreConfig config{
+        "local",
+        "",
+        {
+            {
+                "root",
+                "/foo/bar",
+            },
+        },
+    };
+
+    EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"});
+}
+
+TEST(LocalStore, constructConfig_rootPath)
+{
+    LocalStoreConfig config{"local", "/foo/bar", {}};
+
+    EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"});
+}
+
+} // namespace nix
+#endif

tests/unit/libstore/meson.build

@@ -58,7 +58,11 @@ sources = files(
   'derivation.cc',
   'derived-path.cc',
   'downstream-placeholder.cc',
+  'http-binary-cache-store.cc',
   'legacy-ssh-store.cc',
+  'local-binary-cache-store.cc',
+  'local-overlay-store.cc',
+  'local-store.cc',
   'machines.cc',
   'nar-info-disk-cache.cc',
   'nar-info.cc',
@@ -67,9 +71,11 @@ sources = files(
   'path-info.cc',
   'path.cc',
   'references.cc',
+  's3-binary-cache-store.cc',
   'serve-protocol.cc',
   'ssh-store.cc',
   'store-reference.cc',
+  'uds-remote-store.cc',
   'worker-protocol.cc',
 )
 

tests/unit/libstore/s3-binary-cache-store.cc

@@ -0,0 +1,18 @@
+#if ENABLE_S3
+
+#  include <gtest/gtest.h>
+
+#  include "s3-binary-cache-store.hh"
+
+namespace nix {
+
+TEST(S3BinaryCacheStore, constructConfig)
+{
+    S3BinaryCacheStoreConfig config{"s3", "foobar", {}};
+
+    EXPECT_EQ(config.bucketName, "foobar");
+}
+
+} // namespace nix
+
+#endif

tests/unit/libstore/ssh-store.cc

@@ -1,3 +1,6 @@
+// FIXME: Odd failures for templates that are causing the PR to break
+// for now with discussion with @Ericson2314 to comment out.
+#if 0
 #  include <gtest/gtest.h>
 
 #  include "ssh-store.hh"
@@ -15,7 +18,9 @@ TEST(SSHStore, constructConfig)
                 // TODO #11106, no more split on space
                 "foo bar",
             },
-        }};
+        },
+    };
+
     EXPECT_EQ(
         config.remoteProgram.get(),
         (Strings{
@@ -23,4 +28,28 @@ TEST(SSHStore, constructConfig)
             "bar",
         }));
 }
+
+TEST(MountedSSHStore, constructConfig)
+{
+    MountedSSHStoreConfig config{
+        "mounted-ssh",
+        "localhost",
+        StoreConfig::Params{
+            {
+                "remote-program",
+                // TODO #11106, no more split on space
+                "foo bar",
+            },
+        },
+    };
+
+    EXPECT_EQ(
+        config.remoteProgram.get(),
+        (Strings{
+            "foo",
+            "bar",
+        }));
 }
+
+}
+#endif

tests/unit/libstore/uds-remote-store.cc

@@ -0,0 +1,23 @@
+// FIXME: Odd failures for templates that are causing the PR to break
+// for now with discussion with @Ericson2314 to comment out.
+#if 0
+#  include <gtest/gtest.h>
+
+#  include "uds-remote-store.hh"
+
+namespace nix {
+
+TEST(UDSRemoteStore, constructConfig)
+{
+    UDSRemoteStoreConfig config{"unix", "/tmp/socket", {}};
+
+    EXPECT_EQ(config.path, "/tmp/socket");
+}
+
+TEST(UDSRemoteStore, constructConfigWrongScheme)
+{
+    EXPECT_THROW(UDSRemoteStoreConfig("http", "/tmp/socket", {}), UsageError);
+}
+
+} // namespace nix
+#endif