From 3b7e00ce2215b742d9fdb1b8d4a4d76d349028c7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 18 Nov 2020 14:36:15 +0100 Subject: [PATCH] Move primeCache() to Worker::run() We need the missing path info to communicate the worker's remaining goals to the progress bar. --- src/libstore/build/derivation-goal.hh | 12 +----------- src/libstore/build/local-store-build.cc | 22 ++-------------------- src/libstore/build/substitution-goal.hh | 8 +------- src/libstore/build/worker.cc | 21 ++++++++++++++++++++- 4 files changed, 24 insertions(+), 39 deletions(-) diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 4976207e0..8ee0be9e1 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -40,9 +40,8 @@ struct InitialOutput { std::optional known; }; -class DerivationGoal : public Goal +struct DerivationGoal : public Goal { -private: /* Whether to use an on-disk .drv file. */ bool useDerivation; @@ -246,7 +245,6 @@ private: friend struct RestrictedStore; -public: DerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode = bmNormal); @@ -264,17 +262,11 @@ public: void work() override; - StorePath getDrvPath() - { - return drvPath; - } - /* Add wanted outputs to an already existing derivation goal. */ void addWantedOutputs(const StringSet & outputs); BuildResult getResult() { return result; } -private: /* The states. */ void getDerivation(); void loadDerivation(); @@ -318,8 +310,6 @@ private: /* Run the builder's process. */ void runChild(); - friend int childEntry(void *); - /* Check that the derivation outputs all exist and register them as valid. */ void registerOutputs(); diff --git a/src/libstore/build/local-store-build.cc b/src/libstore/build/local-store-build.cc index a05fb5805..c91cda2fd 100644 --- a/src/libstore/build/local-store-build.cc +++ b/src/libstore/build/local-store-build.cc @@ -5,25 +5,10 @@ namespace nix { -static void primeCache(Store & store, const std::vector & paths) -{ - StorePathSet willBuild, willSubstitute, unknown; - uint64_t downloadSize, narSize; - store.queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); - - if (!willBuild.empty() && 0 == settings.maxBuildJobs && getMachines().empty()) - throw Error( - "%d derivations need to be built, but neither local builds ('--max-jobs') " - "nor remote builds ('--builders') are enabled", willBuild.size()); -} - - void LocalStore::buildPaths(const std::vector & drvPaths, BuildMode buildMode) { Worker worker(*this); - primeCache(*this, drvPaths); - Goals goals; for (auto & path : drvPaths) { if (path.path.isDerivation()) @@ -44,9 +29,8 @@ void LocalStore::buildPaths(const std::vector & drvPaths, ex = i->ex; } if (i->exitCode != Goal::ecSuccess) { - DerivationGoal * i2 = dynamic_cast(i.get()); - if (i2) failed.insert(i2->getDrvPath()); - else failed.insert(dynamic_cast(i.get())->getStorePath()); + if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->drvPath); + else if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->storePath); } } @@ -84,8 +68,6 @@ void LocalStore::ensurePath(const StorePath & path) /* If the path is already valid, we're done. */ if (isValidPath(path)) return; - primeCache(*this, {{path}}); - Worker worker(*this); GoalPtr goal = worker.makeSubstitutionGoal(path); Goals goals = {goal}; diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index 3ae9a9e6b..dee2cecbf 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -8,11 +8,8 @@ namespace nix { class Worker; -class SubstitutionGoal : public Goal +struct SubstitutionGoal : public Goal { - friend class Worker; - -private: /* The store path that should be realised through a substitute. */ StorePath storePath; @@ -56,7 +53,6 @@ private: /* Content address for recomputing store path */ std::optional ca; -public: SubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); ~SubstitutionGoal(); @@ -82,8 +78,6 @@ public: /* Callback used by the worker to write to the log. */ void handleChildOutput(int fd, const string & data) override; void handleEOF(int fd) override; - - StorePath getStorePath() { return storePath; } }; } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 17c10cd71..1f8999a4b 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -207,7 +207,26 @@ void Worker::waitForAWhile(GoalPtr goal) void Worker::run(const Goals & _topGoals) { - for (auto & i : _topGoals) topGoals.insert(i); + std::vector topPaths; + + for (auto & i : _topGoals) { + topGoals.insert(i); + if (auto goal = dynamic_cast(i.get())) { + topPaths.push_back({goal->drvPath, goal->wantedOutputs}); + } else if (auto goal = dynamic_cast(i.get())) { + topPaths.push_back({goal->storePath}); + } + } + + /* Call queryMissing() efficiently query substitutes. */ + StorePathSet willBuild, willSubstitute, unknown; + uint64_t downloadSize, narSize; + store.queryMissing(topPaths, willBuild, willSubstitute, unknown, downloadSize, narSize); + + if (!willBuild.empty() && 0 == settings.maxBuildJobs && getMachines().empty()) + throw Error( + "%d derivations need to be built, but neither local builds ('--max-jobs') " + "nor remote builds ('--builders') are enabled", willBuild.size()); debug("entered goal loop");