From 7d5bdf8b5679cc7b2b9b4d9caf5af9ca52211336 Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Tue, 8 Sep 2020 14:50:23 +0200
Subject: [PATCH] Make the store plugins more introspectable

Directly register the store classes rather than a function to build an
instance of them.
This gives the possibility to introspect static members of the class or
choose different ways of instantiating them.
---
 result                                   |  1 +
 src/libexpr/flake/lockfile.hh            |  2 +-
 src/libfetchers/fetchers.hh              |  2 +-
 src/libstore/dummy-store.cc              | 18 +++-----
 src/libstore/http-binary-cache-store.cc  | 24 +++++------
 src/libstore/legacy-ssh-store.cc         | 15 +++----
 src/libstore/local-binary-cache-store.cc | 23 +++++-----
 src/libstore/remote-store.cc             | 10 +----
 src/libstore/remote-store.hh             |  3 ++
 src/libstore/s3-binary-cache-store.cc    | 15 +++----
 src/libstore/ssh-store.cc                | 14 ++----
 src/libstore/store-api.cc                | 55 +++++++++++++-----------
 src/libstore/store-api.hh                | 31 ++++++++-----
 src/libstore/store.hh                    |  9 ++++
 src/nix/describe-stores.cc               | 30 +++++++++++++
 src/nix/develop.cc                       |  2 +-
 src/nix/diff-closures.cc                 |  2 +-
 src/nix/installables.hh                  |  2 +-
 18 files changed, 141 insertions(+), 117 deletions(-)
 create mode 120000 result
 create mode 100644 src/libstore/store.hh
 create mode 100644 src/nix/describe-stores.cc

diff --git a/result b/result
new file mode 120000
index 000000000..af7bbb6da
--- /dev/null
+++ b/result
@@ -0,0 +1 @@
+/nix/store/9pqfirjppd91mzhkgh8xnn66iwh53zk2-hello-2.10
\ No newline at end of file
diff --git a/src/libexpr/flake/lockfile.hh b/src/libexpr/flake/lockfile.hh
index 5e7cfda3e..9ec8b39c3 100644
--- a/src/libexpr/flake/lockfile.hh
+++ b/src/libexpr/flake/lockfile.hh
@@ -6,7 +6,7 @@
 
 namespace nix {
 class Store;
-struct StorePath;
+class StorePath;
 }
 
 namespace nix::flake {
diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh
index be71b786b..89b1e6e7d 100644
--- a/src/libfetchers/fetchers.hh
+++ b/src/libfetchers/fetchers.hh
@@ -23,7 +23,7 @@ struct InputScheme;
 
 struct Input
 {
-    friend class InputScheme;
+    friend struct InputScheme;
 
     std::shared_ptr<InputScheme> scheme; // note: can be null
     Attrs attrs;
diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc
index 7a5744bc1..a677e201e 100644
--- a/src/libstore/dummy-store.cc
+++ b/src/libstore/dummy-store.cc
@@ -2,17 +2,15 @@
 
 namespace nix {
 
-static std::string uriScheme = "dummy://";
-
 struct DummyStore : public Store
 {
-    DummyStore(const Params & params)
+    DummyStore(const std::string uri, const Params & params)
         : Store(params)
     { }
 
     string getUri() override
     {
-        return uriScheme;
+        return uriPrefixes()[0] + "://";
     }
 
     void queryPathInfoUncached(const StorePath & path,
@@ -21,6 +19,10 @@ struct DummyStore : public Store
         callback(nullptr);
     }
 
+    static std::vector<std::string> uriPrefixes() {
+        return {"dummy"};
+    }
+
     std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override
     { unsupported("queryPathFromHashPart"); }
 
@@ -48,12 +50,6 @@ struct DummyStore : public Store
     { unsupported("buildDerivation"); }
 };
 
-static RegisterStoreImplementation regStore([](
-    const std::string & uri, const Store::Params & params)
-    -> std::shared_ptr<Store>
-{
-    if (uri != uriScheme) return nullptr;
-    return std::make_shared<DummyStore>(params);
-});
+[[maybe_unused]] static RegisterStoreImplementation<DummyStore> regStore();
 
 }
diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc
index 1733239fb..6c3a16faf 100644
--- a/src/libstore/http-binary-cache-store.cc
+++ b/src/libstore/http-binary-cache-store.cc
@@ -24,7 +24,8 @@ private:
 public:
 
     HttpBinaryCacheStore(
-        const Params & params, const Path & _cacheUri)
+        const Path & _cacheUri,
+        const Params & params)
         : BinaryCacheStore(params)
         , cacheUri(_cacheUri)
     {
@@ -55,6 +56,13 @@ public:
         }
     }
 
+    static std::vector<std::string> uriPrefixes()
+    {
+        static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1";
+        auto ret = std::vector<std::string>({"http", "https"});
+        if (forceHttp) ret.push_back("file");
+        return ret;
+    }
 protected:
 
     void maybeDisable()
@@ -162,18 +170,6 @@ protected:
 
 };
 
-static RegisterStoreImplementation regStore([](
-    const std::string & uri, const Store::Params & params)
-    -> std::shared_ptr<Store>
-{
-    static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1";
-    if (std::string(uri, 0, 7) != "http://" &&
-        std::string(uri, 0, 8) != "https://" &&
-        (!forceHttp || std::string(uri, 0, 7) != "file://"))
-        return 0;
-    auto store = std::make_shared<HttpBinaryCacheStore>(params, uri);
-    store->init();
-    return store;
-});
+[[maybe_unused]] static RegisterStoreImplementation<HttpBinaryCacheStore> regStore();
 
 }
diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc
index dc03313f0..72f28a82d 100644
--- a/src/libstore/legacy-ssh-store.cc
+++ b/src/libstore/legacy-ssh-store.cc
@@ -9,8 +9,6 @@
 
 namespace nix {
 
-static std::string uriScheme = "ssh://";
-
 struct LegacySSHStore : public Store
 {
     const Setting<int> maxConnections{this, 1, "max-connections", "maximum number of concurrent SSH connections"};
@@ -37,6 +35,9 @@ struct LegacySSHStore : public Store
 
     SSHMaster master;
 
+    static std::vector<std::string> uriPrefixes() { return {"ssh"}; }
+
+
     LegacySSHStore(const string & host, const Params & params)
         : Store(params)
         , host(host)
@@ -84,7 +85,7 @@ struct LegacySSHStore : public Store
 
     string getUri() override
     {
-        return uriScheme + host;
+        return uriPrefixes()[0] + "://" + host;
     }
 
     void queryPathInfoUncached(const StorePath & path,
@@ -325,12 +326,6 @@ public:
     }
 };
 
-static RegisterStoreImplementation regStore([](
-    const std::string & uri, const Store::Params & params)
-    -> std::shared_ptr<Store>
-{
-    if (std::string(uri, 0, uriScheme.size()) != uriScheme) return 0;
-    return std::make_shared<LegacySSHStore>(std::string(uri, uriScheme.size()), params);
-});
+[[maybe_unused]] static RegisterStoreImplementation<LegacySSHStore> regStore();
 
 }
diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc
index 87d8334d7..752838d3f 100644
--- a/src/libstore/local-binary-cache-store.cc
+++ b/src/libstore/local-binary-cache-store.cc
@@ -13,7 +13,8 @@ private:
 public:
 
     LocalBinaryCacheStore(
-        const Params & params, const Path & binaryCacheDir)
+        const Path & binaryCacheDir,
+        const Params & params)
         : BinaryCacheStore(params)
         , binaryCacheDir(binaryCacheDir)
     {
@@ -26,6 +27,8 @@ public:
         return "file://" + binaryCacheDir;
     }
 
+    static std::vector<std::string> uriPrefixes();
+
 protected:
 
     bool fileExists(const std::string & path) override;
@@ -85,16 +88,14 @@ bool LocalBinaryCacheStore::fileExists(const std::string & path)
     return pathExists(binaryCacheDir + "/" + path);
 }
 
-static RegisterStoreImplementation regStore([](
-    const std::string & uri, const Store::Params & params)
-    -> std::shared_ptr<Store>
+std::vector<std::string> LocalBinaryCacheStore::uriPrefixes()
 {
-    if (getEnv("_NIX_FORCE_HTTP_BINARY_CACHE_STORE") == "1" ||
-        std::string(uri, 0, 7) != "file://")
-        return 0;
-    auto store = std::make_shared<LocalBinaryCacheStore>(params, std::string(uri, 7));
-    store->init();
-    return store;
-});
+    if (getEnv("_NIX_FORCE_HTTP_BINARY_CACHE_STORE") == "1")
+        return {};
+    else
+        return {"file"};
+}
+
+[[maybe_unused]] static RegisterStoreImplementation<LocalBinaryCacheStore> regStore();
 
 }
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index dc61951d3..824b6b140 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -982,14 +982,6 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source *
     return nullptr;
 }
 
-static std::string uriScheme = "unix://";
-
-static RegisterStoreImplementation regStore([](
-    const std::string & uri, const Store::Params & params)
-    -> std::shared_ptr<Store>
-{
-    if (std::string(uri, 0, uriScheme.size()) != uriScheme) return 0;
-    return std::make_shared<UDSRemoteStore>(std::string(uri, uriScheme.size()), params);
-});
+[[maybe_unused]] static RegisterStoreImplementation<UDSRemoteStore> regStore();
 
 }
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index 6f05f2197..07cd5daf8 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -150,6 +150,9 @@ public:
 
     std::string getUri() override;
 
+    static std::vector<std::string> uriPrefixes()
+    { return {"unix"}; }
+
     bool sameMachine() override
     { return true; }
 
diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc
index a0a446bd3..1bf4d5955 100644
--- a/src/libstore/s3-binary-cache-store.cc
+++ b/src/libstore/s3-binary-cache-store.cc
@@ -193,7 +193,8 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore
     S3Helper s3Helper;
 
     S3BinaryCacheStoreImpl(
-        const Params & params, const std::string & bucketName)
+        const std::string & bucketName,
+        const Params & params)
         : S3BinaryCacheStore(params)
         , bucketName(bucketName)
         , s3Helper(profile, region, scheme, endpoint)
@@ -426,17 +427,11 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore
         return paths;
     }
 
+    static std::vector<std::string> uriPrefixes() { return {"s3"}; }
+
 };
 
-static RegisterStoreImplementation regStore([](
-    const std::string & uri, const Store::Params & params)
-    -> std::shared_ptr<Store>
-{
-    if (std::string(uri, 0, 5) != "s3://") return 0;
-    auto store = std::make_shared<S3BinaryCacheStoreImpl>(params, std::string(uri, 5));
-    store->init();
-    return store;
-});
+[[maybe_unused]] static RegisterStoreImplementation<S3BinaryCacheStoreImpl> regStore();
 
 }
 
diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc
index 6cb97c1f1..02208ee82 100644
--- a/src/libstore/ssh-store.cc
+++ b/src/libstore/ssh-store.cc
@@ -8,8 +8,6 @@
 
 namespace nix {
 
-static std::string uriScheme = "ssh-ng://";
-
 class SSHStore : public RemoteStore
 {
 public:
@@ -32,9 +30,11 @@ public:
     {
     }
 
+    static std::vector<std::string> uriPrefixes() { return {"ssh-ng"}; }
+
     std::string getUri() override
     {
-        return uriScheme + host;
+        return uriPrefixes()[0] + "://" + host;
     }
 
     bool sameMachine() override
@@ -76,12 +76,6 @@ ref<RemoteStore::Connection> SSHStore::openConnection()
     return conn;
 }
 
-static RegisterStoreImplementation regStore([](
-    const std::string & uri, const Store::Params & params)
-    -> std::shared_ptr<Store>
-{
-    if (std::string(uri, 0, uriScheme.size()) != uriScheme) return 0;
-    return std::make_shared<SSHStore>(std::string(uri, uriScheme.size()), params);
-});
+[[maybe_unused]] static RegisterStoreImplementation<SSHStore> regStore();
 
 }
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index b3877487c..4a46b5160 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -1009,6 +1009,11 @@ Derivation Store::readDerivation(const StorePath & drvPath)
     }
 }
 
+std::shared_ptr<Config> Store::getConfig()
+{
+    return shared_from_this();
+}
+
 
 }
 
@@ -1019,9 +1024,6 @@ Derivation Store::readDerivation(const StorePath & drvPath)
 
 namespace nix {
 
-
-RegisterStoreImplementation::Implementations * RegisterStoreImplementation::implementations = 0;
-
 /* Split URI into protocol+hierarchy part and its parameter set. */
 std::pair<std::string, Store::Params> splitUriAndParams(const std::string & uri_)
 {
@@ -1035,24 +1037,6 @@ std::pair<std::string, Store::Params> splitUriAndParams(const std::string & uri_
     return {uri, params};
 }
 
-ref<Store> openStore(const std::string & uri_,
-    const Store::Params & extraParams)
-{
-    auto [uri, uriParams] = splitUriAndParams(uri_);
-    auto params = extraParams;
-    params.insert(uriParams.begin(), uriParams.end());
-
-    for (auto fun : *RegisterStoreImplementation::implementations) {
-        auto store = fun(uri, params);
-        if (store) {
-            store->warnUnknownSettings();
-            return ref<Store>(store);
-        }
-    }
-
-    throw Error("don't know how to open Nix store '%s'", uri);
-}
-
 static bool isNonUriPath(const std::string & spec) {
     return
         // is not a URL
@@ -1080,10 +1064,7 @@ StoreType getStoreType(const std::string & uri, const std::string & stateDir)
     }
 }
 
-
-static RegisterStoreImplementation regStore([](
-    const std::string & uri, const Store::Params & params)
-    -> std::shared_ptr<Store>
+std::shared_ptr<Store> openFromNonUri(const std::string & uri, const Store::Params & params)
 {
     switch (getStoreType(uri, get(params, "state").value_or(settings.nixStateDir))) {
         case tDaemon:
@@ -1098,8 +1079,30 @@ static RegisterStoreImplementation regStore([](
         default:
             return nullptr;
     }
-});
+}
 
+ref<Store> openStore(const std::string & uri_,
+    const Store::Params & extraParams)
+{
+    auto [uri, uriParams] = splitUriAndParams(uri_);
+    auto params = extraParams;
+    params.insert(uriParams.begin(), uriParams.end());
+
+    if (auto store = openFromNonUri(uri, params)) {
+        store->warnUnknownSettings();
+        return ref<Store>(store);
+    }
+
+    for (auto implem : *implementations) {
+        auto store = implem.open(uri, params);
+        if (store) {
+            store->warnUnknownSettings();
+            return ref<Store>(store);
+        }
+    }
+
+    throw Error("don't know how to open Nix store '%s'", uri);
+}
 
 std::list<ref<Store>> getDefaultSubstituters()
 {
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 8b42aa6d2..28365542d 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -33,6 +33,7 @@ MakeError(SubstituteGone, Error);
 MakeError(SubstituterDisabled, Error);
 MakeError(BadStorePath, Error);
 
+MakeError(InvalidStoreURI, Error);
 
 class FSAccessor;
 class NarInfoDiskCache;
@@ -199,6 +200,8 @@ protected:
 
     Store(const Params & params);
 
+    std::shared_ptr<Config> getConfig();
+
 public:
 
     virtual ~Store() { }
@@ -744,25 +747,31 @@ StoreType getStoreType(const std::string & uri = settings.storeUri.get(),
    ‘substituters’ option and various legacy options. */
 std::list<ref<Store>> getDefaultSubstituters();
 
+struct StoreFactory
+{
+    std::vector<std::string> uriPrefixes;
+    std::function<std::shared_ptr<Store> (const std::string & uri, const Store::Params & params)> open;
+};
+typedef std::vector<StoreFactory> Implementations;
+static Implementations * implementations = new Implementations;
 
-/* Store implementation registration. */
-typedef std::function<std::shared_ptr<Store>(
-    const std::string & uri, const Store::Params & params)> OpenStore;
-
+template<typename T>
 struct RegisterStoreImplementation
 {
-    typedef std::vector<OpenStore> Implementations;
-    static Implementations * implementations;
-
-    RegisterStoreImplementation(OpenStore fun)
+    RegisterStoreImplementation()
     {
-        if (!implementations) implementations = new Implementations;
-        implementations->push_back(fun);
+        StoreFactory factory{
+            .uriPrefixes = T::uriPrefixes(),
+            .open =
+                ([](const std::string & uri, const Store::Params & params)
+                 -> std::shared_ptr<Store>
+                 { return std::make_shared<T>(uri, params); })
+        };
+        implementations->push_back(factory);
     }
 };
 
 
-
 /* Display a set of paths in human-readable form (i.e., between quotes
    and separated by commas). */
 string showPaths(const PathSet & paths);
diff --git a/src/libstore/store.hh b/src/libstore/store.hh
new file mode 100644
index 000000000..bc73963f3
--- /dev/null
+++ b/src/libstore/store.hh
@@ -0,0 +1,9 @@
+#pragma once
+
+namespace nix {
+
+template<typename T> class BasicStore;
+class StoreConfig;
+typedef BasicStore<StoreConfig> Store;
+
+}
diff --git a/src/nix/describe-stores.cc b/src/nix/describe-stores.cc
new file mode 100644
index 000000000..d84378316
--- /dev/null
+++ b/src/nix/describe-stores.cc
@@ -0,0 +1,30 @@
+#include "command.hh"
+#include "common-args.hh"
+#include "shared.hh"
+#include "store-api.hh"
+
+#include <nlohmann/json.hpp>
+
+using namespace nix;
+
+struct CmdDescribeStores : Command, MixJSON
+{
+    std::string description() override
+    {
+        return "show registered store types and their available options";
+    }
+
+    Category category() override { return catUtility; }
+
+    void run() override
+    {
+
+        if (json) {
+            auto availableStores = *implementations;
+        } else {
+            throw Error("Only json is available for describe-stores");
+        }
+    }
+};
+
+static auto r1 = registerCommand<CmdDescribeStores>("describe-stores");
diff --git a/src/nix/develop.cc b/src/nix/develop.cc
index a2ce9c8c1..f29fa71d2 100644
--- a/src/nix/develop.cc
+++ b/src/nix/develop.cc
@@ -392,7 +392,7 @@ struct CmdDevelop : Common, MixEnvironment
 
             auto bashInstallable = std::make_shared<InstallableFlake>(
                 state,
-                std::move(installable->nixpkgsFlakeRef()),
+                installable->nixpkgsFlakeRef(),
                 Strings{"bashInteractive"},
                 Strings{"legacyPackages." + settings.thisSystem.get() + "."},
                 lockFlags);
diff --git a/src/nix/diff-closures.cc b/src/nix/diff-closures.cc
index 4199dae0f..0dc99d05e 100644
--- a/src/nix/diff-closures.cc
+++ b/src/nix/diff-closures.cc
@@ -81,7 +81,7 @@ void printClosureDiff(
         auto beforeSize = totalSize(beforeVersions);
         auto afterSize = totalSize(afterVersions);
         auto sizeDelta = (int64_t) afterSize - (int64_t) beforeSize;
-        auto showDelta = abs(sizeDelta) >= 8 * 1024;
+        auto showDelta = std::abs(sizeDelta) >= 8 * 1024;
 
         std::set<std::string> removed, unchanged;
         for (auto & [version, _] : beforeVersions)
diff --git a/src/nix/installables.hh b/src/nix/installables.hh
index 41c75a4ed..c7c2f8981 100644
--- a/src/nix/installables.hh
+++ b/src/nix/installables.hh
@@ -69,7 +69,7 @@ struct Installable
 
     virtual FlakeRef nixpkgsFlakeRef() const
     {
-        return std::move(FlakeRef::fromAttrs({{"type","indirect"}, {"id", "nixpkgs"}}));
+        return FlakeRef::fromAttrs({{"type","indirect"}, {"id", "nixpkgs"}});
     }
 };