From 63ebfc73c56978665bf6c58c3e4ad84485355b1a Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 19 May 2021 10:36:48 +0200 Subject: Make `copyPaths` copy the whole realisations closure Otherwise registering the realisations on the remote side might fail as it now expects a complete closure --- src/libstore/store-api.cc | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 93fcb068f..08573f709 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -780,20 +780,40 @@ std::map copyPaths(ref srcStore, ref dstStor RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute) { StorePathSet storePaths; - std::set realisations; + std::set toplevelRealisations; for (auto & path : paths) { storePaths.insert(path.path()); if (auto realisation = std::get_if(&path.raw)) { settings.requireExperimentalFeature("ca-derivations"); - realisations.insert(*realisation); + toplevelRealisations.insert(*realisation); } } auto pathsMap = copyPaths(srcStore, dstStore, storePaths, repair, checkSigs, substitute); + + ThreadPool pool; + try { - for (auto & realisation : realisations) { - dstStore->registerDrvOutput(realisation, checkSigs); - } - } catch (MissingExperimentalFeature & e) { + // Copy the realisation closure + processGraph( + pool, Realisation::closure(*srcStore, toplevelRealisations), + [&](const Realisation& current) -> std::set { + std::set children; + for (const auto& drvOutput : current.drvOutputDeps) { + auto currentChild = srcStore->queryRealisation(drvOutput); + if (!currentChild) + throw Error( + "Incomplete realisation closure: '%s' is a " + "dependency " + "of '%s' but isn’t registered", + drvOutput.to_string(), current.id.to_string()); + children.insert(*currentChild); + } + return children; + }, + [&](const Realisation& current) -> void { + dstStore->registerDrvOutput(current, checkSigs); + }); + } catch (MissingExperimentalFeature& e) { // Don't fail if the remote doesn't support CA derivations is it might // not be within our control to change that, and we might still want // to at least copy the output paths. -- cgit v1.2.3 From 1f3ff0d193c270f7b97af4aa3e463be01dbe5f2d Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 26 May 2021 16:09:02 +0200 Subject: Aso track the output path of the realisation dependencies --- src/libstore/store-api.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 08573f709..6f29c430a 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -798,7 +798,7 @@ std::map copyPaths(ref srcStore, ref dstStor pool, Realisation::closure(*srcStore, toplevelRealisations), [&](const Realisation& current) -> std::set { std::set children; - for (const auto& drvOutput : current.drvOutputDeps) { + for (const auto& [drvOutput, _] : current.dependentRealisations) { auto currentChild = srcStore->queryRealisation(drvOutput); if (!currentChild) throw Error( -- cgit v1.2.3 From 7c077d2a0f4ccdeb84a4ad14ba5871a3fbe170e4 Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 11 Jun 2021 08:37:59 +0200 Subject: Add a ca-derivations required machine feature Make ca-derivations require a `ca-derivations` machine feature, and ca-aware builders expose it. That way, a network of builders can mix ca-aware and non-ca-aware machines, and the scheduler will send them in the right place. --- src/libstore/store-api.cc | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 93fcb068f..6ca8e5b24 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -337,6 +337,13 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath, return info; } +StringSet StoreConfig::getDefaultSystemFeatures() +{ + auto res = settings.systemFeatures.get(); + if (settings.isExperimentalFeatureEnabled("ca-derivations")) + res.insert("ca-derivations"); + return res; +} Store::Store(const Params & params) : StoreConfig(params) -- cgit v1.2.3 From 3b5429aec110a22f8fe0d92b72faf1921b8ede2d Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 2 May 2021 17:24:14 +0200 Subject: Source complete env in `nix-shell` with `__structuredAttrs = true;` This is needed to push the adoption of structured attrs[1] forward. It's now checked if a `__json` exists in the environment-map of the derivation to be openend in a `nix-shell`. Derivations with structured attributes enabled also make use of a file named `.attrs.json` containing every environment variable represented as JSON which is useful for e.g. `exportReferencesGraph`[2]. To provide an environment similar to the build sandbox, `nix-shell` now adds a `.attrs.json` to `cwd` (which is mostly equal to the one in the build sandbox) and removes it using an exit hook when closing the shell. To avoid leaking internals of the build-process to the `nix-shell`, the entire logic to generate JSON and shell code for structured attrs was moved into the `ParsedDerivation` class. [1] https://nixos.mayflower.consulting/blog/2020/01/20/structured-attrs/ [2] https://nixos.org/manual/nix/unstable/expressions/advanced-attributes.html#advanced-attributes --- src/libstore/store-api.cc | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 93fcb068f..cfdfb5269 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -627,6 +627,42 @@ string Store::makeValidityRegistration(const StorePathSet & paths, } +StorePathSet Store::exportReferences(const StorePathSet & storePaths, const StorePathSet & inputPaths) +{ + StorePathSet paths; + + for (auto & storePath : storePaths) { + if (!inputPaths.count(storePath)) + throw BuildError("cannot export references of path '%s' because it is not in the input closure of the derivation", printStorePath(storePath)); + + computeFSClosure({storePath}, paths); + } + + /* If there are derivations in the graph, then include their + outputs as well. This is useful if you want to do things + like passing all build-time dependencies of some path to a + derivation that builds a NixOS DVD image. */ + auto paths2 = paths; + + for (auto & j : paths2) { + if (j.isDerivation()) { + Derivation drv = derivationFromPath(j); + for (auto & k : drv.outputsAndOptPaths(*this)) { + if (!k.second.second) + /* FIXME: I am confused why we are calling + `computeFSClosure` on the output path, rather than + derivation itself. That doesn't seem right to me, so I + won't try to implemented this for CA derivations. */ + throw UnimplementedError("exportReferences on CA derivations is not yet implemented"); + computeFSClosure(*k.second.second, paths); + } + } + } + + return paths; +} + + void Store::pathInfoToJSON(JSONPlaceholder & jsonOut, const StorePathSet & storePaths, bool includeImpureInfo, bool showClosureSize, Base hashBase, -- cgit v1.2.3 From 8d09a4f9a09473e6a32cc118ad10827ec5650700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 22 Jun 2021 09:31:25 +0200 Subject: Remove a useless string split Co-authored-by: Eelco Dolstra --- src/libstore/store-api.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 6f29c430a..7e057b1ea 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -803,8 +803,7 @@ std::map copyPaths(ref srcStore, ref dstStor if (!currentChild) throw Error( "Incomplete realisation closure: '%s' is a " - "dependency " - "of '%s' but isn’t registered", + "dependency of '%s' but isn’t registered", drvOutput.to_string(), current.id.to_string()); children.insert(*currentChild); } -- cgit v1.2.3 From ceda58d112f35a23c7375e0d9b0ebbe53ca93788 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Jun 2021 15:35:14 +0200 Subject: Formatting --- src/libstore/store-api.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 6736adb24..96eb4c3d7 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -803,14 +803,14 @@ std::map copyPaths(ref srcStore, ref dstStor // Copy the realisation closure processGraph( pool, Realisation::closure(*srcStore, toplevelRealisations), - [&](const Realisation& current) -> std::set { + [&](const Realisation & current) -> std::set { std::set children; - for (const auto& [drvOutput, _] : current.dependentRealisations) { + for (const auto & [drvOutput, _] : current.dependentRealisations) { auto currentChild = srcStore->queryRealisation(drvOutput); if (!currentChild) throw Error( - "Incomplete realisation closure: '%s' is a " - "dependency of '%s' but isn’t registered", + "incomplete realisation closure: '%s' is a " + "dependency of '%s' but isn't registered", drvOutput.to_string(), current.id.to_string()); children.insert(*currentChild); } -- cgit v1.2.3 From e9848beca704d27a13e28b4403251725bd485bb2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 16 Jul 2021 09:37:33 +0200 Subject: nix-build: Copy drv closure between eval store and build store --- src/libstore/store-api.cc | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 40a406468..d040560ca 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -9,6 +9,7 @@ #include "url.hh" #include "archive.hh" #include "callback.hh" +#include "remote-store.hh" #include @@ -881,6 +882,16 @@ std::map copyPaths(ref srcStore, ref dstStor for (auto & path : storePaths) pathsMap.insert_or_assign(path, path); + // FIXME: Temporary hack to copy closures in a single round-trip. + if (dynamic_cast(&*dstStore)) { + if (!missing.empty()) { + auto source = sinkToSource([&](Sink & sink) { + srcStore->exportPaths(missing, sink); + }); + dstStore->importPaths(*source, NoCheckSigs); + } + return pathsMap; + } Activity act(*logger, lvlInfo, actCopyPaths, fmt("copying %d paths", missing.size())); @@ -958,6 +969,22 @@ std::map copyPaths(ref srcStore, ref dstStor return pathsMap; } +void copyClosure( + ref srcStore, + ref dstStore, + const RealisedPath::Set & paths, + RepairFlag repair, + CheckSigsFlag checkSigs, + SubstituteFlag substitute) +{ + if (srcStore == dstStore) return; + + RealisedPath::Set closure; + RealisedPath::closure(*srcStore, paths, closure); + + copyPaths(srcStore, dstStore, closure, repair, checkSigs, substitute); +} + std::optional decodeValidPathInfo(const Store & store, std::istream & str, std::optional hashGiven) { std::string path; -- cgit v1.2.3 From 668abd3e5777d29910068b955fb36bf69e7b38b0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 19 Jul 2021 12:01:06 +0200 Subject: copyPaths: Pass store by reference --- src/libstore/store-api.cc | 102 +++++++++++++++++++++++++++------------------- 1 file changed, 61 insertions(+), 41 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index d040560ca..230108f6e 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -771,30 +771,34 @@ const Store::Stats & Store::getStats() } -void copyStorePath(ref srcStore, ref dstStore, - const StorePath & storePath, RepairFlag repair, CheckSigsFlag checkSigs) +void copyStorePath( + Store & srcStore, + Store & dstStore, + const StorePath & storePath, + RepairFlag repair, + CheckSigsFlag checkSigs) { - auto srcUri = srcStore->getUri(); - auto dstUri = dstStore->getUri(); + auto srcUri = srcStore.getUri(); + auto dstUri = dstStore.getUri(); Activity act(*logger, lvlInfo, actCopyPath, srcUri == "local" || srcUri == "daemon" - ? fmt("copying path '%s' to '%s'", srcStore->printStorePath(storePath), dstUri) + ? fmt("copying path '%s' to '%s'", srcStore.printStorePath(storePath), dstUri) : dstUri == "local" || dstUri == "daemon" - ? fmt("copying path '%s' from '%s'", srcStore->printStorePath(storePath), srcUri) - : fmt("copying path '%s' from '%s' to '%s'", srcStore->printStorePath(storePath), srcUri, dstUri), - {srcStore->printStorePath(storePath), srcUri, dstUri}); + ? fmt("copying path '%s' from '%s'", srcStore.printStorePath(storePath), srcUri) + : fmt("copying path '%s' from '%s' to '%s'", srcStore.printStorePath(storePath), srcUri, dstUri), + {srcStore.printStorePath(storePath), srcUri, dstUri}); PushActivity pact(act.id); - auto info = srcStore->queryPathInfo(storePath); + auto info = srcStore.queryPathInfo(storePath); uint64_t total = 0; // recompute store path on the chance dstStore does it differently if (info->ca && info->references.empty()) { auto info2 = make_ref(*info); - info2->path = dstStore->makeFixedOutputPathFromCA(info->path.name(), *info->ca); - if (dstStore->storeDir == srcStore->storeDir) + info2->path = dstStore.makeFixedOutputPathFromCA(info->path.name(), *info->ca); + if (dstStore.storeDir == srcStore.storeDir) assert(info->path == info2->path); info = info2; } @@ -811,17 +815,22 @@ void copyStorePath(ref srcStore, ref dstStore, act.progress(total, info->narSize); }); TeeSink tee { sink, progressSink }; - srcStore->narFromPath(storePath, tee); + srcStore.narFromPath(storePath, tee); }, [&]() { - throw EndOfFile("NAR for '%s' fetched from '%s' is incomplete", srcStore->printStorePath(storePath), srcStore->getUri()); + throw EndOfFile("NAR for '%s' fetched from '%s' is incomplete", srcStore.printStorePath(storePath), srcStore.getUri()); }); - dstStore->addToStore(*info, *source, repair, checkSigs); + dstStore.addToStore(*info, *source, repair, checkSigs); } -std::map copyPaths(ref srcStore, ref dstStore, const RealisedPath::Set & paths, - RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute) +std::map copyPaths( + Store & srcStore, + Store & dstStore, + const RealisedPath::Set & paths, + RepairFlag repair, + CheckSigsFlag checkSigs, + SubstituteFlag substitute) { StorePathSet storePaths; std::set toplevelRealisations; @@ -839,11 +848,11 @@ std::map copyPaths(ref srcStore, ref dstStor try { // Copy the realisation closure processGraph( - pool, Realisation::closure(*srcStore, toplevelRealisations), + pool, Realisation::closure(srcStore, toplevelRealisations), [&](const Realisation & current) -> std::set { std::set children; for (const auto & [drvOutput, _] : current.dependentRealisations) { - auto currentChild = srcStore->queryRealisation(drvOutput); + auto currentChild = srcStore.queryRealisation(drvOutput); if (!currentChild) throw Error( "incomplete realisation closure: '%s' is a " @@ -854,9 +863,9 @@ std::map copyPaths(ref srcStore, ref dstStor return children; }, [&](const Realisation& current) -> void { - dstStore->registerDrvOutput(current, checkSigs); + dstStore.registerDrvOutput(current, checkSigs); }); - } catch (MissingExperimentalFeature& e) { + } catch (MissingExperimentalFeature & e) { // Don't fail if the remote doesn't support CA derivations is it might // not be within our control to change that, and we might still want // to at least copy the output paths. @@ -869,10 +878,15 @@ std::map copyPaths(ref srcStore, ref dstStor return pathsMap; } -std::map copyPaths(ref srcStore, ref dstStore, const StorePathSet & storePaths, - RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute) +std::map copyPaths( + Store & srcStore, + Store & dstStore, + const StorePathSet & storePaths, + RepairFlag repair, + CheckSigsFlag checkSigs, + SubstituteFlag substitute) { - auto valid = dstStore->queryValidPaths(storePaths, substitute); + auto valid = dstStore.queryValidPaths(storePaths, substitute); StorePathSet missing; for (auto & path : storePaths) @@ -883,12 +897,12 @@ std::map copyPaths(ref srcStore, ref dstStor pathsMap.insert_or_assign(path, path); // FIXME: Temporary hack to copy closures in a single round-trip. - if (dynamic_cast(&*dstStore)) { + if (dynamic_cast(&dstStore)) { if (!missing.empty()) { auto source = sinkToSource([&](Sink & sink) { - srcStore->exportPaths(missing, sink); + srcStore.exportPaths(missing, sink); }); - dstStore->importPaths(*source, NoCheckSigs); + dstStore.importPaths(*source, NoCheckSigs); } return pathsMap; } @@ -910,18 +924,21 @@ std::map copyPaths(ref srcStore, ref dstStor StorePathSet(missing.begin(), missing.end()), [&](const StorePath & storePath) { - auto info = srcStore->queryPathInfo(storePath); + auto info = srcStore.queryPathInfo(storePath); auto storePathForDst = storePath; if (info->ca && info->references.empty()) { - storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), *info->ca); - if (dstStore->storeDir == srcStore->storeDir) + storePathForDst = dstStore.makeFixedOutputPathFromCA(storePath.name(), *info->ca); + if (dstStore.storeDir == srcStore.storeDir) assert(storePathForDst == storePath); if (storePathForDst != storePath) - debug("replaced path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); + debug("replaced path '%s' to '%s' for substituter '%s'", + srcStore.printStorePath(storePath), + dstStore.printStorePath(storePathForDst), + dstStore.getUri()); } pathsMap.insert_or_assign(storePath, storePathForDst); - if (dstStore->isValidPath(storePath)) { + if (dstStore.isValidPath(storePath)) { nrDone++; showProgress(); return StorePathSet(); @@ -936,19 +953,22 @@ std::map copyPaths(ref srcStore, ref dstStor [&](const StorePath & storePath) { checkInterrupt(); - auto info = srcStore->queryPathInfo(storePath); + auto info = srcStore.queryPathInfo(storePath); auto storePathForDst = storePath; if (info->ca && info->references.empty()) { - storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), *info->ca); - if (dstStore->storeDir == srcStore->storeDir) + storePathForDst = dstStore.makeFixedOutputPathFromCA(storePath.name(), *info->ca); + if (dstStore.storeDir == srcStore.storeDir) assert(storePathForDst == storePath); if (storePathForDst != storePath) - debug("replaced path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); + debug("replaced path '%s' to '%s' for substituter '%s'", + srcStore.printStorePath(storePath), + dstStore.printStorePath(storePathForDst), + dstStore.getUri()); } pathsMap.insert_or_assign(storePath, storePathForDst); - if (!dstStore->isValidPath(storePathForDst)) { + if (!dstStore.isValidPath(storePathForDst)) { MaintainCount mc(nrRunning); showProgress(); try { @@ -957,7 +977,7 @@ std::map copyPaths(ref srcStore, ref dstStor nrFailed++; if (!settings.keepGoing) throw e; - logger->log(lvlError, fmt("could not copy %s: %s", dstStore->printStorePath(storePath), e.what())); + logger->log(lvlError, fmt("could not copy %s: %s", dstStore.printStorePath(storePath), e.what())); showProgress(); return; } @@ -970,17 +990,17 @@ std::map copyPaths(ref srcStore, ref dstStor } void copyClosure( - ref srcStore, - ref dstStore, + Store & srcStore, + Store & dstStore, const RealisedPath::Set & paths, RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute) { - if (srcStore == dstStore) return; + if (&srcStore == &dstStore) return; RealisedPath::Set closure; - RealisedPath::closure(*srcStore, paths, closure); + RealisedPath::closure(srcStore, paths, closure); copyPaths(srcStore, dstStore, closure, repair, checkSigs, substitute); } -- cgit v1.2.3 From fe1f34fa60ad79e339c38e58af071a44774663f7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 Jul 2021 13:31:09 +0200 Subject: Low-latency closure copy This adds a new store operation 'addMultipleToStore' that reads a number of NARs and ValidPathInfos from a Source, allowing any number of store paths to be copied in a single call. This is much faster on high-latency links when copying a lot of small files, like .drv closures. For example, on a connection with an 50 ms delay: Before: $ nix copy --to 'unix:///tmp/proxy-socket?root=/tmp/dest-chroot' \ /nix/store/90jjw94xiyg5drj70whm9yll6xjj0ca9-hello-2.10.drv \ --derivation --no-check-sigs real 0m57.868s user 0m0.103s sys 0m0.056s After: real 0m0.690s user 0m0.017s sys 0m0.011s --- src/libstore/store-api.cc | 71 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 17 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 230108f6e..970bafd88 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -250,6 +250,20 @@ StorePath Store::addToStore(const string & name, const Path & _srcPath, } +void Store::addMultipleToStore( + Source & source, + RepairFlag repair, + CheckSigsFlag checkSigs) +{ + auto expected = readNum(source); + for (uint64_t i = 0; i < expected; ++i) { + auto info = ValidPathInfo::read(source, *this, 16); + info.ultimate = false; + addToStore(info, source, repair, checkSigs); + } +} + + /* The aim of this function is to compute in one pass the correct ValidPathInfo for the files that we are trying to add to the store. To accomplish that in one @@ -771,6 +785,19 @@ const Store::Stats & Store::getStats() } +static std::string makeCopyPathMessage( + std::string_view srcUri, + std::string_view dstUri, + std::string_view storePath) +{ + return srcUri == "local" || srcUri == "daemon" + ? fmt("copying path '%s' to '%s'", storePath, dstUri) + : dstUri == "local" || dstUri == "daemon" + ? fmt("copying path '%s' from '%s'", storePath, srcUri) + : fmt("copying path '%s' from '%s' to '%s'", storePath, srcUri, dstUri); +} + + void copyStorePath( Store & srcStore, Store & dstStore, @@ -780,14 +807,10 @@ void copyStorePath( { auto srcUri = srcStore.getUri(); auto dstUri = dstStore.getUri(); - + auto storePathS = srcStore.printStorePath(storePath); Activity act(*logger, lvlInfo, actCopyPath, - srcUri == "local" || srcUri == "daemon" - ? fmt("copying path '%s' to '%s'", srcStore.printStorePath(storePath), dstUri) - : dstUri == "local" || dstUri == "daemon" - ? fmt("copying path '%s' from '%s'", srcStore.printStorePath(storePath), srcUri) - : fmt("copying path '%s' from '%s' to '%s'", srcStore.printStorePath(storePath), srcUri, dstUri), - {srcStore.printStorePath(storePath), srcUri, dstUri}); + makeCopyPathMessage(srcUri, dstUri, storePathS), + {storePathS, srcUri, dstUri}); PushActivity pact(act.id); auto info = srcStore.queryPathInfo(storePath); @@ -896,19 +919,31 @@ std::map copyPaths( for (auto & path : storePaths) pathsMap.insert_or_assign(path, path); - // FIXME: Temporary hack to copy closures in a single round-trip. - if (dynamic_cast(&dstStore)) { - if (!missing.empty()) { - auto source = sinkToSource([&](Sink & sink) { - srcStore.exportPaths(missing, sink); - }); - dstStore.importPaths(*source, NoCheckSigs); + Activity act(*logger, lvlInfo, actCopyPaths, fmt("copying %d paths", missing.size())); + + auto sorted = srcStore.topoSortPaths(missing); + std::reverse(sorted.begin(), sorted.end()); + + auto source = sinkToSource([&](Sink & sink) { + sink << sorted.size(); + for (auto & storePath : sorted) { + auto srcUri = srcStore.getUri(); + auto dstUri = dstStore.getUri(); + auto storePathS = srcStore.printStorePath(storePath); + Activity act(*logger, lvlInfo, actCopyPath, + makeCopyPathMessage(srcUri, dstUri, storePathS), + {storePathS, srcUri, dstUri}); + PushActivity pact(act.id); + + auto info = srcStore.queryPathInfo(storePath); + info->write(sink, srcStore, 16); + srcStore.narFromPath(storePath, sink); } - return pathsMap; - } + }); - Activity act(*logger, lvlInfo, actCopyPaths, fmt("copying %d paths", missing.size())); + dstStore.addMultipleToStore(*source, repair, checkSigs); + #if 0 std::atomic nrDone{0}; std::atomic nrFailed{0}; std::atomic bytesExpected{0}; @@ -986,6 +1021,8 @@ std::map copyPaths( nrDone++; showProgress(); }); + #endif + return pathsMap; } -- cgit v1.2.3 From 242f9bf3dc04170502020fb0338b78ea76b9ebac Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 30 Sep 2021 21:31:21 +0000 Subject: `std::visit` by reference I had started the trend of doing `std::visit` by value (because a type error once mislead me into thinking that was the only form that existed). While the optomizer in principle should be able to deal with extra coppying or extra indirection once the lambdas inlined, sticking with by reference is the conventional default. I hope this might even improve performance. --- src/libstore/store-api.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 970bafd88..b5ff3dccf 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -199,10 +199,10 @@ StorePath Store::makeFixedOutputPathFromCA(std::string_view name, ContentAddress { // New template return std::visit(overloaded { - [&](TextHash th) { + [&](const TextHash & th) { return makeTextPath(name, th.hash, references); }, - [&](FixedOutputHash fsh) { + [&](const FixedOutputHash & fsh) { return makeFixedOutputPath(fsh.method, fsh.hash, name, references, hasSelfReference); } }, ca); @@ -1114,10 +1114,10 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const if (! ca) return false; auto caPath = std::visit(overloaded { - [&](TextHash th) { + [&](const TextHash & th) { return store.makeTextPath(path.name(), th.hash, references); }, - [&](FixedOutputHash fsh) { + [&](const FixedOutputHash & fsh) { auto refs = references; bool hasSelfReference = false; if (refs.count(path)) { -- cgit v1.2.3 From 0be8cc1466f317e33977590510bac4b18471f0ce Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 14 Oct 2021 13:28:22 +0200 Subject: pathInfoCache: Use the entire base name as the cache key This fixes a bug in the garbage collector where if a path /nix/store/abcd-foo is valid, but we do a isValidPath("/nix/store/abcd-foo.lock") first, then a negative entry for /nix/store/abcd is added to pathInfoCache, so /nix/store/abcd-foo is subsequently considered invalid and deleted. --- src/libstore/store-api.cc | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b5ff3dccf..10aa1035e 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -414,11 +414,9 @@ StorePathSet Store::queryDerivationOutputs(const StorePath & path) bool Store::isValidPath(const StorePath & storePath) { - std::string hashPart(storePath.hashPart()); - { auto state_(state.lock()); - auto res = state_->pathInfoCache.get(hashPart); + auto res = state_->pathInfoCache.get(std::string(storePath.to_string())); if (res && res->isKnownNow()) { stats.narInfoReadAverted++; return res->didExist(); @@ -426,11 +424,11 @@ bool Store::isValidPath(const StorePath & storePath) } if (diskCache) { - auto res = diskCache->lookupNarInfo(getUri(), hashPart); + auto res = diskCache->lookupNarInfo(getUri(), std::string(storePath.hashPart())); if (res.first != NarInfoDiskCache::oUnknown) { stats.narInfoReadAverted++; auto state_(state.lock()); - state_->pathInfoCache.upsert(hashPart, + state_->pathInfoCache.upsert(std::string(storePath.to_string()), res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue { .value = res.second }); return res.first == NarInfoDiskCache::oValid; } @@ -440,7 +438,7 @@ bool Store::isValidPath(const StorePath & storePath) if (diskCache && !valid) // FIXME: handle valid = true case. - diskCache->upsertNarInfo(getUri(), hashPart, 0); + diskCache->upsertNarInfo(getUri(), std::string(storePath.hashPart()), 0); return valid; } @@ -487,13 +485,11 @@ static bool goodStorePath(const StorePath & expected, const StorePath & actual) void Store::queryPathInfo(const StorePath & storePath, Callback> callback) noexcept { - std::string hashPart; + auto hashPart = std::string(storePath.hashPart()); try { - hashPart = storePath.hashPart(); - { - auto res = state.lock()->pathInfoCache.get(hashPart); + auto res = state.lock()->pathInfoCache.get(std::string(storePath.to_string())); if (res && res->isKnownNow()) { stats.narInfoReadAverted++; if (!res->didExist()) @@ -508,7 +504,7 @@ void Store::queryPathInfo(const StorePath & storePath, stats.narInfoReadAverted++; { auto state_(state.lock()); - state_->pathInfoCache.upsert(hashPart, + state_->pathInfoCache.upsert(std::string(storePath.to_string()), res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{ .value = res.second }); if (res.first == NarInfoDiskCache::oInvalid || !goodStorePath(storePath, res.second->path)) @@ -523,7 +519,7 @@ void Store::queryPathInfo(const StorePath & storePath, auto callbackPtr = std::make_shared(std::move(callback)); queryPathInfoUncached(storePath, - {[this, storePathS{printStorePath(storePath)}, hashPart, callbackPtr](std::future> fut) { + {[this, storePath, hashPart, callbackPtr](std::future> fut) { try { auto info = fut.get(); @@ -533,14 +529,12 @@ void Store::queryPathInfo(const StorePath & storePath, { auto state_(state.lock()); - state_->pathInfoCache.upsert(hashPart, PathInfoCacheValue { .value = info }); + state_->pathInfoCache.upsert(std::string(storePath.to_string()), PathInfoCacheValue { .value = info }); } - auto storePath = parseStorePath(storePathS); - if (!info || !goodStorePath(storePath, info->path)) { stats.narInfoMissing++; - throw InvalidPath("path '%s' is not valid", storePathS); + throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); } (*callbackPtr)(ref(info)); -- cgit v1.2.3 From b9234142f5e3c69f3372f15ab2e6df19c7bf6b64 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Sat, 23 Oct 2021 21:30:51 +0300 Subject: addToStore, addToStoreFromDump: add references argument Allow to pass a set of references to be added as info to the added paths. --- src/libstore/store-api.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b5ff3dccf..84d6db297 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -237,7 +237,7 @@ StorePath Store::computeStorePathForText(const string & name, const string & s, StorePath Store::addToStore(const string & name, const Path & _srcPath, - FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) + FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair, StorePathSet references) { Path srcPath(absPath(_srcPath)); auto source = sinkToSource([&](Sink & sink) { @@ -246,7 +246,7 @@ StorePath Store::addToStore(const string & name, const Path & _srcPath, else readFile(srcPath, sink); }); - return addToStoreFromDump(*source, name, method, hashAlgo, repair); + return addToStoreFromDump(*source, name, method, hashAlgo, repair, references); } -- cgit v1.2.3 From af99941279b80c962ec9cae3e5fa32976a3f5744 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 25 Oct 2021 15:53:01 +0200 Subject: Make experimental-features a proper type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than having them plain strings scattered through the whole codebase, create an enum containing all the known experimental features. This means that - Nix can now `warn` when an unkwown experimental feature is passed (making it much nicer to spot typos and spot deprecated features) - It’s now easy to remove a feature altogether (once the feature isn’t experimental anymore or is dropped) by just removing the field for the enum and letting the compiler point us to all the now invalid usages of it. --- src/libstore/store-api.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b5ff3dccf..3338cdc1b 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -355,7 +355,7 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath, StringSet StoreConfig::getDefaultSystemFeatures() { auto res = settings.systemFeatures.get(); - if (settings.isExperimentalFeatureEnabled("ca-derivations")) + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) res.insert("ca-derivations"); return res; } @@ -860,7 +860,7 @@ std::map copyPaths( for (auto & path : paths) { storePaths.insert(path.path()); if (auto realisation = std::get_if(&path.raw)) { - settings.requireExperimentalFeature("ca-derivations"); + settings.requireExperimentalFeature(Xp::CaDerivations); toplevelRealisations.insert(*realisation); } } @@ -892,7 +892,7 @@ std::map copyPaths( // Don't fail if the remote doesn't support CA derivations is it might // not be within our control to change that, and we might still want // to at least copy the output paths. - if (e.missingFeature == "ca-derivations") + if (e.missingFeature == Xp::CaDerivations) ignoreException(); else throw; -- cgit v1.2.3 From 96670ed2163d3d1a296c9b053833362ec8c06985 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 27 Oct 2021 11:36:51 +0200 Subject: Expose an async interface for `queryRealisation` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doesn’t change much so far because everything is still using it synchronously, but should allow the binary cache to fetch stuff in parallel --- src/libstore/store-api.cc | 68 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b0d3688ce..b8d702200 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -542,6 +542,74 @@ void Store::queryPathInfo(const StorePath & storePath, }}); } +void Store::queryRealisation(const DrvOutput & id, + Callback> callback) noexcept +{ + + try { + if (diskCache) { + auto [cacheOutcome, maybeCachedRealisation] + = diskCache->lookupRealisation(getUri(), id); + switch (cacheOutcome) { + case NarInfoDiskCache::oValid: + debug("Returning a cached realisation for %s", id.to_string()); + callback(maybeCachedRealisation); + return; + case NarInfoDiskCache::oInvalid: + debug( + "Returning a cached missing realisation for %s", + id.to_string()); + callback(nullptr); + return; + case NarInfoDiskCache::oUnknown: + break; + } + } + } catch (...) { + return callback.rethrow(); + } + + auto callbackPtr + = std::make_shared(std::move(callback)); + + queryRealisationUncached( + id, + { [this, id, callbackPtr]( + std::future> fut) { + try { + auto info = fut.get(); + + if (diskCache) { + if (info) + diskCache->upsertRealisation(getUri(), *info); + else + diskCache->upsertAbsentRealisation(getUri(), id); + } + + (*callbackPtr)(std::shared_ptr(info)); + + } catch (...) { + callbackPtr->rethrow(); + } + } }); +} + +std::shared_ptr Store::queryRealisation(const DrvOutput & id) +{ + using RealPtr = std::shared_ptr; + std::promise promise; + + queryRealisation(id, + {[&](std::future result) { + try { + promise.set_value(result.get()); + } catch (...) { + promise.set_exception(std::current_exception()); + } + }}); + + return promise.get_future().get(); +} void Store::substitutePaths(const StorePathSet & paths) { -- cgit v1.2.3 From 0b005bc9d67b3f621bc78e5ecb2cd834172d5563 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Tue, 9 Nov 2021 12:24:49 +0300 Subject: addToStore, addToStoreFromDump: refactor: pass refs by const reference Co-Authored-By: Eelco Dolstra --- src/libstore/store-api.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 84d6db297..329cf679f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -237,7 +237,7 @@ StorePath Store::computeStorePathForText(const string & name, const string & s, StorePath Store::addToStore(const string & name, const Path & _srcPath, - FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair, StorePathSet references) + FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair, const StorePathSet & references) { Path srcPath(absPath(_srcPath)); auto source = sinkToSource([&](Sink & sink) { -- cgit v1.2.3