From bef40c29491ba9b31c85b9b89a5342979344f583 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 29 Jun 2021 21:09:48 +0200 Subject: [PATCH 01/15] Add --eval-store option --- src/libcmd/command.cc | 32 ++++++++++++++++++++++++++++++++ src/libcmd/command.hh | 15 ++++++++++++--- src/libcmd/installables.cc | 13 ------------- src/libexpr/eval.cc | 6 +++++- src/libexpr/eval.hh | 9 ++++++++- 5 files changed, 57 insertions(+), 18 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index 6777b23be..93f20a8c3 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -54,6 +54,38 @@ void StoreCommand::run() run(getStore()); } +EvalCommand::EvalCommand() +{ + // FIXME: move to MixEvalArgs? + addFlag({ + .longName = "eval-store", + .description = "The Nix store to use for evaluations.", + .labels = {"store-url"}, + //.category = ..., + .handler = {&evalStoreUrl}, + }); +} + +EvalCommand::~EvalCommand() +{ + if (evalState) + evalState->printStats(); +} + +ref EvalCommand::getEvalStore() +{ + if (!evalStore) + evalStore = evalStoreUrl ? openStore(*evalStoreUrl) : getStore(); + return ref(evalStore); +} + +ref EvalCommand::getEvalState() +{ + if (!evalState) + evalState = std::make_shared(searchPath, getEvalStore(), getStore()); + return ref(evalState); +} + BuiltPathsCommand::BuiltPathsCommand(bool recursive) : recursive(recursive) { diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index 35b3a384b..3aba8f25f 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -45,11 +45,20 @@ private: struct EvalCommand : virtual StoreCommand, MixEvalArgs { - ref getEvalState(); - - std::shared_ptr evalState; + EvalCommand(); ~EvalCommand(); + + ref getEvalStore(); + + ref getEvalState(); + +private: + std::optional evalStoreUrl; + + std::shared_ptr evalStore; + + std::shared_ptr evalState; }; struct MixFlakeOptions : virtual Args, EvalCommand diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 95327f958..66ec63da2 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -289,19 +289,6 @@ void completeFlakeRefWithFragment( completeFlakeRef(evalState->store, prefix); } -ref EvalCommand::getEvalState() -{ - if (!evalState) - evalState = std::make_shared(searchPath, getStore()); - return ref(evalState); -} - -EvalCommand::~EvalCommand() -{ - if (evalState) - evalState->printStats(); -} - void completeFlakeRef(ref store, std::string_view prefix) { if (prefix == "") diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index c3206a577..b3e952aaa 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -378,7 +378,10 @@ static Strings parseNixPath(const string & s) } -EvalState::EvalState(const Strings & _searchPath, ref store) +EvalState::EvalState( + const Strings & _searchPath, + ref store, + std::shared_ptr buildStore) : sWith(symbols.create("")) , sOutPath(symbols.create("outPath")) , sDrvPath(symbols.create("drvPath")) @@ -411,6 +414,7 @@ EvalState::EvalState(const Strings & _searchPath, ref store) , sEpsilon(symbols.create("")) , repair(NoRepair) , store(store) + , buildStore(buildStore ? buildStore : store) , regexCache(makeRegexCache()) , baseEnv(allocEnv(128)) , staticBaseEnv(false, 0) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index e3eaed6d3..a7aebb8ef 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -94,8 +94,12 @@ public: Value vEmptySet; + /* Store used to materialise .drv files. */ const ref store; + /* Store used to build stuff. */ + const ref buildStore; + private: SrcToStore srcToStore; @@ -128,7 +132,10 @@ private: public: - EvalState(const Strings & _searchPath, ref store); + EvalState( + const Strings & _searchPath, + ref store, + std::shared_ptr buildStore = nullptr); ~EvalState(); void addToSearchPath(const string & s); From 3d9de41a5b86520ff0c994dc7ac1023d2a441083 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 29 Jun 2021 21:17:48 +0200 Subject: [PATCH 02/15] Hacky fast closure copying mechanism --- src/libcmd/installables.cc | 29 +++++++++++++++++++++++++++++ src/libstore/daemon.cc | 11 +++++++++++ src/libstore/remote-store.cc | 10 ++++++++++ src/libstore/remote-store.hh | 2 ++ src/libstore/store-api.hh | 3 ++- src/libstore/worker-protocol.hh | 1 + 6 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 66ec63da2..ca4d509c2 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -12,6 +12,7 @@ #include "eval-cache.hh" #include "url.hh" #include "registry.hh" +#include "remote-store.hh" #include #include @@ -378,6 +379,7 @@ DerivedPaths InstallableValue::toDerivedPaths() DerivedPaths res; std::map> drvsToOutputs; + RealisedPath::Set drvsToCopy; // Group by derivation, helps with .all in particular for (auto & drv : toDerivations()) { @@ -385,11 +387,38 @@ DerivedPaths InstallableValue::toDerivedPaths() if (outputName == "") throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(drv.drvPath)); drvsToOutputs[drv.drvPath].insert(outputName); + drvsToCopy.insert(drv.drvPath); } for (auto & i : drvsToOutputs) res.push_back(DerivedPath::Built { i.first, i.second }); + // FIXME: Temporary hack + if (state->store != state->buildStore) { + RealisedPath::Set closure; + RealisedPath::closure(*state->store, drvsToCopy, closure); + + if (dynamic_cast(&*state->buildStore)) { + StorePathSet closure2; + for (auto & p : closure) + closure2.insert(p.path()); + + auto valid = state->buildStore->queryValidPaths(closure2); + StorePathSet missing; + for (auto & p : closure2) + if (!valid.count(p)) missing.insert(p); + + if (!missing.empty()) { + auto source = sinkToSource([&](Sink & sink) { + state->store->exportPaths(missing, sink); + }); + state->buildStore->importPaths(*source, NoCheckSigs); + } + + } else + copyPaths(state->store, state->buildStore, closure); + } + return res; } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index e06fb9ce2..468152f7f 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -505,6 +505,17 @@ static void performOp(TunnelLogger * logger, ref store, break; } + case wopImportPaths2: { + logger->startWork(); + auto paths = store->importPaths(from, + trusted ? NoCheckSigs : CheckSigs); + logger->stopWork(); + Strings paths2; + for (auto & i : paths) paths2.push_back(store->printStorePath(i)); + to << paths2; + break; + } + case wopBuildPaths: { auto drvs = readDerivedPaths(*store, clientVersion, from); BuildMode mode = bmNormal; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index aec243637..cd2d718ec 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -875,6 +875,16 @@ void RemoteStore::queryMissing(const std::vector & targets, } +StorePaths RemoteStore::importPaths(Source & source, CheckSigsFlag checkSigs) +{ + auto conn(getConnection()); + conn->to << wopImportPaths2; + source.drainInto(conn->to); + conn.processStderr(); + return worker_proto::read(*this, conn->from, Phantom {}); +} + + void RemoteStore::connect() { auto conn(getConnection()); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 6cf76a46d..293393c32 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -112,6 +112,8 @@ public: StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, uint64_t & downloadSize, uint64_t & narSize) override; + StorePaths importPaths(Source & source, CheckSigsFlag checkSigs) override; + void connect() override; unsigned int getProtocol() override; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 4faa65228..356ae615c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -676,7 +676,7 @@ public: the Nix store. Optionally, the contents of the NARs are preloaded into the specified FS accessor to speed up subsequent access. */ - StorePaths importPaths(Source & source, CheckSigsFlag checkSigs = CheckSigs); + virtual StorePaths importPaths(Source & source, CheckSigsFlag checkSigs = CheckSigs); struct Stats { @@ -766,6 +766,7 @@ std::map copyPaths(ref srcStore, ref dstStor RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, SubstituteFlag substitute = NoSubstitute); + std::map copyPaths(ref srcStore, ref dstStore, const StorePathSet & paths, RepairFlag repair = NoRepair, diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index e89183d40..b1888bf01 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -55,6 +55,7 @@ typedef enum { wopQueryDerivationOutputMap = 41, wopRegisterDrvOutput = 42, wopQueryRealisation = 43, + wopImportPaths2 = 44, // hack } WorkerOp; From 2ff3035cf4d5167d180878d69cb47b31890a24c4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 15 Jul 2021 14:28:33 +0200 Subject: [PATCH 03/15] Support --eval-store in nix-instantiate and nix-build --- src/libcmd/command.cc | 8 -------- src/libcmd/command.hh | 2 -- src/libexpr/common-eval-args.cc | 8 ++++++++ src/libexpr/common-eval-args.hh | 3 ++- src/nix-build/nix-build.cc | 3 ++- src/nix-instantiate/nix-instantiate.cc | 1 + 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index 93f20a8c3..4add710ed 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -56,14 +56,6 @@ void StoreCommand::run() EvalCommand::EvalCommand() { - // FIXME: move to MixEvalArgs? - addFlag({ - .longName = "eval-store", - .description = "The Nix store to use for evaluations.", - .labels = {"store-url"}, - //.category = ..., - .handler = {&evalStoreUrl}, - }); } EvalCommand::~EvalCommand() diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index 3aba8f25f..659b13559 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -54,8 +54,6 @@ struct EvalCommand : virtual StoreCommand, MixEvalArgs ref getEvalState(); private: - std::optional evalStoreUrl; - std::shared_ptr evalStore; std::shared_ptr evalState; diff --git a/src/libexpr/common-eval-args.cc b/src/libexpr/common-eval-args.cc index aa14bf79b..fb0932c00 100644 --- a/src/libexpr/common-eval-args.cc +++ b/src/libexpr/common-eval-args.cc @@ -61,6 +61,14 @@ MixEvalArgs::MixEvalArgs() fetchers::overrideRegistry(from.input, to.input, extraAttrs); }} }); + + addFlag({ + .longName = "eval-store", + .description = "The Nix store to use for evaluations.", + .category = category, + .labels = {"store-url"}, + .handler = {&evalStoreUrl}, + }); } Bindings * MixEvalArgs::getAutoArgs(EvalState & state) diff --git a/src/libexpr/common-eval-args.hh b/src/libexpr/common-eval-args.hh index be7fda783..0e113fff1 100644 --- a/src/libexpr/common-eval-args.hh +++ b/src/libexpr/common-eval-args.hh @@ -16,8 +16,9 @@ struct MixEvalArgs : virtual Args Strings searchPath; -private: + std::optional evalStoreUrl; +private: std::map autoArgs; }; diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 29be46cf5..4305c0ac1 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -250,8 +250,9 @@ static void main_nix_build(int argc, char * * argv) throw UsageError("'-p' and '-E' are mutually exclusive"); auto store = openStore(); + auto evalStore = myArgs.evalStoreUrl ? openStore(*myArgs.evalStoreUrl) : store; - auto state = std::make_unique(myArgs.searchPath, store); + auto state = std::make_unique(myArgs.searchPath, evalStore, store); state->repair = repair; auto autoArgs = myArgs.getAutoArgs(*state); diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index 95903d882..a269c5b5c 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -153,6 +153,7 @@ static int main_nix_instantiate(int argc, char * * argv) settings.readOnlyMode = true; auto store = openStore(); + auto evalStore = myArgs.evalStoreUrl ? openStore(*myArgs.evalStoreUrl) : store; auto state = std::make_unique(myArgs.searchPath, store); state->repair = repair; From e9848beca704d27a13e28b4403251725bd485bb2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 16 Jul 2021 09:37:33 +0200 Subject: [PATCH 04/15] nix-build: Copy drv closure between eval store and build store --- src/libcmd/installables.cc | 27 +----------------------- src/libstore/store-api.cc | 27 ++++++++++++++++++++++++ src/libstore/store-api.hh | 6 ++++++ src/nix-build/nix-build.cc | 7 ++++-- src/nix-copy-closure/nix-copy-closure.cc | 5 +---- 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index ca4d509c2..124df34ea 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -12,7 +12,6 @@ #include "eval-cache.hh" #include "url.hh" #include "registry.hh" -#include "remote-store.hh" #include #include @@ -393,31 +392,7 @@ DerivedPaths InstallableValue::toDerivedPaths() for (auto & i : drvsToOutputs) res.push_back(DerivedPath::Built { i.first, i.second }); - // FIXME: Temporary hack - if (state->store != state->buildStore) { - RealisedPath::Set closure; - RealisedPath::closure(*state->store, drvsToCopy, closure); - - if (dynamic_cast(&*state->buildStore)) { - StorePathSet closure2; - for (auto & p : closure) - closure2.insert(p.path()); - - auto valid = state->buildStore->queryValidPaths(closure2); - StorePathSet missing; - for (auto & p : closure2) - if (!valid.count(p)) missing.insert(p); - - if (!missing.empty()) { - auto source = sinkToSource([&](Sink & sink) { - state->store->exportPaths(missing, sink); - }); - state->buildStore->importPaths(*source, NoCheckSigs); - } - - } else - copyPaths(state->store, state->buildStore, closure); - } + copyClosure(state->store, state->buildStore, drvsToCopy); return res; } 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; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 356ae615c..c39d9f894 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -773,6 +773,12 @@ std::map copyPaths(ref srcStore, ref dstStor CheckSigsFlag checkSigs = CheckSigs, SubstituteFlag substitute = NoSubstitute); +/* Copy the closure of `paths` from `srcStore` to `dstStore`. */ +void copyClosure(ref srcStore, ref dstStore, + const RealisedPath::Set & paths, + RepairFlag repair = NoRepair, + CheckSigsFlag checkSigs = CheckSigs, + SubstituteFlag substitute = NoSubstitute); /* Remove the temporary roots file for this process. Any temporary root becomes garbage after this point unless it has been registered diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 4305c0ac1..4ee471520 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -532,6 +532,7 @@ static void main_nix_build(int argc, char * * argv) std::vector pathsToBuild; std::vector> pathsToBuildOrdered; + RealisedPath::Set drvsToCopy; std::map> drvMap; @@ -544,15 +545,17 @@ static void main_nix_build(int argc, char * * argv) pathsToBuild.push_back({drvPath, {outputName}}); pathsToBuildOrdered.push_back({drvPath, {outputName}}); + drvsToCopy.insert(drvPath); auto i = drvMap.find(drvPath); if (i != drvMap.end()) i->second.second.insert(outputName); - else { + else drvMap[drvPath] = {drvMap.size(), {outputName}}; - } } + copyClosure(state->store, state->buildStore, drvsToCopy); + buildPaths(pathsToBuild); if (dryRun) return; diff --git a/src/nix-copy-closure/nix-copy-closure.cc b/src/nix-copy-closure/nix-copy-closure.cc index 02ccbe541..7c38e55e4 100755 --- a/src/nix-copy-closure/nix-copy-closure.cc +++ b/src/nix-copy-closure/nix-copy-closure.cc @@ -54,10 +54,7 @@ static int main_nix_copy_closure(int argc, char ** argv) for (auto & path : storePaths) storePaths2.insert(from->followLinksToStorePath(path)); - RealisedPath::Set closure; - RealisedPath::closure(*from, storePaths2, closure); - - copyPaths(from, to, closure, NoRepair, NoCheckSigs, useSubstitutes); + copyClosure(from, to, storePaths2, NoRepair, NoCheckSigs, useSubstitutes); return 0; } From 732165774663627192161d6074c8faab864dbf3a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 16 Jul 2021 12:47:59 +0200 Subject: [PATCH 05/15] nix-shell: Handle --eval-store correctly --- src/nix-build/nix-build.cc | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 4ee471520..3e4fd8dbd 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -302,8 +302,8 @@ static void main_nix_build(int argc, char * * argv) absolute = canonPath(absPath(i), true); } catch (Error & e) {}; auto [path, outputNames] = parsePathWithOutputs(absolute); - if (store->isStorePath(path) && hasSuffix(path, ".drv")) - drvs.push_back(DrvInfo(*state, store, absolute)); + if (evalStore->isStorePath(path) && hasSuffix(path, ".drv")) + drvs.push_back(DrvInfo(*state, evalStore, absolute)); else /* If we're in a #! script, interpret filenames relative to the script. */ @@ -349,9 +349,10 @@ static void main_nix_build(int argc, char * * argv) throw UsageError("nix-shell requires a single derivation"); auto & drvInfo = drvs.front(); - auto drv = store->derivationFromPath(store->parseStorePath(drvInfo.queryDrvPath())); + auto drv = evalStore->derivationFromPath(evalStore->parseStorePath(drvInfo.queryDrvPath())); std::vector pathsToBuild; + RealisedPath::Set pathsToCopy; /* Figure out what bash shell to use. If $NIX_BUILD_SHELL is not set, then build bashInteractive from @@ -370,7 +371,9 @@ static void main_nix_build(int argc, char * * argv) if (!drv) throw Error("the 'bashInteractive' attribute in did not evaluate to a derivation"); - pathsToBuild.push_back({store->parseStorePath(drv->queryDrvPath())}); + auto bashDrv = store->parseStorePath(drv->queryDrvPath()); + pathsToBuild.push_back({bashDrv}); + pathsToCopy.insert(bashDrv); shell = drv->queryOutPath() + "/bin/bash"; @@ -385,9 +388,16 @@ static void main_nix_build(int argc, char * * argv) for (const auto & input : drv.inputDrvs) if (std::all_of(envExclude.cbegin(), envExclude.cend(), [&](const string & exclude) { return !std::regex_search(store->printStorePath(input.first), std::regex(exclude)); })) + { pathsToBuild.push_back({input.first, input.second}); - for (const auto & src : drv.inputSrcs) + pathsToCopy.insert(input.first); + } + for (const auto & src : drv.inputSrcs) { pathsToBuild.push_back({src}); + pathsToCopy.insert(src); + } + + copyClosure(evalStore, store, pathsToCopy); buildPaths(pathsToBuild); @@ -554,7 +564,7 @@ static void main_nix_build(int argc, char * * argv) drvMap[drvPath] = {drvMap.size(), {outputName}}; } - copyClosure(state->store, state->buildStore, drvsToCopy); + copyClosure(evalStore, store, drvsToCopy); buildPaths(pathsToBuild); From 95e915a993f77cf17124f7866b542939c03d375a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 16 Jul 2021 12:48:47 +0200 Subject: [PATCH 06/15] DummyStore: Remove redundant method --- src/libstore/dummy-store.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 8f26af685..36c6e725c 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -43,11 +43,6 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store RepairFlag repair, CheckSigsFlag checkSigs) override { unsupported("addToStore"); } - StorePath addToStore(const string & name, const Path & srcPath, - FileIngestionMethod method, HashType hashAlgo, - PathFilter & filter, RepairFlag repair) override - { unsupported("addToStore"); } - StorePath addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) override { unsupported("addTextToStore"); } From 8d9f7048cd3b09fb58f6bad0328f644b0ddaaa16 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 16 Jul 2021 16:04:47 +0200 Subject: [PATCH 07/15] Use eval-store in more places In particular, this now works: $ nix path-info --eval-store auto --store https://cache.nixos.org nixpkgs#hello Previously this would fail as it would try to upload the hello .drv to cache.nixos.org. Now the .drv is instantiated in the local store, and then we check for the existence of the outputs in cache.nixos.org. --- src/libcmd/command.cc | 2 +- src/libcmd/command.hh | 17 ++++++++++++----- src/libcmd/installables.cc | 38 +++++++++++++++++++------------------ src/libcmd/installables.hh | 2 +- src/libstore/derivations.cc | 2 +- src/nix/app.cc | 5 +++-- src/nix/build.cc | 5 ++++- src/nix/bundle.cc | 2 +- src/nix/develop.cc | 6 +++--- src/nix/diff-closures.cc | 4 ++-- src/nix/profile.cc | 2 +- src/nix/run.cc | 4 ++-- src/nix/why-depends.cc | 4 ++-- 13 files changed, 53 insertions(+), 40 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index 4add710ed..2daf43aa7 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -115,7 +115,7 @@ void BuiltPathsCommand::run(ref store) for (auto & p : store->queryAllValidPaths()) paths.push_back(BuiltPath::Opaque{p}); } else { - paths = toBuiltPaths(store, realiseMode, operateOn, installables); + paths = toBuiltPaths(getEvalStore(), store, realiseMode, operateOn, installables); if (recursive) { // XXX: This only computes the store path closure, ignoring // intermediate realisations diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index 659b13559..f3625ed0d 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -223,15 +223,21 @@ static RegisterCommand registerCommand2(std::vector && name) return RegisterCommand(std::move(name), [](){ return make_ref(); }); } -BuiltPaths build(ref store, Realise mode, +BuiltPaths build(ref evalStore, ref store, Realise mode, std::vector> installables, BuildMode bMode = bmNormal); -std::set toStorePaths(ref store, - Realise mode, OperateOn operateOn, +std::set toStorePaths( + ref evalStore, + ref store, + Realise mode, + OperateOn operateOn, std::vector> installables); -StorePath toStorePath(ref store, - Realise mode, OperateOn operateOn, +StorePath toStorePath( + ref evalStore, + ref store, + Realise mode, + OperateOn operateOn, std::shared_ptr installable); std::set toDerivations(ref store, @@ -239,6 +245,7 @@ std::set toDerivations(ref store, bool useDeriver = false); BuiltPaths toBuiltPaths( + ref evalStore, ref store, Realise mode, OperateOn operateOn, diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 124df34ea..00f8105ae 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -392,8 +392,6 @@ DerivedPaths InstallableValue::toDerivedPaths() for (auto & i : drvsToOutputs) res.push_back(DerivedPath::Built { i.first, i.second }); - copyClosure(state->store, state->buildStore, drvsToCopy); - return res; } @@ -703,10 +701,10 @@ std::shared_ptr SourceExprCommand::parseInstallable( return installables.front(); } -BuiltPaths getBuiltPaths(ref store, DerivedPaths hopefullyBuiltPaths) +BuiltPaths getBuiltPaths(ref evalStore, ref store, const DerivedPaths & hopefullyBuiltPaths) { BuiltPaths res; - for (auto& b : hopefullyBuiltPaths) + for (auto & b : hopefullyBuiltPaths) std::visit( overloaded{ [&](DerivedPath::Opaque bo) { @@ -714,14 +712,13 @@ BuiltPaths getBuiltPaths(ref store, DerivedPaths hopefullyBuiltPaths) }, [&](DerivedPath::Built bfd) { OutputPathMap outputs; - auto drv = store->readDerivation(bfd.drvPath); - auto outputHashes = staticOutputHashes(*store, drv); + auto drv = evalStore->readDerivation(bfd.drvPath); + auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive auto drvOutputs = drv.outputsAndOptPaths(*store); - for (auto& output : bfd.outputs) { + for (auto & output : bfd.outputs) { if (!outputHashes.count(output)) throw Error( - "the derivation '%s' doesn't have an output " - "named '%s'", + "the derivation '%s' doesn't have an output named '%s'", store->printStorePath(bfd.drvPath), output); if (settings.isExperimentalFeatureEnabled( "ca-derivations")) { @@ -753,7 +750,7 @@ BuiltPaths getBuiltPaths(ref store, DerivedPaths hopefullyBuiltPaths) return res; } -BuiltPaths build(ref store, Realise mode, +BuiltPaths build(ref evalStore, ref store, Realise mode, std::vector> installables, BuildMode bMode) { if (mode == Realise::Nothing) @@ -771,18 +768,19 @@ BuiltPaths build(ref store, Realise mode, else if (mode == Realise::Outputs) store->buildPaths(pathsToBuild, bMode); - return getBuiltPaths(store, pathsToBuild); + return getBuiltPaths(evalStore, store, pathsToBuild); } BuiltPaths toBuiltPaths( + ref evalStore, ref store, Realise mode, OperateOn operateOn, std::vector> installables) { - if (operateOn == OperateOn::Output) { - return build(store, mode, installables); - } else { + if (operateOn == OperateOn::Output) + return build(evalStore, store, mode, installables); + else { if (mode == Realise::Nothing) settings.readOnlyMode = true; @@ -793,23 +791,27 @@ BuiltPaths toBuiltPaths( } } -StorePathSet toStorePaths(ref store, +StorePathSet toStorePaths( + ref evalStore, + ref store, Realise mode, OperateOn operateOn, std::vector> installables) { StorePathSet outPaths; - for (auto & path : toBuiltPaths(store, mode, operateOn, installables)) { + for (auto & path : toBuiltPaths(evalStore, store, mode, operateOn, installables)) { auto thisOutPaths = path.outPaths(); outPaths.insert(thisOutPaths.begin(), thisOutPaths.end()); } return outPaths; } -StorePath toStorePath(ref store, +StorePath toStorePath( + ref evalStore, + ref store, Realise mode, OperateOn operateOn, std::shared_ptr installable) { - auto paths = toStorePaths(store, mode, operateOn, {installable}); + auto paths = toStorePaths(evalStore, store, mode, operateOn, {installable}); if (paths.size() != 1) throw Error("argument '%s' should evaluate to one store path", installable->what()); diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index 298fd48f8..79931ad3e 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -26,7 +26,7 @@ struct App struct UnresolvedApp { App unresolved; - App resolve(ref); + App resolve(ref evalStore, ref store); }; struct Installable diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index f6defd98f..899475860 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -568,7 +568,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m } -std::map staticOutputHashes(Store& store, const Derivation& drv) +std::map staticOutputHashes(Store & store, const Derivation & drv) { std::map res; std::visit(overloaded { diff --git a/src/nix/app.cc b/src/nix/app.cc index 01a0064db..9719a65dd 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -100,7 +100,8 @@ UnresolvedApp Installable::toApp(EvalState & state) throw Error("attribute '%s' has unsupported type '%s'", attrPath, type); } -App UnresolvedApp::resolve(ref store) +// FIXME: move to libcmd +App UnresolvedApp::resolve(ref evalStore, ref store) { auto res = unresolved; @@ -110,7 +111,7 @@ App UnresolvedApp::resolve(ref store) installableContext.push_back( std::make_shared(store, ctxElt.toDerivedPath())); - auto builtContext = build(store, Realise::Outputs, installableContext); + auto builtContext = build(evalStore, store, Realise::Outputs, installableContext); res.program = resolveString(*store, unresolved.program, builtContext); if (!store->isInStore(res.program)) throw Error("app program '%s' is not in the Nix store", res.program); diff --git a/src/nix/build.cc b/src/nix/build.cc index 15923ebc3..13eb66ac6 100644 --- a/src/nix/build.cc +++ b/src/nix/build.cc @@ -52,7 +52,10 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile void run(ref store) override { - auto buildables = build(store, dryRun ? Realise::Nothing : Realise::Outputs, installables, buildMode); + auto buildables = build( + getEvalStore(), store, + dryRun ? Realise::Nothing : Realise::Outputs, + installables, buildMode); if (json) logger->cout("%s", derivedPathsWithHintsToJSON(buildables, store).dump()); diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 88bc3d1d1..cedb5704c 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -69,7 +69,7 @@ struct CmdBundle : InstallableCommand { auto evalState = getEvalState(); - auto app = installable->toApp(*evalState).resolve(store); + auto app = installable->toApp(*evalState).resolve(getEvalStore(), store); auto [bundlerFlakeRef, bundlerName] = parseFlakeRefWithFragment(bundler, absPath(".")); const flake::LockFlags lockFlags{ .writeLockFile = false }; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 9ac2791f8..cb173b91b 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -307,7 +307,7 @@ struct Common : InstallableCommand, MixProfile auto dir = absPath(dir_); auto installable = parseInstallable(store, installable_); auto builtPaths = toStorePaths( - store, Realise::Nothing, OperateOn::Output, {installable}); + getEvalStore(), store, Realise::Nothing, OperateOn::Output, {installable}); for (auto & path: builtPaths) { auto from = store->printStorePath(path); if (script.find(from) == std::string::npos) @@ -495,8 +495,8 @@ struct CmdDevelop : Common, MixEnvironment Strings{"legacyPackages." + settings.thisSystem.get() + "."}, nixpkgsLockFlags); - shell = state->store->printStorePath( - toStorePath(state->store, Realise::Outputs, OperateOn::Output, bashInstallable)) + "/bin/bash"; + shell = store->printStorePath( + toStorePath(getEvalStore(), store, Realise::Outputs, OperateOn::Output, bashInstallable)) + "/bin/bash"; } catch (Error &) { ignoreException(); } diff --git a/src/nix/diff-closures.cc b/src/nix/diff-closures.cc index 0c7d531c1..734c41e0e 100644 --- a/src/nix/diff-closures.cc +++ b/src/nix/diff-closures.cc @@ -131,9 +131,9 @@ struct CmdDiffClosures : SourceExprCommand void run(ref store) override { auto before = parseInstallable(store, _before); - auto beforePath = toStorePath(store, Realise::Outputs, operateOn, before); + auto beforePath = toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, before); auto after = parseInstallable(store, _after); - auto afterPath = toStorePath(store, Realise::Outputs, operateOn, after); + auto afterPath = toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, after); printClosureDiff(store, beforePath, afterPath, ""); } }; diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 511771f89..8cef6d0b6 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -253,7 +253,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile manifest.elements.emplace_back(std::move(element)); } else { - auto buildables = build(store, Realise::Outputs, {installable}, bmNormal); + auto buildables = build(getEvalStore(), store, Realise::Outputs, {installable}, bmNormal); for (auto & buildable : buildables) { ProfileElement element; diff --git a/src/nix/run.cc b/src/nix/run.cc index c0ba05a3e..0c8afec2d 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -93,7 +93,7 @@ struct CmdShell : InstallablesCommand, RunCommon, MixEnvironment void run(ref store) override { - auto outPaths = toStorePaths(store, Realise::Outputs, OperateOn::Output, installables); + auto outPaths = toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, installables); auto accessor = store->getFSAccessor(); @@ -178,7 +178,7 @@ struct CmdRun : InstallableCommand, RunCommon { auto state = getEvalState(); - auto app = installable->toApp(*state).resolve(store); + auto app = installable->toApp(*state).resolve(getEvalStore(), store); Strings allArgs{app.program}; for (auto & i : args) allArgs.push_back(i); diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index 7a4ca5172..2f6b361bb 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -62,9 +62,9 @@ struct CmdWhyDepends : SourceExprCommand void run(ref store) override { auto package = parseInstallable(store, _package); - auto packagePath = toStorePath(store, Realise::Outputs, operateOn, package); + auto packagePath = toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, package); auto dependency = parseInstallable(store, _dependency); - auto dependencyPath = toStorePath(store, Realise::Derivation, operateOn, dependency); + auto dependencyPath = toStorePath(getEvalStore(), store, Realise::Derivation, operateOn, dependency); auto dependencyPathHash = dependencyPath.hashPart(); StorePathSet closure; From 668abd3e5777d29910068b955fb36bf69e7b38b0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 19 Jul 2021 12:01:06 +0200 Subject: [PATCH 08/15] 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); } } }; From eb6db4fd384f34ca426116cd353c02af7d0f9214 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 19 Jul 2021 15:43:08 +0200 Subject: [PATCH 09/15] buildPaths(): Add an evalStore argument With this, we don't have to copy the entire .drv closure to the destination store ahead of time (or at all). Instead, buildPaths() reads .drv files from the eval store and copies inputSrcs to the destination store if it needs to build a derivation. Issue #5025. --- src/libcmd/installables.cc | 2 +- src/libstore/build/derivation-goal.cc | 26 ++++++++++++++------- src/libstore/build/entry-points.cc | 10 ++++---- src/libstore/build/local-derivation-goal.cc | 4 +++- src/libstore/build/worker.cc | 3 ++- src/libstore/build/worker.hh | 3 ++- src/libstore/legacy-ssh-store.cc | 5 +++- src/libstore/realisation.hh | 2 +- src/libstore/remote-store.cc | 5 +++- src/libstore/remote-store.hh | 2 +- src/libstore/store-api.hh | 3 ++- src/nix-build/nix-build.cc | 10 +++----- src/nix/develop.cc | 18 +++++++------- 13 files changed, 55 insertions(+), 38 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 00f8105ae..e3ce564b0 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -766,7 +766,7 @@ BuiltPaths build(ref evalStore, ref store, Realise mode, if (mode == Realise::Nothing) printMissing(store, pathsToBuild, lvlError); else if (mode == Realise::Outputs) - store->buildPaths(pathsToBuild, bMode); + store->buildPaths(pathsToBuild, bMode, evalStore); return getBuiltPaths(evalStore, store, pathsToBuild); } diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 7df597400..dad8717a2 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -165,7 +165,7 @@ void DerivationGoal::getDerivation() /* The first thing to do is to make sure that the derivation exists. If it doesn't, it may be created through a substitute. */ - if (buildMode == bmNormal && worker.store.isValidPath(drvPath)) { + if (buildMode == bmNormal && worker.evalStore.isValidPath(drvPath)) { loadDerivation(); return; } @@ -188,12 +188,12 @@ void DerivationGoal::loadDerivation() /* `drvPath' should already be a root, but let's be on the safe side: if the user forgot to make it a root, we wouldn't want things being garbage collected while we're busy. */ - worker.store.addTempRoot(drvPath); + worker.evalStore.addTempRoot(drvPath); - assert(worker.store.isValidPath(drvPath)); + assert(worker.evalStore.isValidPath(drvPath)); /* Get the derivation. */ - drv = std::make_unique(worker.store.derivationFromPath(drvPath)); + drv = std::make_unique(worker.evalStore.derivationFromPath(drvPath)); haveDerivation(); } @@ -212,8 +212,8 @@ void DerivationGoal::haveDerivation() if (i.second.second) worker.store.addTempRoot(*i.second.second); - auto outputHashes = staticOutputHashes(worker.store, *drv); - for (auto &[outputName, outputHash] : outputHashes) + auto outputHashes = staticOutputHashes(worker.evalStore, *drv); + for (auto & [outputName, outputHash] : outputHashes) initialOutputs.insert({ outputName, InitialOutput{ @@ -337,6 +337,16 @@ void DerivationGoal::gaveUpOnSubstitution() for (auto & i : dynamic_cast(drv.get())->inputDrvs) addWaitee(worker.makeDerivationGoal(i.first, i.second, buildMode == bmRepair ? bmRepair : bmNormal)); + /* Copy the input sources from the eval store to the build + store. */ + if (&worker.evalStore != &worker.store) { + RealisedPath::Set inputSrcs, inputClosure; + for (auto & i : drv->inputSrcs) + inputSrcs.insert(i); + RealisedPath::closure(worker.evalStore, inputSrcs, inputClosure); + copyClosure(worker.evalStore, worker.store, inputClosure); + } + for (auto & i : drv->inputSrcs) { if (worker.store.isValidPath(i)) continue; if (!settings.useSubstitutes) @@ -478,8 +488,8 @@ void DerivationGoal::inputsRealised() /* Add the relevant output closures of the input derivation `i' as input paths. Only add the closures of output paths that are specified as inputs. */ - assert(worker.store.isValidPath(drvPath)); - auto outputs = worker.store.queryPartialDerivationOutputMap(depDrvPath); + assert(worker.evalStore.isValidPath(drvPath)); + auto outputs = worker.evalStore.queryPartialDerivationOutputMap(depDrvPath); for (auto & j : wantedDepOutputs) { if (outputs.count(j) > 0) { auto optRealizedInput = outputs.at(j); diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 732d4785d..96deb81d1 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -6,9 +6,9 @@ namespace nix { -void Store::buildPaths(const std::vector & reqs, BuildMode buildMode) +void Store::buildPaths(const std::vector & reqs, BuildMode buildMode, std::shared_ptr evalStore) { - Worker worker(*this); + Worker worker(*this, evalStore ? *evalStore : *this); Goals goals; for (auto & br : reqs) { @@ -51,7 +51,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode) { - Worker worker(*this); + Worker worker(*this, *this); auto goal = worker.makeBasicDerivationGoal(drvPath, drv, {}, buildMode); BuildResult result; @@ -93,7 +93,7 @@ void Store::ensurePath(const StorePath & path) /* If the path is already valid, we're done. */ if (isValidPath(path)) return; - Worker worker(*this); + Worker worker(*this, *this); GoalPtr goal = worker.makePathSubstitutionGoal(path); Goals goals = {goal}; @@ -111,7 +111,7 @@ void Store::ensurePath(const StorePath & path) void LocalStore::repairPath(const StorePath & path) { - Worker worker(*this); + Worker worker(*this, *this); GoalPtr goal = worker.makePathSubstitutionGoal(path, Repair); Goals goals = {goal}; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 11c99d9f0..990ff60b7 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1254,8 +1254,10 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo return next->queryRealisation(id); } - void buildPaths(const std::vector & paths, BuildMode buildMode) override + void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override { + assert(!evalStore); + if (buildMode != bmNormal) throw Error("unsupported build mode"); StorePathSet newPaths; diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 0f2ade348..a7a6b92a6 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -9,11 +9,12 @@ namespace nix { -Worker::Worker(Store & store) +Worker::Worker(Store & store, Store & evalStore) : act(*logger, actRealise) , actDerivations(*logger, actBuilds) , actSubstitutions(*logger, actCopyPaths) , store(store) + , evalStore(evalStore) { /* Debugging: prevent recursive workers. */ nrLocalBuilds = 0; diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 918de35f6..6a3b99c02 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -110,6 +110,7 @@ public: bool checkMismatch; Store & store; + Store & evalStore; std::unique_ptr hook; @@ -131,7 +132,7 @@ public: it answers with "decline-permanently", we don't try again. */ bool tryBuildHook = true; - Worker(Store & store); + Worker(Store & store, Store & evalStore); ~Worker(); /* Make a goal (with caching). */ diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index edaf75136..45eed5707 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -267,8 +267,11 @@ public: return status; } - void buildPaths(const std::vector & drvPaths, BuildMode buildMode) override + void buildPaths(const std::vector & drvPaths, BuildMode buildMode, std::shared_ptr evalStore) override { + if (evalStore && evalStore.get() != this) + throw Error("building on an SSH store is incompatible with '--eval-store'"); + auto conn(connections->get()); conn->to << cmdBuildPaths; diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 05d2bc44f..9070a6ee2 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -45,7 +45,7 @@ struct Realisation { size_t checkSignatures(const PublicKeys & publicKeys) const; static std::set closure(Store &, const std::set &); - static void closure(Store &, const std::set &, std::set& res); + static void closure(Store &, const std::set &, std::set & res); bool isCompatibleWith(const Realisation & other) const; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index cd2d718ec..77b34d000 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -705,8 +705,11 @@ static void writeDerivedPaths(RemoteStore & store, ConnectionHandle & conn, cons } } -void RemoteStore::buildPaths(const std::vector & drvPaths, BuildMode buildMode) +void RemoteStore::buildPaths(const std::vector & drvPaths, BuildMode buildMode, std::shared_ptr evalStore) { + if (evalStore && evalStore.get() != this) + throw Error("building on a remote store is incompatible with '--eval-store'"); + auto conn(getConnection()); conn->to << wopBuildPaths; assert(GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 293393c32..fbec40b4b 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -85,7 +85,7 @@ public: std::optional queryRealisation(const DrvOutput &) override; - void buildPaths(const std::vector & paths, BuildMode buildMode) override; + void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override; BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode) override; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 80edac244..3ad74f35c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -497,7 +497,8 @@ public: not derivations, substitute them. */ virtual void buildPaths( const std::vector & paths, - BuildMode buildMode = bmNormal); + BuildMode buildMode = bmNormal, + std::shared_ptr evalStore = nullptr); /* Build a single non-materialized derivation (i.e. not from an on-disk .drv file). diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 3b791e619..848b108dd 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -341,7 +341,7 @@ static void main_nix_build(int argc, char * * argv) printMissing(ref(store), willBuild, willSubstitute, unknown, downloadSize, narSize); if (!dryRun) - store->buildPaths(paths, buildMode); + store->buildPaths(paths, buildMode, evalStore); }; if (runEnv) { @@ -397,8 +397,6 @@ static void main_nix_build(int argc, char * * argv) pathsToCopy.insert(src); } - copyClosure(*evalStore, *store, pathsToCopy); - buildPaths(pathsToBuild); if (dryRun) return; @@ -449,7 +447,7 @@ static void main_nix_build(int argc, char * * argv) if (env.count("__json")) { StorePathSet inputs; for (auto & [depDrvPath, wantedDepOutputs] : drv.inputDrvs) { - auto outputs = store->queryPartialDerivationOutputMap(depDrvPath); + auto outputs = evalStore->queryPartialDerivationOutputMap(depDrvPath); for (auto & i : wantedDepOutputs) { auto o = outputs.at(i); store->computeFSClosure(*o, inputs); @@ -564,8 +562,6 @@ static void main_nix_build(int argc, char * * argv) drvMap[drvPath] = {drvMap.size(), {outputName}}; } - copyClosure(*evalStore, *store, drvsToCopy); - buildPaths(pathsToBuild); if (dryRun) return; @@ -578,7 +574,7 @@ static void main_nix_build(int argc, char * * argv) if (counter) drvPrefix += fmt("-%d", counter + 1); - auto builtOutputs = store->queryPartialDerivationOutputMap(drvPath); + auto builtOutputs = evalStore->queryPartialDerivationOutputMap(drvPath); auto maybeOutputPath = builtOutputs.at(outputName); assert(maybeOutputPath); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index cb173b91b..9a93cdb03 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -171,15 +171,15 @@ const static std::string getEnvSh = modified derivation with the same dependencies and nearly the same initial environment variables, that just writes the resulting environment to a file and exits. */ -StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) +static StorePath getDerivationEnvironment(ref store, ref evalStore, const StorePath & drvPath) { - auto drv = store->derivationFromPath(drvPath); + auto drv = evalStore->derivationFromPath(drvPath); auto builder = baseNameOf(drv.builder); if (builder != "bash") throw Error("'nix develop' only works on derivations that use 'bash' as their builder"); - auto getEnvShPath = store->addTextToStore("get-env.sh", getEnvSh, {}); + auto getEnvShPath = evalStore->addTextToStore("get-env.sh", getEnvSh, {}); drv.args = {store->printStorePath(getEnvShPath)}; @@ -205,7 +205,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) output.second = { .output = DerivationOutputInputAddressed { .path = StorePath::dummy } }; drv.env[output.first] = ""; } - Hash h = std::get<0>(hashDerivationModulo(*store, drv, true)); + Hash h = std::get<0>(hashDerivationModulo(*evalStore, drv, true)); for (auto & output : drv.outputs) { auto outPath = store->makeOutputPath(output.first, h, drv.name); @@ -214,12 +214,12 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) } } - auto shellDrvPath = writeDerivation(*store, drv); + auto shellDrvPath = writeDerivation(*evalStore, drv); /* Build the derivation. */ - store->buildPaths({DerivedPath::Built{shellDrvPath}}); + store->buildPaths({DerivedPath::Built{shellDrvPath}}, bmNormal, evalStore); - for (auto & [_0, optPath] : store->queryPartialDerivationOutputMap(shellDrvPath)) { + for (auto & [_0, optPath] : evalStore->queryPartialDerivationOutputMap(shellDrvPath)) { assert(optPath); auto & outPath = *optPath; assert(store->isValidPath(outPath)); @@ -347,7 +347,7 @@ struct Common : InstallableCommand, MixProfile auto & drvPath = *drvs.begin(); - return getDerivationEnvironment(store, drvPath); + return getDerivationEnvironment(store, getEvalStore(), drvPath); } } @@ -361,7 +361,7 @@ struct Common : InstallableCommand, MixProfile debug("reading environment file '%s'", strPath); - return {BuildEnvironment::fromJSON(readFile(strPath)), strPath}; + return {BuildEnvironment::fromJSON(readFile(store->toRealPath(shellOutPath))), strPath}; } }; From a7b7fcfb16bc05cb54bc90f11c84343782e41762 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 22 Jul 2021 22:43:08 +0200 Subject: [PATCH 10/15] Remove redundant RealisedPath::closure() call --- src/libstore/build/derivation-goal.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index dad8717a2..876b8def0 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -340,11 +340,10 @@ void DerivationGoal::gaveUpOnSubstitution() /* Copy the input sources from the eval store to the build store. */ if (&worker.evalStore != &worker.store) { - RealisedPath::Set inputSrcs, inputClosure; + RealisedPath::Set inputSrcs; for (auto & i : drv->inputSrcs) inputSrcs.insert(i); - RealisedPath::closure(worker.evalStore, inputSrcs, inputClosure); - copyClosure(worker.evalStore, worker.store, inputClosure); + copyClosure(worker.evalStore, worker.store, inputSrcs); } for (auto & i : drv->inputSrcs) { From 9957315ce0ec43e122829619174592fd1755177a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 22 Jul 2021 22:50:48 +0200 Subject: [PATCH 11/15] RemoteStore::buildPaths(): Handle evalStore --- src/libstore/remote-store.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 77b34d000..1471520f3 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -707,8 +707,15 @@ static void writeDerivedPaths(RemoteStore & store, ConnectionHandle & conn, cons void RemoteStore::buildPaths(const std::vector & drvPaths, BuildMode buildMode, std::shared_ptr evalStore) { - if (evalStore && evalStore.get() != this) - throw Error("building on a remote store is incompatible with '--eval-store'"); + if (evalStore && evalStore.get() != this) { + /* The remote doesn't have a way to access evalStore, so copy + the .drvs. */ + RealisedPath::Set drvPaths2; + for (auto & i : drvPaths) + if (auto p = std::get_if(&i)) + drvPaths2.insert(p->drvPath); + copyClosure(*evalStore, *this, drvPaths2); + } auto conn(getConnection()); conn->to << wopBuildPaths; From fe1f34fa60ad79e339c38e58af071a44774663f7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 Jul 2021 13:31:09 +0200 Subject: [PATCH 12/15] 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/daemon.cc | 49 ++++++++-------------- src/libstore/path-info.cc | 46 +++++++++++++++++++++ src/libstore/path-info.hh | 5 +++ src/libstore/remote-store.cc | 60 ++++++++++++--------------- src/libstore/remote-store.hh | 9 ++-- src/libstore/store-api.cc | 73 +++++++++++++++++++++++++-------- src/libstore/store-api.hh | 6 +++ src/libstore/worker-protocol.hh | 4 +- 8 files changed, 162 insertions(+), 90 deletions(-) create mode 100644 src/libstore/path-info.cc diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 468152f7f..d68ff64d7 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -243,23 +243,6 @@ struct ClientSettings } }; -static void writeValidPathInfo( - ref store, - unsigned int clientVersion, - Sink & to, - std::shared_ptr info) -{ - to << (info->deriver ? store->printStorePath(*info->deriver) : "") - << info->narHash.to_string(Base16, false); - worker_proto::write(*store, to, info->references); - to << info->registrationTime << info->narSize; - if (GET_PROTOCOL_MINOR(clientVersion) >= 16) { - to << info->ultimate - << info->sigs - << renderContentAddress(info->ca); - } -} - static std::vector readDerivedPaths(Store & store, unsigned int clientVersion, Source & from) { std::vector reqs; @@ -422,9 +405,7 @@ static void performOp(TunnelLogger * logger, ref store, }(); logger->stopWork(); - to << store->printStorePath(pathInfo->path); - writeValidPathInfo(store, clientVersion, to, pathInfo); - + pathInfo->write(to, *store, GET_PROTOCOL_MINOR(clientVersion)); } else { HashType hashAlgo; std::string baseName; @@ -471,6 +452,21 @@ static void performOp(TunnelLogger * logger, ref store, break; } + case wopAddMultipleToStore: { + bool repair, dontCheckSigs; + from >> repair >> dontCheckSigs; + if (!trusted && dontCheckSigs) + dontCheckSigs = false; + + logger->startWork(); + FramedSource source(from); + store->addMultipleToStore(source, + RepairFlag{repair}, + dontCheckSigs ? NoCheckSigs : CheckSigs); + logger->stopWork(); + break; + } + case wopAddTextToStore: { string suffix = readString(from); string s = readString(from); @@ -505,17 +501,6 @@ static void performOp(TunnelLogger * logger, ref store, break; } - case wopImportPaths2: { - logger->startWork(); - auto paths = store->importPaths(from, - trusted ? NoCheckSigs : CheckSigs); - logger->stopWork(); - Strings paths2; - for (auto & i : paths) paths2.push_back(store->printStorePath(i)); - to << paths2; - break; - } - case wopBuildPaths: { auto drvs = readDerivedPaths(*store, clientVersion, from); BuildMode mode = bmNormal; @@ -781,7 +766,7 @@ static void performOp(TunnelLogger * logger, ref store, if (info) { if (GET_PROTOCOL_MINOR(clientVersion) >= 17) to << 1; - writeValidPathInfo(store, clientVersion, to, info); + info->write(to, *store, GET_PROTOCOL_MINOR(clientVersion), false); } else { assert(GET_PROTOCOL_MINOR(clientVersion) >= 17); to << 0; diff --git a/src/libstore/path-info.cc b/src/libstore/path-info.cc new file mode 100644 index 000000000..fda55b2b6 --- /dev/null +++ b/src/libstore/path-info.cc @@ -0,0 +1,46 @@ +#include "path-info.hh" +#include "worker-protocol.hh" + +namespace nix { + +ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format) +{ + return read(source, store, format, store.parseStorePath(readString(source))); +} + +ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format, StorePath && path) +{ + auto deriver = readString(source); + auto narHash = Hash::parseAny(readString(source), htSHA256); + ValidPathInfo info(path, narHash); + if (deriver != "") info.deriver = store.parseStorePath(deriver); + info.references = worker_proto::read(store, source, Phantom {}); + source >> info.registrationTime >> info.narSize; + if (format >= 16) { + source >> info.ultimate; + info.sigs = readStrings(source); + info.ca = parseContentAddressOpt(readString(source)); + } + return info; +} + +void ValidPathInfo::write( + Sink & sink, + const Store & store, + unsigned int format, + bool includePath) const +{ + if (includePath) + sink << store.printStorePath(path); + sink << (deriver ? store.printStorePath(*deriver) : "") + << narHash.to_string(Base16, false); + worker_proto::write(store, sink, references); + sink << registrationTime << narSize; + if (format >= 16) { + sink << ultimate + << sigs + << renderContentAddress(ca); + } +} + +} diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh index de87f8b33..b4b54e593 100644 --- a/src/libstore/path-info.hh +++ b/src/libstore/path-info.hh @@ -105,6 +105,11 @@ struct ValidPathInfo ValidPathInfo(const StorePath & path, Hash narHash) : path(path), narHash(narHash) { }; virtual ~ValidPathInfo() { } + + static ValidPathInfo read(Source & source, const Store & store, unsigned int format); + static ValidPathInfo read(Source & source, const Store & store, unsigned int format, StorePath && path); + + void write(Sink & sink, const Store & store, unsigned int format, bool includePath = true) const; }; typedef std::map ValidPathInfos; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 1471520f3..140f39120 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -386,23 +386,6 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S } -ref RemoteStore::readValidPathInfo(ConnectionHandle & conn, const StorePath & path) -{ - auto deriver = readString(conn->from); - auto narHash = Hash::parseAny(readString(conn->from), htSHA256); - auto info = make_ref(path, narHash); - if (deriver != "") info->deriver = parseStorePath(deriver); - info->references = worker_proto::read(*this, conn->from, Phantom {}); - conn->from >> info->registrationTime >> info->narSize; - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { - conn->from >> info->ultimate; - info->sigs = readStrings(conn->from); - info->ca = parseContentAddressOpt(readString(conn->from)); - } - return info; -} - - void RemoteStore::queryPathInfoUncached(const StorePath & path, Callback> callback) noexcept { @@ -423,7 +406,8 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, bool valid; conn->from >> valid; if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path)); } - info = readValidPathInfo(conn, path); + info = std::make_shared( + ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion), StorePath{path})); } callback(std::move(info)); } catch (...) { callback.rethrow(); } @@ -525,8 +509,8 @@ ref RemoteStore::addCAToStore( }); } - auto path = parseStorePath(readString(conn->from)); - return readValidPathInfo(conn, path); + return make_ref( + ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion))); } else { if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25"); @@ -642,6 +626,25 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, } +void RemoteStore::addMultipleToStore( + Source & source, + RepairFlag repair, + CheckSigsFlag checkSigs) +{ + if (GET_PROTOCOL_MINOR(getConnection()->daemonVersion) >= 32) { + auto conn(getConnection()); + conn->to + << wopAddMultipleToStore + << repair + << !checkSigs; + conn.withFramedSink([&](Sink & sink) { + source.drainInto(sink); + }); + } else + Store::addMultipleToStore(source, repair, checkSigs); +} + + StorePath RemoteStore::addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) { @@ -885,16 +888,6 @@ void RemoteStore::queryMissing(const std::vector & targets, } -StorePaths RemoteStore::importPaths(Source & source, CheckSigsFlag checkSigs) -{ - auto conn(getConnection()); - conn->to << wopImportPaths2; - source.drainInto(conn->to); - conn.processStderr(); - return worker_proto::read(*this, conn->from, Phantom {}); -} - - void RemoteStore::connect() { auto conn(getConnection()); @@ -1021,14 +1014,14 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * return nullptr; } -void ConnectionHandle::withFramedSink(std::function fun) +void ConnectionHandle::withFramedSink(std::function fun) { (*this)->to.flush(); std::exception_ptr ex; - /* Handle log messages / exceptions from the remote on a - separate thread. */ + /* Handle log messages / exceptions from the remote on a separate + thread. */ std::thread stderrThread([&]() { try { @@ -1061,7 +1054,6 @@ void ConnectionHandle::withFramedSink(std::function fun) stderrThread.join(); if (ex) std::rethrow_exception(ex); - } } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index fbec40b4b..8901c79fc 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -78,6 +78,11 @@ public: void addToStore(const ValidPathInfo & info, Source & nar, RepairFlag repair, CheckSigsFlag checkSigs) override; + void addMultipleToStore( + Source & source, + RepairFlag repair, + CheckSigsFlag checkSigs) override; + StorePath addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) override; @@ -112,8 +117,6 @@ public: StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, uint64_t & downloadSize, uint64_t & narSize) override; - StorePaths importPaths(Source & source, CheckSigsFlag checkSigs) override; - void connect() override; unsigned int getProtocol() override; @@ -153,8 +156,6 @@ protected: virtual void narFromPath(const StorePath & path, Sink & sink) override; - ref readValidPathInfo(ConnectionHandle & conn, const StorePath & path); - private: std::atomic_bool failed{false}; 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); - } - return pathsMap; - } - 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); + } + }); + + 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; } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 3ad74f35c..4ed2fd151 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -440,6 +440,12 @@ public: virtual void addToStore(const ValidPathInfo & info, Source & narSource, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs) = 0; + /* Import multiple paths into the store. */ + virtual void addMultipleToStore( + Source & source, + 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. The function object `filter' can be used to exclude files (see diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index b1888bf01..93cf546d2 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -9,7 +9,7 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f -#define PROTOCOL_VERSION (1 << 8 | 31) +#define PROTOCOL_VERSION (1 << 8 | 32) #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) @@ -55,7 +55,7 @@ typedef enum { wopQueryDerivationOutputMap = 41, wopRegisterDrvOutput = 42, wopQueryRealisation = 43, - wopImportPaths2 = 44, // hack + wopAddMultipleToStore = 44, } WorkerOp; From 72c5bac39d0ee37a2a6c02dad5df3e17c02a7995 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 Jul 2021 13:50:18 +0200 Subject: [PATCH 13/15] Revert no longer necessary change --- src/libstore/store-api.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 4ed2fd151..4fb6c40c7 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -683,7 +683,7 @@ public: the Nix store. Optionally, the contents of the NARs are preloaded into the specified FS accessor to speed up subsequent access. */ - virtual StorePaths importPaths(Source & source, CheckSigsFlag checkSigs = CheckSigs); + StorePaths importPaths(Source & source, CheckSigsFlag checkSigs = CheckSigs); struct Stats { From 47002108d1b17e996d2463bbb22f9cffbc59ae71 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 27 Jul 2021 11:16:47 +0200 Subject: [PATCH 14/15] nix-instantiate: Fix --eval-store --- src/nix-instantiate/nix-instantiate.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index a269c5b5c..25d0fa3ba 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -155,7 +155,7 @@ static int main_nix_instantiate(int argc, char * * argv) auto store = openStore(); auto evalStore = myArgs.evalStoreUrl ? openStore(*myArgs.evalStoreUrl) : store; - auto state = std::make_unique(myArgs.searchPath, store); + auto state = std::make_unique(myArgs.searchPath, evalStore, store); state->repair = repair; Bindings & autoArgs = *myArgs.getAutoArgs(*state); From 29e4913f7947730c468c1d96f150648ec59f572d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 27 Jul 2021 11:17:56 +0200 Subject: [PATCH 15/15] Add --eval-store test --- tests/eval-store.sh | 26 ++++++++++++++++++++++++++ tests/local.mk | 3 ++- 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tests/eval-store.sh diff --git a/tests/eval-store.sh b/tests/eval-store.sh new file mode 100644 index 000000000..7bb86d77c --- /dev/null +++ b/tests/eval-store.sh @@ -0,0 +1,26 @@ +source common.sh + +eval_store=$TEST_ROOT/eval-store + +clearStore +rm -rf "$eval_store" + +nix build -f dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result" +[[ -e $TEST_ROOT/result/foobar ]] +(! ls $NIX_STORE_DIR/*.drv) +ls $eval_store/nix/store/*.drv + +clearStore +rm -rf "$eval_store" + +nix-instantiate dependencies.nix --eval-store "$eval_store" +(! ls $NIX_STORE_DIR/*.drv) +ls $eval_store/nix/store/*.drv + +clearStore +rm -rf "$eval_store" + +nix-build dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result" +[[ -e $TEST_ROOT/result/foobar ]] +(! ls $NIX_STORE_DIR/*.drv) +ls $eval_store/nix/store/*.drv diff --git a/tests/local.mk b/tests/local.mk index e16c5a9b7..b100e7f15 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -56,7 +56,8 @@ nix_tests = \ ca/nix-run.sh \ ca/recursive.sh \ ca/concurrent-builds.sh \ - ca/nix-copy.sh + ca/nix-copy.sh \ + eval-store.sh # parallel.sh install-tests += $(foreach x, $(nix_tests), tests/$(x))