From 668abd3e5777d29910068b955fb36bf69e7b38b0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 19 Jul 2021 12:01:06 +0200 Subject: [PATCH] copyPaths: Pass store by reference --- src/build-remote/build-remote.cc | 4 +- src/libstore/build/substitution-goal.cc | 2 +- src/libstore/store-api.cc | 102 ++++++++++++++--------- src/libstore/store-api.hh | 17 ++-- src/nix-build/nix-build.cc | 4 +- src/nix-copy-closure/nix-copy-closure.cc | 2 +- src/nix/copy.cc | 2 +- src/nix/flake.cc | 2 +- 8 files changed, 81 insertions(+), 54 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 0904f2ce9..0559aeaf4 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -270,7 +270,7 @@ connected: { Activity act(*logger, lvlTalkative, actUnknown, fmt("copying dependencies to '%s'", storeUri)); - copyPaths(store, ref(sshStore), store->parseStorePathSet(inputs), NoRepair, NoCheckSigs, substitute); + copyPaths(*store, *sshStore, store->parseStorePathSet(inputs), NoRepair, NoCheckSigs, substitute); } uploadLock = -1; @@ -321,7 +321,7 @@ connected: if (auto localStore = store.dynamic_pointer_cast()) for (auto & path : missingPaths) localStore->locksHeld.insert(store->printStorePath(path)); /* FIXME: ugly */ - copyPaths(ref(sshStore), store, missingPaths, NoRepair, NoCheckSigs, NoSubstitute); + copyPaths(*sshStore, *store, missingPaths, NoRepair, NoCheckSigs, NoSubstitute); } // XXX: Should be done as part of `copyPaths` for (auto & realisation : missingRealisations) { diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index e56cfadbe..29a8cfb87 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -204,7 +204,7 @@ void PathSubstitutionGoal::tryToRun() Activity act(*logger, actSubstitute, Logger::Fields{worker.store.printStorePath(storePath), sub->getUri()}); PushActivity pact(act.id); - copyStorePath(ref(sub), ref(worker.store.shared_from_this()), + copyStorePath(*sub, worker.store, subPath ? *subPath : storePath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs); promise.set_value(); 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); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index c39d9f894..80edac244 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -751,8 +751,12 @@ protected: /* Copy a path from one store to another. */ -void copyStorePath(ref srcStore, ref dstStore, - const StorePath & storePath, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs); +void copyStorePath( + Store & srcStore, + Store & dstStore, + const StorePath & storePath, + RepairFlag repair = NoRepair, + CheckSigsFlag checkSigs = CheckSigs); /* Copy store paths from one store to another. The paths may be copied @@ -761,20 +765,23 @@ void copyStorePath(ref srcStore, ref dstStore, of store paths is not automatically closed; use copyClosure() for that. Returns a map of what each path was copied to the dstStore as. */ -std::map copyPaths(ref srcStore, ref dstStore, +std::map copyPaths( + Store & srcStore, Store & dstStore, const RealisedPath::Set &, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, SubstituteFlag substitute = NoSubstitute); -std::map copyPaths(ref srcStore, ref dstStore, +std::map copyPaths( + Store & srcStore, Store & dstStore, const StorePathSet & paths, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, SubstituteFlag substitute = NoSubstitute); /* Copy the closure of `paths` from `srcStore` to `dstStore`. */ -void copyClosure(ref srcStore, ref dstStore, +void copyClosure( + Store & srcStore, Store & dstStore, const RealisedPath::Set & paths, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 3e4fd8dbd..3b791e619 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -397,7 +397,7 @@ static void main_nix_build(int argc, char * * argv) pathsToCopy.insert(src); } - copyClosure(evalStore, store, pathsToCopy); + copyClosure(*evalStore, *store, pathsToCopy); buildPaths(pathsToBuild); @@ -564,7 +564,7 @@ static void main_nix_build(int argc, char * * argv) drvMap[drvPath] = {drvMap.size(), {outputName}}; } - copyClosure(evalStore, store, drvsToCopy); + copyClosure(*evalStore, *store, drvsToCopy); buildPaths(pathsToBuild); diff --git a/src/nix-copy-closure/nix-copy-closure.cc b/src/nix-copy-closure/nix-copy-closure.cc index 7c38e55e4..841d50fd3 100755 --- a/src/nix-copy-closure/nix-copy-closure.cc +++ b/src/nix-copy-closure/nix-copy-closure.cc @@ -54,7 +54,7 @@ static int main_nix_copy_closure(int argc, char ** argv) for (auto & path : storePaths) storePaths2.insert(from->followLinksToStorePath(path)); - copyClosure(from, to, storePaths2, NoRepair, NoCheckSigs, useSubstitutes); + copyClosure(*from, *to, storePaths2, NoRepair, NoCheckSigs, useSubstitutes); return 0; } diff --git a/src/nix/copy.cc b/src/nix/copy.cc index 674cce4b4..0489dfe06 100644 --- a/src/nix/copy.cc +++ b/src/nix/copy.cc @@ -90,7 +90,7 @@ struct CmdCopy : BuiltPathsCommand } copyPaths( - srcStore, dstStore, stuffToCopy, NoRepair, checkSigs, substitute); + *srcStore, *dstStore, stuffToCopy, NoRepair, checkSigs, substitute); } }; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 23feed24b..abb0fd3b4 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -841,7 +841,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun if (!dryRun && !dstUri.empty()) { ref dstStore = dstUri.empty() ? openStore() : openStore(dstUri); - copyPaths(store, dstStore, sources); + copyPaths(*store, *dstStore, sources); } } };