libstore: move Goal::amDone to Worker

we still mutate goal state to store the results of any given goal run,
but now we also have that information in Worker and could in theory do
something else with it. we could return a map of goal to goal results,
which would also let us better diagnose failures of subgoals (at all).

Change-Id: I1df956bbd9fa8cc9485fb6df32918d68dda3ff48
This commit is contained in:
eldritch horrors 2024-07-26 20:19:21 +02:00
parent dfcab1c3f0
commit e5177dddff
7 changed files with 69 additions and 43 deletions

View file

@ -1534,7 +1534,10 @@ Goal::Finished DerivationGoal::done(
fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl;
}
return amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex));
return Finished{
.result = buildResult.success() ? ecSuccess : ecFailed,
.ex = ex ? std::make_unique<Error>(std::move(*ex)) : nullptr,
};
}

View file

@ -26,7 +26,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::init()
/* If the derivation already exists, were done */
if (worker.store.queryRealisation(id)) {
return amDone(ecSuccess);
return Finished{ecSuccess};
}
subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>();
@ -60,7 +60,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::tryNext()
/* Hack: don't indicate failure if there were no substituters.
In that case the calling derivation should just do a
build. */
return amDone(substituterFailed ? ecFailed : ecNoSubstituters);
return Finished{substituterFailed ? ecFailed : ecNoSubstituters};
}
sub = subs.front();
@ -137,7 +137,9 @@ Goal::WorkResult DrvOutputSubstitutionGoal::outPathValid()
if (nrFailed > 0) {
debug("The output path of the derivation output '%s' could not be substituted", id.to_string());
return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed);
return Finished{
nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed
};
}
worker.store.registerDrvOutput(*outputInfo);
@ -147,7 +149,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::outPathValid()
Goal::WorkResult DrvOutputSubstitutionGoal::finished()
{
trace("finished");
return amDone(ecSuccess);
return Finished{ecSuccess};
}
std::string DrvOutputSubstitutionGoal::key()

View file

@ -45,31 +45,6 @@ void Goal::waiteeDone(GoalPtr waitee, ExitCode result)
}
Goal::Finished Goal::amDone(ExitCode result, std::optional<Error> ex)
{
trace("done");
assert(!exitCode.has_value());
exitCode = result;
if (ex) {
if (!waiters.empty())
logError(ex->info());
else
this->ex = std::make_unique<Error>(std::move(*ex));
}
for (auto & i : waiters) {
GoalPtr goal = i.lock();
if (goal) goal->waiteeDone(shared_from_this(), result);
}
waiters.clear();
worker.removeGoal(shared_from_this());
cleanup();
return Finished{};
}
void Goal::trace(std::string_view s)
{
debug("%1%: %2%", name, s);

View file

@ -105,10 +105,13 @@ struct Goal : public std::enable_shared_from_this<Goal>
public:
struct StillAlive {};
struct Finished {};
struct [[nodiscard]] StillAlive {};
struct [[nodiscard]] Finished {
ExitCode result;
std::unique_ptr<Error> ex;
};
struct WorkResult : std::variant<StillAlive, Finished>
struct [[nodiscard]] WorkResult : std::variant<StillAlive, Finished>
{
WorkResult() = delete;
using variant::variant;
@ -159,8 +162,6 @@ public:
virtual std::string key() = 0;
Finished amDone(ExitCode result, std::optional<Error> ex = {});
virtual void cleanup() { }
/**

View file

@ -35,7 +35,7 @@ Goal::Finished PathSubstitutionGoal::done(
debug(*errorMsg);
buildResult.errorMsg = *errorMsg;
}
return amDone(result);
return Finished{result};
}

View file

@ -139,6 +139,40 @@ static void removeGoal(std::shared_ptr<G> goal, std::map<K, std::weak_ptr<G>> &
}
void Worker::goalFinished(GoalPtr goal, Goal::Finished & f)
{
goal->trace("done");
assert(!goal->exitCode.has_value());
goal->exitCode = f.result;
if (f.ex) {
if (!goal->waiters.empty())
logError(f.ex->info());
else
goal->ex = std::move(f.ex);
}
for (auto & i : goal->waiters) {
if (GoalPtr waiting = i.lock()) {
waiting->waiteeDone(goal, f.result);
}
}
goal->waiters.clear();
removeGoal(goal);
goal->cleanup();
}
void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how)
{
std::visit(
overloaded{
[&](Goal::StillAlive) {},
[&](Goal::Finished & f) { goalFinished(goal, f); },
},
how
);
}
void Worker::removeGoal(GoalPtr goal)
{
if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal))
@ -299,7 +333,7 @@ void Worker::run(const Goals & _topGoals)
awake.clear();
for (auto & goal : awake2) {
checkInterrupt();
goal->work();
handleWorkResult(goal, goal->work());
actDerivations.progress(
doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds
@ -429,9 +463,14 @@ void Worker::waitForInput()
j->respectTimeouts &&
after - j->lastOutput >= std::chrono::seconds(settings.maxSilentTime))
{
goal->timedOut(Error(
handleWorkResult(
goal,
goal->timedOut(Error(
"%1% timed out after %2% seconds of silence",
goal->getName(), settings.maxSilentTime));
goal->getName(),
settings.maxSilentTime
))
);
continue;
}
@ -440,9 +479,12 @@ void Worker::waitForInput()
j->respectTimeouts &&
after - j->timeStarted >= std::chrono::seconds(settings.buildTimeout))
{
goal->timedOut(Error(
"%1% timed out after %2% seconds",
goal->getName(), settings.buildTimeout));
handleWorkResult(
goal,
goal->timedOut(
Error("%1% timed out after %2% seconds", goal->getName(), settings.buildTimeout)
)
);
continue;
}
@ -469,7 +511,7 @@ void Worker::waitForInput()
goal->getName(), rd);
std::string_view data((char *) buffer.data(), rd);
j->lastOutput = after;
goal->handleChildOutput(k, data);
handleWorkResult(goal, goal->handleChildOutput(k, data));
}
}
}

View file

@ -105,6 +105,9 @@ private:
*/
std::map<StorePath, bool> pathContentsGoodCache;
void goalFinished(GoalPtr goal, Goal::Finished & f);
void handleWorkResult(GoalPtr goal, Goal::WorkResult how);
public:
const Activity act;