From 53946fe0171cc30259940b2ac83a6e03782b3bf5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Feb 2025 18:33:00 -0500 Subject: [PATCH 1/4] Narrow scope on some local variables --- src/libstore/build/derivation-goal.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 4d97250d3..401e2cda1 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -235,12 +235,14 @@ Goal::Co DerivationGoal::haveDerivation() } }); - /* Check what outputs paths are not already valid. */ - auto [allValid, validOutputs] = checkPathValidity(); + { + /* Check what outputs paths are not already valid. */ + auto [allValid, validOutputs] = checkPathValidity(); - /* If they are all valid, then we're done. */ - if (allValid && buildMode == bmNormal) { - co_return done(BuildResult::AlreadyValid, std::move(validOutputs)); + /* If they are all valid, then we're done. */ + if (allValid && buildMode == bmNormal) { + co_return done(BuildResult::AlreadyValid, std::move(validOutputs)); + } } /* We are first going to try to create the invalid output paths From 41274f3c3e82d3b7f13147f1828890c1e0e8d141 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Feb 2025 18:33:58 -0500 Subject: [PATCH 2/4] Inline `outputsSubstitutionTried` --- src/libstore/build/derivation-goal.cc | 5 ----- src/libstore/build/derivation-goal.hh | 1 - 2 files changed, 6 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 401e2cda1..2993333fd 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -270,12 +270,7 @@ Goal::Co DerivationGoal::haveDerivation() } if (!waitees.empty()) co_await Suspend{}; /* to prevent hang (no wake-up event) */ - co_return outputsSubstitutionTried(); -} - -Goal::Co DerivationGoal::outputsSubstitutionTried() -{ trace("all outputs substituted (maybe)"); assert(!drv->type().isImpure()); diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index ad3d9ca2a..82fee3539 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -236,7 +236,6 @@ struct DerivationGoal : public Goal Co getDerivation(); Co loadDerivation(); Co haveDerivation(); - Co outputsSubstitutionTried(); Co gaveUpOnSubstitution(); Co closureRepaired(); Co inputsRealised(); From 57463ab910bf5ca31342e73f8f413d67128fb957 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Feb 2025 18:36:58 -0500 Subject: [PATCH 3/4] Inline `closureRepaired` --- src/libstore/build/derivation-goal.cc | 17 ++++++----------- src/libstore/build/derivation-goal.hh | 1 - 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 2993333fd..4d0a7ebc7 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -457,21 +457,16 @@ Goal::Co DerivationGoal::repairClosure() co_return done(BuildResult::AlreadyValid, assertPathValidity()); } else { co_await Suspend{}; - co_return closureRepaired(); + + trace("closure repaired"); + if (nrFailed > 0) + throw Error("some paths in the output closure of derivation '%s' could not be repaired", + worker.store.printStorePath(drvPath)); + co_return done(BuildResult::AlreadyValid, assertPathValidity()); } } -Goal::Co DerivationGoal::closureRepaired() -{ - trace("closure repaired"); - if (nrFailed > 0) - throw Error("some paths in the output closure of derivation '%s' could not be repaired", - worker.store.printStorePath(drvPath)); - co_return done(BuildResult::AlreadyValid, assertPathValidity()); -} - - Goal::Co DerivationGoal::inputsRealised() { trace("all inputs realised"); diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 82fee3539..c21a12e4a 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -237,7 +237,6 @@ struct DerivationGoal : public Goal Co loadDerivation(); Co haveDerivation(); Co gaveUpOnSubstitution(); - Co closureRepaired(); Co inputsRealised(); Co tryToBuild(); virtual Co tryLocalBuild(); From 2297cc0daba549a1b8d2278ffb02bb3edd734f38 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Feb 2025 18:56:42 -0500 Subject: [PATCH 4/4] Inline `getDerivation` and `loadDerivation` --- src/libstore/build/derivation-goal.cc | 80 +++++++++++---------------- src/libstore/build/derivation-goal.hh | 2 - 2 files changed, 33 insertions(+), 49 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 4d0a7ebc7..70d2d30b1 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -36,14 +36,6 @@ namespace nix { -Goal::Co DerivationGoal::init() { - if (useDerivation) { - co_return getDerivation(); - } else { - co_return haveDerivation(); - } -} - DerivationGoal::DerivationGoal(const StorePath & drvPath, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) : Goal(worker, DerivedPath::Built { .drvPath = makeConstantStorePathRef(drvPath), .outputs = wantedOutputs }) @@ -141,50 +133,44 @@ void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) } -Goal::Co DerivationGoal::getDerivation() -{ +Goal::Co DerivationGoal::init() { trace("init"); - /* The first thing to do is to make sure that the derivation - exists. If it doesn't, it may be created through a - substitute. */ - if (buildMode == bmNormal && worker.evalStore.isValidPath(drvPath)) { - co_return loadDerivation(); - } + if (useDerivation) { + /* The first thing to do is to make sure that the derivation + exists. If it doesn't, it may be created through a + substitute. */ - addWaitee(upcast_goal(worker.makePathSubstitutionGoal(drvPath))); - - co_await Suspend{}; - co_return loadDerivation(); -} - - -Goal::Co DerivationGoal::loadDerivation() -{ - trace("loading derivation"); - - if (nrFailed != 0) { - co_return done(BuildResult::MiscFailure, {}, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath))); - } - - /* `drvPath' should already be a root, but let's be on the safe - side: if the user forgot to make it a root, we wouldn't want - things being garbage collected while we're busy. */ - worker.evalStore.addTempRoot(drvPath); - - /* Get the derivation. It is probably in the eval store, but it might be inthe main store: - - - Resolved derivation are resolved against main store realisations, and so must be stored there. - - - Dynamic derivations are built, and so are found in the main store. - */ - for (auto * drvStore : { &worker.evalStore, &worker.store }) { - if (drvStore->isValidPath(drvPath)) { - drv = std::make_unique(drvStore->readDerivation(drvPath)); - break; + if (buildMode != bmNormal || !worker.evalStore.isValidPath(drvPath)) { + addWaitee(upcast_goal(worker.makePathSubstitutionGoal(drvPath))); + co_await Suspend{}; } + + trace("loading derivation"); + + if (nrFailed != 0) { + co_return done(BuildResult::MiscFailure, {}, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath))); + } + + /* `drvPath' should already be a root, but let's be on the safe + side: if the user forgot to make it a root, we wouldn't want + things being garbage collected while we're busy. */ + worker.evalStore.addTempRoot(drvPath); + + /* Get the derivation. It is probably in the eval store, but it might be inthe main store: + + - Resolved derivation are resolved against main store realisations, and so must be stored there. + + - Dynamic derivations are built, and so are found in the main store. + */ + for (auto * drvStore : { &worker.evalStore, &worker.store }) { + if (drvStore->isValidPath(drvPath)) { + drv = std::make_unique(drvStore->readDerivation(drvPath)); + break; + } + } + assert(drv); } - assert(drv); co_return haveDerivation(); } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index c21a12e4a..652fca035 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -233,8 +233,6 @@ struct DerivationGoal : public Goal * The states. */ Co init() override; - Co getDerivation(); - Co loadDerivation(); Co haveDerivation(); Co gaveUpOnSubstitution(); Co inputsRealised();