From a4604f19284254ac98f19a13ff7c2216de7fe176 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 8 Mar 2022 19:50:46 +0100 Subject: Add Store::buildPathsWithResults() This function is like buildPaths(), except that it returns a vector of BuildResults containing the exact statuses and output paths of each derivation / substitution. This is convenient for functions like Installable::build(), because they then don't need to do another series of calls to get the outputs of CA derivations. It's also a precondition to impure derivations, where we *can't* query the output of those derivations since they're not stored in the Nix database. Note that PathSubstitutionGoal can now also return a BuildStatus. --- src/libstore/build/derivation-goal.cc | 177 +++++++++++---------- src/libstore/build/derivation-goal.hh | 31 ++-- src/libstore/build/drv-output-substitution-goal.cc | 4 +- src/libstore/build/entry-points.cc | 51 +++--- src/libstore/build/goal.hh | 4 + src/libstore/build/local-derivation-goal.cc | 96 ++++++----- src/libstore/build/local-derivation-goal.hh | 2 +- src/libstore/build/substitution-goal.cc | 18 ++- src/libstore/build/substitution-goal.hh | 2 + 9 files changed, 209 insertions(+), 176 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index ae250ffaf..cd582bb01 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -135,7 +135,7 @@ void DerivationGoal::killChild() void DerivationGoal::timedOut(Error && ex) { killChild(); - done(BuildResult::TimedOut, ex); + done(BuildResult::TimedOut, {}, ex); } @@ -182,7 +182,7 @@ void DerivationGoal::loadDerivation() trace("loading derivation"); if (nrFailed != 0) { - done(BuildResult::MiscFailure, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath))); + done(BuildResult::MiscFailure, {}, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath))); return; } @@ -215,28 +215,20 @@ void DerivationGoal::haveDerivation() auto outputHashes = staticOutputHashes(worker.evalStore, *drv); for (auto & [outputName, outputHash] : outputHashes) - initialOutputs.insert({ + initialOutputs.insert({ outputName, - InitialOutput{ + InitialOutput { .wanted = true, // Will be refined later .outputHash = outputHash } - }); + }); /* Check what outputs paths are not already valid. */ - checkPathValidity(); - bool allValid = true; - for (auto & [_, status] : initialOutputs) { - if (!status.wanted) continue; - if (!status.known || !status.known->isValid()) { - allValid = false; - break; - } - } + auto [allValid, validOutputs] = checkPathValidity(); /* If they are all valid, then we're done. */ if (allValid && buildMode == bmNormal) { - done(BuildResult::AlreadyValid); + done(BuildResult::AlreadyValid, std::move(validOutputs)); return; } @@ -277,7 +269,7 @@ void DerivationGoal::outputsSubstitutionTried() trace("all outputs substituted (maybe)"); if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) { - done(BuildResult::TransientFailure, + done(BuildResult::TransientFailure, {}, Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", worker.store.printStorePath(drvPath))); return; @@ -301,23 +293,17 @@ void DerivationGoal::outputsSubstitutionTried() return; } - checkPathValidity(); - size_t nrInvalid = 0; - for (auto & [_, status] : initialOutputs) { - if (!status.wanted) continue; - if (!status.known || !status.known->isValid()) - nrInvalid++; - } + auto [allValid, validOutputs] = checkPathValidity(); - if (buildMode == bmNormal && nrInvalid == 0) { - done(BuildResult::Substituted); + if (buildMode == bmNormal && allValid) { + done(BuildResult::Substituted, std::move(validOutputs)); return; } - if (buildMode == bmRepair && nrInvalid == 0) { + if (buildMode == bmRepair && allValid) { repairClosure(); return; } - if (buildMode == bmCheck && nrInvalid > 0) + if (buildMode == bmCheck && !allValid) throw Error("some outputs of '%s' are not valid, so checking is not possible", worker.store.printStorePath(drvPath)); @@ -409,7 +395,7 @@ void DerivationGoal::repairClosure() } if (waitees.empty()) { - done(BuildResult::AlreadyValid); + done(BuildResult::AlreadyValid, assertPathValidity()); return; } @@ -423,7 +409,7 @@ void DerivationGoal::closureRepaired() if (nrFailed > 0) throw Error("some paths in the output closure of derivation '%s' could not be repaired", worker.store.printStorePath(drvPath)); - done(BuildResult::AlreadyValid); + done(BuildResult::AlreadyValid, assertPathValidity()); } @@ -434,7 +420,7 @@ void DerivationGoal::inputsRealised() if (nrFailed != 0) { if (!useDerivation) throw Error("some dependencies of '%s' are missing", worker.store.printStorePath(drvPath)); - done(BuildResult::DependencyFailed, Error( + done(BuildResult::DependencyFailed, {}, Error( "%s dependencies of derivation '%s' failed to build", nrFailed, worker.store.printStorePath(drvPath))); return; @@ -523,10 +509,11 @@ void DerivationGoal::inputsRealised() state = &DerivationGoal::tryToBuild; worker.wakeUp(shared_from_this()); - result = BuildResult(); + buildResult = BuildResult(); } -void DerivationGoal::started() { +void DerivationGoal::started() +{ auto msg = fmt( buildMode == bmRepair ? "repairing outputs of '%s'" : buildMode == bmCheck ? "checking outputs of '%s'" : @@ -588,19 +575,12 @@ void DerivationGoal::tryToBuild() omitted, but that would be less efficient.) Note that since we now hold the locks on the output paths, no other process can build this derivation, so no further checks are necessary. */ - checkPathValidity(); - bool allValid = true; - for (auto & [_, status] : initialOutputs) { - if (!status.wanted) continue; - if (!status.known || !status.known->isValid()) { - allValid = false; - break; - } - } + auto [allValid, validOutputs] = checkPathValidity(); + if (buildMode != bmCheck && allValid) { debug("skipping build of derivation '%s', someone beat us to it", worker.store.printStorePath(drvPath)); outputLocks.setDeletion(true); - done(BuildResult::AlreadyValid); + done(BuildResult::AlreadyValid, std::move(validOutputs)); return; } @@ -626,7 +606,7 @@ void DerivationGoal::tryToBuild() /* Yes, it has started doing so. Wait until we get EOF from the hook. */ actLock.reset(); - result.startTime = time(0); // inexact + buildResult.startTime = time(0); // inexact state = &DerivationGoal::buildDone; started(); return; @@ -830,8 +810,8 @@ void DerivationGoal::buildDone() debug("builder process for '%s' finished", worker.store.printStorePath(drvPath)); - result.timesBuilt++; - result.stopTime = time(0); + buildResult.timesBuilt++; + buildResult.stopTime = time(0); /* So the child is gone now. */ worker.childTerminated(this); @@ -876,11 +856,11 @@ void DerivationGoal::buildDone() /* Compute the FS closure of the outputs and register them as being valid. */ - registerOutputs(); + auto builtOutputs = registerOutputs(); StorePathSet outputPaths; - for (auto & [_, path] : finalOutputs) - outputPaths.insert(path); + for (auto & [_, output] : buildResult.builtOutputs) + outputPaths.insert(output.outPath); runPostBuildHook( worker.store, *logger, @@ -890,7 +870,7 @@ void DerivationGoal::buildDone() if (buildMode == bmCheck) { cleanupPostOutputsRegisteredModeCheck(); - done(BuildResult::Built); + done(BuildResult::Built, std::move(builtOutputs)); return; } @@ -911,6 +891,8 @@ void DerivationGoal::buildDone() outputLocks.setDeletion(true); outputLocks.unlock(); + done(BuildResult::Built, std::move(builtOutputs)); + } catch (BuildError & e) { outputLocks.unlock(); @@ -930,14 +912,13 @@ void DerivationGoal::buildDone() BuildResult::PermanentFailure; } - done(st, e); + done(st, {}, e); return; } - - done(BuildResult::Built); } -void DerivationGoal::resolvedFinished() { +void DerivationGoal::resolvedFinished() +{ assert(resolvedDrvGoal); auto resolvedDrv = *resolvedDrvGoal->drv; @@ -950,11 +931,13 @@ void DerivationGoal::resolvedFinished() { if (realWantedOutputs.empty()) realWantedOutputs = resolvedDrv.outputNames(); + DrvOutputs builtOutputs; + for (auto & wantedOutput : realWantedOutputs) { assert(initialOutputs.count(wantedOutput) != 0); assert(resolvedHashes.count(wantedOutput) != 0); auto realisation = worker.store.queryRealisation( - DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} + DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} ); // We've just built it, but maybe the build failed, in which case the // realisation won't be there @@ -966,10 +949,11 @@ void DerivationGoal::resolvedFinished() { signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); outputPaths.insert(realisation->outPath); + builtOutputs.emplace(realisation->id, *realisation); } else { // If we don't have a realisation, then it must mean that something // failed when building the resolved drv - assert(!result.success()); + assert(!buildResult.success()); } } @@ -981,7 +965,7 @@ void DerivationGoal::resolvedFinished() { ); auto status = [&]() { - auto resolvedResult = resolvedDrvGoal->getResult(); + auto & resolvedResult = resolvedDrvGoal->buildResult; switch (resolvedResult.status) { case BuildResult::AlreadyValid: return BuildResult::ResolvesToAlreadyValid; @@ -990,7 +974,7 @@ void DerivationGoal::resolvedFinished() { } }(); - done(status); + done(status, std::move(builtOutputs)); } HookReply DerivationGoal::tryBuildHook() @@ -1100,7 +1084,7 @@ HookReply DerivationGoal::tryBuildHook() } -void DerivationGoal::registerOutputs() +DrvOutputs DerivationGoal::registerOutputs() { /* When using a build hook, the build hook can register the output as valid (by doing `nix-store --import'). If so we don't have @@ -1109,21 +1093,7 @@ void DerivationGoal::registerOutputs() We can only early return when the outputs are known a priori. For floating content-addressed derivations this isn't the case. */ - for (auto & [outputName, optOutputPath] : worker.store.queryPartialDerivationOutputMap(drvPath)) { - if (!wantOutput(outputName, wantedOutputs)) - continue; - if (!optOutputPath) - throw BuildError( - "output '%s' from derivation '%s' does not have a known output path", - outputName, worker.store.printStorePath(drvPath)); - auto & outputPath = *optOutputPath; - if (!worker.store.isValidPath(outputPath)) - throw BuildError( - "output '%s' from derivation '%s' is supposed to be at '%s' but that path is not valid", - outputName, worker.store.printStorePath(drvPath), worker.store.printStorePath(outputPath)); - - finalOutputs.insert_or_assign(outputName, outputPath); - } + return assertPathValidity(); } Path DerivationGoal::openLogFile() @@ -1185,7 +1155,7 @@ void DerivationGoal::handleChildOutput(int fd, std::string_view data) if (settings.maxLogSize && logSize > settings.maxLogSize) { killChild(); done( - BuildResult::LogLimitExceeded, + BuildResult::LogLimitExceeded, {}, Error("%s killed after writing more than %d bytes of log output", getName(), settings.maxLogSize)); return; @@ -1274,10 +1244,12 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap() } -void DerivationGoal::checkPathValidity() +std::pair DerivationGoal::checkPathValidity() { bool checkHash = buildMode == bmRepair; auto wantedOutputsLeft = wantedOutputs; + DrvOutputs validOutputs; + for (auto & i : queryPartialDerivationOutputMap()) { InitialOutput & info = initialOutputs.at(i.first); info.wanted = wantOutput(i.first, wantedOutputs); @@ -1294,26 +1266,28 @@ void DerivationGoal::checkPathValidity() : PathStatus::Corrupt, }; } + auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first}; if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first}; if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { .path = real->outPath, .status = PathStatus::Valid, }; - } else if (info.known && info.known->status == PathStatus::Valid) { - // We know the output because it' a static output of the + } else if (info.known && info.known->isValid()) { + // We know the output because it's a static output of the // derivation, and the output path is valid, but we don't have // its realisation stored (probably because it has been built - // without the `ca-derivations` experimental flag) + // without the `ca-derivations` experimental flag). worker.store.registerDrvOutput( - Realisation{ + Realisation { drvOutput, info.known->path, } ); } } + if (info.wanted && info.known && info.known->isValid()) + validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path }); } // If we requested all the outputs via the empty set, we are always fine. // If we requested specific elements, the loop above removes all the valid @@ -1322,24 +1296,51 @@ void DerivationGoal::checkPathValidity() throw Error("derivation '%s' does not have wanted outputs %s", worker.store.printStorePath(drvPath), concatStringsSep(", ", quoteStrings(wantedOutputsLeft))); + + bool allValid = true; + for (auto & [_, status] : initialOutputs) { + if (!status.wanted) continue; + if (!status.known || !status.known->isValid()) { + allValid = false; + break; + } + } + + return { allValid, validOutputs }; +} + + +DrvOutputs DerivationGoal::assertPathValidity() +{ + auto [allValid, validOutputs] = checkPathValidity(); + if (!allValid) + throw Error("some outputs are unexpectedly invalid"); + return validOutputs; } -void DerivationGoal::done(BuildResult::Status status, std::optional ex) +void DerivationGoal::done( + BuildResult::Status status, + DrvOutputs builtOutputs, + std::optional ex) { - result.status = status; + buildResult.drvPath = drvPath; + buildResult.status = status; if (ex) - result.errorMsg = ex->what(); - amDone(result.success() ? ecSuccess : ecFailed, ex); - if (result.status == BuildResult::TimedOut) + // FIXME: strip: "error: " + buildResult.errorMsg = ex->what(); + amDone(buildResult.success() ? ecSuccess : ecFailed, ex); + if (buildResult.status == BuildResult::TimedOut) worker.timedOut = true; - if (result.status == BuildResult::PermanentFailure) + if (buildResult.status == BuildResult::PermanentFailure) worker.permanentFailure = true; mcExpectedBuilds.reset(); mcRunningBuilds.reset(); - if (result.success()) { + if (buildResult.success()) { + assert(!builtOutputs.empty()); + buildResult.builtOutputs = std::move(builtOutputs); if (status == BuildResult::Built) worker.doneBuilds++; } else { @@ -1353,7 +1354,7 @@ void DerivationGoal::done(BuildResult::Status status, std::optional ex) if (traceBuiltOutputsFile != "") { std::fstream fs; fs.open(traceBuiltOutputsFile, std::fstream::out); - fs << worker.store.printStorePath(drvPath) << "\t" << result.toString() << std::endl; + fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl; } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 6f158bdbf..ea2db89b2 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -2,7 +2,6 @@ #include "parsed-derivations.hh" #include "lock.hh" -#include "build-result.hh" #include "store-api.hh" #include "pathlocks.hh" #include "goal.hh" @@ -105,20 +104,8 @@ struct DerivationGoal : public Goal typedef void (DerivationGoal::*GoalState)(); GoalState state; - /* The final output paths of the build. - - - For input-addressed derivations, always the precomputed paths - - - For content-addressed derivations, calcuated from whatever the hash - ends up being. (Note that fixed outputs derivations that produce the - "wrong" output still install that data under its true content-address.) - */ - OutputPathMap finalOutputs; - BuildMode buildMode; - BuildResult result; - /* The current round, if we're building multiple times. */ size_t curRound = 1; @@ -153,8 +140,6 @@ struct DerivationGoal : public Goal /* Add wanted outputs to an already existing derivation goal. */ void addWantedOutputs(const StringSet & outputs); - BuildResult getResult() { return result; } - /* The states. */ void getDerivation(); void loadDerivation(); @@ -176,7 +161,7 @@ struct DerivationGoal : public Goal /* Check that the derivation outputs all exist and register them as valid. */ - virtual void registerOutputs(); + virtual DrvOutputs registerOutputs(); /* Open a log file and a pipe to it. */ Path openLogFile(); @@ -211,8 +196,17 @@ struct DerivationGoal : public Goal std::map> queryPartialDerivationOutputMap(); OutputPathMap queryDerivationOutputMap(); - /* Return the set of (in)valid paths. */ - void checkPathValidity(); + /* Update 'initialOutputs' to determine the current status of the + outputs of the derivation. Also returns a Boolean denoting + whether all outputs are valid and non-corrupt, and a + 'DrvOutputs' structure containing the valid and wanted + outputs. */ + std::pair checkPathValidity(); + + /* Aborts if any output is not valid or corrupt, and otherwise + returns a 'DrvOutputs' structure containing the wanted + outputs. */ + DrvOutputs assertPathValidity(); /* Forcibly kill the child process, if any. */ virtual void killChild(); @@ -223,6 +217,7 @@ struct DerivationGoal : public Goal void done( BuildResult::Status status, + DrvOutputs builtOutputs = {}, std::optional ex = {}); StorePathSet exportReferences(const StorePathSet & storePaths); diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index e6e810cdd..946ec1aff 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -32,7 +32,7 @@ void DrvOutputSubstitutionGoal::init() void DrvOutputSubstitutionGoal::tryNext() { - trace("Trying next substituter"); + trace("trying next substituter"); if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal @@ -119,7 +119,7 @@ void DrvOutputSubstitutionGoal::realisationFetched() void DrvOutputSubstitutionGoal::outPathValid() { assert(outputInfo); - trace("Output path substituted"); + trace("output path substituted"); if (nrFailed > 0) { debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 9b4cfd835..b2f87aa82 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -47,6 +47,35 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod } } +std::vector Store::buildPathsWithResults( + const std::vector & reqs, + BuildMode buildMode, + std::shared_ptr evalStore) +{ + Worker worker(*this, evalStore ? *evalStore : *this); + + Goals goals; + for (const auto & br : reqs) { + std::visit(overloaded { + [&](const DerivedPath::Built & bfd) { + goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode)); + }, + [&](const DerivedPath::Opaque & bo) { + goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair)); + }, + }, br.raw()); + } + + worker.run(goals); + + std::vector results; + + for (auto & i : goals) + results.push_back(i->buildResult); + + return results; +} + BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode) { @@ -57,31 +86,11 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat try { worker.run(Goals{goal}); - result = goal->getResult(); + result = goal->buildResult; } catch (Error & e) { result.status = BuildResult::MiscFailure; result.errorMsg = e.msg(); } - // XXX: Should use `goal->queryPartialDerivationOutputMap()` once it's - // extended to return the full realisation for each output - auto staticDrvOutputs = drv.outputsAndOptPaths(*this); - auto outputHashes = staticOutputHashes(*this, drv); - for (auto & [outputName, staticOutput] : staticDrvOutputs) { - auto outputId = DrvOutput{outputHashes.at(outputName), outputName}; - if (staticOutput.second) - result.builtOutputs.insert_or_assign( - outputId, - Realisation{ outputId, *staticOutput.second} - ); - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && !derivationHasKnownOutputPaths(drv.type())) { - auto realisation = this->queryRealisation(outputId); - if (realisation) - result.builtOutputs.insert_or_assign( - outputId, - *realisation - ); - } - } return result; } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 68e08bdf9..fcf3d0084 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -2,6 +2,7 @@ #include "types.hh" #include "store-api.hh" +#include "build-result.hh" namespace nix { @@ -55,6 +56,9 @@ struct Goal : public std::enable_shared_from_this /* Whether the goal is finished. */ ExitCode exitCode; + /* Build result. */ + BuildResult buildResult; + /* Exception containing an error message, if any. */ std::optional ex; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 581d63d0e..a372728f5 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -194,7 +194,7 @@ void LocalDerivationGoal::tryLocalBuild() { outputLocks.unlock(); buildUser.reset(); worker.permanentFailure = true; - done(BuildResult::InputRejected, e); + done(BuildResult::InputRejected, {}, e); return; } @@ -756,7 +756,7 @@ void LocalDerivationGoal::startBuilder() if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term)) throw SysError("putting pseudoterminal into raw mode"); - result.startTime = time(0); + buildResult.startTime = time(0); /* Fork a child to build the package. */ @@ -1260,6 +1260,16 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo } void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override + { + for (auto & result : buildPathsWithResults(paths, buildMode, evalStore)) + if (!result.success()) + result.rethrow(); + } + + std::vector buildPathsWithResults( + const std::vector & paths, + BuildMode buildMode = bmNormal, + std::shared_ptr evalStore = nullptr) override { assert(!evalStore); @@ -1273,26 +1283,13 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo throw InvalidPath("cannot build '%s' in recursive Nix because path is unknown", req.to_string(*next)); } - next->buildPaths(paths, buildMode); - - for (auto & path : paths) { - auto p = std::get_if(&path); - if (!p) continue; - auto & bfd = *p; - auto drv = readDerivation(bfd.drvPath); - auto drvHashes = staticOutputHashes(*this, drv); - auto outputs = next->queryDerivationOutputMap(bfd.drvPath); - for (auto & [outputName, outputPath] : outputs) - if (wantOutput(outputName, bfd.outputs)) { - newPaths.insert(outputPath); - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - auto thisRealisation = next->queryRealisation( - DrvOutput{drvHashes.at(outputName), outputName} - ); - assert(thisRealisation); - newRealisations.insert(*thisRealisation); - } - } + auto results = next->buildPathsWithResults(paths, buildMode); + + for (auto & result : results) { + for (auto & [outputName, output] : result.builtOutputs) { + newPaths.insert(output.outPath); + newRealisations.insert(output); + } } StorePathSet closure; @@ -1301,6 +1298,8 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo goal.addDependency(path); for (auto & real : Realisation::closure(*next, newRealisations)) goal.addedDrvOutputs.insert(real.id); + + return results; } BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, @@ -2069,7 +2068,7 @@ void LocalDerivationGoal::runChild() } -void LocalDerivationGoal::registerOutputs() +DrvOutputs LocalDerivationGoal::registerOutputs() { /* When using a build hook, the build hook can register the output as valid (by doing `nix-store --import'). If so we don't have @@ -2078,10 +2077,8 @@ void LocalDerivationGoal::registerOutputs() We can only early return when the outputs are known a priori. For floating content-addressed derivations this isn't the case. */ - if (hook) { - DerivationGoal::registerOutputs(); - return; - } + if (hook) + return DerivationGoal::registerOutputs(); std::map infos; @@ -2204,6 +2201,8 @@ void LocalDerivationGoal::registerOutputs() std::reverse(sortedOutputNames.begin(), sortedOutputNames.end()); + OutputPathMap finalOutputs; + for (auto & outputName : sortedOutputNames) { auto output = drv->outputs.at(outputName); auto & scratchPath = scratchOutputs.at(outputName); @@ -2340,6 +2339,7 @@ void LocalDerivationGoal::registerOutputs() }; ValidPathInfo newInfo = std::visit(overloaded { + [&](const DerivationOutputInputAddressed & output) { /* input-addressed case */ auto requiredFinalPath = output.path; @@ -2359,6 +2359,7 @@ void LocalDerivationGoal::registerOutputs() newInfo0.references.insert(newInfo0.path); return newInfo0; }, + [&](const DerivationOutputCAFixed & dof) { auto newInfo0 = newInfoFromCA(DerivationOutputCAFloating { .method = dof.hash.method, @@ -2381,14 +2382,17 @@ void LocalDerivationGoal::registerOutputs() } return newInfo0; }, - [&](DerivationOutputCAFloating dof) { + + [&](DerivationOutputCAFloating & dof) { return newInfoFromCA(dof); }, + [&](DerivationOutputDeferred) -> ValidPathInfo { // No derivation should reach that point without having been // rewritten first assert(false); }, + }, output.output); /* FIXME: set proper permissions in restorePath() so @@ -2499,11 +2503,12 @@ void LocalDerivationGoal::registerOutputs() } if (buildMode == bmCheck) { - // In case of FOD mismatches on `--check` an error must be thrown as this is also - // a source for non-determinism. + /* In case of fixed-output derivations, if there are + mismatches on `--check` an error must be thrown as this is + also a source for non-determinism. */ if (delayedException) std::rethrow_exception(delayedException); - return; + return assertPathValidity(); } /* Apply output checks. */ @@ -2515,7 +2520,7 @@ void LocalDerivationGoal::registerOutputs() assert(prevInfos.size() == infos.size()); for (auto i = prevInfos.begin(), j = infos.begin(); i != prevInfos.end(); ++i, ++j) if (!(*i == *j)) { - result.isNonDeterministic = true; + buildResult.isNonDeterministic = true; Path prev = worker.store.printStorePath(i->second.path) + checkSuffix; bool prevExists = keepPreviousRound && pathExists(prev); hintformat hint = prevExists @@ -2553,7 +2558,7 @@ void LocalDerivationGoal::registerOutputs() if (curRound < nrRounds) { prevInfos = std::move(infos); - return; + return {}; } /* Remove the .check directories if we're done. FIXME: keep them @@ -2588,17 +2593,24 @@ void LocalDerivationGoal::registerOutputs() means it's safe to link the derivation to the output hash. We must do that for floating CA derivations, which otherwise couldn't be cached, but it's fine to do in all cases. */ + DrvOutputs builtOutputs; - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - for (auto& [outputName, newInfo] : infos) { - auto thisRealisation = Realisation{ - .id = DrvOutput{initialOutputs.at(outputName).outputHash, - outputName}, - .outPath = newInfo.path}; + for (auto & [outputName, newInfo] : infos) { + auto thisRealisation = Realisation { + .id = DrvOutput { + initialOutputs.at(outputName).outputHash, + outputName + }, + .outPath = newInfo.path + }; + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } + builtOutputs.emplace(thisRealisation.id, thisRealisation); } + + return builtOutputs; } void LocalDerivationGoal::signRealisation(Realisation & realisation) @@ -2607,7 +2619,7 @@ void LocalDerivationGoal::signRealisation(Realisation & realisation) } -void LocalDerivationGoal::checkOutputs(const std::map & outputs) +void LocalDerivationGoal::checkOutputs(const std::map & outputs) { std::map outputsByPath; for (auto & output : outputs) @@ -2679,8 +2691,8 @@ void LocalDerivationGoal::checkOutputs(const std::map & out for (auto & i : *value) { if (worker.store.isStorePath(i)) spec.insert(worker.store.parseStorePath(i)); - else if (finalOutputs.count(i)) - spec.insert(finalOutputs.at(i)); + else if (outputs.count(i)) + spec.insert(outputs.at(i).path); else throw BuildError("derivation contains an illegal reference specifier '%s'", i); } diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 95692c60d..d456e9cae 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -169,7 +169,7 @@ struct LocalDerivationGoal : public DerivationGoal /* Check that the derivation outputs all exist and register them as valid. */ - void registerOutputs() override; + DrvOutputs registerOutputs() override; void signRealisation(Realisation &) override; diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index c1bb1941d..ec500baf8 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -24,6 +24,14 @@ PathSubstitutionGoal::~PathSubstitutionGoal() } +void PathSubstitutionGoal::done(ExitCode result, BuildResult::Status status) +{ + buildResult.outPath = storePath; + buildResult.status = status; + amDone(result); +} + + void PathSubstitutionGoal::work() { (this->*state)(); @@ -38,7 +46,7 @@ void PathSubstitutionGoal::init() /* If the path already exists we're done. */ if (!repair && worker.store.isValidPath(storePath)) { - amDone(ecSuccess); + done(ecSuccess, BuildResult::AlreadyValid); return; } @@ -65,7 +73,7 @@ void PathSubstitutionGoal::tryNext() /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a build. */ - amDone(substituterFailed ? ecFailed : ecNoSubstituters); + done(substituterFailed ? ecFailed : ecNoSubstituters, BuildResult::NoSubstituters); if (substituterFailed) { worker.failedSubstitutions++; @@ -163,7 +171,9 @@ void PathSubstitutionGoal::referencesValid() if (nrFailed > 0) { debug("some references of path '%s' could not be realised", worker.store.printStorePath(storePath)); - amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); + done( + nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed, + BuildResult::DependencyFailed); return; } @@ -268,7 +278,7 @@ void PathSubstitutionGoal::finished() worker.updateProgress(); - amDone(ecSuccess); + done(ecSuccess, BuildResult::Substituted); } diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index d8399d2a8..946f13841 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -53,6 +53,8 @@ struct PathSubstitutionGoal : public Goal /* Content address for recomputing store path */ std::optional ca; + void done(ExitCode result, BuildResult::Status status); + public: PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); ~PathSubstitutionGoal(); -- cgit v1.2.3 From 761242afa08d5c9280ba6bd63a310b4334b83bb2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 9 Mar 2022 12:25:35 +0100 Subject: BuildResult: Use DerivedPath --- src/libstore/build/derivation-goal.cc | 6 +++--- src/libstore/build/drv-output-substitution-goal.cc | 8 ++++++-- src/libstore/build/entry-points.cc | 15 +++++++-------- src/libstore/build/goal.hh | 4 +++- src/libstore/build/substitution-goal.cc | 3 +-- 5 files changed, 20 insertions(+), 16 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index cd582bb01..325635e2e 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -66,7 +66,7 @@ namespace nix { DerivationGoal::DerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker) + : Goal(worker, DerivedPath::Built { .drvPath = drvPath, .outputs = wantedOutputs }) , useDerivation(true) , drvPath(drvPath) , wantedOutputs(wantedOutputs) @@ -85,7 +85,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker) + : Goal(worker, DerivedPath::Built { .drvPath = drvPath, .outputs = wantedOutputs }) , useDerivation(false) , drvPath(drvPath) , wantedOutputs(wantedOutputs) @@ -509,7 +509,7 @@ void DerivationGoal::inputsRealised() state = &DerivationGoal::tryToBuild; worker.wakeUp(shared_from_this()); - buildResult = BuildResult(); + buildResult = BuildResult { .path = buildResult.path }; } void DerivationGoal::started() diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 946ec1aff..e50292c1e 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -6,8 +6,12 @@ namespace nix { -DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal(const DrvOutput& id, Worker & worker, RepairFlag repair, std::optional ca) - : Goal(worker) +DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal( + const DrvOutput & id, + Worker & worker, + RepairFlag repair, + std::optional ca) + : Goal(worker, DerivedPath::Opaque { StorePath::dummy }) , id(id) { state = &DrvOutputSubstitutionGoal::init; diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index b2f87aa82..bea7363db 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -82,17 +82,16 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat Worker worker(*this, *this); auto goal = worker.makeBasicDerivationGoal(drvPath, drv, {}, buildMode); - BuildResult result; - try { worker.run(Goals{goal}); - result = goal->buildResult; + return goal->buildResult; } catch (Error & e) { - result.status = BuildResult::MiscFailure; - result.errorMsg = e.msg(); - } - - return result; + return BuildResult { + .status = BuildResult::MiscFailure, + .errorMsg = e.msg(), + .path = DerivedPath::Built { .drvPath = drvPath }, + }; + }; } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index fcf3d0084..07c752bb9 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -62,7 +62,9 @@ struct Goal : public std::enable_shared_from_this /* Exception containing an error message, if any. */ std::optional ex; - Goal(Worker & worker) : worker(worker) + Goal(Worker & worker, DerivedPath path) + : worker(worker) + , buildResult { .path = std::move(path) } { nrFailed = nrNoSubstituters = nrIncompleteClosure = 0; exitCode = ecBusy; diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index ec500baf8..31e6dbc9f 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -6,7 +6,7 @@ namespace nix { PathSubstitutionGoal::PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair, std::optional ca) - : Goal(worker) + : Goal(worker, DerivedPath::Opaque { storePath }) , storePath(storePath) , repair(repair) , ca(ca) @@ -26,7 +26,6 @@ PathSubstitutionGoal::~PathSubstitutionGoal() void PathSubstitutionGoal::done(ExitCode result, BuildResult::Status status) { - buildResult.outPath = storePath; buildResult.status = status; amDone(result); } -- cgit v1.2.3