1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-09 03:56:01 +01:00

Get rid of DerivationBuilder::note*Mismatch

It's fine to set these worker flags a little later in the control flow,
since we'll be sure to reach those points in the error cases. And doing
that is much nicer than having these tangled callbacks.

I originally made the callbacks to meticulously recreate the exact
behavior which I didn't quite understand. Now, thanks to cleaning up the
error handling, I do understand what is going on, so I can be confident
that this change is safe to make.
This commit is contained in:
John Ericson 2025-08-28 11:01:34 -04:00
parent 169033001d
commit ff961fd9e2
4 changed files with 26 additions and 17 deletions

View file

@ -652,16 +652,6 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
goal.worker.childTerminated(&goal);
}
void noteHashMismatch() override
{
goal.worker.hashMismatch = true;
}
void noteCheckMismatch() override
{
goal.worker.checkMismatch = true;
}
void markContentsGood(const StorePath & path) override
{
goal.worker.markContentsGood(path);
@ -818,6 +808,26 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
// N.B. cannot use `std::visit` with co-routine return
if (auto * ste = std::get_if<0>(&res)) {
outputLocks.unlock();
// Allow selecting a subset of enum values
# pragma GCC diagnostic push
# pragma GCC diagnostic ignored "-Wswitch-enum"
switch (ste->status) {
case BuildResult::HashMismatch:
worker.hashMismatch = true;
/* See header, the protocols don't know about `HashMismatch`
yet, so change it to `OutputRejected`, which they expect
for this case (hash mismatch is a type of output
rejection). */
ste->status = BuildResult::OutputRejected;
break;
case BuildResult::NotDeterministic:
worker.checkMismatch = true;
break;
default:
/* Other statuses need no adjusting */
break;
}
# pragma GCC diagnostic pop
co_return doneFailure(std::move(*ste));
} else if (auto * builtOutputs = std::get_if<1>(&res)) {
StorePathSet outputPaths;

View file

@ -36,6 +36,10 @@ struct BuildResult
NotDeterministic,
ResolvesToAlreadyValid,
NoSubstituters,
/// A certain type of `OutputRejected`. The protocols do not yet
/// know about this one, so change it back to `OutputRejected`
/// before serialization.
HashMismatch,
} status = MiscFailure;
/**

View file

@ -134,9 +134,6 @@ struct DerivationBuilderCallbacks
*/
virtual void childTerminated() = 0;
virtual void noteHashMismatch(void) = 0;
virtual void noteCheckMismatch(void) = 0;
virtual void markContentsGood(const StorePath & path) = 0;
};

View file

@ -1659,9 +1659,8 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
if (wanted != got) {
/* Throw an error after registering the path as
valid. */
miscMethods->noteHashMismatch();
delayedException = std::make_exception_ptr(BuildError(
BuildResult::OutputRejected,
BuildResult::HashMismatch,
"hash mismatch in fixed-output derivation '%s':\n specified: %s\n got: %s",
store.printStorePath(drvPath),
wanted.to_string(HashFormat::SRI, true),
@ -1670,7 +1669,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
if (!newInfo0.references.empty()) {
auto numViolations = newInfo.references.size();
delayedException = std::make_exception_ptr(BuildError(
BuildResult::OutputRejected,
BuildResult::HashMismatch,
"fixed-output derivations must not reference store paths: '%s' references %d distinct paths, e.g. '%s'",
store.printStorePath(drvPath),
numViolations,
@ -1746,7 +1745,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
continue;
ValidPathInfo oldInfo(*store.queryPathInfo(newInfo.path));
if (newInfo.narHash != oldInfo.narHash) {
miscMethods->noteCheckMismatch();
if (settings.runDiffHook || settings.keepFailed) {
auto dst = store.toRealPath(finalDestPath + ".check");
deletePath(dst);