Prevent orphaned build steps

If two active steps of the same build failed, then the first would be
marked as "failed", but the second would end up as "orphaned", causing
it to be marked as "aborted" later on. Now it's correctly marked as
"failed".
This commit is contained in:
Eelco Dolstra 2016-10-26 14:42:28 +02:00
parent 8e1d791d0c
commit 0b00d51baf

View file

@ -108,6 +108,7 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore, Step::ptr step,
Finally clearStep([&]() { Finally clearStep([&]() {
if (stepNr && !stepFinished) { if (stepNr && !stepFinished) {
printError("marking step %d of build %d as orphaned", stepNr, build->id);
auto orphanedSteps_(orphanedSteps.lock()); auto orphanedSteps_(orphanedSteps.lock());
orphanedSteps_->emplace(build->id, stepNr); orphanedSteps_->emplace(build->id, stepNr);
} }
@ -176,6 +177,7 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore, Step::ptr step,
if (result.canRetry) { if (result.canRetry) {
printMsg(lvlError, format("possibly transient failure building %1% on %2%: %3%") printMsg(lvlError, format("possibly transient failure building %1% on %2%: %3%")
% step->drvPath % machine->sshName % result.errorMsg); % step->drvPath % machine->sshName % result.errorMsg);
assert(stepNr);
bool retry; bool retry;
{ {
auto step_(step->state.lock()); auto step_(step->state.lock());
@ -197,6 +199,8 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore, Step::ptr step,
if (result.stepStatus == bsSuccess) { if (result.stepStatus == bsSuccess) {
assert(stepNr);
/* Register success in the database for all Build objects that /* Register success in the database for all Build objects that
have this step as the top-level step. Since the queue have this step as the top-level step. Since the queue
monitor thread may be creating new referring Builds monitor thread may be creating new referring Builds
@ -293,13 +297,17 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore, Step::ptr step,
} else { } else {
/* For standard failures, we don't care about the error
message. */
if (result.stepStatus != bsAborted)
result.errorMsg = "";
/* Register failure in the database for all Build objects that /* Register failure in the database for all Build objects that
directly or indirectly depend on this step. */ directly or indirectly depend on this step. */
std::vector<BuildID> dependentIDs; std::vector<BuildID> dependentIDs;
while (true) { while (true) {
/* Get the builds and steps that depend on this step. */ /* Get the builds and steps that depend on this step. */
std::set<Build::ptr> indirect; std::set<Build::ptr> indirect;
{ {
@ -315,21 +323,17 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore, Step::ptr step,
printMsg(lvlDebug, format("finishing build step %1%") % s->drvPath); printMsg(lvlDebug, format("finishing build step %1%") % s->drvPath);
steps_->erase(s->drvPath); steps_->erase(s->drvPath);
} }
break;
} }
} }
if (indirect.empty() && stepFinished) break;
/* Update the database. */ /* Update the database. */
{ {
auto mc = startDbUpdate(); auto mc = startDbUpdate();
pqxx::work txn(*conn); pqxx::work txn(*conn);
/* For standard failures, we don't care about the error
message. */
if (result.stepStatus != bsAborted)
result.errorMsg = "";
/* Create failed build steps for every build that /* Create failed build steps for every build that
depends on this, except when this step is cached depends on this, except when this step is cached
and is the top-level of that build (since then it's and is the top-level of that build (since then it's
@ -343,9 +347,11 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore, Step::ptr step,
result.stepStatus, result.errorMsg, build == build2 ? 0 : build->id); result.stepStatus, result.errorMsg, build == build2 ? 0 : build->id);
} }
if (result.stepStatus != bsCachedFailure) if (result.stepStatus != bsCachedFailure && !stepFinished) {
assert(stepNr);
finishBuildStep(txn, result.startTime, result.stopTime, result.overhead, finishBuildStep(txn, result.startTime, result.stopTime, result.overhead,
build->id, stepNr, machine->sshName, result.stepStatus, result.errorMsg); build->id, stepNr, machine->sshName, result.stepStatus, result.errorMsg);
}
/* Mark all builds that depend on this derivation as failed. */ /* Mark all builds that depend on this derivation as failed. */
for (auto & build2 : indirect) { for (auto & build2 : indirect) {