Merge pull request #14246 from obsidiansystems/dummy-store-derivations-separately

Make Dummy store store derivations separately

src/libstore-tests/write-derivation.cc

@@ -50,8 +50,8 @@ TEST_F(WriteDerivationTest, addToStoreFromDumpCalledOnce)
     EXPECT_EQ(path1, path2);
     EXPECT_THAT(
         [&] { writeDerivation(*store, drv, Repair); },
-        ::testing::ThrowsMessage<Error>(testing::HasSubstrIgnoreANSIMatcher(
+        ::testing::ThrowsMessage<Error>(
-            "operation 'addToStoreFromDump' is not supported by store 'dummy://'")));
+            testing::HasSubstrIgnoreANSIMatcher("operation 'writeDerivation' is not supported by store 'dummy://'")));
 }
 
 } // namespace nix

src/libstore/derivations.cc

@@ -105,7 +105,7 @@ bool BasicDerivation::isBuiltin() const
     return builder.substr(0, 8) == "builtin:";
 }
 
-StorePath writeDerivation(Store & store, const Derivation & drv, RepairFlag repair, bool readOnly)
+static auto infoForDerivation(Store & store, const Derivation & drv)
 {
     auto references = drv.inputSrcs;
     for (auto & i : drv.inputDrvs.map)
@@ -117,13 +117,32 @@ StorePath writeDerivation(Store & store, const Derivation & drv, RepairFlag repa
     auto contents = drv.unparse(store, false);
     auto hash = hashString(HashAlgorithm::SHA256, contents);
     auto ca = TextInfo{.hash = hash, .references = references};
-    auto path = store.makeFixedOutputPathFromCA(suffix, ca);
+    return std::tuple{
+        suffix,
+        contents,
+        references,
+        store.makeFixedOutputPathFromCA(suffix, ca),
+    };
+}
 
-    if (readOnly || settings.readOnlyMode || (store.isValidPath(path) && !repair))
+StorePath writeDerivation(Store & store, const Derivation & drv, RepairFlag repair, bool readOnly)
+{
+    if (readOnly || settings.readOnlyMode) {
+        auto [_x, _y, _z, path] = infoForDerivation(store, drv);
+        return path;
+    } else
+        return store.writeDerivation(drv, repair);
+}
+
+StorePath Store::writeDerivation(const Derivation & drv, RepairFlag repair)
+{
+    auto [suffix, contents, references, path] = infoForDerivation(*this, drv);
+
+    if (isValidPath(path) && !repair)
         return path;
 
     StringSource s{contents};
-    auto path2 = store.addToStoreFromDump(
+    auto path2 = addToStoreFromDump(
         s,
         suffix,
         FileSerialisationMethod::Flat,

src/libstore/dummy-store.cc

@@ -137,12 +137,40 @@ struct DummyStoreImpl : DummyStore
     void queryPathInfoUncached(
         const StorePath & path, Callback<std::shared_ptr<const ValidPathInfo>> callback) noexcept override
     {
-        bool visited = contents.cvisit(path, [&](const auto & kv) {
+        if (path.isDerivation()) {
-            callback(std::make_shared<ValidPathInfo>(StorePath{kv.first}, kv.second.info));
+            if (auto accessor_ = getMemoryFSAccessor(path)) {
-        });
+                ref<MemorySourceAccessor> accessor = ref{std::move(accessor_)};
+                /* compute path info on demand */
+                auto narHash =
+                    hashPath({accessor, CanonPath::root}, FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256);
+                auto info = std::make_shared<ValidPathInfo>(path, UnkeyedValidPathInfo{narHash.hash});
+                info->narSize = narHash.numBytesDigested;
+                info->ca = ContentAddress{
+                    .method = ContentAddressMethod::Raw::Text,
+                    .hash = hashString(
+                        HashAlgorithm::SHA256,
+                        std::get<MemorySourceAccessor::File::Regular>(accessor->root->raw).contents),
+                };
+                callback(std::move(info));
+                return;
+            }
+        } else {
+            if (contents.cvisit(path, [&](const auto & kv) {
+                    callback(std::make_shared<ValidPathInfo>(StorePath{kv.first}, kv.second.info));
+                }))
+                return;
+        }
 
-        if (!visited)
+        callback(nullptr);
-            callback(nullptr);
+    }
+
+    /**
+     * Do this to avoid `queryPathInfoUncached` computing `PathInfo`
+     * that we don't need just to return a `bool`.
+     */
+    bool isValidPathUncached(const StorePath & path) override
+    {
+        return path.isDerivation() ? derivations.contains(path) : Store::isValidPathUncached(path);
     }
 
     /**
@@ -169,18 +197,25 @@ struct DummyStoreImpl : DummyStore
         if (checkSigs)
             throw Error("checking signatures is not supported for '%s' store", config->getHumanReadableURI());
 
-        auto temp = make_ref<MemorySourceAccessor>();
+        auto accessor = make_ref<MemorySourceAccessor>();
-        MemorySink tempSink{*temp};
+        MemorySink tempSink{*accessor};
         parseDump(tempSink, source);
         auto path = info.path;
 
-        auto accessor = make_ref<MemorySourceAccessor>(std::move(*temp));
+        if (info.path.isDerivation()) {
-        contents.insert(
+            warn("back compat supporting `addToStore` for inserting derivations in dummy store");
-            {path,
+            writeDerivation(
-             PathInfoAndContents{
+                parseDerivation(*this, accessor->readFile(CanonPath::root), Derivation::nameFromPath(info.path)));
-                 std::move(info),
+            return;
-                 accessor,
+        }
-             }});
+
+        contents.insert({
+            path,
+            PathInfoAndContents{
+                std::move(info),
+                accessor,
+            },
+        });
         wholeStoreView->addObject(path.to_string(), accessor);
     }
 
@@ -193,6 +228,9 @@ struct DummyStoreImpl : DummyStore
         const StorePathSet & references = StorePathSet(),
         RepairFlag repair = NoRepair) override
     {
+        if (isDerivation(name))
+            throw Error("Do not insert derivation into dummy store with `addToStoreFromDump`");
+
         if (config->readOnly)
             unsupported("addToStoreFromDump");
 
@@ -239,17 +277,47 @@ struct DummyStoreImpl : DummyStore
 
         auto path = info.path;
         auto accessor = make_ref<MemorySourceAccessor>(std::move(*temp));
-        contents.insert(
+        contents.insert({
-            {path,
+            path,
-             PathInfoAndContents{
+            PathInfoAndContents{
-                 std::move(info),
+                std::move(info),
-                 accessor,
+                accessor,
-             }});
+            },
+        });
         wholeStoreView->addObject(path.to_string(), accessor);
 
         return path;
     }
 
+    StorePath writeDerivation(const Derivation & drv, RepairFlag repair = NoRepair) override
+    {
+        auto drvPath = ::nix::writeDerivation(*this, drv, repair, /*readonly=*/true);
+
+        if (!derivations.contains(drvPath) || repair) {
+            if (config->readOnly)
+                unsupported("writeDerivation");
+            derivations.insert({drvPath, drv});
+        }
+
+        return drvPath;
+    }
+
+    Derivation readDerivation(const StorePath & drvPath) override
+    {
+        if (std::optional res = getConcurrent(derivations, drvPath))
+            return *res;
+        else
+            throw Error("derivation '%s' is not valid", printStorePath(drvPath));
+    }
+
+    /**
+     * No such thing as an "invalid derivation" with the dummy store
+     */
+    Derivation readInvalidDerivation(const StorePath & drvPath) override
+    {
+        return readDerivation(drvPath);
+    }
+
     void registerDrvOutput(const Realisation & output) override
     {
         auto ref = make_ref<UnkeyedRealisation>(output);
@@ -273,13 +341,28 @@ struct DummyStoreImpl : DummyStore
             callback(nullptr);
     }
 
-    std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath & path, bool requireValidPath) override
+    std::shared_ptr<MemorySourceAccessor> getMemoryFSAccessor(const StorePath & path, bool requireValidPath = true)
     {
-        std::shared_ptr<SourceAccessor> res;
+        std::shared_ptr<MemorySourceAccessor> res;
-        contents.cvisit(path, [&](const auto & kv) { res = kv.second.contents.get_ptr(); });
+        if (path.isDerivation())
+            derivations.cvisit(path, [&](const auto & kv) {
+                /* compute path info on demand */
+                auto res2 = make_ref<MemorySourceAccessor>();
+                res2->root = MemorySourceAccessor::File::Regular{
+                    .contents = kv.second.unparse(*this, false),
+                };
+                res = std::move(res2).get_ptr();
+            });
+        else
+            contents.cvisit(path, [&](const auto & kv) { res = kv.second.contents.get_ptr(); });
         return res;
     }
 
+    std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath & path, bool requireValidPath = true) override
+    {
+        return getMemoryFSAccessor(path, requireValidPath);
+    }
+
     ref<SourceAccessor> getFSAccessor(bool requireValidPath) override
     {
         return wholeStoreView;

src/libstore/include/nix/store/dummy-store-impl.hh

@@ -2,6 +2,7 @@
 ///@file
 
 #include "nix/store/dummy-store.hh"
+#include "nix/store/derivations.hh"
 
 #include <boost/unordered/concurrent_flat_map.hpp>
 
@@ -25,11 +26,17 @@ struct DummyStore : virtual Store
     };
 
     /**
-     * This is map conceptually owns the file system objects for each
+     * This map conceptually owns the file system objects for each
      * store object.
      */
     boost::concurrent_flat_map<StorePath, PathInfoAndContents> contents;
 
+    /**
+     * This map conceptually owns every derivation, allowing us to
+     * avoid "on-disk drv format" serialization round-trips.
+     */
+    boost::concurrent_flat_map<StorePath, Derivation> derivations;
+
     /**
      * The build trace maps the pair of a content-addressing (fixed or
      * floating) derivations an one of its output to a

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

@@ -778,15 +778,20 @@ public:
      */
     Derivation derivationFromPath(const StorePath & drvPath);
 
+    /**
+     * Write a derivation to the Nix store, and return its path.
+     */
+    virtual StorePath writeDerivation(const Derivation & drv, RepairFlag repair = NoRepair);
+
     /**
      * Read a derivation (which must already be valid).
      */
-    Derivation readDerivation(const StorePath & drvPath);
+    virtual Derivation readDerivation(const StorePath & drvPath);
 
     /**
      * Read a derivation from a potentially invalid path.
      */
-    Derivation readInvalidDerivation(const StorePath & drvPath);
+    virtual Derivation readInvalidDerivation(const StorePath & drvPath);
 
     /**
      * @param [out] out Place in here the set of all store paths in the

src/libstore/store-api.cc

@@ -1170,7 +1170,7 @@ std::optional<StorePath> Store::getBuildDerivationPath(const StorePath & path)
         // resolved derivation, so we need to get it first
         auto resolvedDrv = drv.tryResolve(*this);
         if (resolvedDrv)
-            return writeDerivation(*this, *resolvedDrv, NoRepair, true);
+            return ::nix::writeDerivation(*this, *resolvedDrv, NoRepair, true);
     }
 
     return path;