Overhaul store subclassing

We embrace virtual the rest of the way, and get rid of the
`assert(false)` 0-param constructors.

We also list config base classes first, so the constructor order is
always:

  1. all the configs
  2. all the stores

Each in the same order
This commit is contained in:
John Ericson 2020-12-20 15:33:12 +00:00
parent ec3e202832
commit 1a1af75338
17 changed files with 48 additions and 44 deletions

View File

@ -24,7 +24,7 @@ struct BinaryCacheStoreConfig : virtual StoreConfig
"enable multi-threading compression, available for xz only currently"}; "enable multi-threading compression, available for xz only currently"};
}; };
class BinaryCacheStore : public Store, public virtual BinaryCacheStoreConfig class BinaryCacheStore : public virtual BinaryCacheStoreConfig, public virtual Store
{ {
private: private:

View File

@ -1985,7 +1985,7 @@ void DerivationGoal::writeStructuredAttrs()
chownToBuilder(tmpDir + "/.attrs.sh"); chownToBuilder(tmpDir + "/.attrs.sh");
} }
struct RestrictedStoreConfig : LocalFSStoreConfig struct RestrictedStoreConfig : virtual LocalFSStoreConfig
{ {
using LocalFSStoreConfig::LocalFSStoreConfig; using LocalFSStoreConfig::LocalFSStoreConfig;
const std::string name() { return "Restricted Store"; } const std::string name() { return "Restricted Store"; }
@ -1994,14 +1994,19 @@ struct RestrictedStoreConfig : LocalFSStoreConfig
/* A wrapper around LocalStore that only allows building/querying of /* A wrapper around LocalStore that only allows building/querying of
paths that are in the input closures of the build or were added via paths that are in the input closures of the build or were added via
recursive Nix calls. */ recursive Nix calls. */
struct RestrictedStore : public LocalFSStore, public virtual RestrictedStoreConfig struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual LocalFSStore
{ {
ref<LocalStore> next; ref<LocalStore> next;
DerivationGoal & goal; DerivationGoal & goal;
RestrictedStore(const Params & params, ref<LocalStore> next, DerivationGoal & goal) RestrictedStore(const Params & params, ref<LocalStore> next, DerivationGoal & goal)
: StoreConfig(params), Store(params), LocalFSStore(params), next(next), goal(goal) : StoreConfig(params)
, LocalFSStoreConfig(params)
, RestrictedStoreConfig(params)
, Store(params)
, LocalFSStore(params)
, next(next), goal(goal)
{ } { }
Path getRealStoreDir() override Path getRealStoreDir() override

View File

@ -9,7 +9,7 @@ struct DummyStoreConfig : virtual StoreConfig {
const std::string name() override { return "Dummy Store"; } const std::string name() override { return "Dummy Store"; }
}; };
struct DummyStore : public Store, public virtual DummyStoreConfig struct DummyStore : public virtual DummyStoreConfig, public virtual Store
{ {
DummyStore(const std::string scheme, const std::string uri, const Params & params) DummyStore(const std::string scheme, const std::string uri, const Params & params)
: DummyStore(params) : DummyStore(params)
@ -17,6 +17,7 @@ struct DummyStore : public Store, public virtual DummyStoreConfig
DummyStore(const Params & params) DummyStore(const Params & params)
: StoreConfig(params) : StoreConfig(params)
, DummyStoreConfig(params)
, Store(params) , Store(params)
{ } { }

View File

@ -15,7 +15,7 @@ struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
const std::string name() override { return "Http Binary Cache Store"; } const std::string name() override { return "Http Binary Cache Store"; }
}; };
class HttpBinaryCacheStore : public BinaryCacheStore, public HttpBinaryCacheStoreConfig class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public virtual BinaryCacheStore
{ {
private: private:
@ -36,6 +36,9 @@ public:
const Path & _cacheUri, const Path & _cacheUri,
const Params & params) const Params & params)
: StoreConfig(params) : StoreConfig(params)
, BinaryCacheStoreConfig(params)
, HttpBinaryCacheStoreConfig(params)
, Store(params)
, BinaryCacheStore(params) , BinaryCacheStore(params)
, cacheUri(scheme + "://" + _cacheUri) , cacheUri(scheme + "://" + _cacheUri)
{ {

View File

@ -22,7 +22,7 @@ struct LegacySSHStoreConfig : virtual StoreConfig
const std::string name() override { return "Legacy SSH Store"; } const std::string name() override { return "Legacy SSH Store"; }
}; };
struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Store
{ {
// Hack for getting remote build log output. // Hack for getting remote build log output.
// Intentionally not in `LegacySSHStoreConfig` so that it doesn't appear in // Intentionally not in `LegacySSHStoreConfig` so that it doesn't appear in
@ -48,6 +48,7 @@ struct LegacySSHStore : public Store, public virtual LegacySSHStoreConfig
LegacySSHStore(const string & scheme, const string & host, const Params & params) LegacySSHStore(const string & scheme, const string & host, const Params & params)
: StoreConfig(params) : StoreConfig(params)
, LegacySSHStoreConfig(params)
, Store(params) , Store(params)
, host(host) , host(host)
, connections(make_ref<Pool<Connection>>( , connections(make_ref<Pool<Connection>>(

View File

@ -11,7 +11,7 @@ struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
const std::string name() override { return "Local Binary Cache Store"; } const std::string name() override { return "Local Binary Cache Store"; }
}; };
class LocalBinaryCacheStore : public BinaryCacheStore, public virtual LocalBinaryCacheStoreConfig class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public virtual BinaryCacheStore
{ {
private: private:
@ -24,6 +24,9 @@ public:
const Path & binaryCacheDir, const Path & binaryCacheDir,
const Params & params) const Params & params)
: StoreConfig(params) : StoreConfig(params)
, BinaryCacheStoreConfig(params)
, LocalBinaryCacheStoreConfig(params)
, Store(params)
, BinaryCacheStore(params) , BinaryCacheStore(params)
, binaryCacheDir(binaryCacheDir) , binaryCacheDir(binaryCacheDir)
{ {

View File

@ -20,7 +20,7 @@ struct LocalFSStoreConfig : virtual StoreConfig
"log", "directory where Nix will store state"}; "log", "directory where Nix will store state"};
}; };
class LocalFSStore : public virtual Store, public virtual LocalFSStoreConfig class LocalFSStore : public virtual LocalFSStoreConfig, public virtual Store
{ {
public: public:

View File

@ -100,6 +100,8 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd)
LocalStore::LocalStore(const Params & params) LocalStore::LocalStore(const Params & params)
: StoreConfig(params) : StoreConfig(params)
, LocalFSStoreConfig(params)
, LocalStoreConfig(params)
, Store(params) , Store(params)
, LocalFSStore(params) , LocalFSStore(params)
, realStoreDir_{this, false, rootDir != "" ? rootDir + "/nix/store" : storeDir, "real", , realStoreDir_{this, false, rootDir != "" ? rootDir + "/nix/store" : storeDir, "real",

View File

@ -43,7 +43,7 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig
}; };
class LocalStore : public LocalFSStore, public virtual LocalStoreConfig class LocalStore : public virtual LocalStoreConfig, public virtual LocalFSStore
{ {
private: private:

View File

@ -77,8 +77,8 @@ void write(const Store & store, Sink & out, const std::optional<ContentAddress>
/* TODO: Separate these store impls into different files, give them better names */ /* TODO: Separate these store impls into different files, give them better names */
RemoteStore::RemoteStore(const Params & params) RemoteStore::RemoteStore(const Params & params)
: Store(params) : RemoteStoreConfig(params)
, RemoteStoreConfig(params) , Store(params)
, connections(make_ref<Pool<Connection>>( , connections(make_ref<Pool<Connection>>(
std::max(1, (int) maxConnections), std::max(1, (int) maxConnections),
[this]() { [this]() {

View File

@ -29,7 +29,7 @@ struct RemoteStoreConfig : virtual StoreConfig
/* FIXME: RemoteStore is a misnomer - should be something like /* FIXME: RemoteStore is a misnomer - should be something like
DaemonStore. */ DaemonStore. */
class RemoteStore : public virtual Store, public virtual RemoteStoreConfig class RemoteStore : public virtual RemoteStoreConfig, public virtual Store
{ {
public: public:

View File

@ -177,6 +177,11 @@ S3Helper::FileTransferResult S3Helper::getObject(
return res; return res;
} }
S3BinaryCacheStore::S3BinaryCacheStore(const Params & params)
: BinaryCacheStoreConfig(params)
, BinaryCacheStore(params)
{ }
struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
{ {
using BinaryCacheStoreConfig::BinaryCacheStoreConfig; using BinaryCacheStoreConfig::BinaryCacheStoreConfig;
@ -195,7 +200,7 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
const std::string name() override { return "S3 Binary Cache Store"; } const std::string name() override { return "S3 Binary Cache Store"; }
}; };
struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCacheStoreConfig struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual S3BinaryCacheStore
{ {
std::string bucketName; std::string bucketName;
@ -208,6 +213,10 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore, virtual S3BinaryCache
const std::string & bucketName, const std::string & bucketName,
const Params & params) const Params & params)
: StoreConfig(params) : StoreConfig(params)
, BinaryCacheStoreConfig(params)
, S3BinaryCacheStoreConfig(params)
, Store(params)
, BinaryCacheStore(params)
, S3BinaryCacheStore(params) , S3BinaryCacheStore(params)
, bucketName(bucketName) , bucketName(bucketName)
, s3Helper(profile, region, scheme, endpoint) , s3Helper(profile, region, scheme, endpoint)

View File

@ -6,13 +6,11 @@
namespace nix { namespace nix {
class S3BinaryCacheStore : public BinaryCacheStore class S3BinaryCacheStore : public virtual BinaryCacheStore
{ {
protected: protected:
S3BinaryCacheStore(const Params & params) S3BinaryCacheStore(const Params & params);
: BinaryCacheStore(params)
{ }
public: public:

View File

@ -20,12 +20,14 @@ struct SSHStoreConfig : virtual RemoteStoreConfig
const std::string name() override { return "SSH Store"; } const std::string name() override { return "SSH Store"; }
}; };
class SSHStore : public virtual RemoteStore, public virtual SSHStoreConfig class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
{ {
public: public:
SSHStore(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) : StoreConfig(params)
, RemoteStoreConfig(params)
, SSHStoreConfig(params)
, Store(params) , Store(params)
, RemoteStore(params) , RemoteStore(params)
, host(host) , host(host)

View File

@ -175,25 +175,7 @@ struct StoreConfig : public Config
{ {
using Config::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); }
virtual ~StoreConfig() { } virtual ~StoreConfig() { }

View File

@ -15,6 +15,9 @@ namespace nix {
UDSRemoteStore::UDSRemoteStore(const Params & params) UDSRemoteStore::UDSRemoteStore(const Params & params)
: StoreConfig(params) : StoreConfig(params)
, LocalFSStoreConfig(params)
, RemoteStoreConfig(params)
, UDSRemoteStoreConfig(params)
, Store(params) , Store(params)
, LocalFSStore(params) , LocalFSStore(params)
, RemoteStore(params) , RemoteStore(params)

View File

@ -14,15 +14,10 @@ struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreCon
{ {
} }
UDSRemoteStoreConfig()
: UDSRemoteStoreConfig(Store::Params({}))
{
}
const std::string name() override { return "Local Daemon Store"; } const std::string name() override { return "Local Daemon Store"; }
}; };
class UDSRemoteStore : public LocalFSStore, public RemoteStore, public virtual UDSRemoteStoreConfig class UDSRemoteStore : public virtual UDSRemoteStoreConfig, public virtual LocalFSStore, public virtual RemoteStore
{ {
public: public: