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 +++++++++++++++++----------------- 1 file changed, 89 insertions(+), 88 deletions(-) (limited to 'src/libstore/build/derivation-goal.cc') 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; } } -- 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 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/libstore/build/derivation-goal.cc') 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() -- cgit v1.2.3