From 28b73cabccc74304d3474aea8c2d06d4c248f811 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 13 Oct 2025 15:04:56 -0400 Subject: [PATCH 1/3] Make reading and writing derivations store methods This allows for different representations. --- src/libstore/derivations.cc | 27 ++++++++++++++++++--- src/libstore/include/nix/store/store-api.hh | 9 +++++-- src/libstore/store-api.cc | 2 +- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index f44bf3e70..20f1d6ca1 100644 --- a/src/libstore/derivations.cc +++ b/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, diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index 8fa13de34..522a9a45f 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/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 diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 08b75c8fa..c292e2e43 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1170,7 +1170,7 @@ std::optional 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; From 136825b4a2700ebbd20f4ba143e9b1819be0537c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 13 Oct 2025 16:00:27 -0400 Subject: [PATCH 2/3] Make Dummy store store derivations separately This makes for more efficiency. Once we have JSON for the dummy store, it will also make for better JSON, too. --- src/libstore-tests/write-derivation.cc | 4 +- src/libstore/dummy-store.cc | 120 ++++++++++++++---- .../include/nix/store/dummy-store-impl.hh | 9 +- 3 files changed, 107 insertions(+), 26 deletions(-) diff --git a/src/libstore-tests/write-derivation.cc b/src/libstore-tests/write-derivation.cc index 3f7de05d3..c320f92fa 100644 --- a/src/libstore-tests/write-derivation.cc +++ b/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(testing::HasSubstrIgnoreANSIMatcher( - "operation 'addToStoreFromDump' is not supported by store 'dummy://'"))); + ::testing::ThrowsMessage( + testing::HasSubstrIgnoreANSIMatcher("operation 'writeDerivation' is not supported by store 'dummy://'"))); } } // namespace nix diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 1333e0aed..d11fef73f 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -137,12 +137,31 @@ struct DummyStoreImpl : DummyStore void queryPathInfoUncached( const StorePath & path, Callback> callback) noexcept override { - bool visited = contents.cvisit(path, [&](const auto & kv) { - callback(std::make_shared(StorePath{kv.first}, kv.second.info)); - }); + if (path.isDerivation()) { + if (auto accessor_ = getMemoryFSAccessor(path)) { + ref 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(path, UnkeyedValidPathInfo{narHash.hash}); + info->narSize = narHash.numBytesDigested; + info->ca = ContentAddress{ + .method = ContentAddressMethod::Raw::Text, + .hash = hashString( + HashAlgorithm::SHA256, + std::get(accessor->root->raw).contents), + }; + callback(std::move(info)); + return; + } + } else { + if (contents.cvisit(path, [&](const auto & kv) { + callback(std::make_shared(StorePath{kv.first}, kv.second.info)); + })) + return; + } - if (!visited) - callback(nullptr); + callback(nullptr); } /** @@ -169,18 +188,25 @@ struct DummyStoreImpl : DummyStore if (checkSigs) throw Error("checking signatures is not supported for '%s' store", config->getHumanReadableURI()); - auto temp = make_ref(); - MemorySink tempSink{*temp}; + auto accessor = make_ref(); + MemorySink tempSink{*accessor}; parseDump(tempSink, source); auto path = info.path; - auto accessor = make_ref(std::move(*temp)); - contents.insert( - {path, - PathInfoAndContents{ - std::move(info), - accessor, - }}); + if (info.path.isDerivation()) { + warn("back compat supporting `addToStore` for inserting derivations in dummy store"); + writeDerivation( + parseDerivation(*this, accessor->readFile(CanonPath::root), Derivation::nameFromPath(info.path))); + return; + } + + contents.insert({ + path, + PathInfoAndContents{ + std::move(info), + accessor, + }, + }); wholeStoreView->addObject(path.to_string(), accessor); } @@ -193,6 +219,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 +268,47 @@ struct DummyStoreImpl : DummyStore auto path = info.path; auto accessor = make_ref(std::move(*temp)); - contents.insert( - {path, - PathInfoAndContents{ - std::move(info), - accessor, - }}); + contents.insert({ + path, + PathInfoAndContents{ + std::move(info), + 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(output); @@ -273,13 +332,28 @@ struct DummyStoreImpl : DummyStore callback(nullptr); } - std::shared_ptr getFSAccessor(const StorePath & path, bool requireValidPath) override + std::shared_ptr getMemoryFSAccessor(const StorePath & path, bool requireValidPath = true) { - std::shared_ptr res; - contents.cvisit(path, [&](const auto & kv) { res = kv.second.contents.get_ptr(); }); + std::shared_ptr res; + if (path.isDerivation()) + derivations.cvisit(path, [&](const auto & kv) { + /* compute path info on demand */ + auto res2 = make_ref(); + 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 getFSAccessor(const StorePath & path, bool requireValidPath = true) override + { + return getMemoryFSAccessor(path, requireValidPath); + } + ref getFSAccessor(bool requireValidPath) override { return wholeStoreView; diff --git a/src/libstore/include/nix/store/dummy-store-impl.hh b/src/libstore/include/nix/store/dummy-store-impl.hh index 4c9f54e98..137f81c9b 100644 --- a/src/libstore/include/nix/store/dummy-store-impl.hh +++ b/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 @@ -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 contents; + /** + * This map conceptually owns every derivation, allowing us to + * avoid "on-disk drv format" serialization round-trips. + */ + boost::concurrent_flat_map derivations; + /** * The build trace maps the pair of a content-addressing (fixed or * floating) derivations an one of its output to a From 18941a2421f40efad264ad1fc4e07339075491c3 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 21 Oct 2025 11:37:14 -0400 Subject: [PATCH 3/3] Optimize `DummyStore::isValidPathUncached` See the API docs for the rationale of why this is needed. --- src/libstore/dummy-store.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index d11fef73f..c45a13cc3 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -164,6 +164,15 @@ struct DummyStoreImpl : DummyStore 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); + } + /** * The dummy store is incapable of *not* trusting! :) */