From 7d5bdf8b5679cc7b2b9b4d9caf5af9ca52211336 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 8 Sep 2020 14:50:23 +0200 Subject: [PATCH 01/25] 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 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 uriPrefixes() { + return {"dummy"}; + } + std::optional 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 -{ - if (uri != uriScheme) return nullptr; - return std::make_shared(params); -}); +[[maybe_unused]] static RegisterStoreImplementation 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 uriPrefixes() + { + static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; + auto ret = std::vector({"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 -{ - 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(params, uri); - store->init(); - return store; -}); +[[maybe_unused]] static RegisterStoreImplementation 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 maxConnections{this, 1, "max-connections", "maximum number of concurrent SSH connections"}; @@ -37,6 +35,9 @@ struct LegacySSHStore : public Store SSHMaster master; + static std::vector 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 -{ - if (std::string(uri, 0, uriScheme.size()) != uriScheme) return 0; - return std::make_shared(std::string(uri, uriScheme.size()), params); -}); +[[maybe_unused]] static RegisterStoreImplementation 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 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 +std::vector LocalBinaryCacheStore::uriPrefixes() { - if (getEnv("_NIX_FORCE_HTTP_BINARY_CACHE_STORE") == "1" || - std::string(uri, 0, 7) != "file://") - return 0; - auto store = std::make_shared(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 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 -{ - if (std::string(uri, 0, uriScheme.size()) != uriScheme) return 0; - return std::make_shared(std::string(uri, uriScheme.size()), params); -}); +[[maybe_unused]] static RegisterStoreImplementation 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 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 uriPrefixes() { return {"s3"}; } + }; -static RegisterStoreImplementation regStore([]( - const std::string & uri, const Store::Params & params) - -> std::shared_ptr -{ - if (std::string(uri, 0, 5) != "s3://") return 0; - auto store = std::make_shared(params, std::string(uri, 5)); - store->init(); - return store; -}); +[[maybe_unused]] static RegisterStoreImplementation 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 uriPrefixes() { return {"ssh-ng"}; } + std::string getUri() override { - return uriScheme + host; + return uriPrefixes()[0] + "://" + host; } bool sameMachine() override @@ -76,12 +76,6 @@ ref SSHStore::openConnection() return conn; } -static RegisterStoreImplementation regStore([]( - const std::string & uri, const Store::Params & params) - -> std::shared_ptr -{ - if (std::string(uri, 0, uriScheme.size()) != uriScheme) return 0; - return std::make_shared(std::string(uri, uriScheme.size()), params); -}); +[[maybe_unused]] static RegisterStoreImplementation 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 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 splitUriAndParams(const std::string & uri_) { @@ -1035,24 +1037,6 @@ std::pair splitUriAndParams(const std::string & uri_ return {uri, params}; } -ref 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); - } - } - - 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 +std::shared_ptr 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 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); + } + + for (auto implem : *implementations) { + auto store = implem.open(uri, params); + if (store) { + store->warnUnknownSettings(); + return ref(store); + } + } + + throw Error("don't know how to open Nix store '%s'", uri); +} std::list> 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 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> getDefaultSubstituters(); +struct StoreFactory +{ + std::vector uriPrefixes; + std::function (const std::string & uri, const Store::Params & params)> open; +}; +typedef std::vector Implementations; +static Implementations * implementations = new Implementations; -/* Store implementation registration. */ -typedef std::function( - const std::string & uri, const Store::Params & params)> OpenStore; - +template struct RegisterStoreImplementation { - typedef std::vector 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 + { return std::make_shared(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 class BasicStore; +class StoreConfig; +typedef BasicStore 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 + +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("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( 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 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"}}); } }; From fa32560169ffa55b9e0b6d5f04c3792acf0c544a Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 9 Sep 2020 11:18:12 +0200 Subject: [PATCH 02/25] Fix the registration of stores --- src/libstore/dummy-store.cc | 2 +- src/libstore/http-binary-cache-store.cc | 2 +- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/local-binary-cache-store.cc | 2 +- src/libstore/remote-store.cc | 2 +- src/libstore/s3-binary-cache-store.cc | 2 +- src/libstore/ssh-store.cc | 2 +- src/libstore/store-api.cc | 3 ++- src/libstore/store-api.hh | 29 +++++++++++++++--------- 9 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index a677e201e..dd1880877 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -50,6 +50,6 @@ struct DummyStore : public Store { unsupported("buildDerivation"); } }; -[[maybe_unused]] static RegisterStoreImplementation regStore(); +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 6c3a16faf..c1abe35cb 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -170,6 +170,6 @@ protected: }; -[[maybe_unused]] static RegisterStoreImplementation regStore(); +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 72f28a82d..552b4b176 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -326,6 +326,6 @@ public: } }; -[[maybe_unused]] static RegisterStoreImplementation regStore(); +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 752838d3f..808a1338f 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -96,6 +96,6 @@ std::vector LocalBinaryCacheStore::uriPrefixes() return {"file"}; } -[[maybe_unused]] static RegisterStoreImplementation regStore(); +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 824b6b140..5f455a62a 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -982,6 +982,6 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * return nullptr; } -[[maybe_unused]] static RegisterStoreImplementation regStore(); +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 1bf4d5955..f426b43a9 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -431,7 +431,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore }; -[[maybe_unused]] static RegisterStoreImplementation regStore(); +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index 02208ee82..8177a95b0 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -76,6 +76,6 @@ ref SSHStore::openConnection() return conn; } -[[maybe_unused]] static RegisterStoreImplementation regStore(); +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 4a46b5160..0f321b434 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1093,7 +1093,7 @@ ref openStore(const std::string & uri_, return ref(store); } - for (auto implem : *implementations) { + for (auto implem : *Implementations::registered) { auto store = implem.open(uri, params); if (store) { store->warnUnknownSettings(); @@ -1136,5 +1136,6 @@ std::list> getDefaultSubstituters() return stores; } +std::vector * Implementations::registered = 0; } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 28365542d..61080978e 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -749,25 +749,32 @@ std::list> getDefaultSubstituters(); struct StoreFactory { - std::vector uriPrefixes; std::function (const std::string & uri, const Store::Params & params)> open; }; -typedef std::vector Implementations; -static Implementations * implementations = new Implementations; +struct Implementations +{ + static std::vector * registered; + + template + static void add() + { + if (!registered) registered = new std::vector(); + StoreFactory factory{ + .open = + ([](const std::string & uri, const Store::Params & params) + -> std::shared_ptr + { return std::make_shared(uri, params); }), + }; + registered->push_back(factory); + } +}; template struct RegisterStoreImplementation { RegisterStoreImplementation() { - StoreFactory factory{ - .uriPrefixes = T::uriPrefixes(), - .open = - ([](const std::string & uri, const Store::Params & params) - -> std::shared_ptr - { return std::make_shared(uri, params); }) - }; - implementations->push_back(factory); + Implementations::add(); } }; From 3b57181f8ed94cfa149ad4319ba96d41c5fbc30e Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 9 Sep 2020 11:29:17 +0200 Subject: [PATCH 03/25] Separate the instantiation and initialisation of the stores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new `init()` method to the `Store` class that is supposed to handle all the effectful initialisation needed to set-up the store. The constructor should remain side-effect free and just initialize the c++ data structure. The goal behind that is that we can create “dummy” instances of each store to query static properties about it (the parameters it accepts for example) --- src/libstore/binary-cache-store.hh | 2 +- src/libstore/dummy-store.cc | 4 ++++ src/libstore/http-binary-cache-store.cc | 6 ++++++ src/libstore/legacy-ssh-store.cc | 3 +++ src/libstore/local-binary-cache-store.cc | 6 ++++++ src/libstore/s3-binary-cache-store.cc | 3 +++ src/libstore/ssh-store.cc | 6 ++++++ src/libstore/store-api.cc | 3 ++- src/libstore/store-api.hh | 16 ++++++++++++---- 9 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 9bcdf5901..881398ea2 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -58,7 +58,7 @@ public: public: - virtual void init(); + virtual void init() override; private: diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index dd1880877..52086445e 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -5,6 +5,10 @@ namespace nix { struct DummyStore : public Store { DummyStore(const std::string uri, const Params & params) + : DummyStore(params) + { } + + DummyStore(const Params & params) : Store(params) { } diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index c1abe35cb..e76bac6e2 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -23,6 +23,12 @@ private: public: + HttpBinaryCacheStore( + const Params & params) + : HttpBinaryCacheStore("dummy", params) + { + } + HttpBinaryCacheStore( const Path & _cacheUri, const Params & params) diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 552b4b176..777ba7520 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -37,6 +37,9 @@ struct LegacySSHStore : public Store static std::vector uriPrefixes() { return {"ssh"}; } + LegacySSHStore(const Params & params) + : LegacySSHStore("dummy", params) + {} LegacySSHStore(const string & host, const Params & params) : Store(params) diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 808a1338f..bcd23eb82 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -12,6 +12,12 @@ private: public: + LocalBinaryCacheStore( + const Params & params) + : LocalBinaryCacheStore("dummy", params) + { + } + LocalBinaryCacheStore( const Path & binaryCacheDir, const Params & params) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index f426b43a9..ab27f203f 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -192,6 +192,9 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore S3Helper s3Helper; + S3BinaryCacheStoreImpl(const Params & params) + : S3BinaryCacheStoreImpl("dummy-bucket", params) {} + S3BinaryCacheStoreImpl( const std::string & bucketName, const Params & params) diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index 8177a95b0..85ddce8be 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -17,6 +17,12 @@ public: const Setting remoteProgram{(Store*) this, "nix-daemon", "remote-program", "path to the nix-daemon executable on the remote system"}; const Setting remoteStore{(Store*) this, "", "remote-store", "URI of the store on the remote system"}; + SSHStore( + const Params & params) + : SSHStore("dummy", params) + { + } + SSHStore(const std::string & host, const Params & params) : Store(params) , RemoteStore(params) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 0f321b434..e14f361bd 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1094,8 +1094,9 @@ ref openStore(const std::string & uri_, } for (auto implem : *Implementations::registered) { - auto store = implem.open(uri, params); + auto store = implem.create(uri, params); if (store) { + store->init(); store->warnUnknownSettings(); return ref(store); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 61080978e..6e081da41 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -200,9 +200,12 @@ protected: Store(const Params & params); - std::shared_ptr getConfig(); - public: + /** + * Perform any necessary effectful operation to make the store up and + * running + */ + virtual void init() {}; virtual ~Store() { } @@ -749,7 +752,8 @@ std::list> getDefaultSubstituters(); struct StoreFactory { - std::function (const std::string & uri, const Store::Params & params)> open; + std::function (const std::string & uri, const Store::Params & params)> create; + std::function (const Store::Params & params)> createDummy; }; struct Implementations { @@ -760,10 +764,14 @@ struct Implementations { if (!registered) registered = new std::vector(); StoreFactory factory{ - .open = + .create = ([](const std::string & uri, const Store::Params & params) -> std::shared_ptr { return std::make_shared(uri, params); }), + .createDummy = + ([](const Store::Params & params) + -> std::shared_ptr + { return std::make_shared(params); }) }; registered->push_back(factory); } From 3c525d15903ea98e5401ff57061295b8c625cc45 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 9 Sep 2020 11:35:33 +0200 Subject: [PATCH 04/25] Complete the `toJSON` instance for `Setting` Don't let it just contain the value, but also the other fields of the setting (description, aliases, etc..) --- src/libstore/globals.cc | 5 ----- src/libutil/config.cc | 32 ++++++++++---------------------- src/libutil/config.hh | 11 +++++++++-- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 4a5971c3f..491c664db 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -162,11 +162,6 @@ template<> std::string BaseSetting::to_string() const else abort(); } -template<> nlohmann::json BaseSetting::toJSON() -{ - return AbstractSetting::toJSON(); -} - template<> void BaseSetting::convertToArg(Args & args, const std::string & category) { args.addFlag({ diff --git a/src/libutil/config.cc b/src/libutil/config.cc index 3cf720bce..faa5cdbeb 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -137,11 +137,7 @@ nlohmann::json Config::toJSON() auto res = nlohmann::json::object(); for (auto & s : _settings) if (!s.second.isAlias) { - auto obj = nlohmann::json::object(); - obj.emplace("description", s.second.setting->description); - obj.emplace("aliases", s.second.setting->aliases); - obj.emplace("value", s.second.setting->toJSON()); - res.emplace(s.first, obj); + res.emplace(s.first, s.second.setting->toJSON()); } return res; } @@ -168,19 +164,21 @@ void AbstractSetting::setDefault(const std::string & str) nlohmann::json AbstractSetting::toJSON() { - return to_string(); + return nlohmann::json(toJSONObject()); +} + +std::map AbstractSetting::toJSONObject() +{ + std::map obj; + obj.emplace("description", description); + obj.emplace("aliases", aliases); + return obj; } void AbstractSetting::convertToArg(Args & args, const std::string & category) { } -template -nlohmann::json BaseSetting::toJSON() -{ - return value; -} - template void BaseSetting::convertToArg(Args & args, const std::string & category) { @@ -259,11 +257,6 @@ template<> std::string BaseSetting::to_string() const return concatStringsSep(" ", value); } -template<> nlohmann::json BaseSetting::toJSON() -{ - return value; -} - template<> void BaseSetting::set(const std::string & str) { value = tokenizeString(str); @@ -274,11 +267,6 @@ template<> std::string BaseSetting::to_string() const return concatStringsSep(" ", value); } -template<> nlohmann::json BaseSetting::toJSON() -{ - return value; -} - template class BaseSetting; template class BaseSetting; template class BaseSetting; diff --git a/src/libutil/config.hh b/src/libutil/config.hh index 2b4265806..be23076b0 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -206,7 +206,9 @@ protected: virtual std::string to_string() const = 0; - virtual nlohmann::json toJSON(); + nlohmann::json toJSON(); + + virtual std::map toJSONObject(); virtual void convertToArg(Args & args, const std::string & category); @@ -251,7 +253,12 @@ public: void convertToArg(Args & args, const std::string & category) override; - nlohmann::json toJSON() override; + std::map toJSONObject() override + { + auto obj = AbstractSetting::toJSONObject(); + obj.emplace("value", value); + return obj; + } }; template From 35042c96230e922b88c8a5cad6a7f5f06792860f Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 9 Sep 2020 11:36:48 +0200 Subject: [PATCH 05/25] Add a default value for the settings The default value is initialized when creating the setting and unchanged after that --- src/libutil/config.hh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libutil/config.hh b/src/libutil/config.hh index be23076b0..42fc856e0 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -222,6 +222,7 @@ class BaseSetting : public AbstractSetting protected: T value; + const T defaultValue; public: @@ -231,6 +232,7 @@ public: const std::set & aliases = {}) : AbstractSetting(name, description, aliases) , value(def) + , defaultValue(def) { } operator const T &() const { return value; } @@ -257,6 +259,7 @@ public: { auto obj = AbstractSetting::toJSONObject(); obj.emplace("value", value); + obj.emplace("defaultValue", defaultValue); return obj; } }; From aa4eac37881258797c241113a5602913e1a5371a Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 9 Sep 2020 13:25:32 +0200 Subject: [PATCH 06/25] fixup! Separate the instantiation and initialisation of the stores --- src/libstore/store-api.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index e14f361bd..831d4f30f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1009,12 +1009,6 @@ Derivation Store::readDerivation(const StorePath & drvPath) } } -std::shared_ptr Store::getConfig() -{ - return shared_from_this(); -} - - } From 22afa8fb4dd7e459faf531dd8a9c3580d02c7e2a Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 10 Sep 2020 10:55:51 +0200 Subject: [PATCH 07/25] Separate store configs from the implems Rework the `Store` hierarchy so that there's now one hierarchy for the store configs and one for the implementations (where each implementation extends the corresponding config). So a class hierarchy like ``` StoreConfig-------->Store | | v v SubStoreConfig----->SubStore | | v v SubSubStoreConfig-->SubSubStore ``` (with virtual inheritance to prevent DDD). The advantage of this architecture is that we can now introspect the configuration of a store without having to instantiate the store itself --- src/libstore/binary-cache-store.cc | 3 +- src/libstore/binary-cache-store.hh | 8 +++- src/libstore/dummy-store.cc | 4 +- src/libstore/http-binary-cache-store.cc | 16 ++++---- src/libstore/legacy-ssh-store.cc | 17 ++++---- src/libstore/local-binary-cache-store.cc | 18 ++++----- src/libstore/local-store.hh | 2 +- src/libstore/remote-store.cc | 10 ++--- src/libstore/remote-store.hh | 34 ++++++++++++---- src/libstore/s3-binary-cache-store.cc | 14 ++++--- src/libstore/ssh-store.cc | 25 ++++++------ src/libstore/store-api.cc | 2 +- src/libstore/store-api.hh | 50 ++++++++++++++---------- 13 files changed, 120 insertions(+), 83 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 5433fe50d..34f844a18 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -22,7 +22,8 @@ namespace nix { BinaryCacheStore::BinaryCacheStore(const Params & params) - : Store(params) + : BinaryCacheStoreConfig(params) + , Store(params) { if (secretKeyFile != "") secretKey = std::unique_ptr(new SecretKey(readFile(secretKeyFile))); diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 881398ea2..f1fc82013 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -11,9 +11,9 @@ namespace nix { struct NarInfo; -class BinaryCacheStore : public Store +struct BinaryCacheStoreConfig : virtual StoreConfig { -public: + using StoreConfig::StoreConfig; const Setting compression{this, "xz", "compression", "NAR compression method ('xz', 'bzip2', or 'none')"}; const Setting writeNARListing{this, false, "write-nar-listing", "whether to write a JSON file listing the files in each NAR"}; @@ -22,6 +22,10 @@ public: const Setting localNarCache{this, "", "local-nar-cache", "path to a local cache of NARs"}; const Setting parallelCompression{this, false, "parallel-compression", "enable multi-threading compression, available for xz only currently"}; +}; + +class BinaryCacheStore : public Store, public virtual BinaryCacheStoreConfig +{ private: diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 52086445e..d4560c13d 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -2,6 +2,8 @@ namespace nix { +struct DummyStoreConfig : StoreConfig {}; + struct DummyStore : public Store { DummyStore(const std::string uri, const Params & params) @@ -54,6 +56,6 @@ struct DummyStore : public Store { unsupported("buildDerivation"); } }; -static RegisterStoreImplementation regStore; +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index e76bac6e2..c3fb455fa 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -7,7 +7,12 @@ namespace nix { MakeError(UploadToHTTP, Error); -class HttpBinaryCacheStore : public BinaryCacheStore +struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +{ + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; +}; + +class HttpBinaryCacheStore : public BinaryCacheStore, public HttpBinaryCacheStoreConfig { private: @@ -23,16 +28,11 @@ private: public: - HttpBinaryCacheStore( - const Params & params) - : HttpBinaryCacheStore("dummy", params) - { - } - HttpBinaryCacheStore( const Path & _cacheUri, const Params & params) : BinaryCacheStore(params) + , HttpBinaryCacheStoreConfig(params) , cacheUri(_cacheUri) { if (cacheUri.back() == '/') @@ -176,6 +176,6 @@ protected: }; -static RegisterStoreImplementation regStore; +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 777ba7520..5a08fbba3 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -9,15 +9,21 @@ namespace nix { -struct LegacySSHStore : public Store +struct LegacySSHStoreConfig : virtual StoreConfig { + using StoreConfig::StoreConfig; const Setting maxConnections{this, 1, "max-connections", "maximum number of concurrent SSH connections"}; const Setting sshKey{this, "", "ssh-key", "path to an SSH private key"}; const Setting compress{this, false, "compress", "whether to compress the connection"}; const Setting remoteProgram{this, "nix-store", "remote-program", "path to the nix-store executable on the remote system"}; const Setting remoteStore{this, "", "remote-store", "URI of the store on the remote system"}; +}; +struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig +{ // Hack for getting remote build log output. + // Intentionally not in `StoreConfig` so that it doesn't appear in the + // documentation const Setting logFD{this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"}; struct Connection @@ -37,12 +43,9 @@ struct LegacySSHStore : public Store static std::vector uriPrefixes() { return {"ssh"}; } - LegacySSHStore(const Params & params) - : LegacySSHStore("dummy", params) - {} - LegacySSHStore(const string & host, const Params & params) - : Store(params) + : LegacySSHStoreConfig(params) + , Store(params) , host(host) , connections(make_ref>( std::max(1, (int) maxConnections), @@ -329,6 +332,6 @@ public: } }; -static RegisterStoreImplementation regStore; +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index bcd23eb82..fc3ae94c2 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -4,7 +4,12 @@ namespace nix { -class LocalBinaryCacheStore : public BinaryCacheStore +struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +{ + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; +}; + +class LocalBinaryCacheStore : public BinaryCacheStore, public virtual LocalBinaryCacheStoreConfig { private: @@ -12,16 +17,11 @@ private: public: - LocalBinaryCacheStore( - const Params & params) - : LocalBinaryCacheStore("dummy", params) - { - } - LocalBinaryCacheStore( const Path & binaryCacheDir, const Params & params) - : BinaryCacheStore(params) + : LocalBinaryCacheStoreConfig(params) + , BinaryCacheStore(params) , binaryCacheDir(binaryCacheDir) { } @@ -102,6 +102,6 @@ std::vector LocalBinaryCacheStore::uriPrefixes() return {"file"}; } -static RegisterStoreImplementation regStore; +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index d5e6d68ef..8b6972847 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -95,7 +95,7 @@ public: private: - Setting requireSigs{(Store*) this, + Setting requireSigs{(StoreConfig*) this, settings.requireSigs, "require-sigs", "whether store paths should have a trusted signature on import"}; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 5f455a62a..619e9a7f1 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -94,6 +94,7 @@ void write(const Store & store, Sink & out, const std::optional & sto /* TODO: Separate these store impls into different files, give them better names */ RemoteStore::RemoteStore(const Params & params) : Store(params) + , RemoteStoreConfig(params) , connections(make_ref>( std::max(1, (int) maxConnections), [this]() { return openConnectionWrapper(); }, @@ -124,6 +125,7 @@ ref RemoteStore::openConnectionWrapper() UDSRemoteStore::UDSRemoteStore(const Params & params) : Store(params) + , UDSRemoteStoreConfig(params) , LocalFSStore(params) , RemoteStore(params) { @@ -131,11 +133,9 @@ UDSRemoteStore::UDSRemoteStore(const Params & params) UDSRemoteStore::UDSRemoteStore(std::string socket_path, const Params & params) - : Store(params) - , LocalFSStore(params) - , RemoteStore(params) - , path(socket_path) + : UDSRemoteStore(params) { + path.emplace(socket_path); } @@ -982,6 +982,6 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * return nullptr; } -static RegisterStoreImplementation regStore; +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 07cd5daf8..ba2f69427 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -16,19 +16,23 @@ struct FdSource; template class Pool; struct ConnectionHandle; +struct RemoteStoreConfig : virtual StoreConfig +{ + using StoreConfig::StoreConfig; + + const Setting maxConnections{(StoreConfig*) this, 1, + "max-connections", "maximum number of concurrent connections to the Nix daemon"}; + + const Setting maxConnectionAge{(StoreConfig*) this, std::numeric_limits::max(), + "max-connection-age", "number of seconds to reuse a connection"}; +}; /* FIXME: RemoteStore is a misnomer - should be something like DaemonStore. */ -class RemoteStore : public virtual Store +class RemoteStore : public virtual Store, public virtual RemoteStoreConfig { public: - const Setting maxConnections{(Store*) this, 1, - "max-connections", "maximum number of concurrent connections to the Nix daemon"}; - - const Setting maxConnectionAge{(Store*) this, std::numeric_limits::max(), - "max-connection-age", "number of seconds to reuse a connection"}; - virtual bool sameMachine() = 0; RemoteStore(const Params & params); @@ -141,7 +145,21 @@ private: }; -class UDSRemoteStore : public LocalFSStore, public RemoteStore +struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreConfig +{ + UDSRemoteStoreConfig(const Store::Params & params) + : LocalFSStoreConfig(params) + , RemoteStoreConfig(params) + { + } + + UDSRemoteStoreConfig() + : UDSRemoteStoreConfig(Store::Params({})) + { + } +}; + +class UDSRemoteStore : public LocalFSStore, public RemoteStore, public virtual UDSRemoteStoreConfig { public: diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index ab27f203f..b75d05555 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -172,8 +172,9 @@ S3Helper::FileTransferResult S3Helper::getObject( return res; } -struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore +struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig { + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; const Setting profile{this, "", "profile", "The name of the AWS configuration profile to use."}; const Setting region{this, Aws::Region::US_EAST_1, "region", {"aws-region"}}; const Setting scheme{this, "", "scheme", "The scheme to use for S3 requests, https by default."}; @@ -185,20 +186,21 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore this, false, "multipart-upload", "whether to use multi-part uploads"}; const Setting bufferSize{ this, 5 * 1024 * 1024, "buffer-size", "size (in bytes) of each part in multi-part uploads"}; +}; +struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCacheStoreConfig +{ std::string bucketName; Stats stats; S3Helper s3Helper; - S3BinaryCacheStoreImpl(const Params & params) - : S3BinaryCacheStoreImpl("dummy-bucket", params) {} - S3BinaryCacheStoreImpl( const std::string & bucketName, const Params & params) - : S3BinaryCacheStore(params) + : S3BinaryCacheStoreConfig(params) + , S3BinaryCacheStore(params) , bucketName(bucketName) , s3Helper(profile, region, scheme, endpoint) { @@ -434,7 +436,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore }; -static RegisterStoreImplementation regStore; +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index 85ddce8be..2b6e2a298 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -8,23 +8,22 @@ namespace nix { -class SSHStore : public RemoteStore +struct SSHStoreConfig : virtual RemoteStoreConfig +{ + using RemoteStoreConfig::RemoteStoreConfig; + const Setting sshKey{(StoreConfig*) this, "", "ssh-key", "path to an SSH private key"}; + const Setting compress{(StoreConfig*) this, false, "compress", "whether to compress the connection"}; + const Setting remoteProgram{(StoreConfig*) this, "nix-daemon", "remote-program", "path to the nix-daemon executable on the remote system"}; + const Setting remoteStore{(StoreConfig*) this, "", "remote-store", "URI of the store on the remote system"}; +}; + +class SSHStore : public RemoteStore, public virtual SSHStoreConfig { public: - const Setting sshKey{(Store*) this, "", "ssh-key", "path to an SSH private key"}; - const Setting compress{(Store*) this, false, "compress", "whether to compress the connection"}; - const Setting remoteProgram{(Store*) this, "nix-daemon", "remote-program", "path to the nix-daemon executable on the remote system"}; - const Setting remoteStore{(Store*) this, "", "remote-store", "URI of the store on the remote system"}; - - SSHStore( - const Params & params) - : SSHStore("dummy", params) - { - } - SSHStore(const std::string & host, const Params & params) : Store(params) + , RemoteStoreConfig(params) , RemoteStore(params) , host(host) , master( @@ -82,6 +81,6 @@ ref SSHStore::openConnection() return conn; } -static RegisterStoreImplementation regStore; +static RegisterStoreImplementation regStore; } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 831d4f30f..fe3f5221c 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -346,7 +346,7 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath, Store::Store(const Params & params) - : Config(params) + : StoreConfig(params) , state({(size_t) pathInfoCacheSize}) { } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 6e081da41..61552f74e 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -145,12 +145,9 @@ struct BuildResult } }; - -class Store : public std::enable_shared_from_this, public Config +struct StoreConfig : public Config { -public: - - typedef std::map Params; + using Config::Config; const PathSetting storeDir_{this, false, settings.nixStore, "store", "path to the Nix store"}; @@ -168,6 +165,14 @@ public: "system-features", "Optional features that the system this store builds on implements (like \"kvm\")."}; +}; + +class Store : public std::enable_shared_from_this, public virtual StoreConfig +{ +public: + + typedef std::map Params; + protected: struct PathInfoCacheValue { @@ -632,22 +637,25 @@ protected: }; - -class LocalFSStore : public virtual Store +struct LocalFSStoreConfig : virtual StoreConfig { -public: - - // FIXME: the (Store*) cast works around a bug in gcc that causes + using StoreConfig::StoreConfig; + // FIXME: the (StoreConfig*) cast works around a bug in gcc that causes // it to omit the call to the Setting constructor. Clang works fine // either way. - const PathSetting rootDir{(Store*) this, true, "", + const PathSetting rootDir{(StoreConfig*) this, true, "", "root", "directory prefixed to all other paths"}; - const PathSetting stateDir{(Store*) this, false, + const PathSetting stateDir{(StoreConfig*) this, false, rootDir != "" ? rootDir + "/nix/var/nix" : settings.nixStateDir, "state", "directory where Nix will store state"}; - const PathSetting logDir{(Store*) this, false, + const PathSetting logDir{(StoreConfig*) this, false, rootDir != "" ? rootDir + "/nix/var/log/nix" : settings.nixLogDir, "log", "directory where Nix will store state"}; +}; + +class LocalFSStore : public virtual Store, public LocalFSStoreConfig +{ +public: const static string drvsLogDir; @@ -753,13 +761,13 @@ std::list> getDefaultSubstituters(); struct StoreFactory { std::function (const std::string & uri, const Store::Params & params)> create; - std::function (const Store::Params & params)> createDummy; + std::function ()> getConfig; }; struct Implementations { static std::vector * registered; - template + template static void add() { if (!registered) registered = new std::vector(); @@ -768,21 +776,21 @@ struct Implementations ([](const std::string & uri, const Store::Params & params) -> std::shared_ptr { return std::make_shared(uri, params); }), - .createDummy = - ([](const Store::Params & params) - -> std::shared_ptr - { return std::make_shared(params); }) + .getConfig = + ([]() + -> std::shared_ptr + { return std::make_shared(); }) }; registered->push_back(factory); } }; -template +template struct RegisterStoreImplementation { RegisterStoreImplementation() { - Implementations::add(); + Implementations::add(); } }; From dae39f0a7abfc41c122f4f2d54d6cf9e26cafe7a Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 10 Sep 2020 11:03:36 +0200 Subject: [PATCH 08/25] Make `nix describe-stores` functional Using the `*Config` class hierarchy --- src/nix/describe-stores.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nix/describe-stores.cc b/src/nix/describe-stores.cc index d84378316..f019ec9c2 100644 --- a/src/nix/describe-stores.cc +++ b/src/nix/describe-stores.cc @@ -20,7 +20,11 @@ struct CmdDescribeStores : Command, MixJSON { if (json) { - auto availableStores = *implementations; + auto res = nlohmann::json::array(); + for (auto & implem : *Implementations::registered) { + auto storeConfig = implem.getConfig(); + std::cout << storeConfig->toJSON().dump() << std::endl; + } } else { throw Error("Only json is available for describe-stores"); } From d184ad1d276006d4d003046710a56a019034a6a1 Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 11 Sep 2020 11:02:38 +0200 Subject: [PATCH 09/25] fixup! Make the store plugins more introspectable --- result | 1 - 1 file changed, 1 deletion(-) delete mode 120000 result diff --git a/result b/result deleted file mode 120000 index af7bbb6da..000000000 --- a/result +++ /dev/null @@ -1 +0,0 @@ -/nix/store/9pqfirjppd91mzhkgh8xnn66iwh53zk2-hello-2.10 \ No newline at end of file From 5895184df44e86ae55270390402c8263b0f24ae2 Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 11 Sep 2020 11:06:18 +0200 Subject: [PATCH 10/25] Correctly call all the parent contructors of the stores Using virtual inheritance means that only the default constructors of the parent classes will be called, which isn't what we want --- src/libstore/build.cc | 2 +- src/libstore/dummy-store.cc | 13 +++++++++---- src/libstore/http-binary-cache-store.cc | 4 +++- src/libstore/legacy-ssh-store.cc | 3 ++- src/libstore/local-binary-cache-store.cc | 4 +++- src/libstore/local-store.cc | 5 ++++- src/libstore/local-store.hh | 15 ++++++++++----- src/libstore/remote-store.cc | 5 ++++- src/libstore/remote-store.hh | 3 ++- src/libstore/s3-binary-cache-store.cc | 4 +++- src/libstore/ssh-store.cc | 7 +++++-- src/libstore/store-api.hh | 2 +- 12 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index ce973862d..4040a3fac 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2883,7 +2883,7 @@ struct RestrictedStore : public LocalFSStore DerivationGoal & goal; RestrictedStore(const Params & params, ref next, DerivationGoal & goal) - : Store(params), LocalFSStore(params), next(next), goal(goal) + : StoreConfig(params), LocalFSStoreConfig(params), Store(params), LocalFSStore(params), next(next), goal(goal) { } Path getRealStoreDir() override diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index d4560c13d..68d03d934 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -2,17 +2,22 @@ namespace nix { -struct DummyStoreConfig : StoreConfig {}; +struct DummyStoreConfig : StoreConfig { + using StoreConfig::StoreConfig; +}; -struct DummyStore : public Store +struct DummyStore : public Store, public virtual DummyStoreConfig { DummyStore(const std::string uri, const Params & params) : DummyStore(params) { } DummyStore(const Params & params) - : Store(params) - { } + : StoreConfig(params) + , DummyStoreConfig(params) + , Store(params) + { + } string getUri() override { diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index c3fb455fa..2c9d61d51 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -31,7 +31,9 @@ public: HttpBinaryCacheStore( const Path & _cacheUri, const Params & params) - : BinaryCacheStore(params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , BinaryCacheStore(params) , HttpBinaryCacheStoreConfig(params) , cacheUri(_cacheUri) { diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 5a08fbba3..bbab452ab 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -44,7 +44,8 @@ struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig static std::vector uriPrefixes() { return {"ssh"}; } LegacySSHStore(const string & host, const Params & params) - : LegacySSHStoreConfig(params) + : StoreConfig(params) + , LegacySSHStoreConfig(params) , Store(params) , host(host) , connections(make_ref>( diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index fc3ae94c2..becd74c75 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -20,7 +20,9 @@ public: LocalBinaryCacheStore( const Path & binaryCacheDir, const Params & params) - : LocalBinaryCacheStoreConfig(params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , LocalBinaryCacheStoreConfig(params) , BinaryCacheStore(params) , binaryCacheDir(binaryCacheDir) { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 0086bb13e..4e9840f3b 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -42,7 +42,10 @@ namespace nix { LocalStore::LocalStore(const Params & params) - : Store(params) + : StoreConfig(params) + , Store(params) + , LocalFSStoreConfig(params) + , LocalStoreConfig(params) , LocalFSStore(params) , realStoreDir_{this, false, rootDir != "" ? rootDir + "/nix/store" : storeDir, "real", "physical path to the Nix store"} diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 8b6972847..0265f0837 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -30,8 +30,17 @@ struct OptimiseStats uint64_t blocksFreed = 0; }; +struct LocalStoreConfig : virtual LocalFSStoreConfig +{ + using LocalFSStoreConfig::LocalFSStoreConfig; -class LocalStore : public LocalFSStore + Setting requireSigs{(StoreConfig*) this, + settings.requireSigs, + "require-sigs", "whether store paths should have a trusted signature on import"}; +}; + + +class LocalStore : public LocalFSStore, public virtual LocalStoreConfig { private: @@ -95,10 +104,6 @@ public: private: - Setting requireSigs{(StoreConfig*) this, - settings.requireSigs, - "require-sigs", "whether store paths should have a trusted signature on import"}; - const PublicKeys & getPublicKeys(); public: diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 619e9a7f1..9bbb69b99 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -124,7 +124,10 @@ ref RemoteStore::openConnectionWrapper() UDSRemoteStore::UDSRemoteStore(const Params & params) - : Store(params) + : StoreConfig(params) + , Store(params) + , LocalFSStoreConfig(params) + , RemoteStoreConfig(params) , UDSRemoteStoreConfig(params) , LocalFSStore(params) , RemoteStore(params) diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index ba2f69427..32daab59a 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -148,7 +148,8 @@ private: struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreConfig { UDSRemoteStoreConfig(const Store::Params & params) - : LocalFSStoreConfig(params) + : StoreConfig(params) + , LocalFSStoreConfig(params) , RemoteStoreConfig(params) { } diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index b75d05555..30ef4082a 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -199,7 +199,9 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCache S3BinaryCacheStoreImpl( const std::string & bucketName, const Params & params) - : S3BinaryCacheStoreConfig(params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , S3BinaryCacheStoreConfig(params) , S3BinaryCacheStore(params) , bucketName(bucketName) , s3Helper(profile, region, scheme, endpoint) diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index 2b6e2a298..da201a9c3 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -11,20 +11,23 @@ namespace nix { struct SSHStoreConfig : virtual RemoteStoreConfig { using RemoteStoreConfig::RemoteStoreConfig; + const Setting sshKey{(StoreConfig*) this, "", "ssh-key", "path to an SSH private key"}; const Setting compress{(StoreConfig*) this, false, "compress", "whether to compress the connection"}; const Setting remoteProgram{(StoreConfig*) this, "nix-daemon", "remote-program", "path to the nix-daemon executable on the remote system"}; const Setting remoteStore{(StoreConfig*) this, "", "remote-store", "URI of the store on the remote system"}; }; -class SSHStore : public RemoteStore, public virtual SSHStoreConfig +class SSHStore : public virtual RemoteStore, public virtual SSHStoreConfig { public: SSHStore(const std::string & host, const Params & params) - : Store(params) + : StoreConfig(params) + , Store(params) , RemoteStoreConfig(params) , RemoteStore(params) + , SSHStoreConfig(params) , host(host) , master( host, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 61552f74e..8e9cb786c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -653,7 +653,7 @@ struct LocalFSStoreConfig : virtual StoreConfig "log", "directory where Nix will store state"}; }; -class LocalFSStore : public virtual Store, public LocalFSStoreConfig +class LocalFSStore : public virtual Store, public virtual LocalFSStoreConfig { public: From 7f103dcddd4cf3c0748b7a379a117f3262c4c5f7 Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 11 Sep 2020 11:11:05 +0200 Subject: [PATCH 11/25] Properly filter the stores according to their declared uriSchemes When opening a store, only try the stores whose `uriSchemes()` include the current one --- src/libstore/dummy-store.cc | 6 ++--- src/libstore/http-binary-cache-store.cc | 9 +++---- src/libstore/legacy-ssh-store.cc | 6 ++--- src/libstore/local-binary-cache-store.cc | 5 ++-- src/libstore/remote-store.cc | 5 +++- src/libstore/remote-store.hh | 4 ++-- src/libstore/s3-binary-cache-store.cc | 3 ++- src/libstore/ssh-store.cc | 6 ++--- src/libstore/store-api.cc | 30 ++++++++++++++++-------- src/libstore/store-api.hh | 10 ++++---- 10 files changed, 51 insertions(+), 33 deletions(-) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 68d03d934..210e5b11c 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -8,7 +8,7 @@ struct DummyStoreConfig : StoreConfig { struct DummyStore : public Store, public virtual DummyStoreConfig { - DummyStore(const std::string uri, const Params & params) + DummyStore(const std::string scheme, const std::string uri, const Params & params) : DummyStore(params) { } @@ -21,7 +21,7 @@ struct DummyStore : public Store, public virtual DummyStoreConfig string getUri() override { - return uriPrefixes()[0] + "://"; + return *uriSchemes().begin(); } void queryPathInfoUncached(const StorePath & path, @@ -30,7 +30,7 @@ struct DummyStore : public Store, public virtual DummyStoreConfig callback(nullptr); } - static std::vector uriPrefixes() { + static std::set uriSchemes() { return {"dummy"}; } diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 2c9d61d51..095eaa82c 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -29,13 +29,14 @@ private: public: HttpBinaryCacheStore( + const std::string & scheme, const Path & _cacheUri, const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) , BinaryCacheStore(params) , HttpBinaryCacheStoreConfig(params) - , cacheUri(_cacheUri) + , cacheUri(scheme + "://" + _cacheUri) { if (cacheUri.back() == '/') cacheUri.pop_back(); @@ -64,11 +65,11 @@ public: } } - static std::vector uriPrefixes() + static std::set uriSchemes() { static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; - auto ret = std::vector({"http", "https"}); - if (forceHttp) ret.push_back("file"); + auto ret = std::set({"http", "https"}); + if (forceHttp) ret.insert("file"); return ret; } protected: diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index bbab452ab..6dc83e288 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -41,9 +41,9 @@ struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig SSHMaster master; - static std::vector uriPrefixes() { return {"ssh"}; } + static std::set uriSchemes() { return {"ssh"}; } - LegacySSHStore(const string & host, const Params & params) + LegacySSHStore(const string & scheme, const string & host, const Params & params) : StoreConfig(params) , LegacySSHStoreConfig(params) , Store(params) @@ -92,7 +92,7 @@ struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig string getUri() override { - return uriPrefixes()[0] + "://" + host; + return *uriSchemes().begin() + "://" + host; } void queryPathInfoUncached(const StorePath & path, diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index becd74c75..4d11e0f2a 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -18,6 +18,7 @@ private: public: LocalBinaryCacheStore( + [[maybe_unused]] const std::string scheme, const Path & binaryCacheDir, const Params & params) : StoreConfig(params) @@ -35,7 +36,7 @@ public: return "file://" + binaryCacheDir; } - static std::vector uriPrefixes(); + static std::set uriSchemes(); protected: @@ -96,7 +97,7 @@ bool LocalBinaryCacheStore::fileExists(const std::string & path) return pathExists(binaryCacheDir + "/" + path); } -std::vector LocalBinaryCacheStore::uriPrefixes() +std::set LocalBinaryCacheStore::uriSchemes() { if (getEnv("_NIX_FORCE_HTTP_BINARY_CACHE_STORE") == "1") return {}; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 9bbb69b99..561502cbe 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -135,7 +135,10 @@ UDSRemoteStore::UDSRemoteStore(const Params & params) } -UDSRemoteStore::UDSRemoteStore(std::string socket_path, const Params & params) +UDSRemoteStore::UDSRemoteStore( + [[maybe_unused]] const std::string scheme, + std::string socket_path, + const Params & params) : UDSRemoteStore(params) { path.emplace(socket_path); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 32daab59a..cceb8d185 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -165,11 +165,11 @@ class UDSRemoteStore : public LocalFSStore, public RemoteStore, public virtual U public: UDSRemoteStore(const Params & params); - UDSRemoteStore(std::string path, const Params & params); + UDSRemoteStore(const std::string scheme, std::string path, const Params & params); std::string getUri() override; - static std::vector uriPrefixes() + static std::set uriSchemes() { return {"unix"}; } bool sameMachine() override diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 30ef4082a..020b20c37 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -197,6 +197,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCache S3Helper s3Helper; S3BinaryCacheStoreImpl( + [[maybe_unused]] const std::string & scheme, const std::string & bucketName, const Params & params) : StoreConfig(params) @@ -434,7 +435,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCache return paths; } - static std::vector uriPrefixes() { return {"s3"}; } + static std::set uriSchemes() { return {"s3"}; } }; diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index da201a9c3..13265e127 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -22,7 +22,7 @@ class SSHStore : public virtual RemoteStore, public virtual SSHStoreConfig { public: - SSHStore(const std::string & host, const Params & params) + SSHStore([[maybe_unused]] const std::string & scheme, const std::string & host, const Params & params) : StoreConfig(params) , Store(params) , RemoteStoreConfig(params) @@ -38,11 +38,11 @@ public: { } - static std::vector uriPrefixes() { return {"ssh-ng"}; } + static std::set uriSchemes() { return {"ssh-ng"}; } std::string getUri() override { - return uriPrefixes()[0] + "://" + host; + return *uriSchemes().begin() + "://" + host; } bool sameMachine() override diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index fe3f5221c..9252c85e8 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1078,25 +1078,35 @@ std::shared_ptr openFromNonUri(const std::string & uri, const Store::Para ref openStore(const std::string & uri_, const Store::Params & extraParams) { - auto [uri, uriParams] = splitUriAndParams(uri_); auto params = extraParams; - params.insert(uriParams.begin(), uriParams.end()); + try { + auto parsedUri = parseURL(uri_); + params.insert(parsedUri.query.begin(), parsedUri.query.end()); - if (auto store = openFromNonUri(uri, params)) { - store->warnUnknownSettings(); - return ref(store); + auto baseURI = parsedUri.authority.value_or("") + parsedUri.path; + + for (auto implem : *Implementations::registered) { + if (implem.uriSchemes.count(parsedUri.scheme)) { + auto store = implem.create(parsedUri.scheme, baseURI, params); + if (store) { + store->init(); + store->warnUnknownSettings(); + return ref(store); + } + } + } } + catch (BadURL) { + auto [uri, uriParams] = splitUriAndParams(uri_); + params.insert(uriParams.begin(), uriParams.end()); - for (auto implem : *Implementations::registered) { - auto store = implem.create(uri, params); - if (store) { - store->init(); + if (auto store = openFromNonUri(uri, params)) { store->warnUnknownSettings(); return ref(store); } } - throw Error("don't know how to open Nix store '%s'", uri); + throw Error("don't know how to open Nix store '%s'", uri_); } std::list> getDefaultSubstituters() diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 8e9cb786c..bb7d8036b 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -760,7 +760,8 @@ std::list> getDefaultSubstituters(); struct StoreFactory { - std::function (const std::string & uri, const Store::Params & params)> create; + std::set uriSchemes; + std::function (const std::string & scheme, const std::string & uri, const Store::Params & params)> create; std::function ()> getConfig; }; struct Implementations @@ -773,13 +774,14 @@ struct Implementations if (!registered) registered = new std::vector(); StoreFactory factory{ .create = - ([](const std::string & uri, const Store::Params & params) + ([](const std::string & scheme, const std::string & uri, const Store::Params & params) -> std::shared_ptr - { return std::make_shared(uri, params); }), + { return std::make_shared(scheme, uri, params); }), .getConfig = ([]() -> std::shared_ptr - { return std::make_shared(); }) + { return std::make_shared(StringMap({})); }), + .uriSchemes = T::uriSchemes() }; registered->push_back(factory); } From 1129913c4e175f4890a7029d22a301676f4cc3ca Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 11 Sep 2020 11:12:25 +0200 Subject: [PATCH 12/25] fixup! Correctly call all the parent contructors of the stores --- src/libstore/store-api.hh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index bb7d8036b..6f7893e41 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -148,6 +148,8 @@ struct BuildResult struct StoreConfig : public Config { using Config::Config; + StoreConfig() = delete; + const PathSetting storeDir_{this, false, settings.nixStore, "store", "path to the Nix store"}; From 29a632386e358b8cd72cf0545917e324bcfba16e Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 11 Sep 2020 11:13:41 +0200 Subject: [PATCH 13/25] fixup! Make the store plugins more introspectable --- src/libstore/store.hh | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 src/libstore/store.hh diff --git a/src/libstore/store.hh b/src/libstore/store.hh deleted file mode 100644 index bc73963f3..000000000 --- a/src/libstore/store.hh +++ /dev/null @@ -1,9 +0,0 @@ -#pragma once - -namespace nix { - -template class BasicStore; -class StoreConfig; -typedef BasicStore Store; - -} From d65962db4d5c1a2c8f456e3ae6bb789eb3a1e744 Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 11 Sep 2020 14:07:02 +0200 Subject: [PATCH 14/25] Make uri schemes grammar more RFC-compliant Allow `-` and `.` in the RFC schemes as stated by [RFC3986](https://tools.ietf.org/html/rfc3986#section-3.1). Practically, this is needed so that `ssh-ng` is a valid URI scheme --- src/libutil/url.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/url.hh b/src/libutil/url.hh index 2ef88ef2a..1f716ba10 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -31,7 +31,7 @@ ParsedURL parseURL(const std::string & url); // URI stuff. const static std::string pctEncoded = "(?:%[0-9a-fA-F][0-9a-fA-F])"; -const static std::string schemeRegex = "(?:[a-z+]+)"; +const static std::string schemeRegex = "(?:[a-z+.-]+)"; const static std::string ipv6AddressRegex = "(?:\\[[0-9a-fA-F:]+\\])"; const static std::string unreservedRegex = "(?:[a-zA-Z0-9-._~])"; const static std::string subdelimsRegex = "(?:[!$&'\"()*+,;=])"; From f24f0888f93b0881e2cefa1ceb4c8fc187c94f21 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 14 Sep 2020 11:18:45 +0200 Subject: [PATCH 15/25] Document the new store hierarchy --- src/libstore/build.cc | 2 +- src/libstore/dummy-store.cc | 3 +- src/libstore/http-binary-cache-store.cc | 2 -- src/libstore/legacy-ssh-store.cc | 1 - src/libstore/local-binary-cache-store.cc | 2 -- src/libstore/local-store.cc | 2 -- src/libstore/remote-store.cc | 3 -- src/libstore/s3-binary-cache-store.cc | 2 -- src/libstore/ssh-store.cc | 2 -- src/libstore/store-api.hh | 46 +++++++++++++++++++++++- 10 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 4040a3fac..18bb9b12e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2883,7 +2883,7 @@ struct RestrictedStore : public LocalFSStore DerivationGoal & goal; RestrictedStore(const Params & params, ref next, DerivationGoal & goal) - : StoreConfig(params), LocalFSStoreConfig(params), Store(params), LocalFSStore(params), next(next), goal(goal) + : StoreConfig(params), Store(params), LocalFSStore(params), next(next), goal(goal) { } Path getRealStoreDir() override diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 210e5b11c..074953b1d 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -2,7 +2,7 @@ namespace nix { -struct DummyStoreConfig : StoreConfig { +struct DummyStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; }; @@ -14,7 +14,6 @@ struct DummyStore : public Store, public virtual DummyStoreConfig DummyStore(const Params & params) : StoreConfig(params) - , DummyStoreConfig(params) , Store(params) { } diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 095eaa82c..bd65273eb 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -33,9 +33,7 @@ public: const Path & _cacheUri, const Params & params) : StoreConfig(params) - , BinaryCacheStoreConfig(params) , BinaryCacheStore(params) - , HttpBinaryCacheStoreConfig(params) , cacheUri(scheme + "://" + _cacheUri) { if (cacheUri.back() == '/') diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 6dc83e288..ce9051d20 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -45,7 +45,6 @@ struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig LegacySSHStore(const string & scheme, const string & host, const Params & params) : StoreConfig(params) - , LegacySSHStoreConfig(params) , Store(params) , host(host) , connections(make_ref>( diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 4d11e0f2a..b893befbc 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -22,8 +22,6 @@ public: const Path & binaryCacheDir, const Params & params) : StoreConfig(params) - , BinaryCacheStoreConfig(params) - , LocalBinaryCacheStoreConfig(params) , BinaryCacheStore(params) , binaryCacheDir(binaryCacheDir) { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 4e9840f3b..c618203f0 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -44,8 +44,6 @@ namespace nix { LocalStore::LocalStore(const Params & params) : StoreConfig(params) , Store(params) - , LocalFSStoreConfig(params) - , LocalStoreConfig(params) , LocalFSStore(params) , realStoreDir_{this, false, rootDir != "" ? rootDir + "/nix/store" : storeDir, "real", "physical path to the Nix store"} diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 561502cbe..4c8ffe71e 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -126,9 +126,6 @@ ref RemoteStore::openConnectionWrapper() UDSRemoteStore::UDSRemoteStore(const Params & params) : StoreConfig(params) , Store(params) - , LocalFSStoreConfig(params) - , RemoteStoreConfig(params) - , UDSRemoteStoreConfig(params) , LocalFSStore(params) , RemoteStore(params) { diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 020b20c37..e4548dd22 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -201,8 +201,6 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCache const std::string & bucketName, const Params & params) : StoreConfig(params) - , BinaryCacheStoreConfig(params) - , S3BinaryCacheStoreConfig(params) , S3BinaryCacheStore(params) , bucketName(bucketName) , s3Helper(profile, region, scheme, endpoint) diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index 13265e127..fac251d8f 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -25,9 +25,7 @@ public: SSHStore([[maybe_unused]] const std::string & scheme, const std::string & host, const Params & params) : StoreConfig(params) , Store(params) - , RemoteStoreConfig(params) , RemoteStore(params) - , SSHStoreConfig(params) , host(host) , master( host, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 6f7893e41..a67123ff0 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -24,6 +24,31 @@ namespace nix { +/** + * About the class hierarchy of the store implementations: + * + * Each store type `Foo` consists of two classes: + * + * 1. A class `FooConfig : virtual StoreConfig` that contains the configuration + * for the store + * + * It should only contain members of type `const Setting` (or subclasses + * of it) and inherit the constructors of `StoreConfig` + * (`using StoreConfig::StoreConfig`). + * + * 2. A class `Foo : virtual Store, virtual FooConfig` that contains the + * implementation of the store. + * + * This class is expected to have a constructor `Foo(const Params & params)` + * that calls `StoreConfig(params)` (otherwise you're gonna encounter an + * `assertion failure` when trying to instantiate it). + * + * You can then register the new store using: + * + * ``` + * cpp static RegisterStoreImplementation regStore; + * ``` + */ MakeError(SubstError, Error); MakeError(BuildError, Error); // denotes a permanent build failure @@ -148,7 +173,26 @@ struct BuildResult struct StoreConfig : public Config { using Config::Config; - StoreConfig() = delete; + + /** + * When constructing a store implementation, we pass in a map `params` of + * parameters that's supposed to initialize the associated config. + * To do that, we must use the `StoreConfig(StringMap & params)` + * constructor, so we'd like to `delete` its default constructor to enforce + * it. + * + * However, actually deleting it means that all the subclasses of + * `StoreConfig` will have their default constructor deleted (because it's + * supposed to call the deleted default constructor of `StoreConfig`). But + * because we're always using virtual inheritance, the constructors of + * child classes will never implicitely call this one, so deleting it will + * be more painful than anything else. + * + * So we `assert(false)` here to ensure at runtime that the right + * constructor is always called without having to redefine a custom + * constructor for each `*Config` class. + */ + StoreConfig() { assert(false); } const PathSetting storeDir_{this, false, settings.nixStore, From b73adacc1ebda8a790cd8e0b0988c3b0607d947a Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 14 Sep 2020 14:04:02 +0200 Subject: [PATCH 16/25] Add a name to the stores So that it can be printed by `nix describe-stores` --- src/libstore/build.cc | 7 ++++++- src/libstore/dummy-store.cc | 2 ++ src/libstore/http-binary-cache-store.cc | 2 ++ src/libstore/legacy-ssh-store.cc | 2 ++ src/libstore/local-binary-cache-store.cc | 2 ++ src/libstore/local-store.hh | 2 ++ src/libstore/remote-store.hh | 2 ++ src/libstore/s3-binary-cache-store.cc | 2 ++ src/libstore/ssh-store.cc | 2 ++ src/libstore/store-api.hh | 1 + src/nix/describe-stores.cc | 13 +++++++------ 11 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 18bb9b12e..6e55f83d5 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2872,11 +2872,16 @@ void DerivationGoal::writeStructuredAttrs() chownToBuilder(tmpDir + "/.attrs.sh"); } +struct RestrictedStoreConfig : LocalFSStoreConfig +{ + using LocalFSStoreConfig::LocalFSStoreConfig; + const std::string name() { return "Restricted Store"; } +}; /* A wrapper around LocalStore that only allows building/querying of paths that are in the input closures of the build or were added via recursive Nix calls. */ -struct RestrictedStore : public LocalFSStore +struct RestrictedStore : public LocalFSStore, public virtual RestrictedStoreConfig { ref next; diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 074953b1d..128832e60 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -4,6 +4,8 @@ namespace nix { struct DummyStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; + + const std::string name() override { return "Dummy Store"; } }; struct DummyStore : public Store, public virtual DummyStoreConfig diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index bd65273eb..f4ab15a10 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -10,6 +10,8 @@ MakeError(UploadToHTTP, Error); struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig { using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + + const std::string name() override { return "Http Binary Cache Store"; } }; class HttpBinaryCacheStore : public BinaryCacheStore, public HttpBinaryCacheStoreConfig diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index ce9051d20..6563c001f 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -17,6 +17,8 @@ struct LegacySSHStoreConfig : virtual StoreConfig const Setting compress{this, false, "compress", "whether to compress the connection"}; const Setting remoteProgram{this, "nix-store", "remote-program", "path to the nix-store executable on the remote system"}; const Setting remoteStore{this, "", "remote-store", "URI of the store on the remote system"}; + + const std::string name() override { return "Legacy SSH Store"; } }; struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index b893befbc..357bc74df 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -7,6 +7,8 @@ namespace nix { struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig { using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + + const std::string name() override { return "Local Binary Cache Store"; } }; class LocalBinaryCacheStore : public BinaryCacheStore, public virtual LocalBinaryCacheStoreConfig diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 0265f0837..e7c9d1605 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -37,6 +37,8 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig Setting requireSigs{(StoreConfig*) this, settings.requireSigs, "require-sigs", "whether store paths should have a trusted signature on import"}; + + const std::string name() override { return "Local Store"; } }; diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index cceb8d185..a23690830 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -158,6 +158,8 @@ struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreCon : UDSRemoteStoreConfig(Store::Params({})) { } + + const std::string name() override { return "Local Daemon Store"; } }; class UDSRemoteStore : public LocalFSStore, public RemoteStore, public virtual UDSRemoteStoreConfig diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index e4548dd22..0b9c9bb31 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -186,6 +186,8 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig this, false, "multipart-upload", "whether to use multi-part uploads"}; const Setting 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"; } }; struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCacheStoreConfig diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index fac251d8f..df383bba6 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -16,6 +16,8 @@ struct SSHStoreConfig : virtual RemoteStoreConfig const Setting compress{(StoreConfig*) this, false, "compress", "whether to compress the connection"}; const Setting remoteProgram{(StoreConfig*) this, "nix-daemon", "remote-program", "path to the nix-daemon executable on the remote system"}; const Setting remoteStore{(StoreConfig*) this, "", "remote-store", "URI of the store on the remote system"}; + + const std::string name() override { return "SSH Store"; } }; class SSHStore : public virtual RemoteStore, public virtual SSHStoreConfig diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a67123ff0..81b979b73 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -194,6 +194,7 @@ struct StoreConfig : public Config */ StoreConfig() { assert(false); } + virtual const std::string name() = 0; const PathSetting storeDir_{this, false, settings.nixStore, "store", "path to the Nix store"}; diff --git a/src/nix/describe-stores.cc b/src/nix/describe-stores.cc index f019ec9c2..027187bd9 100644 --- a/src/nix/describe-stores.cc +++ b/src/nix/describe-stores.cc @@ -18,13 +18,14 @@ struct CmdDescribeStores : Command, MixJSON void run() override { - + auto res = nlohmann::json::object(); + for (auto & implem : *Implementations::registered) { + auto storeConfig = implem.getConfig(); + auto storeName = storeConfig->name(); + res[storeName] = storeConfig->toJSON(); + } if (json) { - auto res = nlohmann::json::array(); - for (auto & implem : *Implementations::registered) { - auto storeConfig = implem.getConfig(); - std::cout << storeConfig->toJSON().dump() << std::endl; - } + std::cout << res; } else { throw Error("Only json is available for describe-stores"); } From 634cb2a5aec64164dd7fa9467d9cdb1569611c21 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 14 Sep 2020 14:04:21 +0200 Subject: [PATCH 17/25] Add a markdown output to `nix describe-stores` --- src/nix/describe-stores.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/nix/describe-stores.cc b/src/nix/describe-stores.cc index 027187bd9..0cc2d9337 100644 --- a/src/nix/describe-stores.cc +++ b/src/nix/describe-stores.cc @@ -27,7 +27,16 @@ struct CmdDescribeStores : Command, MixJSON if (json) { std::cout << res; } else { - throw Error("Only json is available for describe-stores"); + for (auto & [storeName, storeConfig] : res.items()) { + std::cout << "## " << storeName << std::endl << std::endl; + for (auto & [optionName, optionDesc] : storeConfig.items()) { + std::cout << "### " << optionName << std::endl << std::endl; + std::cout << optionDesc["description"].get() << std::endl; + std::cout << "default: " << optionDesc["defaultValue"] << std::endl < Date: Mon, 14 Sep 2020 14:12:47 +0200 Subject: [PATCH 18/25] Fix build issues with gcc --- src/libstore/local-binary-cache-store.cc | 2 +- src/libstore/remote-store.cc | 2 +- src/libstore/s3-binary-cache-store.cc | 2 +- src/libstore/ssh-store.cc | 2 +- src/libstore/store-api.hh | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 357bc74df..b5744448e 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -20,7 +20,7 @@ private: public: LocalBinaryCacheStore( - [[maybe_unused]] const std::string scheme, + const std::string scheme, const Path & binaryCacheDir, const Params & params) : StoreConfig(params) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 4c8ffe71e..1abe236f7 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -133,7 +133,7 @@ UDSRemoteStore::UDSRemoteStore(const Params & params) UDSRemoteStore::UDSRemoteStore( - [[maybe_unused]] const std::string scheme, + const std::string scheme, std::string socket_path, const Params & params) : UDSRemoteStore(params) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 0b9c9bb31..b3541d7b8 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -199,7 +199,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCache S3Helper s3Helper; S3BinaryCacheStoreImpl( - [[maybe_unused]] const std::string & scheme, + const std::string & scheme, const std::string & bucketName, const Params & params) : StoreConfig(params) diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index df383bba6..8b6e48fb0 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -24,7 +24,7 @@ class SSHStore : public virtual RemoteStore, public virtual SSHStoreConfig { public: - SSHStore([[maybe_unused]] const std::string & scheme, const std::string & host, const Params & params) + SSHStore(const std::string & scheme, const std::string & host, const Params & params) : StoreConfig(params) , Store(params) , RemoteStore(params) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 81b979b73..1bea4837b 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -820,6 +820,7 @@ struct Implementations { if (!registered) registered = new std::vector(); StoreFactory factory{ + .uriSchemes = T::uriSchemes(), .create = ([](const std::string & scheme, const std::string & uri, const Store::Params & params) -> std::shared_ptr @@ -827,8 +828,7 @@ struct Implementations .getConfig = ([]() -> std::shared_ptr - { return std::make_shared(StringMap({})); }), - .uriSchemes = T::uriSchemes() + { return std::make_shared(StringMap({})); }) }; registered->push_back(factory); } From a1e82ba450f274966dc45375b42b5d4413fff77d Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 14 Sep 2020 15:35:17 +0200 Subject: [PATCH 19/25] fixup! Add a default value for the settings --- src/libutil/tests/config.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/tests/config.cc b/src/libutil/tests/config.cc index c5abefe11..c7777a21f 100644 --- a/src/libutil/tests/config.cc +++ b/src/libutil/tests/config.cc @@ -161,7 +161,7 @@ namespace nix { Setting setting{&config, "", "name-of-the-setting", "description"}; setting.assign("value"); - ASSERT_EQ(config.toJSON().dump(), R"#({"name-of-the-setting":{"aliases":[],"description":"description\n","value":"value"}})#"); + ASSERT_EQ(config.toJSON().dump(), R"#({"name-of-the-setting":{"aliases":[],"defaultValue":"","description":"description\n","value":"value"}})#"); } TEST(Config, setSettingAlias) { From fc2d31c423377cea2355e7f7a4ce75e64a7f3620 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 15 Sep 2020 09:10:37 +0200 Subject: [PATCH 20/25] Add `(StoreConfig*)` casts to work around a GCC bug Work around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80431 that was already there in the code but was accidentally removed in the last commits --- src/libstore/binary-cache-store.hh | 12 ++++++------ src/libstore/legacy-ssh-store.cc | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index f1fc82013..4b779cdd4 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -15,12 +15,12 @@ struct BinaryCacheStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; - const Setting compression{this, "xz", "compression", "NAR compression method ('xz', 'bzip2', or 'none')"}; - const Setting writeNARListing{this, false, "write-nar-listing", "whether to write a JSON file listing the files in each NAR"}; - const Setting writeDebugInfo{this, false, "index-debug-info", "whether to index DWARF debug info files by build ID"}; - const Setting secretKeyFile{this, "", "secret-key", "path to secret key used to sign the binary cache"}; - const Setting localNarCache{this, "", "local-nar-cache", "path to a local cache of NARs"}; - const Setting parallelCompression{this, false, "parallel-compression", + const Setting compression{(StoreConfig*) this, "xz", "compression", "NAR compression method ('xz', 'bzip2', or 'none')"}; + const Setting writeNARListing{(StoreConfig*) this, false, "write-nar-listing", "whether to write a JSON file listing the files in each NAR"}; + const Setting writeDebugInfo{(StoreConfig*) this, false, "index-debug-info", "whether to index DWARF debug info files by build ID"}; + const Setting secretKeyFile{(StoreConfig*) this, "", "secret-key", "path to secret key used to sign the binary cache"}; + const Setting localNarCache{(StoreConfig*) this, "", "local-nar-cache", "path to a local cache of NARs"}; + const Setting parallelCompression{(StoreConfig*) this, false, "parallel-compression", "enable multi-threading compression, available for xz only currently"}; }; diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 6563c001f..e9478c1d5 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -12,11 +12,11 @@ namespace nix { struct LegacySSHStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; - const Setting maxConnections{this, 1, "max-connections", "maximum number of concurrent SSH connections"}; - const Setting sshKey{this, "", "ssh-key", "path to an SSH private key"}; - const Setting compress{this, false, "compress", "whether to compress the connection"}; - const Setting remoteProgram{this, "nix-store", "remote-program", "path to the nix-store executable on the remote system"}; - const Setting remoteStore{this, "", "remote-store", "URI of the store on the remote system"}; + const Setting maxConnections{(StoreConfig*) this, 1, "max-connections", "maximum number of concurrent SSH connections"}; + const Setting sshKey{(StoreConfig*) this, "", "ssh-key", "path to an SSH private key"}; + const Setting compress{(StoreConfig*) this, false, "compress", "whether to compress the connection"}; + const Setting remoteProgram{(StoreConfig*) this, "nix-store", "remote-program", "path to the nix-store executable on the remote system"}; + const Setting remoteStore{(StoreConfig*) this, "", "remote-store", "URI of the store on the remote system"}; const std::string name() override { return "Legacy SSH Store"; } }; @@ -24,9 +24,9 @@ struct LegacySSHStoreConfig : virtual StoreConfig struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig { // Hack for getting remote build log output. - // Intentionally not in `StoreConfig` so that it doesn't appear in the - // documentation - const Setting logFD{this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"}; + // Intentionally not in `LegacySSHStoreConfig` so that it doesn't appear in + // the documentation + const Setting logFD{(StoreConfig*) this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"}; struct Connection { From 93c0e14a308d513cf61daa7a003417e42a7dc2f8 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 15 Sep 2020 11:34:20 +0200 Subject: [PATCH 21/25] Include the full nlohmann/json header in config.hh It is apparently required for using `toJSONObject()`, which we do inside the header file (because it's in a template). This was accidentally working when building Nix itself (presumably because `config.hh` was always included after `nlohman/json.hpp`) but caused a (pretty dirty) build failure in the perl bindings package. --- src/libutil/config.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/config.hh b/src/libutil/config.hh index 42fc856e0..18fa9dea8 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -4,7 +4,7 @@ #include "types.hh" -#include +#include #pragma once From e0817cbcdcdbec81d7ce2f5141b4f99bbc2bece7 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 16 Sep 2020 09:06:35 +0200 Subject: [PATCH 22/25] Don't include nlohmann/json.hpp in config.hh Instead make a separate header with the template implementation of `BaseSetting::toJSONObj` that can be included where needed --- src/libstore/globals.hh | 1 + src/libutil/abstractsettingtojson.hh | 15 +++++++++++++++ src/libutil/config.cc | 1 + src/libutil/config.hh | 10 ++-------- 4 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 src/libutil/abstractsettingtojson.hh diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 8a2d3ff75..02721285a 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -2,6 +2,7 @@ #include "types.hh" #include "config.hh" +#include "abstractsettingtojson.hh" #include "util.hh" #include diff --git a/src/libutil/abstractsettingtojson.hh b/src/libutil/abstractsettingtojson.hh new file mode 100644 index 000000000..b3fbc84f7 --- /dev/null +++ b/src/libutil/abstractsettingtojson.hh @@ -0,0 +1,15 @@ +#pragma once + +#include +#include "config.hh" + +namespace nix { +template +std::map BaseSetting::toJSONObject() +{ + auto obj = AbstractSetting::toJSONObject(); + obj.emplace("value", value); + obj.emplace("defaultValue", defaultValue); + return obj; +} +} diff --git a/src/libutil/config.cc b/src/libutil/config.cc index faa5cdbeb..309d23b40 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -1,5 +1,6 @@ #include "config.hh" #include "args.hh" +#include "abstractsettingtojson.hh" #include diff --git a/src/libutil/config.hh b/src/libutil/config.hh index 18fa9dea8..1f5f4e7b9 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -4,7 +4,7 @@ #include "types.hh" -#include +#include #pragma once @@ -255,13 +255,7 @@ public: void convertToArg(Args & args, const std::string & category) override; - std::map toJSONObject() override - { - auto obj = AbstractSetting::toJSONObject(); - obj.emplace("value", value); - obj.emplace("defaultValue", defaultValue); - return obj; - } + std::map toJSONObject() override; }; template From d72927aa7a16be56b134dfadf34d5bbdb751f264 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 16 Sep 2020 13:51:53 +0200 Subject: [PATCH 23/25] Fix the s3 store Add some necessary casts in the initialisation of the store's config --- src/libstore/s3-binary-cache-store.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index b3541d7b8..d43f267e0 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -175,17 +175,17 @@ S3Helper::FileTransferResult S3Helper::getObject( struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig { using BinaryCacheStoreConfig::BinaryCacheStoreConfig; - const Setting profile{this, "", "profile", "The name of the AWS configuration profile to use."}; - const Setting region{this, Aws::Region::US_EAST_1, "region", {"aws-region"}}; - const Setting scheme{this, "", "scheme", "The scheme to use for S3 requests, https by default."}; - const Setting endpoint{this, "", "endpoint", "An optional override of the endpoint to use when talking to S3."}; - const Setting narinfoCompression{this, "", "narinfo-compression", "compression method for .narinfo files"}; - const Setting lsCompression{this, "", "ls-compression", "compression method for .ls files"}; - const Setting logCompression{this, "", "log-compression", "compression method for log/* files"}; + const Setting profile{(StoreConfig*) this, "", "profile", "The name of the AWS configuration profile to use."}; + const Setting region{(StoreConfig*) this, Aws::Region::US_EAST_1, "region", {"aws-region"}}; + const Setting scheme{(StoreConfig*) this, "", "scheme", "The scheme to use for S3 requests, https by default."}; + const Setting endpoint{(StoreConfig*) this, "", "endpoint", "An optional override of the endpoint to use when talking to S3."}; + const Setting narinfoCompression{(StoreConfig*) this, "", "narinfo-compression", "compression method for .narinfo files"}; + const Setting lsCompression{(StoreConfig*) this, "", "ls-compression", "compression method for .ls files"}; + const Setting logCompression{(StoreConfig*) this, "", "log-compression", "compression method for log/* files"}; const Setting multipartUpload{ - this, false, "multipart-upload", "whether to use multi-part uploads"}; + (StoreConfig*) this, false, "multipart-upload", "whether to use multi-part uploads"}; const Setting bufferSize{ - this, 5 * 1024 * 1024, "buffer-size", "size (in bytes) of each part in multi-part uploads"}; + (StoreConfig*) 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"; } }; From c29624bf7d58aeb698ddd12b983465d355fccf79 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 16 Sep 2020 13:52:15 +0200 Subject: [PATCH 24/25] Add a test for `nix describe-stores` Doesn't test much, but at least ensures that the command runs properly --- tests/describe-stores.sh | 8 ++++++++ tests/local.mk | 1 + 2 files changed, 9 insertions(+) create mode 100644 tests/describe-stores.sh diff --git a/tests/describe-stores.sh b/tests/describe-stores.sh new file mode 100644 index 000000000..3fea61483 --- /dev/null +++ b/tests/describe-stores.sh @@ -0,0 +1,8 @@ +source common.sh + +# Query an arbitrary value in `nix describe-stores --json`'s output just to +# check that it has the right structure +[[ $(nix --experimental-features 'nix-command flakes' describe-stores --json | jq '.["SSH Store"]["compress"]["defaultValue"]') == false ]] + +# Ensure that the output of `nix describe-stores` isn't empty +[[ -n $(nix --experimental-features 'nix-command flakes' describe-stores) ]] diff --git a/tests/local.mk b/tests/local.mk index 5d25de019..594901504 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -32,6 +32,7 @@ nix_tests = \ post-hook.sh \ function-trace.sh \ recursive.sh \ + describe-stores.sh \ flakes.sh \ content-addressed.sh # parallel.sh From 77a0e2c5beba478f4cfc9f3c9516e516a5da43c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 16 Sep 2020 14:00:21 +0200 Subject: [PATCH 25/25] Remove useless exception copy Co-authored-by: Eelco Dolstra --- src/libstore/store-api.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 9252c85e8..76cbc0605 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1096,7 +1096,7 @@ ref openStore(const std::string & uri_, } } } - catch (BadURL) { + catch (BadURL &) { auto [uri, uriParams] = splitUriAndParams(uri_); params.insert(uriParams.begin(), uriParams.end());