From ba1a256d0875592b28d902f3e40663b2adedfe9c Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 23 Feb 2021 14:12:11 +0100 Subject: [PATCH] Make `DerivationGoal::drv` a full Derivation This field used to be a `BasicDerivation`, but this `BasicDerivation` was downcasted to a `Derivation` when needed (implicitely or not), so we might as well make it a full `Derivation` and upcast it when needed. This also allows getting rid of a weird duplication in the way we compute the static output hashes for the derivation. We had to do it differently and in a different place depending on whether the derivation was a full derivation or just a basic drv, but we can now do it unconditionally on the full derivation. Fix #4559 --- src/libstore/build/derivation-goal.cc | 37 ++++++++++----------------- src/libstore/build/derivation-goal.hh | 2 +- 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index d8a89a2d0..804a79e4c 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -123,17 +123,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation , wantedOutputs(wantedOutputs) , buildMode(buildMode) { - this->drv = std::make_unique(BasicDerivation(drv)); - - auto outputHashes = staticOutputHashes(worker.store, drv); - for (auto &[outputName, outputHash] : outputHashes) - initialOutputs.insert({ - outputName, - InitialOutput{ - .wanted = true, // Will be refined later - .outputHash = outputHash - } - }); + this->drv = std::make_unique(drv); state = &DerivationGoal::haveDerivation; name = fmt( @@ -271,18 +261,8 @@ void DerivationGoal::loadDerivation() auto fullDrv = new Derivation(worker.store.derivationFromPath(drvPath)); - auto outputHashes = staticOutputHashes(worker.store, *fullDrv); - for (auto &[outputName, outputHash] : outputHashes) - initialOutputs.insert({ - outputName, - InitialOutput{ - .wanted = true, // Will be refined later - .outputHash = outputHash - } - }); - /* Get the derivation. */ - drv = std::unique_ptr(fullDrv); + drv = std::unique_ptr(fullDrv); haveDerivation(); } @@ -301,6 +281,16 @@ void DerivationGoal::haveDerivation() if (i.second.second) worker.store.addTempRoot(*i.second.second); + auto outputHashes = staticOutputHashes(worker.store, *drv); + for (auto &[outputName, outputHash] : outputHashes) + initialOutputs.insert({ + outputName, + InitialOutput{ + .wanted = true, // Will be refined later + .outputHash = outputHash + } + }); + /* Check what outputs paths are not already valid. */ checkPathValidity(); bool allValid = true; @@ -3517,10 +3507,9 @@ void DerivationGoal::registerOutputs() but it's fine to do in all cases. */ if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - auto outputHashes = staticOutputHashes(worker.store, *drv); for (auto& [outputName, newInfo] : infos) worker.store.registerDrvOutput(Realisation{ - .id = DrvOutput{outputHashes.at(outputName), outputName}, + .id = DrvOutput{initialOutputs.at(outputName).outputHash, outputName}, .outPath = newInfo.path}); } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 761100d3a..6dc164922 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -64,7 +64,7 @@ struct DerivationGoal : public Goal bool retrySubstitution; /* The derivation stored at drvPath. */ - std::unique_ptr drv; + std::unique_ptr drv; std::unique_ptr parsedDrv;