diff options
Diffstat (limited to 'src/libstore/build')
-rw-r--r-- | src/libstore/build/derivation-goal.cc | 33 | ||||
-rw-r--r-- | src/libstore/build/local-derivation-goal.cc | 118 | ||||
-rw-r--r-- | src/libstore/build/worker.cc | 7 |
3 files changed, 95 insertions, 63 deletions
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index d095a0f02..3fff2385f 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -984,21 +984,28 @@ void DerivationGoal::resolvedFinished() realWantedOutputs = resolvedDrv.outputNames(); for (auto & wantedOutput : realWantedOutputs) { - assert(initialOutputs.count(wantedOutput) != 0); - assert(resolvedHashes.count(wantedOutput) != 0); - auto realisation = resolvedResult.builtOutputs.at( - DrvOutput { resolvedHashes.at(wantedOutput), wantedOutput }); + auto initialOutput = get(initialOutputs, wantedOutput); + auto resolvedHash = get(resolvedHashes, wantedOutput); + if ((!initialOutput) || (!resolvedHash)) + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,resolve)", + worker.store.printStorePath(drvPath), wantedOutput); + auto realisation = get(resolvedResult.builtOutputs, DrvOutput { *resolvedHash, wantedOutput }); + if (!realisation) + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,realisation)", + worker.store.printStorePath(resolvedDrvGoal->drvPath), wantedOutput); if (drv->type().isPure()) { - auto newRealisation = realisation; - newRealisation.id = DrvOutput { initialOutputs.at(wantedOutput).outputHash, wantedOutput }; + auto newRealisation = *realisation; + newRealisation.id = DrvOutput { initialOutput->outputHash, wantedOutput }; newRealisation.signatures.clear(); if (!drv->type().isFixed()) - newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); + newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation->outPath); signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); } - outputPaths.insert(realisation.outPath); - builtOutputs.emplace(realisation.id, realisation); + outputPaths.insert(realisation->outPath); + builtOutputs.emplace(realisation->id, *realisation); } runPostBuildHook( @@ -1294,7 +1301,11 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() DrvOutputs validOutputs; for (auto & i : queryPartialDerivationOutputMap()) { - InitialOutput & info = initialOutputs.at(i.first); + auto initialOutput = get(initialOutputs, i.first); + if (!initialOutput) + // this is an invalid output, gets catched with (!wantedOutputsLeft.empty()) + continue; + auto & info = *initialOutput; info.wanted = wantOutput(i.first, wantedOutputs); if (info.wanted) wantedOutputsLeft.erase(i.first); @@ -1309,7 +1320,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() : PathStatus::Corrupt, }; } - auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first}; + auto drvOutput = DrvOutput{info.outputHash, i.first}; if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 4c91fa4fb..3ac9c20f9 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -14,6 +14,7 @@ #include "worker-protocol.hh" #include "topo-sort.hh" #include "callback.hh" +#include "json-utils.hh" #include <regex> #include <queue> @@ -56,8 +57,6 @@ #include <pwd.h> #include <grp.h> -#include <nlohmann/json.hpp> - namespace nix { void handleDiffHook( @@ -482,7 +481,7 @@ void LocalDerivationGoal::startBuilder() temporary build directory. The text files have the format used by `nix-store --register-validity'. However, the deriver fields are left empty. */ - auto s = get(drv->env, "exportReferencesGraph").value_or(""); + auto s = getOr(drv->env, "exportReferencesGraph", ""); Strings ss = tokenizeString<Strings>(s); if (ss.size() % 2 != 0) throw BuildError("odd number of tokens in 'exportReferencesGraph': '%1%'", s); @@ -989,7 +988,7 @@ void LocalDerivationGoal::initTmpDir() { there is no size constraint). */ if (!parsedDrv->getStructuredAttrs()) { - StringSet passAsFile = tokenizeString<StringSet>(get(drv->env, "passAsFile").value_or("")); + StringSet passAsFile = tokenizeString<StringSet>(getOr(drv->env, "passAsFile", "")); for (auto & i : drv->env) { if (passAsFile.find(i.first) == passAsFile.end()) { env[i.first] = i.second; @@ -2128,12 +2127,22 @@ DrvOutputs LocalDerivationGoal::registerOutputs() std::map<std::string, std::variant<AlreadyRegistered, PerhapsNeedToRegister>> outputReferencesIfUnregistered; std::map<std::string, struct stat> outputStats; for (auto & [outputName, _] : drv->outputs) { - auto actualPath = toRealPathChroot(worker.store.printStorePath(scratchOutputs.at(outputName))); + auto scratchOutput = get(scratchOutputs, outputName); + if (!scratchOutput) + throw BuildError( + "builder for '%s' has no scratch output for '%s'", + worker.store.printStorePath(drvPath), outputName); + auto actualPath = toRealPathChroot(worker.store.printStorePath(*scratchOutput)); outputsToSort.insert(outputName); /* Updated wanted info to remove the outputs we definitely don't need to register */ - auto & initialInfo = initialOutputs.at(outputName); + auto initialOutput = get(initialOutputs, outputName); + if (!initialOutput) + throw BuildError( + "builder for '%s' has no initial output for '%s'", + worker.store.printStorePath(drvPath), outputName); + auto & initialInfo = *initialOutput; /* Don't register if already valid, and not checking */ initialInfo.wanted = buildMode == bmCheck @@ -2185,6 +2194,11 @@ DrvOutputs LocalDerivationGoal::registerOutputs() auto sortedOutputNames = topoSort(outputsToSort, {[&](const std::string & name) { + auto orifu = get(outputReferencesIfUnregistered, name); + if (!orifu) + throw BuildError( + "no output reference for '%s' in build of '%s'", + name, worker.store.printStorePath(drvPath)); return std::visit(overloaded { /* Since we'll use the already installed versions of these, we can treat them as leaves and ignore any references they @@ -2199,7 +2213,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() referencedOutputs.insert(o); return referencedOutputs; }, - }, outputReferencesIfUnregistered.at(name)); + }, *orifu); }}, {[&](const std::string & path, const std::string & parent) { // TODO with more -vvvv also show the temporary paths for manual inspection. @@ -2213,9 +2227,10 @@ DrvOutputs LocalDerivationGoal::registerOutputs() OutputPathMap finalOutputs; for (auto & outputName : sortedOutputNames) { - auto output = drv->outputs.at(outputName); - auto & scratchPath = scratchOutputs.at(outputName); - auto actualPath = toRealPathChroot(worker.store.printStorePath(scratchPath)); + auto output = get(drv->outputs, outputName); + auto scratchPath = get(scratchOutputs, outputName); + assert(output && scratchPath); + auto actualPath = toRealPathChroot(worker.store.printStorePath(*scratchPath)); auto finish = [&](StorePath finalStorePath) { /* Store the final path */ @@ -2223,10 +2238,13 @@ DrvOutputs LocalDerivationGoal::registerOutputs() /* The rewrite rule will be used in downstream outputs that refer to use. This is why the topological sort is essential to do first before this for loop. */ - if (scratchPath != finalStorePath) - outputRewrites[std::string { scratchPath.hashPart() }] = std::string { finalStorePath.hashPart() }; + if (*scratchPath != finalStorePath) + outputRewrites[std::string { scratchPath->hashPart() }] = std::string { finalStorePath.hashPart() }; }; + auto orifu = get(outputReferencesIfUnregistered, outputName); + assert(orifu); + std::optional<StorePathSet> referencesOpt = std::visit(overloaded { [&](const AlreadyRegistered & skippedFinalPath) -> std::optional<StorePathSet> { finish(skippedFinalPath.path); @@ -2235,7 +2253,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() [&](const PerhapsNeedToRegister & r) -> std::optional<StorePathSet> { return r.refs; }, - }, outputReferencesIfUnregistered.at(outputName)); + }, *orifu); if (!referencesOpt) continue; @@ -2268,25 +2286,29 @@ DrvOutputs LocalDerivationGoal::registerOutputs() for (auto & r : references) { auto name = r.name(); auto origHash = std::string { r.hashPart() }; - if (r == scratchPath) + if (r == *scratchPath) { res.first = true; - else if (outputRewrites.count(origHash) == 0) - res.second.insert(r); - else { - std::string newRef = outputRewrites.at(origHash); + } else if (auto outputRewrite = get(outputRewrites, origHash)) { + std::string newRef = *outputRewrite; newRef += '-'; newRef += name; res.second.insert(StorePath { newRef }); + } else { + res.second.insert(r); } } return res; }; auto newInfoFromCA = [&](const DerivationOutput::CAFloating outputHash) -> ValidPathInfo { - auto & st = outputStats.at(outputName); + auto st = get(outputStats, outputName); + if (!st) + throw BuildError( + "output path %1% without valid stats info", + actualPath); if (outputHash.method == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ - if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) + if (!S_ISREG(st->st_mode) || (st->st_mode & S_IXUSR) != 0) throw BuildError( "output path '%1%' should be a non-executable regular file " "since recursive hashing is not enabled (outputHashMode=flat)", @@ -2294,7 +2316,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() } rewriteOutput(); /* FIXME optimize and deduplicate with addToStore */ - std::string oldHashPart { scratchPath.hashPart() }; + std::string oldHashPart { scratchPath->hashPart() }; HashModuloSink caSink { outputHash.hashType, oldHashPart }; switch (outputHash.method) { case FileIngestionMethod::Recursive: @@ -2313,7 +2335,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() outputPathName(drv->name, outputName), refs.second, refs.first); - if (scratchPath != finalPath) { + if (*scratchPath != finalPath) { // Also rewrite the output path auto source = sinkToSource([&](Sink & nextSink) { StringSink sink; @@ -2354,9 +2376,9 @@ DrvOutputs LocalDerivationGoal::registerOutputs() auto requiredFinalPath = output.path; /* Preemptively add rewrite rule for final hash, as that is what the NAR hash will use rather than normalized-self references */ - if (scratchPath != requiredFinalPath) + if (*scratchPath != requiredFinalPath) outputRewrites.insert_or_assign( - std::string { scratchPath.hashPart() }, + std::string { scratchPath->hashPart() }, std::string { requiredFinalPath.hashPart() }); rewriteOutput(); auto narHashAndSize = hashPath(htSHA256, actualPath); @@ -2409,7 +2431,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() }); }, - }, output.raw()); + }, output->raw()); /* FIXME: set proper permissions in restorePath() so we don't have to do another traversal. */ @@ -2425,7 +2447,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() derivations. */ PathLocks dynamicOutputLock; dynamicOutputLock.setDeletion(true); - auto optFixedPath = output.path(worker.store, drv->name, outputName); + auto optFixedPath = output->path(worker.store, drv->name, outputName); if (!optFixedPath || worker.store.printStorePath(*optFixedPath) != finalDestPath) { @@ -2491,11 +2513,10 @@ DrvOutputs LocalDerivationGoal::registerOutputs() /* For debugging, print out the referenced and unreferenced paths. */ for (auto & i : inputPaths) { - auto j = references.find(i); - if (j == references.end()) - debug("unreferenced input: '%1%'", worker.store.printStorePath(i)); - else + if (references.count(i)) debug("referenced input: '%1%'", worker.store.printStorePath(i)); + else + debug("unreferenced input: '%1%'", worker.store.printStorePath(i)); } if (curRound == nrRounds) { @@ -2612,9 +2633,11 @@ DrvOutputs LocalDerivationGoal::registerOutputs() DrvOutputs builtOutputs; for (auto & [outputName, newInfo] : infos) { + auto oldinfo = get(initialOutputs, outputName); + assert(oldinfo); auto thisRealisation = Realisation { .id = DrvOutput { - initialOutputs.at(outputName).outputHash, + oldinfo->outputHash, outputName }, .outPath = newInfo.path @@ -2710,9 +2733,10 @@ void LocalDerivationGoal::checkOutputs(const std::map<std::string, ValidPathInfo for (auto & i : *value) { if (worker.store.isStorePath(i)) spec.insert(worker.store.parseStorePath(i)); - else if (outputs.count(i)) - spec.insert(outputs.at(i).path); - else throw BuildError("derivation contains an illegal reference specifier '%s'", i); + else if (auto output = get(outputs, i)) + spec.insert(output->path); + else + throw BuildError("derivation contains an illegal reference specifier '%s'", i); } auto used = recursive @@ -2751,24 +2775,18 @@ void LocalDerivationGoal::checkOutputs(const std::map<std::string, ValidPathInfo }; if (auto structuredAttrs = parsedDrv->getStructuredAttrs()) { - auto outputChecks = structuredAttrs->find("outputChecks"); - if (outputChecks != structuredAttrs->end()) { - auto output = outputChecks->find(outputName); - - if (output != outputChecks->end()) { + if (auto outputChecks = get(*structuredAttrs, "outputChecks")) { + if (auto output = get(*outputChecks, outputName)) { Checks checks; - auto maxSize = output->find("maxSize"); - if (maxSize != output->end()) + if (auto maxSize = get(*output, "maxSize")) checks.maxSize = maxSize->get<uint64_t>(); - auto maxClosureSize = output->find("maxClosureSize"); - if (maxClosureSize != output->end()) + if (auto maxClosureSize = get(*output, "maxClosureSize")) checks.maxClosureSize = maxClosureSize->get<uint64_t>(); - auto get = [&](const std::string & name) -> std::optional<Strings> { - auto i = output->find(name); - if (i != output->end()) { + auto get_ = [&](const std::string & name) -> std::optional<Strings> { + if (auto i = get(*output, name)) { Strings res; for (auto j = i->begin(); j != i->end(); ++j) { if (!j->is_string()) @@ -2781,10 +2799,10 @@ void LocalDerivationGoal::checkOutputs(const std::map<std::string, ValidPathInfo return {}; }; - checks.allowedReferences = get("allowedReferences"); - checks.allowedRequisites = get("allowedRequisites"); - checks.disallowedReferences = get("disallowedReferences"); - checks.disallowedRequisites = get("disallowedRequisites"); + checks.allowedReferences = get_("allowedReferences"); + checks.allowedRequisites = get_("allowedRequisites"); + checks.disallowedReferences = get_("disallowedReferences"); + checks.disallowedRequisites = get_("disallowedRequisites"); applyChecks(checks); } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index f72c1cc9c..b192fbc77 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -350,7 +350,7 @@ void Worker::waitForInput() become `available'. Note that `available' (i.e., non-blocking) includes EOF. */ std::vector<struct pollfd> pollStatus; - std::map <int, int> fdToPollStatus; + std::map<int, size_t> fdToPollStatus; for (auto & i : children) { for (auto & j : i.fds) { pollStatus.push_back((struct pollfd) { .fd = j, .events = POLLIN }); @@ -380,7 +380,10 @@ void Worker::waitForInput() std::set<int> fds2(j->fds); std::vector<unsigned char> buffer(4096); for (auto & k : fds2) { - if (pollStatus.at(fdToPollStatus.at(k)).revents) { + const auto fdPollStatusId = get(fdToPollStatus, k); + assert(fdPollStatusId); + assert(*fdPollStatusId < pollStatus.size()); + if (pollStatus.at(*fdPollStatusId).revents) { ssize_t rd = ::read(k, buffer.data(), buffer.size()); // FIXME: is there a cleaner way to handle pt close // than EIO? Is this even standard? |