From 87c8d3d702123528ac068bb703232e24431c535e Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 27 Jan 2021 10:03:05 +0100 Subject: [PATCH] Register the realisations for unresolved drvs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once a build is done, get back to the original derivation, and register all the newly built outputs for this derivation. This allows Nix to work properly with derivations that don't have all their build inputs available − thus allowing garbage collection and (once it's implemented) binary substitution --- src/libstore/build/derivation-goal.cc | 54 ++++++++++++++++++++++++++- src/libstore/build/derivation-goal.hh | 3 ++ src/libstore/derivations.cc | 9 ++++- src/libstore/local-store.cc | 2 +- src/libstore/local-store.hh | 2 +- src/libstore/store-api.cc | 15 +------- src/libstore/store-api.hh | 6 --- 7 files changed, 67 insertions(+), 24 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index eeaec4f2c..315cf3f0a 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -506,6 +506,7 @@ void DerivationGoal::inputsRealised() Derivation drvResolved { *std::move(attempt) }; auto pathResolved = writeDerivation(worker.store, drvResolved); + resolvedDrv = drvResolved; auto msg = fmt("Resolved derivation: '%s' -> '%s'", worker.store.printStorePath(drvPath), @@ -1019,7 +1020,45 @@ void DerivationGoal::buildDone() } void DerivationGoal::resolvedFinished() { - done(BuildResult::Built); + assert(resolvedDrv); + + // If the derivation was originally a full `Derivation` (and not just + // a `BasicDerivation`, we must retrieve it because the `staticOutputHashes` + // will be wrong otherwise + Derivation fullDrv = *drv; + if (auto upcasted = dynamic_cast(drv.get())) + fullDrv = *upcasted; + + auto originalHashes = staticOutputHashes(worker.store, fullDrv); + auto resolvedHashes = staticOutputHashes(worker.store, *resolvedDrv); + + // `wantedOutputs` might be empty, which means “all the outputs” + auto realWantedOutputs = wantedOutputs; + if (realWantedOutputs.empty()) + realWantedOutputs = resolvedDrv->outputNames(); + + for (auto & wantedOutput : realWantedOutputs) { + assert(originalHashes.count(wantedOutput) != 0); + assert(resolvedHashes.count(wantedOutput) != 0); + auto realisation = worker.store.queryRealisation( + DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} + ); + // We've just built it, but maybe the build failed, in which case the + // realisation won't be there + if (realisation) { + auto newRealisation = *realisation; + newRealisation.id = DrvOutput{originalHashes.at(wantedOutput), wantedOutput}; + worker.store.registerDrvOutput(newRealisation); + } else { + // If we don't have a realisation, then it must mean that something + // failed when building the resolved drv + assert(!result.success()); + } + } + + // This is potentially a bit fishy in terms of error reporting. Not sure + // how to do it in a cleaner way + amDone(nrFailed == 0 ? ecSuccess : ecFailed, ex); } HookReply DerivationGoal::tryBuildHook() @@ -3804,6 +3843,19 @@ void DerivationGoal::checkPathValidity() : PathStatus::Corrupt, }; } + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + Derivation fullDrv = *drv; + if (auto upcasted = dynamic_cast(drv.get())) + fullDrv = *upcasted; + auto outputHashes = staticOutputHashes(worker.store, fullDrv); + if (auto real = worker.store.queryRealisation( + DrvOutput{outputHashes.at(i.first), i.first})) { + info.known = { + .path = real->outPath, + .status = PathStatus::Valid, + }; + } + } initialOutputs.insert_or_assign(i.first, info); } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 8ee0be9e1..b7b85c85d 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -48,6 +48,9 @@ struct DerivationGoal : public Goal /* The path of the derivation. */ StorePath drvPath; + /* The path of the corresponding resolved derivation */ + std::optional resolvedDrv; + /* The specific outputs that we need to build. Empty means all of them. */ StringSet wantedOutputs; diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 7466c7d41..4b774c42a 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -756,8 +756,13 @@ std::optional Derivation::tryResolveUncached(Store & store) { StringSet newOutputNames; for (auto & outputName : input.second) { auto actualPathOpt = inputDrvOutputs.at(outputName); - if (!actualPathOpt) + if (!actualPathOpt) { + warn("Input %s!%s missing, aborting the resolving", + store.printStorePath(input.first), + outputName + ); return std::nullopt; + } auto actualPath = *actualPathOpt; inputRewrites.emplace( downstreamPlaceholder(store, input.first, outputName), @@ -782,6 +787,8 @@ std::optional Derivation::tryResolve(Store& store, const StoreP // This is quite dirty and leaky, but will disappear once #4340 is merged static Sync>> resolutionsCache; + debug("Trying to resolve %s", store.printStorePath(drvPath)); + { auto resolutions = resolutionsCache.lock(); auto resolvedDrvIter = resolutions->find(drvPath); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index f45af2bac..e06c47cde 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -883,7 +883,7 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path) std::map> -LocalStore::queryDerivationOutputMapNoResolve(const StorePath& path_) +LocalStore::queryPartialDerivationOutputMap(const StorePath& path_) { auto path = path_; auto outputs = retrySQLite>>([&]() { diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 9d235ba0a..780cc0f07 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -127,7 +127,7 @@ public: StorePathSet queryValidDerivers(const StorePath & path) override; - std::map> queryDerivationOutputMapNoResolve(const StorePath & path) override; + std::map> queryPartialDerivationOutputMap(const StorePath & path) override; std::optional queryPathFromHashPart(const std::string & hashPart) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 37c11fe86..2658f7617 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -366,7 +366,7 @@ bool Store::PathInfoCacheValue::isKnownNow() return std::chrono::steady_clock::now() < time_point + ttl; } -std::map> Store::queryDerivationOutputMapNoResolve(const StorePath & path) +std::map> Store::queryPartialDerivationOutputMap(const StorePath & path) { std::map> outputs; auto drv = readInvalidDerivation(path); @@ -376,19 +376,6 @@ std::map> Store::queryDerivationOutputMapN return outputs; } -std::map> Store::queryPartialDerivationOutputMap(const StorePath & path) -{ - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - auto resolvedDrv = Derivation::tryResolve(*this, path); - if (resolvedDrv) { - auto resolvedDrvPath = writeDerivation(*this, *resolvedDrv, NoRepair, true); - if (isValidPath(resolvedDrvPath)) - return queryDerivationOutputMapNoResolve(resolvedDrvPath); - } - } - return queryDerivationOutputMapNoResolve(path); -} - OutputPathMap Store::queryDerivationOutputMap(const StorePath & path) { auto resp = queryPartialDerivationOutputMap(path); OutputPathMap result; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 9e98eb8f9..6dcd43ed1 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -415,12 +415,6 @@ public: `std::nullopt`. */ virtual std::map> queryPartialDerivationOutputMap(const StorePath & path); - /* - * Similar to `queryPartialDerivationOutputMap`, but doesn't try to resolve - * the derivation - */ - virtual std::map> queryDerivationOutputMapNoResolve(const StorePath & path); - /* Query the mapping outputName=>outputPath for the given derivation. Assume every output has a mapping and throw an exception otherwise. */ OutputPathMap queryDerivationOutputMap(const StorePath & path);