wroclaw-git-backups/nix
1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-07-06 21:41:48 +02:00
Code Activity

Inline the try-catch BuildError in the hook case

Browse source 
In the local building case, there is many things which can through
`BuildError`, but in the hook case there is just this one. We can
therefore simplify the code by "cinching" down the logic just to the
spot the error is thrown.

There is other code outside `libstore/build` which also uses
`BuildError`, but I believe those cases are mistakes. The point of
`BuildError` is the narrow technical use-cases of "errors which should
not be fatal with `--keep-going`". Using it outside the
building/scheduling code doesn't really make sense in that regard. It
seems likely that those usages were instead merely because "oh, this
error has something to do with building, so I guess `BuildError` is
better than `Error`".

It is quite likely that I myself used `BuildError` incorrectly as
described above :).
This commit is contained in:
John Ericson 2025-03-12 16:26:16 -04:00
parent a39ed67180
commit 06af9cb532
1 changed files with 38 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -922,51 +922,16 @@ Goal::Co DerivationGoal::hookDone()
/* Close the log file. */
closeLogFile();
try {
/* Check the exit status. */
if (!statusOk(status)) {
auto msg = fmt("builder for '%s' %s",
Magenta(worker.store.printStorePath(drvPath)),
statusToString(status));
/* Check the exit status. */
if (!statusOk(status)) {
auto msg = fmt("builder for '%s' %s",
Magenta(worker.store.printStorePath(drvPath)),
statusToString(status));
appendLogTailErrorMsg(worker, drvPath, logTail, msg);
appendLogTailErrorMsg(worker, drvPath, logTail, msg);
auto e = BuildError(msg);
throw BuildError(msg);
}
/* Compute the FS closure of the outputs and register them as
being valid. */
auto builtOutputs =
/* When using a build hook, the build hook can register the output
as valid (by doing `nix-store --import'). If so we don't have
to do anything here.
We can only early return when the outputs are known a priori. For
floating content-addressing derivations this isn't the case.
*/
assertPathValidity();
StorePathSet outputPaths;
for (auto & [_, output] : builtOutputs)
outputPaths.insert(output.outPath);
runPostBuildHook(
worker.store,
*logger,
drvPath,
outputPaths
);
/* It is now safe to delete the lock files, since all future
lockers will see that the output paths are valid; they will
not create new lock files with the same names as the old
(unlinked) lock files. */
outputLocks.setDeletion(true);
outputLocks.unlock();
co_return done(BuildResult::Built, std::move(builtOutputs));
} catch (BuildError & e) {
outputLocks.unlock();
BuildResult::Status st = BuildResult::MiscFailure;
@ -988,6 +953,37 @@ Goal::Co DerivationGoal::hookDone()
co_return done(st, {}, std::move(e));
}
/* Compute the FS closure of the outputs and register them as
being valid. */
auto builtOutputs =
/* When using a build hook, the build hook can register the output
as valid (by doing `nix-store --import'). If so we don't have
to do anything here.
We can only early return when the outputs are known a priori. For
floating content-addressing derivations this isn't the case.
*/
assertPathValidity();
StorePathSet outputPaths;
for (auto & [_, output] : builtOutputs)
outputPaths.insert(output.outPath);
runPostBuildHook(
worker.store,
*logger,
drvPath,
outputPaths
);
/* It is now safe to delete the lock files, since all future
lockers will see that the output paths are valid; they will
not create new lock files with the same names as the old
(unlinked) lock files. */
outputLocks.setDeletion(true);
outputLocks.unlock();
co_return done(BuildResult::Built, std::move(builtOutputs));
}
Goal::Co DerivationGoal::resolvedFinished()