Skip to content

Commit

Permalink
More properly track the status of CA builds
Browse files Browse the repository at this point in the history
Make the build of unresolved derivations return the same status as the
resolved one, except in the case of an `AlreadyValid` in which case it
will return `ResolvesToAlreadyValid` to mean that the outputs of the unresolved
derivation weren’t known, but the resolved one is.
  • Loading branch information
thufschmitt committed Dec 13, 2021
1 parent f133001 commit 55dbb7f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
26 changes: 17 additions & 9 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,6 @@ void DerivationGoal::inputsRealised()
Derivation drvResolved { *std::move(attempt) };

auto pathResolved = writeDerivation(worker.store, drvResolved);
resolvedDrv = drvResolved;

auto msg = fmt("Resolved derivation: '%s' -> '%s'",
worker.store.printStorePath(drvPath),
Expand All @@ -475,9 +474,9 @@ void DerivationGoal::inputsRealised()
worker.store.printStorePath(pathResolved),
});

auto resolvedGoal = worker.makeDerivationGoal(
resolvedDrvGoal = worker.makeDerivationGoal(
pathResolved, wantedOutputs, buildMode);
addWaitee(resolvedGoal);
addWaitee(resolvedDrvGoal);

state = &DerivationGoal::resolvedFinished;
return;
Expand Down Expand Up @@ -938,16 +937,17 @@ void DerivationGoal::buildDone()
}

void DerivationGoal::resolvedFinished() {
assert(resolvedDrv);
assert(resolvedDrvGoal);
auto resolvedDrv = *resolvedDrvGoal->drv;

auto resolvedHashes = staticOutputHashes(worker.store, *resolvedDrv);
auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv);

StorePathSet outputPaths;

// `wantedOutputs` might be empty, which means “all the outputs”
auto realWantedOutputs = wantedOutputs;
if (realWantedOutputs.empty())
realWantedOutputs = resolvedDrv->outputNames();
realWantedOutputs = resolvedDrv.outputNames();

for (auto & wantedOutput : realWantedOutputs) {
assert(initialOutputs.count(wantedOutput) != 0);
Expand Down Expand Up @@ -979,9 +979,17 @@ void DerivationGoal::resolvedFinished() {
outputPaths
);

// This is potentially a bit fishy in terms of error reporting. Not sure
// how to do it in a cleaner way
amDone(nrFailed == 0 ? ecSuccess : ecFailed, ex);
auto status = [&]() {
auto resolvedResult = resolvedDrvGoal->getResult();
switch (resolvedResult.status) {
case BuildResult::AlreadyValid:
return BuildResult::ResolvesToAlreadyValid;
default:
return resolvedResult.status;
}
}();

done(status);
}

HookReply DerivationGoal::tryBuildHook()
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/build/derivation-goal.hh
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ struct DerivationGoal : public Goal
/* The path of the derivation. */
StorePath drvPath;

/* The path of the corresponding resolved derivation */
std::optional<BasicDerivation> resolvedDrv;
/* The goal for the corresponding resolved derivation */
std::shared_ptr<DerivationGoal> resolvedDrvGoal;

/* The specific outputs that we need to build. Empty means all of
them. */
Expand Down
3 changes: 2 additions & 1 deletion src/libstore/store-api.hh
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ struct BuildResult
DependencyFailed,
LogLimitExceeded,
NotDeterministic,
ResolvesToAlreadyValid,
} status = MiscFailure;
std::string errorMsg;

Expand All @@ -170,7 +171,7 @@ struct BuildResult
time_t startTime = 0, stopTime = 0;

bool success() {
return status == Built || status == Substituted || status == AlreadyValid;
return status == Built || status == Substituted || status == AlreadyValid || status == ResolvesToAlreadyValid;
}
};

Expand Down

0 comments on commit 55dbb7f

Please sign in to comment.