Merge pull request #13764 from obsidiansystems/simplify-store-dir

Simplify "Store dir" superclass

Author: Ericson2314

src/libstore/include/nix/store/store-api.hh

@@ -81,6 +81,25 @@ struct MissingPaths
     uint64_t narSize{0};
 };
 
+/**
+ * Need to make this a separate class so I can get the right
+ * initialization order in the constructor for `StoreConfig`.
+ */
+struct StoreConfigBase : Config
+{
+    using Config::Config;
+
+    const PathSetting storeDir_{
+        this,
+        settings.nixStore,
+        "store",
+        R"(
+          Logical location of the Nix store, usually
+          `/nix/store`. Note that you can only copy store paths
+          between stores if they have the same `store` setting.
+        )"};
+};
+
 /**
  * About the class hierarchy of the store types:
  *
@@ -107,10 +126,17 @@ struct MissingPaths
  * ```
  * cpp static RegisterStoreImplementation<FooConfig> regStore;
  * ```
+ *
+ * @note The order of `StoreConfigBase` and then `StorerConfig` is
+ * very important. This ensures that `StoreConfigBase::storeDir_`
+ * is initialized before we have our one chance (because references are
+ * immutable) to initialize `StoreConfig::storeDir`.
  */
-struct StoreConfig : public StoreDirConfig
+struct StoreConfig : public StoreConfigBase, public StoreDirConfig
 {
-    using StoreDirConfig::StoreDirConfig;
+    using Params = StoreReference::Params;
+
+    StoreConfig(const Params & params);
 
     StoreConfig() = delete;
 
@@ -233,7 +259,7 @@ struct StoreConfig : public StoreDirConfig
  * underlying resource, which could be an external process (daemon
  * server), file system state, etc.
  */
-class Store : public std::enable_shared_from_this<Store>, public MixStoreDirMethods
+class Store : public std::enable_shared_from_this<Store>, public StoreDirConfig
 {
 public:
 

src/libstore/include/nix/store/store-dir-config.hh

@@ -18,15 +18,17 @@ MakeError(BadStorePath, Error);
 MakeError(BadStorePathName, BadStorePath);
 
 /**
- * @todo This should just be part of `StoreDirConfig`. However, it would
- * be a huge amount of churn if `Store` didn't have these methods
+ * @todo This should just be inherited by `StoreConfig`. However, it
+ * would be a huge amount of churn if `Store` didn't have these methods
  * anymore, forcing a bunch of code to go from `store.method(...)` to
  * `store.config.method(...)`.
  *
- * So we instead pull out the methods into their own mix-in, so can put
- * them directly on the Store too.
+ * @todo this should not have "config" in its name, because it no longer
+ * uses the configuration system for `storeDir` --- in fact, `storeDir`
+ * isn't even owned, but a mere reference. But doing that rename would
+ * cause a bunch of churn.
  */
-struct MixStoreDirMethods
+struct StoreDirConfig
 {
     const Path & storeDir;
 
@@ -96,40 +98,4 @@ struct MixStoreDirMethods
         PathFilter & filter = defaultPathFilter) const;
 };
 
-/**
- * Need to make this a separate class so I can get the right
- * initialization order in the constructor for `StoreDirConfig`.
- */
-struct StoreDirConfigBase : Config
-{
-    using Config::Config;
-
-    const PathSetting storeDir_{
-        this,
-        settings.nixStore,
-        "store",
-        R"(
-          Logical location of the Nix store, usually
-          `/nix/store`. Note that you can only copy store paths
-          between stores if they have the same `store` setting.
-        )"};
-};
-
-/**
- * The order of `StoreDirConfigBase` and then `MixStoreDirMethods` is
- * very important. This ensures that `StoreDirConfigBase::storeDir_`
- * is initialized before we have our one chance (because references are
- * immutable) to initialize `MixStoreDirMethods::storeDir`.
- */
-struct StoreDirConfig : StoreDirConfigBase, MixStoreDirMethods
-{
-    using Params = StringMap;
-
-    StoreDirConfig(const Params & params);
-
-    StoreDirConfig() = delete;
-
-    virtual ~StoreDirConfig() = default;
-};
-
 } // namespace nix

src/libstore/store-api.cc

@@ -27,12 +27,18 @@ using json = nlohmann::json;
 
 namespace nix {
 
-bool MixStoreDirMethods::isInStore(PathView path) const
+StoreConfig::StoreConfig(const Params & params)
+    : StoreConfigBase(params)
+    , StoreDirConfig{storeDir_}
+{
+}
+
+bool StoreDirConfig::isInStore(PathView path) const
 {
     return isInDir(path, storeDir);
 }
 
-std::pair<StorePath, Path> MixStoreDirMethods::toStorePath(PathView path) const
+std::pair<StorePath, Path> StoreDirConfig::toStorePath(PathView path) const
 {
     if (!isInStore(path))
         throw Error("path '%1%' is not in the Nix store", path);
@@ -293,7 +299,7 @@ StringSet Store::Config::getDefaultSystemFeatures()
 }
 
 Store::Store(const Store::Config & config)
-    : MixStoreDirMethods{config}
+    : StoreDirConfig{config}
     , config{config}
     , state({(size_t) config.pathInfoCacheSize})
 {
@@ -1082,7 +1088,7 @@ decodeValidPathInfo(const Store & store, std::istream & str, std::optional<HashR
     return std::optional<ValidPathInfo>(std::move(info));
 }
 
-std::string MixStoreDirMethods::showPaths(const StorePathSet & paths) const
+std::string StoreDirConfig::showPaths(const StorePathSet & paths) const
 {
     std::string s;
     for (auto & i : paths) {

src/libstore/store-dir-config.cc

@@ -6,7 +6,7 @@
 
 namespace nix {
 
-StorePath MixStoreDirMethods::parseStorePath(std::string_view path) const
+StorePath StoreDirConfig::parseStorePath(std::string_view path) const
 {
     // On Windows, `/nix/store` is not a canonical path. More broadly it
     // is unclear whether this function should be using the native
@@ -25,7 +25,7 @@ StorePath MixStoreDirMethods::parseStorePath(std::string_view path) const
     return StorePath(baseNameOf(p));
 }
 
-std::optional<StorePath> MixStoreDirMethods::maybeParseStorePath(std::string_view path) const
+std::optional<StorePath> StoreDirConfig::maybeParseStorePath(std::string_view path) const
 {
     try {
         return parseStorePath(path);
@@ -34,25 +34,25 @@ std::optional<StorePath> MixStoreDirMethods::maybeParseStorePath(std::string_vie
     }
 }
 
-bool MixStoreDirMethods::isStorePath(std::string_view path) const
+bool StoreDirConfig::isStorePath(std::string_view path) const
 {
     return (bool) maybeParseStorePath(path);
 }
 
-StorePathSet MixStoreDirMethods::parseStorePathSet(const PathSet & paths) const
+StorePathSet StoreDirConfig::parseStorePathSet(const PathSet & paths) const
 {
     StorePathSet res;
     for (auto & i : paths)
         res.insert(parseStorePath(i));
     return res;
 }
 
-std::string MixStoreDirMethods::printStorePath(const StorePath & path) const
+std::string StoreDirConfig::printStorePath(const StorePath & path) const
 {
     return (storeDir + "/").append(path.to_string());
 }
 
-PathSet MixStoreDirMethods::printStorePathSet(const StorePathSet & paths) const
+PathSet StoreDirConfig::printStorePathSet(const StorePathSet & paths) const
 {
     PathSet res;
     for (auto & i : paths)
@@ -69,28 +69,28 @@ also update the user-visible behavior, please update the specification
 to match.
 */
 
-StorePath MixStoreDirMethods::makeStorePath(std::string_view type, std::string_view hash, std::string_view name) const
+StorePath StoreDirConfig::makeStorePath(std::string_view type, std::string_view hash, std::string_view name) const
 {
     /* e.g., "source:sha256:1abc...:/nix/store:foo.tar.gz" */
     auto s = std::string(type) + ":" + std::string(hash) + ":" + storeDir + ":" + std::string(name);
     auto h = compressHash(hashString(HashAlgorithm::SHA256, s), 20);
     return StorePath(h, name);
 }
 
-StorePath MixStoreDirMethods::makeStorePath(std::string_view type, const Hash & hash, std::string_view name) const
+StorePath StoreDirConfig::makeStorePath(std::string_view type, const Hash & hash, std::string_view name) const
 {
     return makeStorePath(type, hash.to_string(HashFormat::Base16, true), name);
 }
 
-StorePath MixStoreDirMethods::makeOutputPath(std::string_view id, const Hash & hash, std::string_view name) const
+StorePath StoreDirConfig::makeOutputPath(std::string_view id, const Hash & hash, std::string_view name) const
 {
     return makeStorePath("output:" + std::string{id}, hash, outputPathName(name, id));
 }
 
 /* Stuff the references (if any) into the type.  This is a bit
    hacky, but we can't put them in, say, <s2> (per the grammar above)
    since that would be ambiguous. */
-static std::string makeType(const MixStoreDirMethods & store, std::string && type, const StoreReferences & references)
+static std::string makeType(const StoreDirConfig & store, std::string && type, const StoreReferences & references)
 {
     for (auto & i : references.others) {
         type += ":";
@@ -101,7 +101,7 @@ static std::string makeType(const MixStoreDirMethods & store, std::string && typ
     return std::move(type);
 }
 
-StorePath MixStoreDirMethods::makeFixedOutputPath(std::string_view name, const FixedOutputInfo & info) const
+StorePath StoreDirConfig::makeFixedOutputPath(std::string_view name, const FixedOutputInfo & info) const
 {
     if (info.method == FileIngestionMethod::Git
         && !(info.hash.algo == HashAlgorithm::SHA1 || info.hash.algo == HashAlgorithm::SHA256)) {
@@ -126,7 +126,7 @@ StorePath MixStoreDirMethods::makeFixedOutputPath(std::string_view name, const F
 }
 
 StorePath
-MixStoreDirMethods::makeFixedOutputPathFromCA(std::string_view name, const ContentAddressWithReferences & ca) const
+StoreDirConfig::makeFixedOutputPathFromCA(std::string_view name, const ContentAddressWithReferences & ca) const
 {
     // New template
     return std::visit(
@@ -148,7 +148,7 @@ MixStoreDirMethods::makeFixedOutputPathFromCA(std::string_view name, const Conte
         ca.raw);
 }
 
-std::pair<StorePath, Hash> MixStoreDirMethods::computeStorePath(
+std::pair<StorePath, Hash> StoreDirConfig::computeStorePath(
     std::string_view name,
     const SourcePath & path,
     ContentAddressMethod method,
@@ -173,10 +173,4 @@ std::pair<StorePath, Hash> MixStoreDirMethods::computeStorePath(
     };
 }
 
-StoreDirConfig::StoreDirConfig(const Params & params)
-    : StoreDirConfigBase(params)
-    , MixStoreDirMethods{storeDir_}
-{
-}
-
 } // namespace nix