From 95f47c28fb8786f8d8d529192465bb6ec20db46b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Fri, 3 Jun 2022 17:01:16 +0200 Subject: [PATCH 1/9] Make nix copy parallel again FILLME --- src/libstore/store-api.cc | 207 ++++++++++++++++++++------------------ src/libstore/store-api.hh | 7 ++ 2 files changed, 117 insertions(+), 97 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 8861274a2d5c..0084516664f5 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -258,6 +258,86 @@ StorePath Store::addToStore( return addToStoreFromDump(*source, name, method, hashAlgo, repair, references); } +void Store::addMultipleToStore( + std::vector>> & pathsToCopy, + Activity & act, + RepairFlag repair, + CheckSigsFlag checkSigs) +{ + std::atomic nrDone{0}; + std::atomic nrFailed{0}; + std::atomic bytesExpected{0}; + std::atomic nrRunning{0}; + + using PathWithInfo = std::pair>; + + std::map infosMap; + StorePathSet storePathsToAdd; + for (auto & thingToAdd : pathsToCopy) { + infosMap.insert_or_assign(thingToAdd.first.path, &thingToAdd); + storePathsToAdd.insert(thingToAdd.first.path); + } + + auto showProgress = [&]() { + act.progress(nrDone, pathsToCopy.size(), nrRunning, nrFailed); + }; + + ThreadPool pool; + + processGraph(pool, + storePathsToAdd, + + [&](const StorePath & path) { + auto & [info, source] = *infosMap.at(path); + /* auto storePathForDst = info.storePath; */ + /* if (info->ca && info->references.empty()) { */ + /* 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()); */ + /* } */ + /* pathsMap.insert_or_assign(storePath, storePathForDst); */ + + if (isValidPath(info.path)) { + nrDone++; + showProgress(); + return StorePathSet(); + } + + bytesExpected += info.narSize; + act.setExpected(actCopyPath, bytesExpected); + + return info.references; + }, + + [&](const StorePath & path) { + checkInterrupt(); + + auto & [info, source] = *infosMap.at(path); + + if (!isValidPath(info.path)) { + MaintainCount mc(nrRunning); + showProgress(); + try { + addToStore(info, *source, repair, checkSigs); + } catch (Error &e) { + nrFailed++; + if (!settings.keepGoing) + throw e; + printMsg(lvlError, "could not copy %s: %s", printStorePath(path), e.what()); + showProgress(); + return; + } + } + + nrDone++; + showProgress(); + }); +} void Store::addMultipleToStore( Source & source, @@ -998,106 +1078,39 @@ std::map copyPaths( 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); - } - }); + /* 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); */ + /* } */ + /* }); */ + + std::vector>> pathsToCopy; + + for (auto & missingPath : missing) { + auto info = srcStore.queryPathInfo(missingPath); + auto source = sinkToSource([&](Sink & sink) { + srcStore.narFromPath(missingPath, sink); + }); + pathsToCopy.push_back(std::pair{*info, std::move(source)}); + } - dstStore.addMultipleToStore(*source, repair, checkSigs); + dstStore.addMultipleToStore(pathsToCopy, act, repair, checkSigs); #if 0 - std::atomic nrDone{0}; - std::atomic nrFailed{0}; - std::atomic bytesExpected{0}; - std::atomic nrRunning{0}; - - auto showProgress = [&]() { - act.progress(nrDone, missing.size(), nrRunning, nrFailed); - }; - - ThreadPool pool; - - processGraph(pool, - StorePathSet(missing.begin(), missing.end()), - - [&](const StorePath & 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) - assert(storePathForDst == storePath); - if (storePathForDst != storePath) - 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)) { - nrDone++; - showProgress(); - return StorePathSet(); - } - - bytesExpected += info->narSize; - act.setExpected(actCopyPath, bytesExpected); - - return info->references; - }, - - [&](const StorePath & storePath) { - checkInterrupt(); - - 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) - assert(storePathForDst == storePath); - if (storePathForDst != storePath) - 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)) { - MaintainCount mc(nrRunning); - showProgress(); - try { - copyStorePath(srcStore, dstStore, storePath, repair, checkSigs); - } catch (Error &e) { - nrFailed++; - if (!settings.keepGoing) - throw e; - printMsg(lvlError, "could not copy %s: %s", dstStore.printStorePath(storePath), e.what()); - showProgress(); - return; - } - } - - nrDone++; - showProgress(); - }); #endif return pathsMap; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 0c8a4db5667c..d934979cf451 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -1,5 +1,6 @@ #pragma once +#include "nar-info.hh" #include "realisation.hh" #include "path.hh" #include "derived-path.hh" @@ -364,6 +365,12 @@ public: Source & source, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs); + virtual void addMultipleToStore( + std::vector>> & pathsToCopy, + Activity & act, + RepairFlag repair = NoRepair, + CheckSigsFlag checkSigs = CheckSigs + ); /* Copy the contents of a path to the store and register the validity the resulting path. The resulting path is returned. From cb0553ecd0122f693653c1ac82beb26d127ce3cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 8 Jun 2022 14:03:46 +0200 Subject: [PATCH 2/9] Restore the "low-latency" ssh copying --- src/libstore/remote-store.cc | 17 +++++++++++++ src/libstore/remote-store.hh | 8 ++++++ src/libstore/store-api.cc | 49 ++++++++++++++++-------------------- src/libstore/store-api.hh | 6 ++++- 4 files changed, 51 insertions(+), 29 deletions(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index bc36aef5d0fa..ad2e5c18af6c 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -673,6 +673,23 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, } +void RemoteStore::addMultipleToStore( + PathsSource & pathsToCopy, + Activity & act, + RepairFlag repair, + CheckSigsFlag checkSigs) +{ + auto source = sinkToSource([&](Sink & sink) { + sink << pathsToCopy.size(); + for (auto & [pathInfo, pathSource] : pathsToCopy) { + pathInfo.write(sink, *this, 16); + pathSource->drainInto(sink); + } + }); + + addMultipleToStore(*source, repair, checkSigs); +} + void RemoteStore::addMultipleToStore( Source & source, RepairFlag repair, diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 8493be6fc544..5a599997e6af 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -88,6 +88,14 @@ public: RepairFlag repair, CheckSigsFlag checkSigs) override; + void addMultipleToStore( + PathsSource & pathsToCopy, + Activity & act, + RepairFlag repair, + CheckSigsFlag checkSigs) override; + + + StorePath addTextToStore( std::string_view name, std::string_view s, diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 0084516664f5..eeec6c6f7a73 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -259,7 +259,7 @@ StorePath Store::addToStore( } void Store::addMultipleToStore( - std::vector>> & pathsToCopy, + PathsSource & pathsToCopy, Activity & act, RepairFlag repair, CheckSigsFlag checkSigs) @@ -1072,37 +1072,33 @@ std::map copyPaths( for (auto & path : storePaths) if (!valid.count(path)) missing.insert(path); + Activity act(*logger, lvlInfo, actCopyPaths, fmt("copying %d paths", missing.size())); + + // In the general case, `addMultipleToStore` requires a sorted list of + // store paths to add, so sort them right now + auto sortedMissing = srcStore.topoSortPaths(missing); + std::reverse(sortedMissing.begin(), sortedMissing.end()); + std::map pathsMap; for (auto & path : storePaths) pathsMap.insert_or_assign(path, path); - Activity act(*logger, lvlInfo, actCopyPaths, fmt("copying %d paths", missing.size())); + Store::PathsSource pathsToCopy; - /* 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); */ - /* } */ - /* }); */ - - std::vector>> pathsToCopy; - - for (auto & missingPath : missing) { + for (auto & missingPath : sortedMissing) { auto info = srcStore.queryPathInfo(missingPath); auto source = sinkToSource([&](Sink & sink) { + + // We can reasonably assume that the copy will happen whenever we + // read the path, so log something about that at that point + auto srcUri = srcStore.getUri(); + auto dstUri = dstStore.getUri(); + auto storePathS = srcStore.printStorePath(missingPath); + Activity act(*logger, lvlInfo, actCopyPath, + makeCopyPathMessage(srcUri, dstUri, storePathS), + {storePathS, srcUri, dstUri}); + PushActivity pact(act.id); + srcStore.narFromPath(missingPath, sink); }); pathsToCopy.push_back(std::pair{*info, std::move(source)}); @@ -1110,9 +1106,6 @@ std::map copyPaths( dstStore.addMultipleToStore(pathsToCopy, act, repair, checkSigs); - #if 0 - #endif - return pathsMap; } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index d934979cf451..c0a61115be28 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -360,13 +360,17 @@ public: virtual void addToStore(const ValidPathInfo & info, Source & narSource, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs) = 0; + // A list of paths infos along with a source providing the content of the + // associated store path + using PathsSource = std::vector>>; + /* Import multiple paths into the store. */ virtual void addMultipleToStore( Source & source, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs); virtual void addMultipleToStore( - std::vector>> & pathsToCopy, + PathsSource & pathsToCopy, Activity & act, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs From 480c2b6699e6afc6a746ba8a72319f197c3556ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 8 Jun 2022 15:13:11 +0200 Subject: [PATCH 3/9] Rewrite the CA paths when moving them between store Bring back the possibility to copy CA paths with no reference (like the outputs of FO derivations or stuff imported at eval time) between stores that have a different prefix. --- src/libstore/store-api.cc | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index eeec6c6f7a73..61a12e84afd4 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -289,18 +289,6 @@ void Store::addMultipleToStore( [&](const StorePath & path) { auto & [info, source] = *infosMap.at(path); - /* auto storePathForDst = info.storePath; */ - /* if (info->ca && info->references.empty()) { */ - /* 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()); */ - /* } */ - /* pathsMap.insert_or_assign(storePath, storePathForDst); */ if (isValidPath(info.path)) { nrDone++; @@ -1085,10 +1073,32 @@ std::map copyPaths( Store::PathsSource pathsToCopy; + auto computeStorePathForDst = [&](const ValidPathInfo & currentPathInfo) -> StorePath { + auto storePathForSrc = currentPathInfo.path; + auto storePathForDst = storePathForSrc; + if (currentPathInfo.ca && currentPathInfo.references.empty()) { + storePathForDst = dstStore.makeFixedOutputPathFromCA(storePathForSrc.name(), *currentPathInfo.ca); + if (dstStore.storeDir == srcStore.storeDir) + assert(storePathForDst == storePathForSrc); + if (storePathForDst != storePathForSrc) + debug("replaced path '%s' to '%s' for substituter '%s'", + srcStore.printStorePath(storePathForSrc), + dstStore.printStorePath(storePathForDst), + dstStore.getUri()); + } + return storePathForDst; + }; + for (auto & missingPath : sortedMissing) { auto info = srcStore.queryPathInfo(missingPath); - auto source = sinkToSource([&](Sink & sink) { + auto storePathForDst = computeStorePathForDst(*info); + pathsMap.insert_or_assign(missingPath, storePathForDst); + + ValidPathInfo infoForDst = *info; + infoForDst.path = storePathForDst; + + auto source = sinkToSource([&](Sink & sink) { // We can reasonably assume that the copy will happen whenever we // read the path, so log something about that at that point auto srcUri = srcStore.getUri(); @@ -1101,7 +1111,7 @@ std::map copyPaths( srcStore.narFromPath(missingPath, sink); }); - pathsToCopy.push_back(std::pair{*info, std::move(source)}); + pathsToCopy.push_back(std::pair{infoForDst, std::move(source)}); } dstStore.addMultipleToStore(pathsToCopy, act, repair, checkSigs); From 34d90fbe224e7b626366f98ea1bc9e370d9cb534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 8 Jun 2022 15:25:52 +0200 Subject: [PATCH 4/9] Mention the parallel copy in the release notes --- doc/manual/src/release-notes/rl-next.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 52e3b6240ef1..6c2c4689cd16 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -2,3 +2,7 @@ * Nix can now be built with LTO by passing `--enable-lto` to `configure`. LTO is currently only supported when building with GCC. + +* `nix copy` now copies the store paths in parallel as much as possible (again). + This doesn't apply for the `daemon` and `ssh-ng` stores which copy everything + in one batch to avoid latencies issues. From 56f6f3725f4fbeeb7900ae95bd71d559695b3dc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 19 Jul 2022 19:46:00 +0200 Subject: [PATCH 5/9] Don't ultimately trust the signed paths Like the old implem did (and like you'd want it to be anyways) --- src/libstore/store-api.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 61a12e84afd4..06d03bff26d0 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -305,7 +305,9 @@ void Store::addMultipleToStore( [&](const StorePath & path) { checkInterrupt(); - auto & [info, source] = *infosMap.at(path); + auto & [info_, source] = *infosMap.at(path); + auto info = info_; + info.ultimate = false; if (!isValidPath(info.path)) { MaintainCount mc(nrRunning); From b97cda5fa7ca3b7de990cce59ad5a1f797c0be7d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 22 Aug 2022 15:30:38 +0200 Subject: [PATCH 6/9] Indentation --- src/libstore/remote-store.hh | 10 ++++------ src/libstore/store-api.hh | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 5a599997e6af..11d089cd2450 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -89,12 +89,10 @@ public: CheckSigsFlag checkSigs) override; void addMultipleToStore( - PathsSource & pathsToCopy, - Activity & act, - RepairFlag repair, - CheckSigsFlag checkSigs) override; - - + PathsSource & pathsToCopy, + Activity & act, + RepairFlag repair, + CheckSigsFlag checkSigs) override; StorePath addTextToStore( std::string_view name, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index c0a61115be28..c8a667c6db21 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -369,12 +369,12 @@ public: Source & source, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs); + virtual void addMultipleToStore( PathsSource & pathsToCopy, Activity & act, RepairFlag repair = NoRepair, - CheckSigsFlag checkSigs = CheckSigs - ); + CheckSigsFlag checkSigs = CheckSigs); /* Copy the contents of a path to the store and register the validity the resulting path. The resulting path is returned. From 71e42be2ada6327e099d0aa539098d3ec4513fd0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 23 Aug 2022 14:14:47 +0200 Subject: [PATCH 7/9] Fix a hang in nix-copy-ssh.sh This hang for some reason didn't trigger in the Nix build, but did running 'make installcheck' interactively. What happened: * Store::addMultipleToStore() calls a SinkToSource object to copy a path, which in turn calls LegacySSHStore::narFromPath(), which acquires a connection. * The SinkToSource object is not destroyed after the last bytes has been read, so the coroutine's stack is still alive and its destructors are not run. So the connection is not released. * Then when the next path is copied, because max-connections = 1, LegacySSHStore::narFromPath() hangs forever waiting for a connection to be released. The fix is to make sure that the source object is destroyed when we're done with it. --- src/libstore/store-api.cc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 1406bf65719e..9c3a0b3d6316 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -11,6 +11,7 @@ #include "archive.hh" #include "callback.hh" #include "remote-store.hh" +#include "finally.hh" #include @@ -271,7 +272,7 @@ void Store::addMultipleToStore( using PathWithInfo = std::pair>; - std::map infosMap; + std::map infosMap; StorePathSet storePathsToAdd; for (auto & thingToAdd : pathsToCopy) { infosMap.insert_or_assign(thingToAdd.first.path, &thingToAdd); @@ -288,7 +289,8 @@ void Store::addMultipleToStore( storePathsToAdd, [&](const StorePath & path) { - auto & [info, source] = *infosMap.at(path); + + auto & [info, _] = *infosMap.at(path); if (isValidPath(info.path)) { nrDone++; @@ -309,12 +311,21 @@ void Store::addMultipleToStore( auto info = info_; info.ultimate = false; + /* Make sure that the Source object is destroyed when + we're done. In particular, a SinkToSource object must + be destroyed to ensure that the destructors on its + stack frame are run; this includes + LegacySSHStore::narFromPath()'s connection lock. */ + Finally cleanupSource{[&]() { + source.reset(); + }}; + if (!isValidPath(info.path)) { MaintainCount mc(nrRunning); showProgress(); try { addToStore(info, *source, repair, checkSigs); - } catch (Error &e) { + } catch (Error & e) { nrFailed++; if (!settings.keepGoing) throw e; From bfcca946f2e14ac72e6d42fbe5142338d606077f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 24 Aug 2022 14:11:03 +0200 Subject: [PATCH 8/9] Fix macOS build --- src/libstore/store-api.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 9c3a0b3d6316..2cd6c15ec6e1 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -307,8 +307,9 @@ void Store::addMultipleToStore( [&](const StorePath & path) { checkInterrupt(); - auto & [info_, source] = *infosMap.at(path); + auto & [info_, source_] = *infosMap.at(path); auto info = info_; + auto source = std::move(source_); info.ultimate = false; /* Make sure that the Source object is destroyed when From 1649136ea75a8376f9a3ed86eed485f071dcc686 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 24 Aug 2022 14:49:58 +0200 Subject: [PATCH 9/9] Remove redundant Finally --- src/libstore/store-api.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 2cd6c15ec6e1..86b12257a269 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -11,7 +11,6 @@ #include "archive.hh" #include "callback.hh" #include "remote-store.hh" -#include "finally.hh" #include @@ -309,7 +308,6 @@ void Store::addMultipleToStore( auto & [info_, source_] = *infosMap.at(path); auto info = info_; - auto source = std::move(source_); info.ultimate = false; /* Make sure that the Source object is destroyed when @@ -317,9 +315,7 @@ void Store::addMultipleToStore( be destroyed to ensure that the destructors on its stack frame are run; this includes LegacySSHStore::narFromPath()'s connection lock. */ - Finally cleanupSource{[&]() { - source.reset(); - }}; + auto source = std::move(source_); if (!isValidPath(info.path)) { MaintainCount mc(nrRunning);