diff options
Diffstat (limited to 'src/libstore/build')
-rw-r--r-- | src/libstore/build/derivation-goal.cc | 154 | ||||
-rw-r--r-- | src/libstore/build/derivation-goal.hh | 235 | ||||
-rw-r--r-- | src/libstore/build/drv-output-substitution-goal.cc | 23 | ||||
-rw-r--r-- | src/libstore/build/drv-output-substitution-goal.hh | 49 | ||||
-rw-r--r-- | src/libstore/build/entry-points.cc | 49 | ||||
-rw-r--r-- | src/libstore/build/goal.cc | 27 | ||||
-rw-r--r-- | src/libstore/build/goal.hh | 102 | ||||
-rw-r--r-- | src/libstore/build/hook-instance.cc | 5 | ||||
-rw-r--r-- | src/libstore/build/hook-instance.hh | 17 | ||||
-rw-r--r-- | src/libstore/build/local-derivation-goal.cc | 223 | ||||
-rw-r--r-- | src/libstore/build/local-derivation-goal.hh | 225 | ||||
-rw-r--r-- | src/libstore/build/personality.hh | 1 | ||||
-rw-r--r-- | src/libstore/build/substitution-goal.cc | 13 | ||||
-rw-r--r-- | src/libstore/build/substitution-goal.hh | 65 | ||||
-rw-r--r-- | src/libstore/build/worker.cc | 40 | ||||
-rw-r--r-- | src/libstore/build/worker.hh | 204 |
16 files changed, 974 insertions, 458 deletions
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 2021d0023..df7d21e54 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -145,8 +145,20 @@ void DerivationGoal::work() void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) { auto newWanted = wantedOutputs.union_(outputs); - if (!newWanted.isSubsetOf(wantedOutputs)) - needRestart = true; + switch (needRestart) { + case NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed: + if (!newWanted.isSubsetOf(wantedOutputs)) + needRestart = NeedRestartForMoreOutputs::OutputsAddedDoNeed; + break; + case NeedRestartForMoreOutputs::OutputsAddedDoNeed: + /* No need to check whether we added more outputs, because a + restart is already queued up. */ + break; + case NeedRestartForMoreOutputs::BuildInProgressWillNotNeed: + /* We are already building all outputs, so it doesn't matter if + we now want more. */ + break; + }; wantedOutputs = newWanted; } @@ -199,10 +211,10 @@ void DerivationGoal::haveDerivation() parsedDrv = std::make_unique<ParsedDerivation>(drvPath, *drv); if (!drv->type().hasKnownOutputPaths()) - settings.requireExperimentalFeature(Xp::CaDerivations); + experimentalFeatureSettings.require(Xp::CaDerivations); if (!drv->type().isPure()) { - settings.requireExperimentalFeature(Xp::ImpureDerivations); + experimentalFeatureSettings.require(Xp::ImpureDerivations); for (auto & [outputName, output] : drv->outputs) { auto randomPath = StorePath::random(outputPathName(drv->name, outputName)); @@ -262,11 +274,13 @@ void DerivationGoal::haveDerivation() ) ) ); - else + else { + auto * cap = getDerivationCA(*drv); addWaitee(upcast_goal(worker.makePathSubstitutionGoal( status.known->path, buildMode == bmRepair ? Repair : NoRepair, - getDerivationCA(*drv)))); + cap ? std::optional { *cap } : std::nullopt))); + } } if (waitees.empty()) /* to prevent hang (no wake-up event) */ @@ -297,12 +311,29 @@ void DerivationGoal::outputsSubstitutionTried() In particular, it may be the case that the hole in the closure is an output of the current derivation, which causes a loop if retried. */ - if (nrIncompleteClosure > 0 && nrIncompleteClosure == nrFailed) retrySubstitution = true; + { + bool substitutionFailed = + nrIncompleteClosure > 0 && + nrIncompleteClosure == nrFailed; + switch (retrySubstitution) { + case RetrySubstitution::NoNeed: + if (substitutionFailed) + retrySubstitution = RetrySubstitution::YesNeed; + break; + case RetrySubstitution::YesNeed: + // Should not be able to reach this state from here. + assert(false); + break; + case RetrySubstitution::AlreadyRetried: + debug("substitution failed again, but we already retried once. Not retrying again."); + break; + } + } nrFailed = nrNoSubstituters = nrIncompleteClosure = 0; - if (needRestart) { - needRestart = false; + if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) { + needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; haveDerivation(); return; } @@ -330,13 +361,17 @@ void DerivationGoal::outputsSubstitutionTried() produced using a substitute. So we have to build instead. */ void DerivationGoal::gaveUpOnSubstitution() { + /* At this point we are building all outputs, so if more are wanted there + is no need to restart. */ + needRestart = NeedRestartForMoreOutputs::BuildInProgressWillNotNeed; + /* The inputs must be built before we can build this goal. */ inputDrvOutputs.clear(); if (useDerivation) for (auto & i : dynamic_cast<Derivation *>(drv.get())->inputDrvs) { /* Ensure that pure, non-fixed-output derivations don't depend on impure derivations. */ - if (settings.isExperimentalFeatureEnabled(Xp::ImpureDerivations) && drv->type().isPure() && !drv->type().isFixed()) { + if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && drv->type().isPure() && !drv->type().isFixed()) { auto inputDrv = worker.evalStore.readDerivation(i.first); if (!inputDrv.type().isPure()) throw Error("pure derivation '%s' depends on impure derivation '%s'", @@ -451,8 +486,8 @@ void DerivationGoal::inputsRealised() return; } - if (retrySubstitution && !retriedSubstitution) { - retriedSubstitution = true; + if (retrySubstitution == RetrySubstitution::YesNeed) { + retrySubstitution = RetrySubstitution::AlreadyRetried; haveDerivation(); return; } @@ -477,7 +512,7 @@ void DerivationGoal::inputsRealised() ca.fixed /* Can optionally resolve if fixed, which is good for avoiding unnecessary rebuilds. */ - ? settings.isExperimentalFeatureEnabled(Xp::CaDerivations) + ? experimentalFeatureSettings.isEnabled(Xp::CaDerivations) /* Must resolve if floating and there are any inputs drvs. */ : true); @@ -488,7 +523,7 @@ void DerivationGoal::inputsRealised() }, drvType.raw()); if (resolveDrv && !fullDrv.inputDrvs.empty()) { - settings.requireExperimentalFeature(Xp::CaDerivations); + experimentalFeatureSettings.require(Xp::CaDerivations); /* We are be able to resolve this derivation based on the now-known results of dependencies. If so, we become a @@ -570,8 +605,6 @@ void DerivationGoal::inputsRealised() build hook. */ state = &DerivationGoal::tryToBuild; worker.wakeUp(shared_from_this()); - - buildResult = BuildResult { .path = buildResult.path }; } void DerivationGoal::started() @@ -732,7 +765,7 @@ void replaceValidPath(const Path & storePath, const Path & tmpPath) tmpPath (the replacement), so we have to move it out of the way first. We'd better not be interrupted here, because if we're repairing (say) Glibc, we end up with a broken system. */ - Path oldPath = (format("%1%.old-%2%-%3%") % storePath % getpid() % random()).str(); + Path oldPath = fmt("%1%.old-%2%-%3%", storePath, getpid(), random()); if (pathExists(storePath)) movePath(storePath, oldPath); @@ -911,7 +944,11 @@ void DerivationGoal::buildDone() msg += line; msg += "\n"; } - msg += fmt("For full logs, run '" ANSI_BOLD "nix log %s" ANSI_NORMAL "'.", + auto nixLogCommand = experimentalFeatureSettings.isEnabled(Xp::NixCommand) + ? "nix log" + : "nix-store -l"; + msg += fmt("For full logs, run '" ANSI_BOLD "%s %s" ANSI_NORMAL "'.", + nixLogCommand, worker.store.printStorePath(drvPath)); } @@ -978,50 +1015,40 @@ void DerivationGoal::resolvedFinished() auto resolvedDrv = *resolvedDrvGoal->drv; auto & resolvedResult = resolvedDrvGoal->buildResult; - DrvOutputs builtOutputs; + SingleDrvOutputs builtOutputs; if (resolvedResult.success()) { auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); StorePathSet outputPaths; - // `wantedOutputs` might merely indicate “all the outputs” - auto realWantedOutputs = std::visit(overloaded { - [&](const OutputsSpec::All &) { - return resolvedDrv.outputNames(); - }, - [&](const OutputsSpec::Names & names) { - return static_cast<std::set<std::string>>(names); - }, - }, wantedOutputs.raw()); - - for (auto & wantedOutput : realWantedOutputs) { - auto initialOutput = get(initialOutputs, wantedOutput); - auto resolvedHash = get(resolvedHashes, wantedOutput); + for (auto & outputName : resolvedDrv.outputNames()) { + auto initialOutput = get(initialOutputs, outputName); + auto resolvedHash = get(resolvedHashes, outputName); if ((!initialOutput) || (!resolvedHash)) throw Error( "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,resolve)", - worker.store.printStorePath(drvPath), wantedOutput); + worker.store.printStorePath(drvPath), outputName); auto realisation = [&]{ - auto take1 = get(resolvedResult.builtOutputs, DrvOutput { *resolvedHash, wantedOutput }); + auto take1 = get(resolvedResult.builtOutputs, outputName); if (take1) return *take1; /* The above `get` should work. But sateful tracking of outputs in resolvedResult, this can get out of sync with the store, which is our actual source of truth. For now we just check the store directly if it fails. */ - auto take2 = worker.evalStore.queryRealisation(DrvOutput { *resolvedHash, wantedOutput }); + auto take2 = worker.evalStore.queryRealisation(DrvOutput { *resolvedHash, outputName }); if (take2) return *take2; throw Error( "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,realisation)", - worker.store.printStorePath(resolvedDrvGoal->drvPath), wantedOutput); + worker.store.printStorePath(resolvedDrvGoal->drvPath), outputName); }(); if (drv->type().isPure()) { auto newRealisation = realisation; - newRealisation.id = DrvOutput { initialOutput->outputHash, wantedOutput }; + newRealisation.id = DrvOutput { initialOutput->outputHash, outputName }; newRealisation.signatures.clear(); if (!drv->type().isFixed()) newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); @@ -1029,7 +1056,7 @@ void DerivationGoal::resolvedFinished() worker.store.registerDrvOutput(newRealisation); } outputPaths.insert(realisation.outPath); - builtOutputs.emplace(realisation.id, realisation); + builtOutputs.emplace(outputName, realisation); } runPostBuildHook( @@ -1125,7 +1152,7 @@ HookReply DerivationGoal::tryBuildHook() /* Tell the hook all the inputs that have to be copied to the remote system. */ - worker_proto::write(worker.store, hook->sink, inputPaths); + workerProtoWrite(worker.store, hook->sink, inputPaths); /* Tell the hooks the missing outputs that have to be copied back from the remote system. */ @@ -1136,7 +1163,7 @@ HookReply DerivationGoal::tryBuildHook() if (buildMode != bmCheck && status.known && status.known->isValid()) continue; missingOutputs.insert(outputName); } - worker_proto::write(worker.store, hook->sink, missingOutputs); + workerProtoWrite(worker.store, hook->sink, missingOutputs); } hook->sink = FdSink(); @@ -1154,7 +1181,7 @@ HookReply DerivationGoal::tryBuildHook() } -DrvOutputs DerivationGoal::registerOutputs() +SingleDrvOutputs 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 @@ -1316,7 +1343,7 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap() } -std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() +std::pair<bool, SingleDrvOutputs> DerivationGoal::checkPathValidity() { if (!drv->type().isPure()) return { false, {} }; @@ -1329,7 +1356,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() return static_cast<StringSet>(names); }, }, wantedOutputs.raw()); - DrvOutputs validOutputs; + SingleDrvOutputs validOutputs; for (auto & i : queryPartialDerivationOutputMap()) { auto initialOutput = get(initialOutputs, i.first); @@ -1352,7 +1379,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() }; } auto drvOutput = DrvOutput{info.outputHash, i.first}; - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { + if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { .path = real->outPath, @@ -1371,8 +1398,8 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() ); } } - if (info.wanted && info.known && info.known->isValid()) - validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path }); + if (info.known && info.known->isValid()) + validOutputs.emplace(i.first, Realisation { drvOutput, info.known->path }); } // If we requested all the outputs, we are always fine. @@ -1396,7 +1423,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() } -DrvOutputs DerivationGoal::assertPathValidity() +SingleDrvOutputs DerivationGoal::assertPathValidity() { auto [allValid, validOutputs] = checkPathValidity(); if (!allValid) @@ -1407,7 +1434,7 @@ DrvOutputs DerivationGoal::assertPathValidity() void DerivationGoal::done( BuildResult::Status status, - DrvOutputs builtOutputs, + SingleDrvOutputs builtOutputs, std::optional<Error> ex) { buildResult.status = status; @@ -1422,8 +1449,9 @@ void DerivationGoal::done( mcRunningBuilds.reset(); if (buildResult.success()) { - assert(!builtOutputs.empty()); - buildResult.builtOutputs = std::move(builtOutputs); + auto wantedBuiltOutputs = filterDrvOutputs(wantedOutputs, std::move(builtOutputs)); + assert(!wantedBuiltOutputs.empty()); + buildResult.builtOutputs = std::move(wantedBuiltOutputs); if (status == BuildResult::Built) worker.doneBuilds++; } else { @@ -1448,12 +1476,28 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) { Goal::waiteeDone(waitee, result); - if (waitee->buildResult.success()) - if (auto bfd = std::get_if<DerivedPath::Built>(&waitee->buildResult.path)) - for (auto & [output, realisation] : waitee->buildResult.builtOutputs) + if (!useDerivation) return; + auto & fullDrv = *dynamic_cast<Derivation *>(drv.get()); + + auto * dg = dynamic_cast<DerivationGoal *>(&*waitee); + if (!dg) return; + + auto outputs = fullDrv.inputDrvs.find(dg->drvPath); + if (outputs == fullDrv.inputDrvs.end()) return; + + for (auto & outputName : outputs->second) { + auto buildResult = dg->getBuildResult(DerivedPath::Built { + .drvPath = dg->drvPath, + .outputs = OutputsSpec::Names { outputName }, + }); + if (buildResult.success()) { + auto i = buildResult.builtOutputs.find(outputName); + if (i != buildResult.builtOutputs.end()) inputDrvOutputs.insert_or_assign( - { bfd->drvPath, output.outputName }, - realisation.outPath); + { dg->drvPath, outputName }, + i->second.outPath); + } + } } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 707e38b4b..ee8f06f25 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -1,4 +1,5 @@ #pragma once +///@file #include "parsed-derivations.hh" #include "lock.hh" @@ -15,8 +16,10 @@ struct HookInstance; typedef enum {rpAccept, rpDecline, rpPostpone} HookReply; -/* Unless we are repairing, we don't both to test validity and just assume it, - so the choices are `Absent` or `Valid`. */ +/** + * Unless we are repairing, we don't both to test validity and just assume it, + * so the choices are `Absent` or `Valid`. + */ enum struct PathStatus { Corrupt, Absent, @@ -26,11 +29,15 @@ enum struct PathStatus { struct InitialOutputStatus { StorePath path; PathStatus status; - /* Valid in the store, and additionally non-corrupt if we are repairing */ + /** + * Valid in the store, and additionally non-corrupt if we are repairing + */ bool isValid() const { return status == PathStatus::Valid; } - /* Merely present, allowed to be corrupt */ + /** + * Merely present, allowed to be corrupt + */ bool isPresent() const { return status == PathStatus::Corrupt || status == PathStatus::Valid; @@ -45,59 +52,123 @@ struct InitialOutput { struct DerivationGoal : public Goal { - /* Whether to use an on-disk .drv file. */ + /** + * Whether to use an on-disk .drv file. + */ bool useDerivation; - /* The path of the derivation. */ + /** The path of the derivation. */ StorePath drvPath; - /* The goal for the corresponding resolved derivation */ + /** + * The goal for the corresponding resolved derivation + */ std::shared_ptr<DerivationGoal> resolvedDrvGoal; - /* The specific outputs that we need to build. Empty means all of - them. */ + /** + * The specific outputs that we need to build. Empty means all of + * them. + */ OutputsSpec wantedOutputs; - /* Mapping from input derivations + output names to actual store - paths. This is filled in by waiteeDone() as each dependency - finishes, before inputsRealised() is reached, */ + /** + * Mapping from input derivations + output names to actual store + * paths. This is filled in by waiteeDone() as each dependency + * finishes, before inputsRealised() is reached. + */ std::map<std::pair<StorePath, std::string>, StorePath> inputDrvOutputs; - /* Whether additional wanted outputs have been added. */ - bool needRestart = false; - - /* Whether to retry substituting the outputs after building the - inputs. This is done in case of an incomplete closure. */ - bool retrySubstitution = false; - - /* Whether we've retried substitution, in which case we won't try - again. */ - bool retriedSubstitution = false; - - /* The derivation stored at drvPath. */ + /** + * See `needRestart`; just for that field. + */ + enum struct NeedRestartForMoreOutputs { + /** + * The goal state machine is progressing based on the current value of + * `wantedOutputs. No actions are needed. + */ + OutputsUnmodifedDontNeed, + /** + * `wantedOutputs` has been extended, but the state machine is + * proceeding according to its old value, so we need to restart. + */ + OutputsAddedDoNeed, + /** + * The goal state machine has progressed to the point of doing a build, + * in which case all outputs will be produced, so extensions to + * `wantedOutputs` no longer require a restart. + */ + BuildInProgressWillNotNeed, + }; + + /** + * Whether additional wanted outputs have been added. + */ + NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; + + /** + * See `retrySubstitution`; just for that field. + */ + enum RetrySubstitution { + /** + * No issues have yet arose, no need to restart. + */ + NoNeed, + /** + * Something failed and there is an incomplete closure. Let's retry + * substituting. + */ + YesNeed, + /** + * We are current or have already retried substitution, and whether or + * not something goes wrong we will not retry again. + */ + AlreadyRetried, + }; + + /** + * Whether to retry substituting the outputs after building the + * inputs. This is done in case of an incomplete closure. + */ + RetrySubstitution retrySubstitution = RetrySubstitution::NoNeed; + + /** + * The derivation stored at drvPath. + */ std::unique_ptr<Derivation> drv; std::unique_ptr<ParsedDerivation> parsedDrv; - /* The remainder is state held during the build. */ + /** + * The remainder is state held during the build. + */ - /* Locks on (fixed) output paths. */ + /** + * Locks on (fixed) output paths. + */ PathLocks outputLocks; - /* All input paths (that is, the union of FS closures of the - immediate input paths). */ + /** + * All input paths (that is, the union of FS closures of the + * immediate input paths). + */ StorePathSet inputPaths; std::map<std::string, InitialOutput> initialOutputs; - /* File descriptor for the log file. */ + /** + * File descriptor for the log file. + */ AutoCloseFD fdLogFile; std::shared_ptr<BufferedSink> logFileSink, logSink; - /* Number of bytes received from the builder's stdout/stderr. */ + /** + * Number of bytes received from the builder's stdout/stderr. + */ unsigned long logSize; - /* The most recent log lines. */ + /** + * The most recent log lines. + */ std::list<std::string> logTail; std::string currentLogLine; @@ -105,10 +176,14 @@ struct DerivationGoal : public Goal std::string currentHookLine; - /* The build hook. */ + /** + * The build hook. + */ std::unique_ptr<HookInstance> hook; - /* The sort of derivation we are building. */ + /** + * The sort of derivation we are building. + */ DerivationType derivationType; typedef void (DerivationGoal::*GoalState)(); @@ -120,12 +195,16 @@ struct DerivationGoal : public Goal std::unique_ptr<Activity> act; - /* Activity that denotes waiting for a lock. */ + /** + * Activity that denotes waiting for a lock. + */ std::unique_ptr<Activity> actLock; std::map<ActivityId, Activity> builderActivities; - /* The remote machine on which we're building. */ + /** + * The remote machine on which we're building. + */ std::string machineName; DerivationGoal(const StorePath & drvPath, @@ -142,10 +221,14 @@ struct DerivationGoal : public Goal void work() override; - /* Add wanted outputs to an already existing derivation goal. */ + /** + * Add wanted outputs to an already existing derivation goal. + */ void addWantedOutputs(const OutputsSpec & outputs); - /* The states. */ + /** + * The states. + */ void getDerivation(); void loadDerivation(); void haveDerivation(); @@ -159,28 +242,42 @@ struct DerivationGoal : public Goal void resolvedFinished(); - /* Is the build hook willing to perform the build? */ + /** + * Is the build hook willing to perform the build? + */ HookReply tryBuildHook(); virtual int getChildStatus(); - /* Check that the derivation outputs all exist and register them - as valid. */ - virtual DrvOutputs registerOutputs(); + /** + * Check that the derivation outputs all exist and register them + * as valid. + */ + virtual SingleDrvOutputs registerOutputs(); - /* Open a log file and a pipe to it. */ + /** + * Open a log file and a pipe to it. + */ Path openLogFile(); - /* Sign the newly built realisation if the store allows it */ + /** + * Sign the newly built realisation if the store allows it + */ virtual void signRealisation(Realisation&) {} - /* Close the log file. */ + /** + * Close the log file. + */ void closeLogFile(); - /* Close the read side of the logger pipe. */ + /** + * Close the read side of the logger pipe. + */ virtual void closeReadPipes(); - /* Cleanup hooks for buildDone() */ + /** + * Cleanup hooks for buildDone() + */ virtual void cleanupHookFinally(); virtual void cleanupPreChildKill(); virtual void cleanupPostChildKill(); @@ -190,30 +287,38 @@ struct DerivationGoal : public Goal virtual bool isReadDesc(int fd); - /* Callback used by the worker to write to the log. */ + /** + * Callback used by the worker to write to the log. + */ void handleChildOutput(int fd, std::string_view data) override; void handleEOF(int fd) override; void flushLine(); - /* Wrappers around the corresponding Store methods that first consult the - derivation. This is currently needed because when there is no drv file - there also is no DB entry. */ + /** + * Wrappers around the corresponding Store methods that first consult the + * derivation. This is currently needed because when there is no drv file + * there also is no DB entry. + */ std::map<std::string, std::optional<StorePath>> queryPartialDerivationOutputMap(); OutputPathMap queryDerivationOutputMap(); - /* 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<bool, DrvOutputs> 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. */ + /** + * 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 + * 'SingleDrvOutputs' structure containing the valid outputs. + */ + std::pair<bool, SingleDrvOutputs> checkPathValidity(); + + /** + * Aborts if any output is not valid or corrupt, and otherwise + * returns a 'SingleDrvOutputs' structure containing all outputs. + */ + SingleDrvOutputs assertPathValidity(); + + /** + * Forcibly kill the child process, if any. + */ virtual void killChild(); void repairClosure(); @@ -222,12 +327,14 @@ struct DerivationGoal : public Goal void done( BuildResult::Status status, - DrvOutputs builtOutputs = {}, + SingleDrvOutputs builtOutputs = {}, std::optional<Error> ex = {}); void waiteeDone(GoalPtr waitee, ExitCode result) override; StorePathSet exportReferences(const StorePathSet & storePaths); + + JobCategory jobCategory() override { return JobCategory::Build; }; }; MakeError(NotDeterministic, BuildError); diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index b7f7b5ab1..b30957c84 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -61,20 +61,25 @@ void DrvOutputSubstitutionGoal::tryNext() // FIXME: Make async // outputInfo = sub->queryRealisation(id); - outPipe.create(); - promise = decltype(promise)(); + + /* The callback of the curl download below can outlive `this` (if + some other error occurs), so it must not touch `this`. So put + the shared state in a separate refcounted object. */ + downloadState = std::make_shared<DownloadState>(); + downloadState->outPipe.create(); sub->queryRealisation( - id, { [&](std::future<std::shared_ptr<const Realisation>> res) { + id, + { [downloadState(downloadState)](std::future<std::shared_ptr<const Realisation>> res) { try { - Finally updateStats([this]() { outPipe.writeSide.close(); }); - promise.set_value(res.get()); + Finally updateStats([&]() { downloadState->outPipe.writeSide.close(); }); + downloadState->promise.set_value(res.get()); } catch (...) { - promise.set_exception(std::current_exception()); + downloadState->promise.set_exception(std::current_exception()); } } }); - worker.childStarted(shared_from_this(), {outPipe.readSide.get()}, true, false); + worker.childStarted(shared_from_this(), {downloadState->outPipe.readSide.get()}, true, false); state = &DrvOutputSubstitutionGoal::realisationFetched; } @@ -84,7 +89,7 @@ void DrvOutputSubstitutionGoal::realisationFetched() worker.childTerminated(this); try { - outputInfo = promise.get_future().get(); + outputInfo = downloadState->promise.get_future().get(); } catch (std::exception & e) { printError(e.what()); substituterFailed = true; @@ -155,7 +160,7 @@ void DrvOutputSubstitutionGoal::work() void DrvOutputSubstitutionGoal::handleEOF(int fd) { - if (fd == outPipe.readSide.get()) worker.wakeUp(shared_from_this()); + if (fd == downloadState->outPipe.readSide.get()) worker.wakeUp(shared_from_this()); } diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 948dbda8f..5d1253a71 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -1,4 +1,5 @@ #pragma once +///@file #include "store-api.hh" #include "goal.hh" @@ -10,31 +11,47 @@ namespace nix { class Worker; -// Substitution of a derivation output. -// This is done in three steps: -// 1. Fetch the output info from a substituter -// 2. Substitute the corresponding output path -// 3. Register the output info +/** + * Substitution of a derivation output. + * This is done in three steps: + * 1. Fetch the output info from a substituter + * 2. Substitute the corresponding output path + * 3. Register the output info + */ class DrvOutputSubstitutionGoal : public Goal { -private: - // The drv output we're trying to substitue + + /** + * The drv output we're trying to substitute + */ DrvOutput id; - // The realisation corresponding to the given output id. - // Will be filled once we can get it. + /** + * The realisation corresponding to the given output id. + * Will be filled once we can get it. + */ std::shared_ptr<const Realisation> outputInfo; - /* The remaining substituters. */ + /** + * The remaining substituters. + */ std::list<ref<Store>> subs; - /* The current substituter. */ + /** + * The current substituter. + */ std::shared_ptr<Store> sub; - Pipe outPipe; - std::thread thr; - std::promise<std::shared_ptr<const Realisation>> promise; + struct DownloadState + { + Pipe outPipe; + std::promise<std::shared_ptr<const Realisation>> promise; + }; + + std::shared_ptr<DownloadState> downloadState; - /* Whether a substituter failed. */ + /** + * Whether a substituter failed. + */ bool substituterFailed = false; public: @@ -55,6 +72,8 @@ public: void work() override; void handleEOF(int fd) override; + + JobCategory jobCategory() override { return JobCategory::Substitution; }; }; } diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 2925fe3ca..edd6cb6d2 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -10,16 +10,8 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod 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()); - } + for (auto & br : reqs) + goals.insert(worker.makeGoal(br, buildMode)); worker.run(goals); @@ -47,7 +39,7 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod } } -std::vector<BuildResult> Store::buildPathsWithResults( +std::vector<KeyedBuildResult> Store::buildPathsWithResults( const std::vector<DerivedPath> & reqs, BuildMode buildMode, std::shared_ptr<Store> evalStore) @@ -55,23 +47,23 @@ std::vector<BuildResult> Store::buildPathsWithResults( 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()); + std::vector<std::pair<const DerivedPath &, GoalPtr>> state; + + for (const auto & req : reqs) { + auto goal = worker.makeGoal(req, buildMode); + goals.insert(goal); + state.push_back({req, goal}); } worker.run(goals); - std::vector<BuildResult> results; + std::vector<KeyedBuildResult> results; - for (auto & i : goals) - results.push_back(i->buildResult); + for (auto & [req, goalPtr] : state) + results.emplace_back(KeyedBuildResult { + goalPtr->getBuildResult(req), + /* .path = */ req, + }); return results; } @@ -84,15 +76,14 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat try { worker.run(Goals{goal}); - return goal->buildResult; + return goal->getBuildResult(DerivedPath::Built { + .drvPath = drvPath, + .outputs = OutputsSpec::All {}, + }); } catch (Error & e) { return BuildResult { .status = BuildResult::MiscFailure, .errorMsg = e.msg(), - .path = DerivedPath::Built { - .drvPath = drvPath, - .outputs = OutputsSpec::All { }, - }, }; }; } @@ -119,7 +110,7 @@ void Store::ensurePath(const StorePath & path) } -void LocalStore::repairPath(const StorePath & path) +void Store::repairPath(const StorePath & path) { Worker worker(*this, *this); GoalPtr goal = worker.makePathSubstitutionGoal(path, Repair); diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 58e805f55..ca7097a68 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -11,6 +11,29 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { } +BuildResult Goal::getBuildResult(const DerivedPath & req) { + BuildResult res { buildResult }; + + if (auto pbp = std::get_if<DerivedPath::Built>(&req)) { + auto & bp = *pbp; + + /* Because goals are in general shared between derived paths + that share the same derivation, we need to filter their + results to get back just the results we care about. + */ + + for (auto it = res.builtOutputs.begin(); it != res.builtOutputs.end();) { + if (bp.outputs.contains(it->first)) + ++it; + else + it = res.builtOutputs.erase(it); + } + } + + return res; +} + + void addToWeakGoals(WeakGoals & goals, GoalPtr p) { if (goals.find(p) != goals.end()) @@ -78,9 +101,9 @@ void Goal::amDone(ExitCode result, std::optional<Error> ex) } -void Goal::trace(const FormatOrString & fs) +void Goal::trace(std::string_view s) { - debug("%1%: %2%", name, fs.s); + debug("%1%: %2%", name, s); } } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 35121c5d9..a313bf22c 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -1,4 +1,5 @@ #pragma once +///@file #include "types.hh" #include "store-api.hh" @@ -6,11 +7,15 @@ namespace nix { -/* Forward definition. */ +/** + * Forward definition. + */ struct Goal; class Worker; -/* A pointer to a goal. */ +/** + * A pointer to a goal. + */ typedef std::shared_ptr<Goal> GoalPtr; typedef std::weak_ptr<Goal> WeakGoalPtr; @@ -18,53 +23,102 @@ struct CompareGoalPtrs { bool operator() (const GoalPtr & a, const GoalPtr & b) const; }; -/* Set of goals. */ +/** + * Set of goals. + */ typedef std::set<GoalPtr, CompareGoalPtrs> Goals; typedef std::set<WeakGoalPtr, std::owner_less<WeakGoalPtr>> WeakGoals; -/* A map of paths to goals (and the other way around). */ +/** + * A map of paths to goals (and the other way around). + */ typedef std::map<StorePath, WeakGoalPtr> WeakGoalMap; +/** + * Used as a hint to the worker on how to schedule a particular goal. For example, + * builds are typically CPU- and memory-bound, while substitutions are I/O bound. + * Using this information, the worker might decide to schedule more or fewer goals + * of each category in parallel. + */ +enum struct JobCategory { + Build, + Substitution, +}; + struct Goal : public std::enable_shared_from_this<Goal> { typedef enum {ecBusy, ecSuccess, ecFailed, ecNoSubstituters, ecIncompleteClosure} ExitCode; - /* Backlink to the worker. */ + /** + * Backlink to the worker. + */ Worker & worker; - /* Goals that this goal is waiting for. */ + /** + * Goals that this goal is waiting for. + */ Goals waitees; - /* Goals waiting for this one to finish. Must use weak pointers - here to prevent cycles. */ + /** + * Goals waiting for this one to finish. Must use weak pointers + * here to prevent cycles. + */ WeakGoals waiters; - /* Number of goals we are/were waiting for that have failed. */ + /** + * Number of goals we are/were waiting for that have failed. + */ size_t nrFailed = 0; - /* Number of substitution goals we are/were waiting for that - failed because there are no substituters. */ + /** + * Number of substitution goals we are/were waiting for that + * failed because there are no substituters. + */ size_t nrNoSubstituters = 0; - /* Number of substitution goals we are/were waiting for that - failed because they had unsubstitutable references. */ + /** + * Number of substitution goals we are/were waiting for that + * failed because they had unsubstitutable references. + */ size_t nrIncompleteClosure = 0; - /* Name of this goal for debugging purposes. */ + /** + * Name of this goal for debugging purposes. + */ std::string name; - /* Whether the goal is finished. */ + /** + * Whether the goal is finished. + */ ExitCode exitCode = ecBusy; - /* Build result. */ +protected: + /** + * Build result. + */ BuildResult buildResult; - /* Exception containing an error message, if any. */ +public: + + /** + * Project a `BuildResult` with just the information that pertains + * to the given request. + * + * In general, goals may be aliased between multiple requests, and + * the stored `BuildResult` has information for the union of all + * requests. We don't want to leak what the other request are for + * sake of both privacy and determinism, and this "safe accessor" + * ensures we don't. + */ + BuildResult getBuildResult(const DerivedPath &); + + /** + * Exception containing an error message, if any. + */ std::optional<Error> ex; Goal(Worker & worker, DerivedPath path) : worker(worker) - , buildResult { .path = std::move(path) } { } virtual ~Goal() @@ -88,16 +142,18 @@ struct Goal : public std::enable_shared_from_this<Goal> abort(); } - void trace(const FormatOrString & fs); + void trace(std::string_view s); std::string getName() { return name; } - /* Callback in case of a timeout. It should wake up its waiters, - get rid of any running child processes that are being monitored - by the worker (important!), etc. */ + /** + * Callback in case of a timeout. It should wake up its waiters, + * get rid of any running child processes that are being monitored + * by the worker (important!), etc. + */ virtual void timedOut(Error && ex) = 0; virtual std::string key() = 0; @@ -105,6 +161,8 @@ struct Goal : public std::enable_shared_from_this<Goal> void amDone(ExitCode result, std::optional<Error> ex = {}); virtual void cleanup() { } + + virtual JobCategory jobCategory() = 0; }; void addToWeakGoals(WeakGoals & goals, GoalPtr p); diff --git a/src/libstore/build/hook-instance.cc b/src/libstore/build/hook-instance.cc index cb58a1f02..075ad554f 100644 --- a/src/libstore/build/hook-instance.cc +++ b/src/libstore/build/hook-instance.cc @@ -35,7 +35,10 @@ HookInstance::HookInstance() /* Fork the hook. */ pid = startProcess([&]() { - commonChildInit(fromHook); + if (dup2(fromHook.writeSide.get(), STDERR_FILENO) == -1) + throw SysError("cannot pipe standard error into log file"); + + commonChildInit(); if (chdir("/") == -1) throw SysError("changing into /"); diff --git a/src/libstore/build/hook-instance.hh b/src/libstore/build/hook-instance.hh index 9e8cff128..d84f62877 100644 --- a/src/libstore/build/hook-instance.hh +++ b/src/libstore/build/hook-instance.hh @@ -1,4 +1,5 @@ #pragma once +///@file #include "logging.hh" #include "serialise.hh" @@ -7,16 +8,24 @@ namespace nix { struct HookInstance { - /* Pipes for talking to the build hook. */ + /** + * Pipes for talking to the build hook. + */ Pipe toHook; - /* Pipe for the hook's standard output/error. */ + /** + * Pipe for the hook's standard output/error. + */ Pipe fromHook; - /* Pipe for the builder's standard output/error. */ + /** + * Pipe for the builder's standard output/error. + */ Pipe builderOut; - /* The process ID of the hook. */ + /** + * The process ID of the hook. + */ Pid pid; FdSink sink; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index a961d8eed..05d6685da 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -292,7 +292,7 @@ void LocalDerivationGoal::closeReadPipes() if (hook) { DerivationGoal::closeReadPipes(); } else - builderOut.readSide = -1; + builderOut.close(); } @@ -413,7 +413,7 @@ void LocalDerivationGoal::startBuilder() ) { #if __linux__ - settings.requireExperimentalFeature(Xp::Cgroups); + experimentalFeatureSettings.require(Xp::Cgroups); auto cgroupFS = getCgroupFS(); if (!cgroupFS) @@ -650,7 +650,7 @@ void LocalDerivationGoal::startBuilder() /* Clean up the chroot directory automatically. */ autoDelChroot = std::make_shared<AutoDelete>(chrootRootDir); - printMsg(lvlChatty, format("setting up chroot environment in '%1%'") % chrootRootDir); + printMsg(lvlChatty, "setting up chroot environment in '%1%'", chrootRootDir); // FIXME: make this 0700 if (mkdir(chrootRootDir.c_str(), buildUser && buildUser->getUIDCount() != 1 ? 0755 : 0750) == -1) @@ -753,8 +753,7 @@ void LocalDerivationGoal::startBuilder() throw Error("home directory '%1%' exists; please remove it to assure purity of builds without sandboxing", homeDir); if (useChroot && settings.preBuildHook != "" && dynamic_cast<Derivation *>(drv.get())) { - printMsg(lvlChatty, format("executing pre-build hook '%1%'") - % settings.preBuildHook); + printMsg(lvlChatty, "executing pre-build hook '%1%'", settings.preBuildHook); auto args = useChroot ? Strings({worker.store.printStorePath(drvPath), chrootRootDir}) : Strings({ worker.store.printStorePath(drvPath) }); enum BuildHookState { @@ -803,15 +802,13 @@ void LocalDerivationGoal::startBuilder() /* Create the log file. */ Path logFile = openLogFile(); - /* Create a pipe to get the output of the builder. */ - //builderOut.create(); - - builderOut.readSide = posix_openpt(O_RDWR | O_NOCTTY); - if (!builderOut.readSide) + /* Create a pseudoterminal to get the output of the builder. */ + builderOut = posix_openpt(O_RDWR | O_NOCTTY); + if (!builderOut) throw SysError("opening pseudoterminal master"); // FIXME: not thread-safe, use ptsname_r - std::string slaveName(ptsname(builderOut.readSide.get())); + std::string slaveName = ptsname(builderOut.get()); if (buildUser) { if (chmod(slaveName.c_str(), 0600)) @@ -822,34 +819,34 @@ void LocalDerivationGoal::startBuilder() } #if __APPLE__ else { - if (grantpt(builderOut.readSide.get())) + if (grantpt(builderOut.get())) throw SysError("granting access to pseudoterminal slave"); } #endif - #if 0 - // Mount the pt in the sandbox so that the "tty" command works. - // FIXME: this doesn't work with the new devpts in the sandbox. - if (useChroot) - dirsInChroot[slaveName] = {slaveName, false}; - #endif - - if (unlockpt(builderOut.readSide.get())) + if (unlockpt(builderOut.get())) throw SysError("unlocking pseudoterminal"); - builderOut.writeSide = open(slaveName.c_str(), O_RDWR | O_NOCTTY); - if (!builderOut.writeSide) - throw SysError("opening pseudoterminal slave"); + /* Open the slave side of the pseudoterminal and use it as stderr. */ + auto openSlave = [&]() + { + AutoCloseFD builderOut = open(slaveName.c_str(), O_RDWR | O_NOCTTY); + if (!builderOut) + throw SysError("opening pseudoterminal slave"); - // Put the pt into raw mode to prevent \n -> \r\n translation. - struct termios term; - if (tcgetattr(builderOut.writeSide.get(), &term)) - throw SysError("getting pseudoterminal attributes"); + // Put the pt into raw mode to prevent \n -> \r\n translation. + struct termios term; + if (tcgetattr(builderOut.get(), &term)) + throw SysError("getting pseudoterminal attributes"); - cfmakeraw(&term); + cfmakeraw(&term); - if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term)) - throw SysError("putting pseudoterminal into raw mode"); + if (tcsetattr(builderOut.get(), TCSANOW, &term)) + throw SysError("putting pseudoterminal into raw mode"); + + if (dup2(builderOut.get(), STDERR_FILENO) == -1) + throw SysError("cannot pipe standard error into log file"); + }; buildResult.startTime = time(0); @@ -898,7 +895,16 @@ void LocalDerivationGoal::startBuilder() usingUserNamespace = userNamespacesSupported(); + Pipe sendPid; + sendPid.create(); + Pid helper = startProcess([&]() { + sendPid.readSide.close(); + + /* We need to open the slave early, before + CLONE_NEWUSER. Otherwise we get EPERM when running as + root. */ + openSlave(); /* Drop additional groups here because we can't do it after we've created the new user namespace. FIXME: @@ -920,11 +926,12 @@ void LocalDerivationGoal::startBuilder() pid_t child = startProcess([&]() { runChild(); }, options); - writeFull(builderOut.writeSide.get(), - fmt("%d %d\n", usingUserNamespace, child)); + writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); _exit(0); }); + sendPid.writeSide.close(); + if (helper.wait() != 0) throw Error("unable to start build process"); @@ -936,10 +943,9 @@ void LocalDerivationGoal::startBuilder() userNamespaceSync.writeSide = -1; }); - auto ss = tokenizeString<std::vector<std::string>>(readLine(builderOut.readSide.get())); - assert(ss.size() == 2); - usingUserNamespace = ss[0] == "1"; - pid = string2Int<pid_t>(ss[1]).value(); + auto ss = tokenizeString<std::vector<std::string>>(readLine(sendPid.readSide.get())); + assert(ss.size() == 1); + pid = string2Int<pid_t>(ss[0]).value(); if (usingUserNamespace) { /* Set the UID/GID mapping of the builder's user namespace @@ -994,21 +1000,21 @@ void LocalDerivationGoal::startBuilder() #endif { pid = startProcess([&]() { + openSlave(); runChild(); }); } /* parent */ pid.setSeparatePG(true); - builderOut.writeSide = -1; - worker.childStarted(shared_from_this(), {builderOut.readSide.get()}, true, true); + worker.childStarted(shared_from_this(), {builderOut.get()}, true, true); /* Check if setting up the build environment failed. */ std::vector<std::string> msgs; while (true) { std::string msg = [&]() { try { - return readLine(builderOut.readSide.get()); + return readLine(builderOut.get()); } catch (Error & e) { auto status = pid.wait(); e.addTrace({}, "while waiting for the build environment for '%s' to initialize (%s, previous messages: %s)", @@ -1020,7 +1026,7 @@ void LocalDerivationGoal::startBuilder() }(); if (msg.substr(0, 1) == "\2") break; if (msg.substr(0, 1) == "\1") { - FdSource source(builderOut.readSide.get()); + FdSource source(builderOut.get()); auto ex = readError(source); ex.addTrace({}, "while setting up the build environment"); throw ex; @@ -1104,7 +1110,7 @@ void LocalDerivationGoal::initEnv() env["NIX_STORE"] = worker.store.storeDir; /* The maximum number of cores to utilize for parallel building. */ - env["NIX_BUILD_CORES"] = (format("%d") % settings.buildCores).str(); + env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores); initTmpDir(); @@ -1155,10 +1161,10 @@ void LocalDerivationGoal::writeStructuredAttrs() writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); chownToBuilder(tmpDir + "/.attrs.sh"); - env["NIX_ATTRS_SH_FILE"] = tmpDir + "/.attrs.sh"; + env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox + "/.attrs.sh"; writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); chownToBuilder(tmpDir + "/.attrs.json"); - env["NIX_ATTRS_JSON_FILE"] = tmpDir + "/.attrs.json"; + env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox + "/.attrs.json"; } } @@ -1329,7 +1335,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo result.rethrow(); } - std::vector<BuildResult> buildPathsWithResults( + std::vector<KeyedBuildResult> buildPathsWithResults( const std::vector<DerivedPath> & paths, BuildMode buildMode = bmNormal, std::shared_ptr<Store> evalStore = nullptr) override @@ -1409,12 +1415,15 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo virtual void addBuildLog(const StorePath & path, std::string_view log) override { unsupported("addBuildLog"); } + + std::optional<TrustedFlag> isTrustedClient() override + { return NotTrusted; } }; void LocalDerivationGoal::startDaemon() { - settings.requireExperimentalFeature(Xp::RecursiveNix); + experimentalFeatureSettings.require(Xp::RecursiveNix); Store::Params params; params["path-info-cache-size"] = "0"; @@ -1461,7 +1470,7 @@ void LocalDerivationGoal::startDaemon() FdSink to(remote.get()); try { daemon::processConnection(store, from, to, - daemon::NotTrusted, daemon::Recursive); + NotTrusted, daemon::Recursive); debug("terminated daemon connection"); } catch (SysError &) { ignoreException(); @@ -1650,7 +1659,7 @@ void LocalDerivationGoal::runChild() try { /* child */ - commonChildInit(builderOut); + commonChildInit(); try { setupSeccomp(); @@ -1767,6 +1776,9 @@ void LocalDerivationGoal::runChild() for (auto & path : { "/etc/resolv.conf", "/etc/services", "/etc/hosts" }) if (pathExists(path)) ss.push_back(path); + + if (settings.caFile != "") + dirsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true); } for (auto & i : ss) dirsInChroot.emplace(i, i); @@ -2063,7 +2075,7 @@ void LocalDerivationGoal::runChild() /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms to find temporary directories, so we want to open up a broader place for them to dump their files, if needed. */ - Path globalTmpDir = canonPath(getEnv("TMPDIR").value_or("/tmp"), true); + Path globalTmpDir = canonPath(getEnvNonEmpty("TMPDIR").value_or("/tmp"), true); /* They don't like trailing slashes on subpath directives */ if (globalTmpDir.back() == '/') globalTmpDir.pop_back(); @@ -2165,7 +2177,7 @@ void LocalDerivationGoal::runChild() } -DrvOutputs LocalDerivationGoal::registerOutputs() +SingleDrvOutputs 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 @@ -2274,7 +2286,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() bool discardReferences = false; if (auto structuredAttrs = parsedDrv->getStructuredAttrs()) { if (auto udr = get(*structuredAttrs, "unsafeDiscardReferences")) { - settings.requireExperimentalFeature(Xp::DiscardReferences); + experimentalFeatureSettings.require(Xp::DiscardReferences); if (auto output = get(*udr, outputName)) { if (!output->is_boolean()) throw Error("attribute 'unsafeDiscardReferences.\"%s\"' of derivation '%s' must be a Boolean", outputName, drvPath.to_string()); @@ -2386,27 +2398,26 @@ DrvOutputs LocalDerivationGoal::registerOutputs() } }; - auto rewriteRefs = [&]() -> std::pair<bool, StorePathSet> { + auto rewriteRefs = [&]() -> StoreReferences { /* In the CA case, we need the rewritten refs to calculate the final path, therefore we look for a *non-rewritten self-reference, and use a bool rather try to solve the computationally intractable fixed point. */ - std::pair<bool, StorePathSet> res { - false, - {}, + StoreReferences res { + .self = false, }; for (auto & r : references) { auto name = r.name(); auto origHash = std::string { r.hashPart() }; if (r == *scratchPath) { - res.first = true; + res.self = true; } else if (auto outputRewrite = get(outputRewrites, origHash)) { std::string newRef = *outputRewrite; newRef += '-'; newRef += name; - res.second.insert(StorePath { newRef }); + res.others.insert(StorePath { newRef }); } else { - res.second.insert(r); + res.others.insert(r); } } return res; @@ -2418,39 +2429,57 @@ DrvOutputs LocalDerivationGoal::registerOutputs() throw BuildError( "output path %1% without valid stats info", actualPath); - if (outputHash.method == FileIngestionMethod::Flat) { + if (outputHash.method == ContentAddressMethod { FileIngestionMethod::Flat } || + outputHash.method == ContentAddressMethod { TextIngestionMethod {} }) + { /* The output path should be a regular file without execute permission. */ 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)", + "since recursive hashing is not enabled (one of outputHashMode={flat,text} is true)", actualPath); } rewriteOutput(); /* FIXME optimize and deduplicate with addToStore */ std::string oldHashPart { scratchPath->hashPart() }; HashModuloSink caSink { outputHash.hashType, oldHashPart }; - switch (outputHash.method) { - case FileIngestionMethod::Recursive: - dumpPath(actualPath, caSink); - break; - case FileIngestionMethod::Flat: - readFile(actualPath, caSink); - break; - } + std::visit(overloaded { + [&](const TextIngestionMethod &) { + readFile(actualPath, caSink); + }, + [&](const FileIngestionMethod & m2) { + switch (m2) { + case FileIngestionMethod::Recursive: + dumpPath(actualPath, caSink); + break; + case FileIngestionMethod::Flat: + readFile(actualPath, caSink); + break; + } + }, + }, outputHash.method.raw); auto got = caSink.finish().first; - auto refs = rewriteRefs(); - - auto finalPath = worker.store.makeFixedOutputPath( - outputHash.method, - got, - outputPathName(drv->name, outputName), - refs.second, - refs.first); - if (*scratchPath != finalPath) { + + auto optCA = ContentAddressWithReferences::fromPartsOpt( + outputHash.method, + std::move(got), + rewriteRefs()); + if (!optCA) { + // TODO track distinct failure modes separately (at the time of + // writing there is just one but `nullopt` is unclear) so this + // message can't get out of sync. + throw BuildError("output path '%s' has illegal content address, probably a spurious self-reference with text hashing"); + } + ValidPathInfo newInfo0 { + worker.store, + outputPathName(drv->name, outputName), + *std::move(optCA), + Hash::dummy, + }; + if (*scratchPath != newInfo0.path) { // Also rewrite the output path auto source = sinkToSource([&](Sink & nextSink) { - RewritingSink rsink2(oldHashPart, std::string(finalPath.hashPart()), nextSink); + RewritingSink rsink2(oldHashPart, std::string(newInfo0.path.hashPart()), nextSink); dumpPath(actualPath, rsink2); rsink2.flush(); }); @@ -2461,19 +2490,8 @@ DrvOutputs LocalDerivationGoal::registerOutputs() } HashResult narHashAndSize = hashPath(htSHA256, actualPath); - ValidPathInfo newInfo0 { - finalPath, - narHashAndSize.first, - }; - + newInfo0.narHash = narHashAndSize.first; newInfo0.narSize = narHashAndSize.second; - newInfo0.ca = FixedOutputHash { - .method = outputHash.method, - .hash = got, - }; - newInfo0.references = refs.second; - if (refs.first) - newInfo0.references.insert(newInfo0.path); assert(newInfo0.ca); return newInfo0; @@ -2495,22 +2513,23 @@ DrvOutputs LocalDerivationGoal::registerOutputs() ValidPathInfo newInfo0 { requiredFinalPath, narHashAndSize.first }; newInfo0.narSize = narHashAndSize.second; auto refs = rewriteRefs(); - newInfo0.references = refs.second; - if (refs.first) + newInfo0.references = std::move(refs.others); + if (refs.self) newInfo0.references.insert(newInfo0.path); return newInfo0; }, [&](const DerivationOutput::CAFixed & dof) { + auto wanted = dof.ca.getHash(); + auto newInfo0 = newInfoFromCA(DerivationOutput::CAFloating { - .method = dof.hash.method, - .hashType = dof.hash.hash.type, + .method = dof.ca.getMethod(), + .hashType = wanted.type, }); /* Check wanted hash */ - const Hash & wanted = dof.hash.hash; assert(newInfo0.ca); - auto got = getContentAddressHash(*newInfo0.ca); + auto got = newInfo0.ca->getHash(); if (wanted != got) { /* Throw an error after registering the path as valid. */ @@ -2521,6 +2540,11 @@ DrvOutputs LocalDerivationGoal::registerOutputs() wanted.to_string(SRI, true), got.to_string(SRI, true))); } + if (!newInfo0.references.empty()) + delayedException = std::make_exception_ptr( + BuildError("illegal path references in fixed-output derivation '%s'", + worker.store.printStorePath(drvPath))); + return newInfo0; }, @@ -2682,7 +2706,7 @@ DrvOutputs 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; + SingleDrvOutputs builtOutputs; for (auto & [outputName, newInfo] : infos) { auto oldinfo = get(initialOutputs, outputName); @@ -2694,14 +2718,13 @@ DrvOutputs LocalDerivationGoal::registerOutputs() }, .outPath = newInfo.path }; - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) + if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && drv->type().isPure()) { signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } - if (wantedOutputs.contains(outputName)) - builtOutputs.emplace(thisRealisation.id, thisRealisation); + builtOutputs.emplace(outputName, thisRealisation); } return builtOutputs; @@ -2892,7 +2915,7 @@ void LocalDerivationGoal::deleteTmpDir(bool force) bool LocalDerivationGoal::isReadDesc(int fd) { return (hook && DerivationGoal::isReadDesc(fd)) || - (!hook && fd == builderOut.readSide.get()); + (!hook && fd == builderOut.get()); } diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 34c4e9187..9acd7593d 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -1,4 +1,5 @@ #pragma once +///@file #include "derivation-goal.hh" #include "local-store.hh" @@ -9,48 +10,75 @@ struct LocalDerivationGoal : public DerivationGoal { LocalStore & getLocalStore(); - /* User selected for running the builder. */ + /** + * User selected for running the builder. + */ std::unique_ptr<UserLock> buildUser; - /* The process ID of the builder. */ + /** + * The process ID of the builder. + */ Pid pid; - /* The cgroup of the builder, if any. */ + /** + * The cgroup of the builder, if any. + */ std::optional<Path> cgroup; - /* The temporary directory. */ + /** + * The temporary directory. + */ Path tmpDir; - /* The path of the temporary directory in the sandbox. */ + /** + * The path of the temporary directory in the sandbox. + */ Path tmpDirInSandbox; - /* Pipe for the builder's standard output/error. */ - Pipe builderOut; + /** + * Master side of the pseudoterminal used for the builder's + * standard output/error. + */ + AutoCloseFD builderOut; - /* Pipe for synchronising updates to the builder namespaces. */ + /** + * Pipe for synchronising updates to the builder namespaces. + */ Pipe userNamespaceSync; - /* The mount namespace and user namespace of the builder, used to add additional - paths to the sandbox as a result of recursive Nix calls. */ + /** + * The mount namespace and user namespace of the builder, used to add additional + * paths to the sandbox as a result of recursive Nix calls. + */ AutoCloseFD sandboxMountNamespace; AutoCloseFD sandboxUserNamespace; - /* On Linux, whether we're doing the build in its own user - namespace. */ + /** + * On Linux, whether we're doing the build in its own user + * namespace. + */ bool usingUserNamespace = true; - /* Whether we're currently doing a chroot build. */ + /** + * Whether we're currently doing a chroot build. + */ bool useChroot = false; Path chrootRootDir; - /* RAII object to delete the chroot directory. */ + /** + * RAII object to delete the chroot directory. + */ std::shared_ptr<AutoDelete> autoDelChroot; - /* Whether to run the build in a private network namespace. */ + /** + * Whether to run the build in a private network namespace. + */ bool privateNetwork = false; - /* Stuff we need to pass to initChild(). */ + /** + * Stuff we need to pass to initChild(). + */ struct ChrootPath { Path source; bool optional; @@ -69,30 +97,35 @@ struct LocalDerivationGoal : public DerivationGoal SandboxProfile additionalSandboxProfile; #endif - /* Hash rewriting. */ + /** + * Hash rewriting. + */ StringMap inputRewrites, outputRewrites; typedef map<StorePath, StorePath> RedirectedOutputs; RedirectedOutputs redirectedOutputs; - /* The outputs paths used during the build. - - - Input-addressed derivations or fixed content-addressed outputs are - sometimes built when some of their outputs already exist, and can not - be hidden via sandboxing. We use temporary locations instead and - rewrite after the build. Otherwise the regular predetermined paths are - put here. - - - Floating content-addressed derivations do not know their final build - output paths until the outputs are hashed, so random locations are - used, and then renamed. The randomness helps guard against hidden - self-references. + /** + * The outputs paths used during the build. + * + * - Input-addressed derivations or fixed content-addressed outputs are + * sometimes built when some of their outputs already exist, and can not + * be hidden via sandboxing. We use temporary locations instead and + * rewrite after the build. Otherwise the regular predetermined paths are + * put here. + * + * - Floating content-addressed derivations do not know their final build + * output paths until the outputs are hashed, so random locations are + * used, and then renamed. The randomness helps guard against hidden + * self-references. */ OutputPathMap scratchOutputs; - /* Path registration info from the previous round, if we're - building multiple times. Since this contains the hash, it - allows us to compare whether two rounds produced the same - result. */ + /** + * Path registration info from the previous round, if we're + * building multiple times. Since this contains the hash, it + * allows us to compare whether two rounds produced the same + * result. + */ std::map<Path, ValidPathInfo> prevInfos; uid_t sandboxUid() { return usingUserNamespace ? (!buildUser || buildUser->getUIDCount() == 1 ? 1000 : 0) : buildUser->getUID(); } @@ -100,25 +133,37 @@ struct LocalDerivationGoal : public DerivationGoal const static Path homeDir; - /* The recursive Nix daemon socket. */ + /** + * The recursive Nix daemon socket. + */ AutoCloseFD daemonSocket; - /* The daemon main thread. */ + /** + * The daemon main thread. + */ std::thread daemonThread; - /* The daemon worker threads. */ + /** + * The daemon worker threads. + */ std::vector<std::thread> daemonWorkerThreads; - /* Paths that were added via recursive Nix calls. */ + /** + * Paths that were added via recursive Nix calls. + */ StorePathSet addedPaths; - /* Realisations that were added via recursive Nix calls. */ + /** + * Realisations that were added via recursive Nix calls. + */ std::set<DrvOutput> addedDrvOutputs; - /* Recursive Nix calls are only allowed to build or realize paths - in the original input closure or added via a recursive Nix call - (so e.g. you can't do 'nix-store -r /nix/store/<bla>' where - /nix/store/<bla> is some arbitrary path in a binary cache). */ + /** + * Recursive Nix calls are only allowed to build or realize paths + * in the original input closure or added via a recursive Nix call + * (so e.g. you can't do 'nix-store -r /nix/store/<bla>' where + * /nix/store/<bla> is some arbitrary path in a binary cache). + */ bool isAllowed(const StorePath & path) { return inputPaths.count(path) || addedPaths.count(path); @@ -136,55 +181,81 @@ struct LocalDerivationGoal : public DerivationGoal virtual ~LocalDerivationGoal() override; - /* Whether we need to perform hash rewriting if there are valid output paths. */ + /** + * Whether we need to perform hash rewriting if there are valid output paths. + */ bool needsHashRewrite(); - /* The additional states. */ + /** + * The additional states. + */ void tryLocalBuild() override; - /* Start building a derivation. */ + /** + * Start building a derivation. + */ void startBuilder(); - /* Fill in the environment for the builder. */ + /** + * Fill in the environment for the builder. + */ void initEnv(); - /* Setup tmp dir location. */ + /** + * Setup tmp dir location. + */ void initTmpDir(); - /* Write a JSON file containing the derivation attributes. */ + /** + * Write a JSON file containing the derivation attributes. + */ void writeStructuredAttrs(); void startDaemon(); void stopDaemon(); - /* Add 'path' to the set of paths that may be referenced by the - outputs, and make it appear in the sandbox. */ + /** + * Add 'path' to the set of paths that may be referenced by the + * outputs, and make it appear in the sandbox. + */ void addDependency(const StorePath & path); - /* Make a file owned by the builder. */ + /** + * Make a file owned by the builder. + */ void chownToBuilder(const Path & path); int getChildStatus() override; - /* Run the builder's process. */ + /** + * Run the builder's process. + */ void runChild(); - /* Check that the derivation outputs all exist and register them - as valid. */ - DrvOutputs registerOutputs() override; + /** + * Check that the derivation outputs all exist and register them + * as valid. + */ + SingleDrvOutputs registerOutputs() override; void signRealisation(Realisation &) override; - /* Check that an output meets the requirements specified by the - 'outputChecks' attribute (or the legacy - '{allowed,disallowed}{References,Requisites}' attributes). */ + /** + * Check that an output meets the requirements specified by the + * 'outputChecks' attribute (or the legacy + * '{allowed,disallowed}{References,Requisites}' attributes). + */ void checkOutputs(const std::map<std::string, ValidPathInfo> & outputs); - /* Close the read side of the logger pipe. */ + /** + * Close the read side of the logger pipe. + */ void closeReadPipes() override; - /* Cleanup hooks for buildDone() */ + /** + * Cleanup hooks for buildDone() + */ void cleanupHookFinally() override; void cleanupPreChildKill() override; void cleanupPostChildKill() override; @@ -194,24 +265,36 @@ struct LocalDerivationGoal : public DerivationGoal bool isReadDesc(int fd) override; - /* Delete the temporary directory, if we have one. */ + /** + * Delete the temporary directory, if we have one. + */ void deleteTmpDir(bool force); - /* Forcibly kill the child process, if any. */ + /** + * Forcibly kill the child process, if any. + */ void killChild() override; - /* Kill any processes running under the build user UID or in the - cgroup of the build. */ + /** + * Kill any processes running under the build user UID or in the + * cgroup of the build. + */ void killSandbox(bool getStats); - /* Create alternative path calculated from but distinct from the - input, so we can avoid overwriting outputs (or other store paths) - that already exist. */ + /** + * Create alternative path calculated from but distinct from the + * input, so we can avoid overwriting outputs (or other store paths) + * that already exist. + */ StorePath makeFallbackPath(const StorePath & path); - /* Make a path to another based on the output name along with the - derivation hash. */ - /* FIXME add option to randomize, so we can audit whether our - rewrites caught everything */ + + /** + * Make a path to another based on the output name along with the + * derivation hash. + * + * @todo Add option to randomize, so we can audit whether our + * rewrites caught everything + */ StorePath makeFallbackPath(std::string_view outputName); }; diff --git a/src/libstore/build/personality.hh b/src/libstore/build/personality.hh index 30e4f4062..91b730fab 100644 --- a/src/libstore/build/personality.hh +++ b/src/libstore/build/personality.hh @@ -1,4 +1,5 @@ #pragma once +///@file #include <string> diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 2af105b4d..93867007d 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -95,7 +95,9 @@ void PathSubstitutionGoal::tryNext() subs.pop_front(); if (ca) { - subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); + subPath = sub->makeFixedOutputPathFromCA( + std::string { storePath.name() }, + ContentAddressWithReferences::withoutRefs(*ca)); if (sub->storeDir == worker.store.storeDir) assert(subPath == storePath); } else if (sub->storeDir != worker.store.storeDir) { @@ -198,11 +200,10 @@ void PathSubstitutionGoal::tryToRun() { trace("trying to run"); - /* Make sure that we are allowed to start a build. Note that even - if maxBuildJobs == 0 (no local builds allowed), we still allow - a substituter to run. This is because substitutions cannot be - distributed to another machine via the build hook. */ - if (worker.getNrLocalBuilds() >= std::max(1U, (unsigned int) settings.maxBuildJobs)) { + /* Make sure that we are allowed to start a substitution. Note that even + if maxSubstitutionJobs == 0, we still allow a substituter to run. This + prevents infinite waiting. */ + if (worker.getNrSubstitutions() >= std::max(1U, (unsigned int) settings.maxSubstitutionJobs)) { worker.waitForBuildSlot(shared_from_this()); return; } diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index a73f8e666..9fc041920 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -1,4 +1,5 @@ #pragma once +///@file #include "lock.hh" #include "store-api.hh" @@ -10,38 +11,58 @@ class Worker; struct PathSubstitutionGoal : public Goal { - /* The store path that should be realised through a substitute. */ + /** + * The store path that should be realised through a substitute. + */ StorePath storePath; - /* The path the substituter refers to the path as. This will be - different when the stores have different names. */ + /** + * The path the substituter refers to the path as. This will be + * different when the stores have different names. + */ std::optional<StorePath> subPath; - /* The remaining substituters. */ + /** + * The remaining substituters. + */ std::list<ref<Store>> subs; - /* The current substituter. */ + /** + * The current substituter. + */ std::shared_ptr<Store> sub; - /* Whether a substituter failed. */ + /** + * Whether a substituter failed. + */ bool substituterFailed = false; - /* Path info returned by the substituter's query info operation. */ + /** + * Path info returned by the substituter's query info operation. + */ std::shared_ptr<const ValidPathInfo> info; - /* Pipe for the substituter's standard output. */ + /** + * Pipe for the substituter's standard output. + */ Pipe outPipe; - /* The substituter thread. */ + /** + * The substituter thread. + */ std::thread thr; std::promise<void> promise; - /* Whether to try to repair a valid path. */ + /** + * Whether to try to repair a valid path. + */ RepairFlag repair; - /* Location where we're downloading the substitute. Differs from - storePath when doing a repair. */ + /** + * Location where we're downloading the substitute. Differs from + * storePath when doing a repair. + */ Path destPath; std::unique_ptr<MaintainCount<uint64_t>> maintainExpectedSubstitutions, @@ -50,7 +71,9 @@ struct PathSubstitutionGoal : public Goal typedef void (PathSubstitutionGoal::*GoalState)(); GoalState state; - /* Content address for recomputing store path */ + /** + * Content address for recomputing store path + */ std::optional<ContentAddress> ca; void done( @@ -64,16 +87,20 @@ public: void timedOut(Error && ex) override { abort(); }; + /** + * We prepend "a$" to the key name to ensure substitution goals + * happen before derivation goals. + */ std::string key() override { - /* "a$" ensures substitution goals happen before derivation - goals. */ return "a$" + std::string(storePath.name()) + "$" + worker.store.printStorePath(storePath); } void work() override; - /* The states. */ + /** + * The states. + */ void init(); void tryNext(); void gotInfo(); @@ -81,11 +108,15 @@ public: void tryToRun(); void finished(); - /* Callback used by the worker to write to the log. */ + /** + * Callback used by the worker to write to the log. + */ void handleChildOutput(int fd, std::string_view data) override; void handleEOF(int fd) override; void cleanup() override; + + JobCategory jobCategory() override { return JobCategory::Substitution; }; }; } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index f775f8486..ee334d54a 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -18,6 +18,7 @@ Worker::Worker(Store & store, Store & evalStore) { /* Debugging: prevent recursive workers. */ nrLocalBuilds = 0; + nrSubstitutions = 0; lastWokenUp = steady_time_point::min(); permanentFailure = false; timedOut = false; @@ -92,6 +93,7 @@ std::shared_ptr<PathSubstitutionGoal> Worker::makePathSubstitutionGoal(const Sto return goal; } + std::shared_ptr<DrvOutputSubstitutionGoal> Worker::makeDrvOutputSubstitutionGoal(const DrvOutput& id, RepairFlag repair, std::optional<ContentAddress> ca) { std::weak_ptr<DrvOutputSubstitutionGoal> & goal_weak = drvOutputSubstitutionGoals[id]; @@ -104,6 +106,20 @@ std::shared_ptr<DrvOutputSubstitutionGoal> Worker::makeDrvOutputSubstitutionGoal return goal; } + +GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) +{ + return std::visit(overloaded { + [&](const DerivedPath::Built & bfd) -> GoalPtr { + return makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode); + }, + [&](const DerivedPath::Opaque & bo) -> GoalPtr { + return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); + }, + }, req.raw()); +} + + template<typename K, typename G> static void removeGoal(std::shared_ptr<G> goal, std::map<K, std::weak_ptr<G>> & goalMap) { @@ -161,6 +177,12 @@ unsigned Worker::getNrLocalBuilds() } +unsigned Worker::getNrSubstitutions() +{ + return nrSubstitutions; +} + + void Worker::childStarted(GoalPtr goal, const std::set<int> & fds, bool inBuildSlot, bool respectTimeouts) { @@ -172,7 +194,10 @@ void Worker::childStarted(GoalPtr goal, const std::set<int> & fds, child.inBuildSlot = inBuildSlot; child.respectTimeouts = respectTimeouts; children.emplace_back(child); - if (inBuildSlot) nrLocalBuilds++; + if (inBuildSlot) { + if (goal->jobCategory() == JobCategory::Substitution) nrSubstitutions++; + else nrLocalBuilds++; + } } @@ -183,8 +208,13 @@ void Worker::childTerminated(Goal * goal, bool wakeSleepers) if (i == children.end()) return; if (i->inBuildSlot) { - assert(nrLocalBuilds > 0); - nrLocalBuilds--; + if (goal->jobCategory() == JobCategory::Substitution) { + assert(nrSubstitutions > 0); + nrSubstitutions--; + } else { + assert(nrLocalBuilds > 0); + nrLocalBuilds--; + } } children.erase(i); @@ -205,7 +235,9 @@ void Worker::childTerminated(Goal * goal, bool wakeSleepers) void Worker::waitForBuildSlot(GoalPtr goal) { debug("wait for build slot"); - if (getNrLocalBuilds() < settings.maxBuildJobs) + bool isSubstitutionGoal = goal->jobCategory() == JobCategory::Substitution; + if ((!isSubstitutionGoal && getNrLocalBuilds() < settings.maxBuildJobs) || + (isSubstitutionGoal && getNrSubstitutions() < settings.maxSubstitutionJobs)) wakeUp(goal); /* we can do it right away */ else addToWeakGoals(wantingToBuild, goal); diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 6d68d3cf1..63624d910 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -1,4 +1,5 @@ #pragma once +///@file #include "types.hh" #include "lock.hh" @@ -16,24 +17,29 @@ struct DerivationGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; -/* Workaround for not being able to declare a something like - - class PathSubstitutionGoal : public Goal; - - even when Goal is a complete type. - - This is still a static cast. The purpose of exporting it is to define it in - a place where `PathSubstitutionGoal` is concrete, and use it in a place where it - is opaque. */ +/** + * Workaround for not being able to declare a something like + * + * ```c++ + * class PathSubstitutionGoal : public Goal; + * ``` + * even when Goal is a complete type. + * + * This is still a static cast. The purpose of exporting it is to define it in + * a place where `PathSubstitutionGoal` is concrete, and use it in a place where it + * is opaque. + */ GoalPtr upcast_goal(std::shared_ptr<PathSubstitutionGoal> subGoal); GoalPtr upcast_goal(std::shared_ptr<DrvOutputSubstitutionGoal> subGoal); typedef std::chrono::time_point<std::chrono::steady_clock> steady_time_point; -/* A mapping used to remember for each child process to what goal it - belongs, and file descriptors for receiving log data and output - path creation commands. */ +/** + * A mapping used to remember for each child process to what goal it + * belongs, and file descriptors for receiving log data and output + * path creation commands. + */ struct Child { WeakGoalPtr goal; @@ -41,14 +47,19 @@ struct Child std::set<int> fds; bool respectTimeouts; bool inBuildSlot; - steady_time_point lastOutput; /* time we last got output on stdout/stderr */ + /** + * Time we last got output on stdout/stderr + */ + steady_time_point lastOutput; steady_time_point timeStarted; }; /* Forward definition. */ struct HookInstance; -/* The worker class. */ +/** + * The worker class. + */ class Worker { private: @@ -56,38 +67,63 @@ private: /* Note: the worker should only have strong pointers to the top-level goals. */ - /* The top-level goals of the worker. */ + /** + * The top-level goals of the worker. + */ Goals topGoals; - /* Goals that are ready to do some work. */ + /** + * Goals that are ready to do some work. + */ WeakGoals awake; - /* Goals waiting for a build slot. */ + /** + * Goals waiting for a build slot. + */ WeakGoals wantingToBuild; - /* Child processes currently running. */ + /** + * Child processes currently running. + */ std::list<Child> children; - /* Number of build slots occupied. This includes local builds and - substitutions but not remote builds via the build hook. */ + /** + * Number of build slots occupied. This includes local builds but does not + * include substitutions or remote builds via the build hook. + */ unsigned int nrLocalBuilds; - /* Maps used to prevent multiple instantiations of a goal for the - same derivation / path. */ + /** + * Number of substitution slots occupied. + */ + unsigned int nrSubstitutions; + + /** + * Maps used to prevent multiple instantiations of a goal for the + * same derivation / path. + */ std::map<StorePath, std::weak_ptr<DerivationGoal>> derivationGoals; std::map<StorePath, std::weak_ptr<PathSubstitutionGoal>> substitutionGoals; std::map<DrvOutput, std::weak_ptr<DrvOutputSubstitutionGoal>> drvOutputSubstitutionGoals; - /* Goals waiting for busy paths to be unlocked. */ + /** + * Goals waiting for busy paths to be unlocked. + */ WeakGoals waitingForAnyGoal; - /* Goals sleeping for a few seconds (polling a lock). */ + /** + * Goals sleeping for a few seconds (polling a lock). + */ WeakGoals waitingForAWhile; - /* Last time the goals in `waitingForAWhile' where woken up. */ + /** + * Last time the goals in `waitingForAWhile` where woken up. + */ steady_time_point lastWokenUp; - /* Cache for pathContentsGood(). */ + /** + * Cache for pathContentsGood(). + */ std::map<StorePath, bool> pathContentsGoodCache; public: @@ -96,17 +132,25 @@ public: const Activity actDerivations; const Activity actSubstitutions; - /* Set if at least one derivation had a BuildError (i.e. permanent - failure). */ + /** + * Set if at least one derivation had a BuildError (i.e. permanent + * failure). + */ bool permanentFailure; - /* Set if at least one derivation had a timeout. */ + /** + * Set if at least one derivation had a timeout. + */ bool timedOut; - /* Set if at least one derivation fails with a hash mismatch. */ + /** + * Set if at least one derivation fails with a hash mismatch. + */ bool hashMismatch; - /* Set if at least one derivation is not deterministic in check mode. */ + /** + * Set if at least one derivation is not deterministic in check mode. + */ bool checkMismatch; Store & store; @@ -128,16 +172,22 @@ public: uint64_t expectedNarSize = 0; uint64_t doneNarSize = 0; - /* Whether to ask the build hook if it can build a derivation. If - it answers with "decline-permanently", we don't try again. */ + /** + * Whether to ask the build hook if it can build a derivation. If + * it answers with "decline-permanently", we don't try again. + */ bool tryBuildHook = true; Worker(Store & store, Store & evalStore); ~Worker(); - /* Make a goal (with caching). */ + /** + * Make a goal (with caching). + */ - /* derivation goal */ + /** + * @ref DerivationGoal "derivation goal" + */ private: std::shared_ptr<DerivationGoal> makeDerivationGoalCommon( const StorePath & drvPath, const OutputsSpec & wantedOutputs, @@ -150,56 +200,92 @@ public: const StorePath & drvPath, const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); - /* substitution goal */ + /** + * @ref SubstitutionGoal "substitution goal" + */ std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(const DrvOutput & id, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); - /* Remove a dead goal. */ + /** + * Make a goal corresponding to the `DerivedPath`. + * + * It will be a `DerivationGoal` for a `DerivedPath::Built` or + * a `SubstitutionGoal` for a `DerivedPath::Opaque`. + */ + GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal); + + /** + * Remove a dead goal. + */ void removeGoal(GoalPtr goal); - /* Wake up a goal (i.e., there is something for it to do). */ + /** + * Wake up a goal (i.e., there is something for it to do). + */ void wakeUp(GoalPtr goal); - /* Return the number of local build and substitution processes - currently running (but not remote builds via the build - hook). */ + /** + * Return the number of local build processes currently running (but not + * remote builds via the build hook). + */ unsigned int getNrLocalBuilds(); - /* Registers a running child process. `inBuildSlot' means that - the process counts towards the jobs limit. */ + /** + * Return the number of substitution processes currently running. + */ + unsigned int getNrSubstitutions(); + + /** + * Registers a running child process. `inBuildSlot` means that + * the process counts towards the jobs limit. + */ void childStarted(GoalPtr goal, const std::set<int> & fds, bool inBuildSlot, bool respectTimeouts); - /* Unregisters a running child process. `wakeSleepers' should be - false if there is no sense in waking up goals that are sleeping - because they can't run yet (e.g., there is no free build slot, - or the hook would still say `postpone'). */ + /** + * Unregisters a running child process. `wakeSleepers` should be + * false if there is no sense in waking up goals that are sleeping + * because they can't run yet (e.g., there is no free build slot, + * or the hook would still say `postpone`). + */ void childTerminated(Goal * goal, bool wakeSleepers = true); - /* Put `goal' to sleep until a build slot becomes available (which - might be right away). */ + /** + * Put `goal` to sleep until a build slot becomes available (which + * might be right away). + */ void waitForBuildSlot(GoalPtr goal); - /* Wait for any goal to finish. Pretty indiscriminate way to - wait for some resource that some other goal is holding. */ + /** + * Wait for any goal to finish. Pretty indiscriminate way to + * wait for some resource that some other goal is holding. + */ void waitForAnyGoal(GoalPtr goal); - /* Wait for a few seconds and then retry this goal. Used when - waiting for a lock held by another process. This kind of - polling is inefficient, but POSIX doesn't really provide a way - to wait for multiple locks in the main select() loop. */ + /** + * Wait for a few seconds and then retry this goal. Used when + * waiting for a lock held by another process. This kind of + * polling is inefficient, but POSIX doesn't really provide a way + * to wait for multiple locks in the main select() loop. + */ void waitForAWhile(GoalPtr goal); - /* Loop until the specified top-level goals have finished. */ + /** + * Loop until the specified top-level goals have finished. + */ void run(const Goals & topGoals); - /* Wait for input to become available. */ + /** + * Wait for input to become available. + */ void waitForInput(); unsigned int exitStatus(); - /* Check whether the given valid path exists and has the right - contents. */ + /** + * Check whether the given valid path exists and has the right + * contents. + */ bool pathContentsGood(const StorePath & path); void markContentsGood(const StorePath & path); |