nixos/nix
1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-12-14 04:51:05 +01:00
Code Activity

Simplify handling of statuses for build errors

Browse source 
Instead of passing them around separately, or doing finicky logic in a
try-catch block to recover them, just make `BuildError` always contain a
status, and make it the thrower's responsibility to set it. This is much
more simple and explicit.

Once that change is done, split the `done` functions of `DerivationGoal`
and `DerivationBuildingGoal` into separate success and failure
functions, which ends up being easier to understand and hardly any
duplication.

Also, change the handling of failures in resolved cases to use
`BuildResult::DependencyFailed` and a new message. This is because the
underlying derivation will also get its message printed --- which is
good, because in general the resolved derivation is not unique. One dyn
drv test had to be updated, but CA (and dyn drv) is experimental, so I
do not mind.

Finally, delete `SubstError` because it is unused.
This commit is contained in:
John Ericson 2025-08-27 18:58:58 -04:00
parent 0590b13156
commit 169033001d
16 changed files with 153 additions and 92 deletions
Download patch file Download diff file Expand all files Collapse all files

83
src/libstore/build/derivation-building-goal.cc
View file

@ -118,7 +118,7 @@ void DerivationBuildingGoal::timedOut(Error && ex)
killChild();
// We're not inside a coroutine, hence we can't use co_return here.
// Thus we ignore the return value.
[[maybe_unused]] Done _ = done(BuildResult::TimedOut, {}, std::move(ex));
[[maybe_unused]] Done _ = doneFailure({BuildResult::TimedOut, std::move(ex)});
}
/**
@ -258,7 +258,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
nrFailed,
nrFailed == 1 ? "dependency" : "dependencies");
msg += showKnownOutputs(worker.store, *drv);
co_return done(BuildResult::DependencyFailed, {}, Error(msg));
co_return doneFailure(BuildError(BuildResult::DependencyFailed, msg));
}
/* Gather information necessary for computing the closure and/or
@ -359,9 +359,9 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
auto resolvedResult = resolvedDrvGoal->buildResult;
SingleDrvOutputs builtOutputs;
if (resolvedResult.success()) {
SingleDrvOutputs builtOutputs;
auto resolvedHashes = staticOutputHashes(worker.store, drvResolved);
StorePathSet outputPaths;
@ -411,13 +411,19 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
}
runPostBuildHook(worker.store, *logger, drvPath, outputPaths);
auto status = resolvedResult.status;
if (status == BuildResult::AlreadyValid)
status = BuildResult::ResolvesToAlreadyValid;
co_return doneSuccess(status, std::move(builtOutputs));
} else {
co_return doneFailure({
BuildResult::DependencyFailed,
"build of resolved derivation '%s' failed",
worker.store.printStorePath(pathResolved),
});
}
auto status = resolvedResult.status;
if (status == BuildResult::AlreadyValid)
status = BuildResult::ResolvesToAlreadyValid;
co_return done(status, std::move(builtOutputs));
}
/* If we get this far, we know no dynamic drvs inputs */
@ -542,7 +548,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
debug("skipping build of derivation '%s', someone beat us to it", worker.store.printStorePath(drvPath));
outputLocks.setDeletion(true);
outputLocks.unlock();
co_return done(BuildResult::AlreadyValid, std::move(validOutputs));
co_return doneSuccess(BuildResult::AlreadyValid, std::move(validOutputs));
}
/* If any of the outputs already exist but are not valid, delete
@ -752,7 +758,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
} catch (BuildError & e) {
outputLocks.unlock();
worker.permanentFailure = true;
co_return done(BuildResult::InputRejected, {}, std::move(e));
co_return doneFailure(std::move(e));
}
/* If we have to wait and retry (see below), then `builder` will
@ -800,7 +806,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
builder.reset();
outputLocks.unlock();
worker.permanentFailure = true;
co_return done(BuildResult::InputRejected, {}, std::move(e));
co_return doneFailure(std::move(e)); // InputRejected
}
started();
@ -812,7 +818,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
// N.B. cannot use `std::visit` with co-routine return
if (auto * ste = std::get_if<0>(&res)) {
outputLocks.unlock();
co_return done(std::move(ste->first), {}, std::move(ste->second));
co_return doneFailure(std::move(*ste));
} else if (auto * builtOutputs = std::get_if<1>(&res)) {
StorePathSet outputPaths;
for (auto & [_, output] : *builtOutputs)
@ -825,7 +831,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
(unlinked) lock files. */
outputLocks.setDeletion(true);
outputLocks.unlock();
co_return done(BuildResult::Built, std::move(*builtOutputs));
co_return doneSuccess(BuildResult::Built, std::move(*builtOutputs));
} else {
unreachable();
}
@ -970,7 +976,7 @@ Goal::Co DerivationBuildingGoal::hookDone()
/* TODO (once again) support fine-grained error codes, see issue #12641. */
co_return done(BuildResult::MiscFailure, {}, BuildError(msg));
co_return doneFailure(BuildError{BuildResult::MiscFailure, msg});
}
/* Compute the FS closure of the outputs and register them as
@ -997,7 +1003,7 @@ Goal::Co DerivationBuildingGoal::hookDone()
outputLocks.setDeletion(true);
outputLocks.unlock();
co_return done(BuildResult::Built, std::move(builtOutputs));
co_return doneSuccess(BuildResult::Built, std::move(builtOutputs));
}
HookReply DerivationBuildingGoal::tryBuildHook()
@ -1179,10 +1185,11 @@ void DerivationBuildingGoal::handleChildOutput(Descriptor fd, std::string_view d
killChild();
// We're not inside a coroutine, hence we can't use co_return here.
// Thus we ignore the return value.
[[maybe_unused]] Done _ = done(
[[maybe_unused]] Done _ = doneFailure(BuildError(
BuildResult::LogLimitExceeded,
{},
Error("%s killed after writing more than %d bytes of log output", getName(), settings.maxLogSize));
"%s killed after writing more than %d bytes of log output",
getName(),
settings.maxLogSize));
return;
}
@ -1343,13 +1350,27 @@ SingleDrvOutputs DerivationBuildingGoal::assertPathValidity()
return validOutputs;
}
Goal::Done
DerivationBuildingGoal::done(BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional<Error> ex)
Goal::Done DerivationBuildingGoal::doneSuccess(BuildResult::Status status, SingleDrvOutputs builtOutputs)
{
outputLocks.unlock();
buildResult.status = status;
if (ex)
buildResult.errorMsg = fmt("%s", Uncolored(ex->info().msg));
assert(buildResult.success());
mcRunningBuilds.reset();
buildResult.builtOutputs = std::move(builtOutputs);
if (status == BuildResult::Built)
worker.doneBuilds++;
worker.updateProgress();
return amDone(ecSuccess, std::nullopt);
}
Goal::Done DerivationBuildingGoal::doneFailure(BuildError ex)
{
buildResult.status = ex.status;
buildResult.errorMsg = fmt("%s", Uncolored(ex.info().msg));
if (buildResult.status == BuildResult::TimedOut)
worker.timedOut = true;
if (buildResult.status == BuildResult::PermanentFailure)
@ -1357,18 +1378,12 @@ DerivationBuildingGoal::done(BuildResult::Status status, SingleDrvOutputs builtO
mcRunningBuilds.reset();
if (buildResult.success()) {
buildResult.builtOutputs = std::move(builtOutputs);
if (status == BuildResult::Built)
worker.doneBuilds++;
} else {
if (status != BuildResult::DependencyFailed)
worker.failedBuilds++;
}
if (ex.status != BuildResult::DependencyFailed)
worker.failedBuilds++;
worker.updateProgress();
return amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex));
return amDone(ecFailed, {std::move(ex)});
}
} // namespace nix

5
src/libstore/build/derivation-check.cc
View file

@ -1,6 +1,7 @@
#include <queue>
#include "nix/store/store-api.hh"
#include "nix/store/build-result.hh"
#include "derivation-check.hh"
@ -54,6 +55,7 @@ void checkOutputs(
auto applyChecks = [&](const DerivationOptions::OutputChecks & checks) {
if (checks.maxSize && info.narSize > *checks.maxSize)
throw BuildError(
BuildResult::OutputRejected,
"path '%s' is too large at %d bytes; limit is %d bytes",
store.printStorePath(info.path),
info.narSize,
@ -63,6 +65,7 @@ void checkOutputs(
uint64_t closureSize = getClosure(info.path).second;
if (closureSize > *checks.maxClosureSize)
throw BuildError(
BuildResult::OutputRejected,
"closure of path '%s' is too large at %d bytes; limit is %d bytes",
store.printStorePath(info.path),
closureSize,
@ -83,6 +86,7 @@ void checkOutputs(
std::string outputsListing =
concatMapStringsSep(", ", outputs, [](auto & o) { return o.first; });
throw BuildError(
BuildResult::OutputRejected,
"derivation '%s' output check for '%s' contains an illegal reference specifier '%s',"
" expected store path or output name (one of [%s])",
store.printStorePath(drvPath),
@ -115,6 +119,7 @@ void checkOutputs(
badPathsStr += store.printStorePath(i);
}
throw BuildError(
BuildResult::OutputRejected,
"output '%s' is not allowed to refer to the following paths:%s",
store.printStorePath(info.path),
badPathsStr);

50
src/libstore/build/derivation-goal.cc
View file

@ -94,7 +94,7 @@ Goal::Co DerivationGoal::haveDerivation()
/* If they are all valid, then we're done. */
if (checkResult && checkResult->second == PathStatus::Valid && buildMode == bmNormal) {
co_return done(BuildResult::AlreadyValid, checkResult->first);
co_return doneSuccess(BuildResult::AlreadyValid, checkResult->first);
}
Goals waitees;
@ -122,12 +122,10 @@ Goal::Co DerivationGoal::haveDerivation()
assert(!drv->type().isImpure());
if (nrFailed > 0 && nrFailed > nrNoSubstituters && !settings.tryFallback) {
co_return done(
co_return doneFailure(BuildError(
BuildResult::TransientFailure,
{},
Error(
"some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ",
worker.store.printStorePath(drvPath)));
"some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ",
worker.store.printStorePath(drvPath)));
}
nrFailed = nrNoSubstituters = 0;
@ -137,7 +135,7 @@ Goal::Co DerivationGoal::haveDerivation()
bool allValid = checkResult && checkResult->second == PathStatus::Valid;
if (buildMode == bmNormal && allValid) {
co_return done(BuildResult::Substituted, checkResult->first);
co_return doneSuccess(BuildResult::Substituted, checkResult->first);
}
if (buildMode == bmRepair && allValid) {
co_return repairClosure();
@ -281,7 +279,7 @@ Goal::Co DerivationGoal::repairClosure()
"some paths in the output closure of derivation '%s' could not be repaired",
worker.store.printStorePath(drvPath));
}
co_return done(BuildResult::AlreadyValid, assertPathValidity());
co_return doneSuccess(BuildResult::AlreadyValid, assertPathValidity());
}
std::optional<std::pair<Realisation, PathStatus>> DerivationGoal::checkPathValidity()
@ -339,12 +337,27 @@ Realisation DerivationGoal::assertPathValidity()
return checkResult->first;
}
Goal::Done
DerivationGoal::done(BuildResult::Status status, std::optional<Realisation> builtOutput, std::optional<Error> ex)
Goal::Done DerivationGoal::doneSuccess(BuildResult::Status status, Realisation builtOutput)
{
buildResult.status = status;
if (ex)
buildResult.errorMsg = fmt("%s", Uncolored(ex->info().msg));
assert(buildResult.success());
mcExpectedBuilds.reset();
buildResult.builtOutputs = {{wantedOutput, std::move(builtOutput)}};
if (status == BuildResult::Built)
worker.doneBuilds++;
worker.updateProgress();
return amDone(ecSuccess, std::nullopt);
}
Goal::Done DerivationGoal::doneFailure(BuildError ex)
{
buildResult.status = ex.status;
buildResult.errorMsg = fmt("%s", Uncolored(ex.info().msg));
if (buildResult.status == BuildResult::TimedOut)
worker.timedOut = true;
if (buildResult.status == BuildResult::PermanentFailure)
@ -352,19 +365,12 @@ DerivationGoal::done(BuildResult::Status status, std::optional<Realisation> buil
mcExpectedBuilds.reset();
if (buildResult.success()) {
assert(builtOutput);
buildResult.builtOutputs = {{wantedOutput, std::move(*builtOutput)}};
if (status == BuildResult::Built)
worker.doneBuilds++;
} else {
if (status != BuildResult::DependencyFailed)
worker.failedBuilds++;
}
if (ex.status != BuildResult::DependencyFailed)
worker.failedBuilds++;
worker.updateProgress();
return amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex));
return amDone(ecFailed, {std::move(ex)});
}
} // namespace nix