From ba85e501ced59838d36f36cb545bcc6f7bc30561 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 2 Aug 2024 17:00:57 +0200 Subject: [PATCH] libstore: make Worker status flags private Change-Id: I16ec8994c6448d70b686a2e4c10f19d4e240750d --- src/libstore/build/derivation-goal.cc | 8 ++-- src/libstore/build/derivation-goal.hh | 3 ++ src/libstore/build/goal.hh | 4 ++ src/libstore/build/local-derivation-goal.cc | 9 +++-- src/libstore/build/worker.cc | 9 +++-- src/libstore/build/worker.hh | 42 ++++++++++----------- 6 files changed, 42 insertions(+), 33 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index ace00b1fb..aa89f9e7d 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1506,10 +1506,6 @@ Goal::Finished DerivationGoal::done( buildResult.status = status; if (ex) buildResult.errorMsg = fmt("%s", Uncolored(ex->info().msg)); - if (buildResult.status == BuildResult::TimedOut) - worker.timedOut = true; - if (buildResult.status == BuildResult::PermanentFailure) - worker.permanentFailure = true; mcExpectedBuilds.reset(); mcRunningBuilds.reset(); @@ -1535,6 +1531,10 @@ Goal::Finished DerivationGoal::done( return Finished{ .result = buildResult.success() ? ecSuccess : ecFailed, .ex = ex ? std::make_unique(std::move(*ex)) : nullptr, + .permanentFailure = buildResult.status == BuildResult::PermanentFailure, + .timedOut = buildResult.status == BuildResult::TimedOut, + .hashMismatch = anyHashMismatchSeen, + .checkMismatch = anyCheckMismatchSeen, }; } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index c43e2aed5..257282308 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -107,6 +107,9 @@ struct DerivationGoal : public Goal */ NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; + bool anyHashMismatchSeen = false; + bool anyCheckMismatchSeen = false; + /** * See `retrySubstitution`; just for that field. */ diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 83f52927e..5d7bb72b8 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -115,6 +115,10 @@ public: struct [[nodiscard]] Finished { ExitCode result; std::unique_ptr ex; + bool permanentFailure = false; + bool timedOut = false; + bool hashMismatch = false; + bool checkMismatch = false; }; struct [[nodiscard]] WorkResult : std::variant< diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 3618aa191..afc741397 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -235,8 +235,9 @@ Goal::WorkResult LocalDerivationGoal::tryLocalBuild() } catch (BuildError & e) { outputLocks.unlock(); buildUser.reset(); - worker.permanentFailure = true; - return done(BuildResult::InputRejected, {}, std::move(e)); + auto report = done(BuildResult::InputRejected, {}, std::move(e)); + report.permanentFailure = true; + return report; } /* This state will be reached when we get EOF on the child's @@ -2195,7 +2196,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() if (wanted != got) { /* Throw an error after registering the path as valid. */ - worker.hashMismatch = true; + anyHashMismatchSeen = true; // XXX: shameless layering violation hack that makes the hash mismatch error at least not utterly worthless auto guessedUrl = getOr(drv->env, "urls", getOr(drv->env, "url", "(unknown)")); delayedException = std::make_exception_ptr( @@ -2282,7 +2283,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() if (!worker.store.isValidPath(newInfo.path)) continue; ValidPathInfo oldInfo(*worker.store.queryPathInfo(newInfo.path)); if (newInfo.narHash != oldInfo.narHash) { - worker.checkMismatch = true; + anyCheckMismatchSeen = true; if (settings.runDiffHook || settings.keepFailed) { auto dst = worker.store.toRealPath(finalDestPath + checkSuffix); deletePath(dst); diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 712af6c7e..556cc9035 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -21,10 +21,6 @@ Worker::Worker(Store & store, Store & evalStore) nrLocalBuilds = 0; nrSubstitutions = 0; lastWokenUp = steady_time_point::min(); - permanentFailure = false; - timedOut = false; - hashMismatch = false; - checkMismatch = false; } @@ -145,6 +141,11 @@ void Worker::goalFinished(GoalPtr goal, Goal::Finished & f) assert(!goal->exitCode.has_value()); goal->exitCode = f.result; + permanentFailure |= f.permanentFailure; + timedOut |= f.timedOut; + hashMismatch |= f.hashMismatch; + checkMismatch |= f.checkMismatch; + if (f.ex) { if (!goal->waiters.empty()) logError(f.ex->info()); diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 20aa34e74..6e7c1c626 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -105,6 +105,27 @@ private: */ std::map pathContentsGoodCache; + /** + * Set if at least one derivation had a BuildError (i.e. permanent + * failure). + */ + bool permanentFailure = false; + + /** + * Set if at least one derivation had a timeout. + */ + bool timedOut = false; + + /** + * Set if at least one derivation fails with a hash mismatch. + */ + bool hashMismatch = false; + + /** + * Set if at least one derivation is not deterministic in check mode. + */ + bool checkMismatch = false; + void goalFinished(GoalPtr goal, Goal::Finished & f); void handleWorkResult(GoalPtr goal, Goal::WorkResult how); @@ -133,27 +154,6 @@ public: const Activity actDerivations; const Activity actSubstitutions; - /** - * Set if at least one derivation had a BuildError (i.e. permanent - * failure). - */ - bool permanentFailure; - - /** - * Set if at least one derivation had a timeout. - */ - bool timedOut; - - /** - * Set if at least one derivation fails with a hash mismatch. - */ - bool hashMismatch; - - /** - * Set if at least one derivation is not deterministic in check mode. - */ - bool checkMismatch; - Store & store; Store & evalStore;