From ce749454dc3e7685092cafdb4d1e05876a065b07 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 5 Oct 2025 21:54:59 +0300 Subject: [PATCH] Revert "Merge pull request #14022 from obsidiansystems/derivation-resolution-goal" This reverts commit d02dca099f2f7411489b57fc5c97968013498f9a, reversing changes made to 9bd09155ac7659f07dfefbd47e4e76ec499f38cd. --- .../build/derivation-building-goal.cc | 223 +++++++++++++++++- src/libstore/build/derivation-goal.cc | 97 +------- .../build/derivation-resolution-goal.cc | 210 ----------------- src/libstore/build/worker.cc | 24 +- .../store/build/derivation-building-goal.hh | 19 +- .../nix/store/build/derivation-goal.hh | 8 +- .../store/build/derivation-resolution-goal.hh | 82 ------- .../include/nix/store/build/worker.hh | 20 +- src/libstore/include/nix/store/meson.build | 1 - src/libstore/meson.build | 1 - tests/functional/build.sh | 9 +- tests/functional/ca/issue-13247.sh | 5 +- 12 files changed, 237 insertions(+), 462 deletions(-) delete mode 100644 src/libstore/build/derivation-resolution-goal.cc delete mode 100644 src/libstore/include/nix/store/build/derivation-resolution-goal.hh diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index fa819c96b..001816ca0 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -1,5 +1,6 @@ #include "nix/store/build/derivation-building-goal.hh" #include "nix/store/build/derivation-env-desugar.hh" +#include "nix/store/build/derivation-trampoline-goal.hh" #ifndef _WIN32 // TODO enable build hook on Windows # include "nix/store/build/hook-instance.hh" # include "nix/store/build/derivation-builder.hh" @@ -26,8 +27,8 @@ namespace nix { DerivationBuildingGoal::DerivationBuildingGoal( - const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode, bool storeDerivation) - : Goal(worker, gaveUpOnSubstitution(storeDerivation)) + const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode) + : Goal(worker, gaveUpOnSubstitution()) , drvPath(drvPath) , buildMode(buildMode) { @@ -124,10 +125,50 @@ static void runPostBuildHook( /* At least one of the output paths could not be produced using a substitute. So we have to build instead. */ -Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) +Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() { Goals waitees; + std::map, GoalPtr, value_comparison> inputGoals; + + { + std::function, const DerivedPathMap::ChildNode &)> + addWaiteeDerivedPath; + + addWaiteeDerivedPath = [&](ref inputDrv, + const DerivedPathMap::ChildNode & inputNode) { + if (!inputNode.value.empty()) { + auto g = worker.makeGoal( + DerivedPath::Built{ + .drvPath = inputDrv, + .outputs = inputNode.value, + }, + buildMode == bmRepair ? bmRepair : bmNormal); + inputGoals.insert_or_assign(inputDrv, g); + waitees.insert(std::move(g)); + } + for (const auto & [outputName, childNode] : inputNode.childMap) + addWaiteeDerivedPath( + make_ref(SingleDerivedPath::Built{inputDrv, outputName}), childNode); + }; + + for (const auto & [inputDrvPath, inputNode] : drv->inputDrvs.map) { + /* Ensure that pure, non-fixed-output derivations don't + depend on impure derivations. */ + if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && !drv->type().isImpure() + && !drv->type().isFixed()) { + auto inputDrv = worker.evalStore.readDerivation(inputDrvPath); + if (inputDrv.type().isImpure()) + throw Error( + "pure derivation '%s' depends on impure derivation '%s'", + worker.store.printStorePath(drvPath), + worker.store.printStorePath(inputDrvPath)); + } + + addWaiteeDerivedPath(makeConstantStorePathRef(inputDrvPath), inputNode); + } + } + /* Copy the input sources from the eval store to the build store. @@ -172,17 +213,177 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) /* Determine the full set of input paths. */ - if (storeDerivation) { - assert(drv->inputDrvs.map.empty()); - /* Store the resolved derivation, as part of the record of - what we're actually building */ - writeDerivation(worker.store, *drv); - } - + /* First, the input derivations. */ { + auto & fullDrv = *drv; + + auto drvType = fullDrv.type(); + bool resolveDrv = + std::visit( + overloaded{ + [&](const DerivationType::InputAddressed & ia) { + /* must resolve if deferred. */ + return ia.deferred; + }, + [&](const DerivationType::ContentAddressed & ca) { + return !fullDrv.inputDrvs.map.empty() + && (ca.fixed + /* Can optionally resolve if fixed, which is good + for avoiding unnecessary rebuilds. */ + ? experimentalFeatureSettings.isEnabled(Xp::CaDerivations) + /* Must resolve if floating and there are any inputs + drvs. */ + : true); + }, + [&](const DerivationType::Impure &) { return true; }}, + drvType.raw) + /* no inputs are outputs of dynamic derivations */ + || std::ranges::any_of(fullDrv.inputDrvs.map.begin(), fullDrv.inputDrvs.map.end(), [](auto & pair) { + return !pair.second.childMap.empty(); + }); + + if (resolveDrv && !fullDrv.inputDrvs.map.empty()) { + experimentalFeatureSettings.require(Xp::CaDerivations); + + /* We are be able to resolve this derivation based on the + now-known results of dependencies. If so, we become a + stub goal aliasing that resolved derivation goal. */ + std::optional attempt = fullDrv.tryResolve( + worker.store, + [&](ref drvPath, const std::string & outputName) -> std::optional { + auto mEntry = get(inputGoals, drvPath); + if (!mEntry) + return std::nullopt; + + auto & buildResult = (*mEntry)->buildResult; + return std::visit( + overloaded{ + [](const BuildResult::Failure &) -> std::optional { return std::nullopt; }, + [&](const BuildResult::Success & success) -> std::optional { + auto i = get(success.builtOutputs, outputName); + if (!i) + return std::nullopt; + + return i->outPath; + }, + }, + buildResult.inner); + }); + if (!attempt) { + /* TODO (impure derivations-induced tech debt) (see below): + The above attempt should have found it, but because we manage + inputDrvOutputs statefully, sometimes it gets out of sync with + the real source of truth (store). So we query the store + directly if there's a problem. */ + attempt = fullDrv.tryResolve(worker.store, &worker.evalStore); + } + assert(attempt); + Derivation drvResolved{std::move(*attempt)}; + + auto pathResolved = writeDerivation(worker.store, drvResolved); + + auto msg = + fmt("resolved derivation: '%s' -> '%s'", + worker.store.printStorePath(drvPath), + worker.store.printStorePath(pathResolved)); + act = std::make_unique( + *logger, + lvlInfo, + actBuildWaiting, + msg, + Logger::Fields{ + worker.store.printStorePath(drvPath), + worker.store.printStorePath(pathResolved), + }); + + /* TODO https://github.com/NixOS/nix/issues/13247 we should + let the calling goal do this, so it has a change to pass + just the output(s) it cares about. */ + auto resolvedDrvGoal = + worker.makeDerivationTrampolineGoal(pathResolved, OutputsSpec::All{}, drvResolved, buildMode); + { + Goals waitees{resolvedDrvGoal}; + co_await await(std::move(waitees)); + } + + trace("resolved derivation finished"); + + auto resolvedResult = resolvedDrvGoal->buildResult; + + // No `std::visit` for coroutines yet + if (auto * successP = resolvedResult.tryGetSuccess()) { + auto & success = *successP; + SingleDrvOutputs builtOutputs; + + auto outputHashes = staticOutputHashes(worker.evalStore, *drv); + auto resolvedHashes = staticOutputHashes(worker.store, drvResolved); + + StorePathSet outputPaths; + + for (auto & outputName : drvResolved.outputNames()) { + auto outputHash = get(outputHashes, outputName); + auto resolvedHash = get(resolvedHashes, outputName); + if ((!outputHash) || (!resolvedHash)) + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)", + worker.store.printStorePath(drvPath), + outputName); + + auto realisation = [&] { + auto take1 = get(success.builtOutputs, outputName); + if (take1) + return *take1; + + /* The above `get` should work. But stateful tracking of + outputs in resolvedResult, this can get out of sync with the + store, which is our actual source of truth. For now we just + check the store directly if it fails. */ + auto take2 = worker.evalStore.queryRealisation(DrvOutput{*resolvedHash, outputName}); + if (take2) + return *take2; + + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)", + worker.store.printStorePath(pathResolved), + outputName); + }(); + + if (!drv->type().isImpure()) { + auto newRealisation = realisation; + newRealisation.id = DrvOutput{*outputHash, outputName}; + newRealisation.signatures.clear(); + if (!drv->type().isFixed()) { + auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store; + newRealisation.dependentRealisations = + drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore); + } + worker.store.signRealisation(newRealisation); + worker.store.registerDrvOutput(newRealisation); + } + outputPaths.insert(realisation.outPath); + builtOutputs.emplace(outputName, realisation); + } + + runPostBuildHook(worker.store, *logger, drvPath, outputPaths); + + auto status = success.status; + if (status == BuildResult::Success::AlreadyValid) + status = BuildResult::Success::ResolvesToAlreadyValid; + + co_return doneSuccess(success.status, std::move(builtOutputs)); + } else if (resolvedResult.tryGetFailure()) { + co_return doneFailure({ + BuildResult::Failure::DependencyFailed, + "build of resolved derivation '%s' failed", + worker.store.printStorePath(pathResolved), + }); + } else + assert(false); + } + /* If we get this far, we know no dynamic drvs inputs */ - for (auto & [depDrvPath, depNode] : drv->inputDrvs.map) { + for (auto & [depDrvPath, depNode] : fullDrv.inputDrvs.map) { for (auto & outputName : depNode.value) { /* Don't need to worry about `inputGoals`, because impure derivations are always resolved above. Can diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index cc3ba2b7b..5dfc334a8 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1,6 +1,5 @@ #include "nix/store/build/derivation-goal.hh" #include "nix/store/build/derivation-building-goal.hh" -#include "nix/store/build/derivation-resolution-goal.hh" #ifndef _WIN32 // TODO enable build hook on Windows # include "nix/store/build/hook-instance.hh" # include "nix/store/build/derivation-builder.hh" @@ -30,9 +29,8 @@ DerivationGoal::DerivationGoal( const Derivation & drv, const OutputName & wantedOutput, Worker & worker, - BuildMode buildMode, - bool storeDerivation) - : Goal(worker, haveDerivation(storeDerivation)) + BuildMode buildMode) + : Goal(worker, haveDerivation()) , drvPath(drvPath) , wantedOutput(wantedOutput) , outputHash{[&] { @@ -66,7 +64,7 @@ std::string DerivationGoal::key() }.to_string(worker.store); } -Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) +Goal::Co DerivationGoal::haveDerivation() { trace("have derivation"); @@ -148,96 +146,9 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) worker.store.printStorePath(drvPath)); } - auto resolutionGoal = worker.makeDerivationResolutionGoal(drvPath, *drv, buildMode); - { - Goals waitees{resolutionGoal}; - co_await await(std::move(waitees)); - } - if (nrFailed != 0) { - co_return doneFailure({BuildResult::Failure::DependencyFailed, "resolution failed"}); - } - - if (resolutionGoal->resolvedDrv) { - auto & [pathResolved, drvResolved] = *resolutionGoal->resolvedDrv; - - auto resolvedDrvGoal = - worker.makeDerivationGoal(pathResolved, drvResolved, wantedOutput, buildMode, /*storeDerivation=*/true); - { - Goals waitees{resolvedDrvGoal}; - co_await await(std::move(waitees)); - } - - trace("resolved derivation finished"); - - auto resolvedResult = resolvedDrvGoal->buildResult; - - // No `std::visit` for coroutines yet - if (auto * successP = resolvedResult.tryGetSuccess()) { - auto & success = *successP; - auto outputHashes = staticOutputHashes(worker.evalStore, *drv); - auto resolvedHashes = staticOutputHashes(worker.store, drvResolved); - - StorePathSet outputPaths; - - auto outputHash = get(outputHashes, wantedOutput); - auto resolvedHash = get(resolvedHashes, wantedOutput); - if ((!outputHash) || (!resolvedHash)) - throw Error( - "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)", - worker.store.printStorePath(drvPath), - wantedOutput); - - auto realisation = [&] { - auto take1 = get(success.builtOutputs, wantedOutput); - if (take1) - return *take1; - - /* The above `get` should work. But stateful tracking of - outputs in resolvedResult, this can get out of sync with the - store, which is our actual source of truth. For now we just - check the store directly if it fails. */ - auto take2 = worker.evalStore.queryRealisation(DrvOutput{*resolvedHash, wantedOutput}); - if (take2) - return *take2; - - throw Error( - "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)", - worker.store.printStorePath(pathResolved), - wantedOutput); - }(); - - if (!drv->type().isImpure()) { - auto newRealisation = realisation; - newRealisation.id = DrvOutput{*outputHash, wantedOutput}; - newRealisation.signatures.clear(); - if (!drv->type().isFixed()) { - auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store; - newRealisation.dependentRealisations = - drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore); - } - worker.store.signRealisation(newRealisation); - worker.store.registerDrvOutput(newRealisation); - } - outputPaths.insert(realisation.outPath); - - auto status = success.status; - if (status == BuildResult::Success::AlreadyValid) - status = BuildResult::Success::ResolvesToAlreadyValid; - - co_return doneSuccess(status, std::move(realisation)); - } else if (resolvedResult.tryGetFailure()) { - co_return doneFailure({ - BuildResult::Failure::DependencyFailed, - "build of resolved derivation '%s' failed", - worker.store.printStorePath(pathResolved), - }); - } else - assert(false); - } - /* Give up on substitution for the output we want, actually build this derivation */ - auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode, storeDerivation); + auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode); /* We will finish with it ourselves, as if we were the derivational goal. */ g->preserveException = true; diff --git a/src/libstore/build/derivation-resolution-goal.cc b/src/libstore/build/derivation-resolution-goal.cc deleted file mode 100644 index 584169ef3..000000000 --- a/src/libstore/build/derivation-resolution-goal.cc +++ /dev/null @@ -1,210 +0,0 @@ -#include "nix/store/build/derivation-resolution-goal.hh" -#include "nix/store/build/derivation-env-desugar.hh" -#include "nix/store/build/worker.hh" -#include "nix/util/util.hh" -#include "nix/store/common-protocol.hh" -#include "nix/store/globals.hh" - -#include -#include -#include - -#include - -namespace nix { - -DerivationResolutionGoal::DerivationResolutionGoal( - const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode) - : Goal(worker, resolveDerivation()) - , drvPath(drvPath) -{ - drv = std::make_unique(drv_); - - name = fmt("building of '%s' from in-memory derivation", worker.store.printStorePath(drvPath)); - trace("created"); - - /* Prevent the .chroot directory from being - garbage-collected. (See isActiveTempFile() in gc.cc.) */ - worker.store.addTempRoot(this->drvPath); -} - -void DerivationResolutionGoal::timedOut(Error && ex) {} - -std::string DerivationResolutionGoal::key() -{ - /* Ensure that derivations get built in order of their name, - i.e. a derivation named "aardvark" always comes before - "baboon". And substitution goals always happen before - derivation goals (due to "bd$"). */ - return "rd$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath); -} - -/** - * Used for `inputGoals` local variable below - */ -struct value_comparison -{ - template - bool operator()(const ref & lhs, const ref & rhs) const - { - return *lhs < *rhs; - } -}; - -/* At least one of the output paths could not be - produced using a substitute. So we have to build instead. */ -Goal::Co DerivationResolutionGoal::resolveDerivation() -{ - Goals waitees; - - std::map, GoalPtr, value_comparison> inputGoals; - - { - std::function, const DerivedPathMap::ChildNode &)> - addWaiteeDerivedPath; - - addWaiteeDerivedPath = [&](ref inputDrv, - const DerivedPathMap::ChildNode & inputNode) { - if (!inputNode.value.empty()) { - auto g = worker.makeGoal( - DerivedPath::Built{ - .drvPath = inputDrv, - .outputs = inputNode.value, - }, - buildMode == bmRepair ? bmRepair : bmNormal); - inputGoals.insert_or_assign(inputDrv, g); - waitees.insert(std::move(g)); - } - for (const auto & [outputName, childNode] : inputNode.childMap) - addWaiteeDerivedPath( - make_ref(SingleDerivedPath::Built{inputDrv, outputName}), childNode); - }; - - for (const auto & [inputDrvPath, inputNode] : drv->inputDrvs.map) { - /* Ensure that pure, non-fixed-output derivations don't - depend on impure derivations. */ - if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && !drv->type().isImpure() - && !drv->type().isFixed()) { - auto inputDrv = worker.evalStore.readDerivation(inputDrvPath); - if (inputDrv.type().isImpure()) - throw Error( - "pure derivation '%s' depends on impure derivation '%s'", - worker.store.printStorePath(drvPath), - worker.store.printStorePath(inputDrvPath)); - } - - addWaiteeDerivedPath(makeConstantStorePathRef(inputDrvPath), inputNode); - } - } - - co_await await(std::move(waitees)); - - trace("all inputs realised"); - - if (nrFailed != 0) { - auto msg = - fmt("Cannot build '%s'.\n" - "Reason: " ANSI_RED "%d %s failed" ANSI_NORMAL ".", - Magenta(worker.store.printStorePath(drvPath)), - nrFailed, - nrFailed == 1 ? "dependency" : "dependencies"); - msg += showKnownOutputs(worker.store, *drv); - co_return amDone(ecFailed, {BuildError(BuildResult::Failure::DependencyFailed, msg)}); - } - - /* Gather information necessary for computing the closure and/or - running the build hook. */ - - /* Determine the full set of input paths. */ - - /* First, the input derivations. */ - { - auto & fullDrv = *drv; - - auto drvType = fullDrv.type(); - bool resolveDrv = - std::visit( - overloaded{ - [&](const DerivationType::InputAddressed & ia) { - /* must resolve if deferred. */ - return ia.deferred; - }, - [&](const DerivationType::ContentAddressed & ca) { - return !fullDrv.inputDrvs.map.empty() - && (ca.fixed - /* Can optionally resolve if fixed, which is good - for avoiding unnecessary rebuilds. */ - ? experimentalFeatureSettings.isEnabled(Xp::CaDerivations) - /* Must resolve if floating and there are any inputs - drvs. */ - : true); - }, - [&](const DerivationType::Impure &) { return true; }}, - drvType.raw) - /* no inputs are outputs of dynamic derivations */ - || std::ranges::any_of(fullDrv.inputDrvs.map.begin(), fullDrv.inputDrvs.map.end(), [](auto & pair) { - return !pair.second.childMap.empty(); - }); - - if (resolveDrv && !fullDrv.inputDrvs.map.empty()) { - experimentalFeatureSettings.require(Xp::CaDerivations); - - /* We are be able to resolve this derivation based on the - now-known results of dependencies. If so, we become a - stub goal aliasing that resolved derivation goal. */ - std::optional attempt = fullDrv.tryResolve( - worker.store, - [&](ref drvPath, const std::string & outputName) -> std::optional { - auto mEntry = get(inputGoals, drvPath); - if (!mEntry) - return std::nullopt; - - auto & buildResult = (*mEntry)->buildResult; - return std::visit( - overloaded{ - [](const BuildResult::Failure &) -> std::optional { return std::nullopt; }, - [&](const BuildResult::Success & success) -> std::optional { - auto i = get(success.builtOutputs, outputName); - if (!i) - return std::nullopt; - - return i->outPath; - }, - }, - buildResult.inner); - }); - if (!attempt) { - /* TODO (impure derivations-induced tech debt) (see below): - The above attempt should have found it, but because we manage - inputDrvOutputs statefully, sometimes it gets out of sync with - the real source of truth (store). So we query the store - directly if there's a problem. */ - attempt = fullDrv.tryResolve(worker.store, &worker.evalStore); - } - assert(attempt); - - auto pathResolved = writeDerivation(worker.store, *attempt, NoRepair, /*readOnly =*/true); - - auto msg = - fmt("resolved derivation: '%s' -> '%s'", - worker.store.printStorePath(drvPath), - worker.store.printStorePath(pathResolved)); - act = std::make_unique( - *logger, - lvlInfo, - actBuildWaiting, - msg, - Logger::Fields{ - worker.store.printStorePath(drvPath), - worker.store.printStorePath(pathResolved), - }); - - resolvedDrv = - std::make_unique>(std::move(pathResolved), *std::move(attempt)); - } - } - - co_return amDone(ecSuccess, std::nullopt); -} - -} // namespace nix diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 53175a8c4..3e6e0bef0 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -4,7 +4,6 @@ #include "nix/store/build/substitution-goal.hh" #include "nix/store/build/drv-output-substitution-goal.hh" #include "nix/store/build/derivation-goal.hh" -#include "nix/store/build/derivation-resolution-goal.hh" #include "nix/store/build/derivation-building-goal.hh" #include "nix/store/build/derivation-trampoline-goal.hh" #ifndef _WIN32 // TODO Enable building on Windows @@ -76,26 +75,15 @@ std::shared_ptr Worker::makeDerivationTrampolineGoal( } std::shared_ptr Worker::makeDerivationGoal( - const StorePath & drvPath, - const Derivation & drv, - const OutputName & wantedOutput, - BuildMode buildMode, - bool storeDerivation) + const StorePath & drvPath, const Derivation & drv, const OutputName & wantedOutput, BuildMode buildMode) { - return initGoalIfNeeded( - derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode, storeDerivation); + return initGoalIfNeeded(derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode); } -std::shared_ptr -Worker::makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode) +std::shared_ptr +Worker::makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode) { - return initGoalIfNeeded(derivationResolutionGoals[drvPath], drvPath, drv, *this, buildMode); -} - -std::shared_ptr Worker::makeDerivationBuildingGoal( - const StorePath & drvPath, const Derivation & drv, BuildMode buildMode, bool storeDerivation) -{ - return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode, storeDerivation); + return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode); } std::shared_ptr @@ -170,8 +158,6 @@ void Worker::removeGoal(GoalPtr goal) nix::removeGoal(drvGoal, derivationTrampolineGoals.map); else if (auto drvGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvGoal, derivationGoals); - else if (auto drvResolutionGoal = std::dynamic_pointer_cast(goal)) - nix::removeGoal(drvResolutionGoal, derivationResolutionGoals); else if (auto drvBuildingGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvBuildingGoal, derivationBuildingGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index ab063ff3f..edb496024 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -29,21 +29,8 @@ typedef enum { rpAccept, rpDecline, rpPostpone } HookReply; */ struct DerivationBuildingGoal : public Goal { - /** - * @param storeDerivation Whether to store the derivation in - * `worker.store`. This is useful for newly-resolved derivations. In this - * case, the derivation was not created a priori, e.g. purely (or close - * enough) from evaluation of the Nix language, but also depends on the - * exact content produced by upstream builds. It is strongly advised to - * have a permanent record of such a resolved derivation in order to - * faithfully reconstruct the build history. - */ DerivationBuildingGoal( - const StorePath & drvPath, - const Derivation & drv, - Worker & worker, - BuildMode buildMode = bmNormal, - bool storeDerivation = false); + const StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal); ~DerivationBuildingGoal(); private: @@ -113,7 +100,7 @@ private: /** * The states. */ - Co gaveUpOnSubstitution(bool storeDerivation); + Co gaveUpOnSubstitution(); Co tryToBuild(); /** @@ -168,7 +155,7 @@ private: JobCategory jobCategory() const override { - return JobCategory::Administration; + return JobCategory::Build; }; }; diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index 353e7c489..e05bf1c0b 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -40,16 +40,12 @@ struct DerivationGoal : public Goal */ OutputName wantedOutput; - /** - * @param storeDerivation See `DerivationBuildingGoal`. This is just passed along. - */ DerivationGoal( const StorePath & drvPath, const Derivation & drv, const OutputName & wantedOutput, Worker & worker, - BuildMode buildMode = bmNormal, - bool storeDerivation = false); + BuildMode buildMode = bmNormal); ~DerivationGoal() = default; void timedOut(Error && ex) override @@ -84,7 +80,7 @@ private: /** * The states. */ - Co haveDerivation(bool storeDerivation); + Co haveDerivation(); /** * Return `std::nullopt` if the output is unknown, e.g. un unbuilt diff --git a/src/libstore/include/nix/store/build/derivation-resolution-goal.hh b/src/libstore/include/nix/store/build/derivation-resolution-goal.hh deleted file mode 100644 index ebaab4f06..000000000 --- a/src/libstore/include/nix/store/build/derivation-resolution-goal.hh +++ /dev/null @@ -1,82 +0,0 @@ -#pragma once -///@file - -#include "nix/store/derivations.hh" -#include "nix/store/derivation-options.hh" -#include "nix/store/build/derivation-building-misc.hh" -#include "nix/store/store-api.hh" -#include "nix/store/build/goal.hh" - -namespace nix { - -struct BuilderFailureError; - -/** - * A goal for resolving a derivation. Resolving a derivation (@see - * `Derivation::tryResolve`) simplifies its inputs, replacing - * `inputDrvs` with `inputSrcs. - * - * Conceptually, we resolve all derivations. For input-addressed - * derivations (that don't transtively depend on content-addressed - * derivations), however, we don't actually use the resolved derivation, - * because the output paths would appear invalid (if we tried to verify - * them), since they are computed from the original, unresolved inputs. - * - * That said, if we ever made the new flavor of input-addressing as described - * in issue #9259, then the input-addressing would be based on the resolved - * inputs, and we like the CA case *would* use the output of this goal. - * - * (The point of this discussion is not to randomly stuff information on - * a yet-unimplemented feature (issue #9259) in the codebase, but - * rather, to illustrate that there is no inherent tension between - * explicit derivation resolution and input-addressing in general. That - * tension only exists with the type of input-addressing we've - * historically used.) - */ -struct DerivationResolutionGoal : public Goal -{ - DerivationResolutionGoal( - const StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal); - - /** - * If the derivation needed to be resolved, this is resulting - * resolved derivations and its path. - */ - std::unique_ptr> resolvedDrv; - - void timedOut(Error && ex) override; - -private: - - /** - * The path of the derivation. - */ - StorePath drvPath; - - /** - * The derivation stored at drvPath. - */ - std::unique_ptr drv; - - /** - * The remainder is state held during the build. - */ - - BuildMode buildMode; - - std::unique_ptr act; - - std::string key() override; - - /** - * The states. - */ - Co resolveDerivation(); - - JobCategory jobCategory() const override - { - return JobCategory::Administration; - }; -}; - -} // namespace nix diff --git a/src/libstore/include/nix/store/build/worker.hh b/src/libstore/include/nix/store/build/worker.hh index 9767590ac..a6de780c1 100644 --- a/src/libstore/include/nix/store/build/worker.hh +++ b/src/libstore/include/nix/store/build/worker.hh @@ -16,7 +16,6 @@ namespace nix { /* Forward definition. */ struct DerivationTrampolineGoal; struct DerivationGoal; -struct DerivationResolutionGoal; struct DerivationBuildingGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; @@ -112,7 +111,6 @@ private: DerivedPathMap>> derivationTrampolineGoals; std::map>> derivationGoals; - std::map> derivationResolutionGoals; std::map> derivationBuildingGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; @@ -223,23 +221,13 @@ public: const StorePath & drvPath, const Derivation & drv, const OutputName & wantedOutput, - BuildMode buildMode = bmNormal, - bool storeDerivation = false); + BuildMode buildMode = bmNormal); /** - * @ref DerivationResolutionGoal "derivation resolution goal" + * @ref DerivationBuildingGoal "derivation goal" */ - std::shared_ptr - makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode = bmNormal); - - /** - * @ref DerivationBuildingGoal "derivation building goal" - */ - std::shared_ptr makeDerivationBuildingGoal( - const StorePath & drvPath, - const Derivation & drv, - BuildMode buildMode = bmNormal, - bool storeDerivation = false); + std::shared_ptr + makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode = bmNormal); /** * @ref PathSubstitutionGoal "substitution goal" diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index 1f04e357a..c9e4c36dd 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -18,7 +18,6 @@ headers = [ config_pub_h ] + files( 'build/derivation-building-misc.hh', 'build/derivation-env-desugar.hh', 'build/derivation-goal.hh', - 'build/derivation-resolution-goal.hh', 'build/derivation-trampoline-goal.hh', 'build/drv-output-substitution-goal.hh', 'build/goal.hh', diff --git a/src/libstore/meson.build b/src/libstore/meson.build index e220e65cd..a3502c2e0 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -302,7 +302,6 @@ sources = files( 'build/derivation-check.cc', 'build/derivation-env-desugar.cc', 'build/derivation-goal.cc', - 'build/derivation-resolution-goal.cc', 'build/derivation-trampoline-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', diff --git a/tests/functional/build.sh b/tests/functional/build.sh index c9a39438d..0a19ff7da 100755 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -178,8 +178,7 @@ test "$(<<<"$out" grep -cE '^error:')" = 4 out="$(nix build -f fod-failing.nix -L x4 2>&1)" && status=0 || status=$? test "$status" = 1 -# Precise number of errors depends on daemon version / goal refactorings -(( "$(<<<"$out" grep -cE '^error:')" >= 2 )) +test "$(<<<"$out" grep -cE '^error:')" = 2 if isDaemonNewer "2.29pre"; then <<<"$out" grepQuiet -E "error: Cannot build '.*-x4\\.drv'" @@ -187,13 +186,11 @@ if isDaemonNewer "2.29pre"; then else <<<"$out" grepQuiet -E "error: 1 dependencies of derivation '.*-x4\\.drv' failed to build" fi -# Either x2 or x3 could have failed, x4 depends on both symmetrically -<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x[23]\\.drv'" +<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'" out="$(nix build -f fod-failing.nix -L x4 --keep-going 2>&1)" && status=0 || status=$? test "$status" = 1 -# Precise number of errors depends on daemon version / goal refactorings -(( "$(<<<"$out" grep -cE '^error:')" >= 3 )) +test "$(<<<"$out" grep -cE '^error:')" = 3 if isDaemonNewer "2.29pre"; then <<<"$out" grepQuiet -E "error: Cannot build '.*-x4\\.drv'" <<<"$out" grepQuiet -E "Reason: 2 dependencies failed." diff --git a/tests/functional/ca/issue-13247.sh b/tests/functional/ca/issue-13247.sh index 705919513..686d90ced 100755 --- a/tests/functional/ca/issue-13247.sh +++ b/tests/functional/ca/issue-13247.sh @@ -65,4 +65,7 @@ buildViaSubstitute use-a-prime-more-outputs^first # Should only fetch the output we asked for [[ -d "$(jq -r <"$TEST_ROOT"/a.json '.[0].outputs.out')" ]] [[ -f "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.first')" ]] -[[ ! -e "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.second')" ]] + +# Output should *not* be here, this is the bug +[[ -e "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.second')" ]] +skipTest "bug is not yet fixed"