Merge pull request #4385 from obsidiansystems/store-subclass

Overhaul store subclassing
This commit is contained in:
Eelco Dolstra 2020-12-21 12:51:36 +01:00 committed by GitHub
commit 9fab14adbc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
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"};
};
class BinaryCacheStore : public Store, public virtual BinaryCacheStoreConfig
class BinaryCacheStore : public virtual BinaryCacheStoreConfig, public virtual Store
{
private:

View file

@ -1985,7 +1985,7 @@ void DerivationGoal::writeStructuredAttrs()
chownToBuilder(tmpDir + "/.attrs.sh");
}
struct RestrictedStoreConfig : LocalFSStoreConfig
struct RestrictedStoreConfig : virtual LocalFSStoreConfig
{
using LocalFSStoreConfig::LocalFSStoreConfig;
const std::string name() { return "Restricted Store"; }
@ -1994,14 +1994,19 @@ struct RestrictedStoreConfig : LocalFSStoreConfig
/* 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, public virtual RestrictedStoreConfig
struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual LocalFSStore
{
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

View file

@ -9,7 +9,7 @@ struct DummyStoreConfig : virtual StoreConfig {
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(params)
@ -17,6 +17,7 @@ struct DummyStore : public Store, public virtual DummyStoreConfig
DummyStore(const Params & params)
: StoreConfig(params)
, DummyStoreConfig(params)
, Store(params)
{ }

View file

@ -15,7 +15,7 @@ struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
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:
@ -36,6 +36,9 @@ public:
const Path & _cacheUri,
const Params & params)
: StoreConfig(params)
, BinaryCacheStoreConfig(params)
, HttpBinaryCacheStoreConfig(params)
, Store(params)
, BinaryCacheStore(params)
, cacheUri(scheme + "://" + _cacheUri)
{

View file

@ -22,7 +22,7 @@ struct LegacySSHStoreConfig : virtual StoreConfig
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.
// 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)
: StoreConfig(params)
, LegacySSHStoreConfig(params)
, Store(params)
, host(host)
, 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"; }
};
class LocalBinaryCacheStore : public BinaryCacheStore, public virtual LocalBinaryCacheStoreConfig
class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public virtual BinaryCacheStore
{
private:
@ -24,6 +24,9 @@ public:
const Path & binaryCacheDir,
const Params & params)
: StoreConfig(params)
, BinaryCacheStoreConfig(params)
, LocalBinaryCacheStoreConfig(params)
, Store(params)
, BinaryCacheStore(params)
, binaryCacheDir(binaryCacheDir)
{

View file

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

View file

@ -100,6 +100,8 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd)
LocalStore::LocalStore(const Params & params)
: StoreConfig(params)
, LocalFSStoreConfig(params)
, LocalStoreConfig(params)
, Store(params)
, LocalFSStore(params)
, 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:

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 */
RemoteStore::RemoteStore(const Params & params)
: Store(params)
, RemoteStoreConfig(params)
: RemoteStoreConfig(params)
, Store(params)
, connections(make_ref<Pool<Connection>>(
std::max(1, (int) maxConnections),
[this]() {

View file

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

View file

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

View file

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

View file

@ -20,12 +20,14 @@ struct SSHStoreConfig : virtual RemoteStoreConfig
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:
SSHStore(const std::string & scheme, const std::string & host, const Params & params)
: StoreConfig(params)
, RemoteStoreConfig(params)
, SSHStoreConfig(params)
, Store(params)
, RemoteStore(params)
, host(host)

View file

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

View file

@ -15,6 +15,9 @@ namespace nix {
UDSRemoteStore::UDSRemoteStore(const Params & params)
: StoreConfig(params)
, LocalFSStoreConfig(params)
, RemoteStoreConfig(params)
, UDSRemoteStoreConfig(params)
, Store(params)
, LocalFSStore(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"; }
};
class UDSRemoteStore : public LocalFSStore, public RemoteStore, public virtual UDSRemoteStoreConfig
class UDSRemoteStore : public virtual UDSRemoteStoreConfig, public virtual LocalFSStore, public virtual RemoteStore
{
public: