From f3ff1da42479eda7826d1bcb10e3a067e62a2daa Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 20 Jul 2015 03:15:45 +0200 Subject: daemon: Better distinguish build statuses. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Nix itself, the new 'BuildResult' type is returned by the new 'buildDerivation' method, which we don't have and need. * nix/libstore/build.cc (Goal)[cancel]: Remove. [timeOut]: New pure virtual method. (DerivationGoal)[result]: New field. [cancel]: Remove. [timedOut, getResult, done]: New methods. (DerivationGoal::cancel): Remove. (DerivationGoal::timedOut): New method. (DerivationGoal::haveDerivation): Call 'done' instead of 'amDone'. (DerivationGoal::outputsSubstituted): Ditto. (DerivationGoal::inputsRealised): Ditto. (DerivationGoal::buildDone): Ditto. (DerivationGoal::handleChildOutput): Call 'timedOut' instead of 'cancel'. (DerivationGoal::done): New method. (SubstitutionGoal)[cancel]: Remove. [timedOut]: New method. (SubstitutionGoal::cancel): Remove. (SubstitutionGoal::timedOut): New method. (Worker::waitForInput): Use it. * nix/libstore/store-api.hh (BuildResult): New struct. Co-authored-by: Ludovic Courtès --- nix/libstore/build.cc | 93 +++++++++++++++++++++++++++++------------------ nix/libstore/store-api.hh | 24 ++++++++++++ 2 files changed, 82 insertions(+), 35 deletions(-) diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index 47c7f9728e..ff7b2c4771 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -84,6 +84,7 @@ struct HookInstance; /* A pointer to a goal. */ class Goal; +class DerivationGoal; typedef std::shared_ptr GoalPtr; typedef std::weak_ptr WeakGoalPtr; @@ -174,10 +175,10 @@ public: return exitCode; } - /* Cancel the goal. It should wake up its waiters, get rid of any - running child processes that are being monitored by the worker - (important!), etc. */ - virtual void cancel(bool timeout) = 0; + /* 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() = 0; virtual string key() = 0; @@ -799,11 +800,13 @@ private: result. */ ValidPathInfos prevInfos; + BuildResult result; + public: DerivationGoal(const Path & drvPath, const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode = bmNormal); ~DerivationGoal(); - void cancel(bool timeout); + void timedOut() override; string key() { @@ -824,6 +827,8 @@ public: /* Add wanted outputs to an already existing derivation goal. */ void addWantedOutputs(const StringSet & outputs); + BuildResult getResult() { return result; } + private: /* The states. */ void init(); @@ -874,6 +879,8 @@ private: Path addHashRewrite(const Path & path); void repairClosure(); + + void done(BuildResult::Status status, const string & msg = ""); }; @@ -933,12 +940,12 @@ void DerivationGoal::killChild() } -void DerivationGoal::cancel(bool timeout) +void DerivationGoal::timedOut() { - if (settings.printBuildTrace && timeout) + if (settings.printBuildTrace) printMsg(lvlError, format("@ build-failed %1% - timeout") % drvPath); killChild(); - amDone(ecFailed); + done(BuildResult::TimedOut); } @@ -991,8 +998,8 @@ void DerivationGoal::haveDerivation() trace("loading derivation"); if (nrFailed != 0) { - printMsg(lvlError, format("cannot build missing derivation `%1%'") % drvPath); - amDone(ecFailed); + printMsg(lvlError, format("cannot build missing derivation ‘%1%’") % drvPath); + done(BuildResult::MiscFailure); return; } @@ -1014,7 +1021,7 @@ void DerivationGoal::haveDerivation() /* If they are all valid, then we're done. */ if (invalidOutputs.size() == 0 && buildMode == bmNormal) { - amDone(ecSuccess); + done(BuildResult::AlreadyValid); return; } @@ -1059,7 +1066,7 @@ void DerivationGoal::outputsSubstituted() unsigned int nrInvalid = checkPathValidity(false, buildMode == bmRepair).size(); if (buildMode == bmNormal && nrInvalid == 0) { - amDone(ecSuccess); + done(BuildResult::Substituted); return; } if (buildMode == bmRepair && nrInvalid == 0) { @@ -1132,7 +1139,7 @@ void DerivationGoal::repairClosure() } if (waitees.empty()) { - amDone(ecSuccess); + done(BuildResult::AlreadyValid); return; } @@ -1144,8 +1151,8 @@ void DerivationGoal::closureRepaired() { trace("closure repaired"); if (nrFailed > 0) - throw Error(format("some paths in the output closure of derivation `%1%' could not be repaired") % drvPath); - amDone(ecSuccess); + throw Error(format("some paths in the output closure of derivation ‘%1%’ could not be repaired") % drvPath); + done(BuildResult::AlreadyValid); } @@ -1157,7 +1164,7 @@ void DerivationGoal::inputsRealised() printMsg(lvlError, format("cannot build derivation `%1%': %2% dependencies couldn't be built") % drvPath % nrFailed); - amDone(ecFailed); + done(BuildResult::DependencyFailed); return; } @@ -1286,7 +1293,7 @@ void DerivationGoal::tryToBuild() if (buildMode != bmCheck && validPaths.size() == drv.outputs.size()) { debug(format("skipping build of derivation `%1%', someone beat us to it") % drvPath); outputLocks.setDeletion(true); - amDone(ecSuccess); + done(BuildResult::AlreadyValid); return; } @@ -1358,7 +1365,7 @@ void DerivationGoal::tryToBuild() printMsg(lvlError, format("@ build-failed %1% - %2% %3%") % drvPath % 0 % e.msg()); worker.permanentFailure = true; - amDone(ecFailed); + done(BuildResult::InputRejected, e.msg()); return; } @@ -1473,7 +1480,7 @@ void DerivationGoal::buildDone() registerOutputs(); if (buildMode == bmCheck) { - amDone(ecSuccess); + done(BuildResult::Built); return; } @@ -1508,10 +1515,12 @@ void DerivationGoal::buildDone() outputLocks.unlock(); buildUser.release(); + BuildResult::Status st = BuildResult::MiscFailure; + if (hook && WIFEXITED(status) && WEXITSTATUS(status) == 101) { if (settings.printBuildTrace) printMsg(lvlError, format("@ build-failed %1% - timeout") % drvPath); - worker.timedOut = true; + st = BuildResult::TimedOut; } else if (hook && (!WIFEXITED(status) || WEXITSTATUS(status) != 100)) { @@ -1524,7 +1533,11 @@ void DerivationGoal::buildDone() if (settings.printBuildTrace) printMsg(lvlError, format("@ build-failed %1% - %2% %3%") % drvPath % 1 % e.msg()); - worker.permanentFailure = !fixedOutput && !diskFull; + + st = + statusOk(status) ? BuildResult::OutputRejected : + fixedOutput || diskFull ? BuildResult::TransientFailure : + BuildResult::PermanentFailure; /* Register the outputs of this build as "failed" so we won't try to build them again (negative caching). @@ -1538,7 +1551,7 @@ void DerivationGoal::buildDone() worker.store.registerFailedPath(i->second.path); } - amDone(ecFailed); + done(st, e.msg()); return; } @@ -1548,7 +1561,7 @@ void DerivationGoal::buildDone() if (settings.printBuildTrace) printMsg(lvlError, format("@ build-succeeded %1% -") % drvPath); - amDone(ecSuccess); + done(BuildResult::Built); } @@ -2579,7 +2592,7 @@ void DerivationGoal::handleChildOutput(int fd, const string & data) printMsg(lvlError, format("%1% killed after writing more than %2% bytes of log output") % getName() % settings.maxLogSize); - cancel(true); // not really a timeout, but close enough + timedOut(); // not really a timeout, but close enough return; } if (verbosity >= settings.buildVerbosity) @@ -2628,8 +2641,7 @@ bool DerivationGoal::pathFailed(const Path & path) if (settings.printBuildTrace) printMsg(lvlError, format("@ build-failed %1% - cached") % drvPath); - worker.permanentFailure = true; - amDone(ecFailed); + done(BuildResult::CachedFailure); return true; } @@ -2649,6 +2661,18 @@ Path DerivationGoal::addHashRewrite(const Path & path) } +void DerivationGoal::done(BuildResult::Status status, const string & msg) +{ + result.status = status; + result.errorMsg = msg; + amDone(result.success() ? ecSuccess : ecFailed); + if (result.status == BuildResult::TimedOut) + worker.timedOut = true; + if (result.status == BuildResult::PermanentFailure || result.status == BuildResult::CachedFailure) + worker.permanentFailure = true; +} + + ////////////////////////////////////////////////////////////////////// @@ -2698,7 +2722,7 @@ public: SubstitutionGoal(const Path & storePath, Worker & worker, bool repair = false); ~SubstitutionGoal(); - void cancel(bool timeout); + void timedOut(); string key() { @@ -2743,9 +2767,9 @@ SubstitutionGoal::~SubstitutionGoal() } -void SubstitutionGoal::cancel(bool timeout) +void SubstitutionGoal::timedOut() { - if (settings.printBuildTrace && timeout) + if (settings.printBuildTrace) printMsg(lvlError, format("@ substituter-failed %1% timeout") % storePath); if (pid != -1) { pid_t savedPid = pid; @@ -3066,7 +3090,8 @@ Worker::~Worker() } -GoalPtr Worker::makeDerivationGoal(const Path & path, const StringSet & wantedOutputs, BuildMode buildMode) +GoalPtr Worker::makeDerivationGoal(const Path & path, + const StringSet & wantedOutputs, BuildMode buildMode) { GoalPtr goal = derivationGoals[path].lock(); if (!goal) { @@ -3323,7 +3348,7 @@ void Worker::waitForInput() /* Since goals may be canceled from inside the loop below (causing them go be erased from the `children' map), we have to be careful that we don't keep iterators alive across calls to - cancel(). */ + timedOut(). */ set pids; foreach (Children::iterator, i, children) pids.insert(i->first); @@ -3365,8 +3390,7 @@ void Worker::waitForInput() printMsg(lvlError, format("%1% timed out after %2% seconds of silence") % goal->getName() % settings.maxSilentTime); - goal->cancel(true); - timedOut = true; + goal->timedOut(); } else if (goal->getExitCode() == Goal::ecBusy && @@ -3377,8 +3401,7 @@ void Worker::waitForInput() printMsg(lvlError, format("%1% timed out after %2% seconds") % goal->getName() % settings.buildTimeout); - goal->cancel(true); - timedOut = true; + goal->timedOut(); } } diff --git a/nix/libstore/store-api.hh b/nix/libstore/store-api.hh index 9403cbee19..3e982f6dd3 100644 --- a/nix/libstore/store-api.hh +++ b/nix/libstore/store-api.hh @@ -106,6 +106,30 @@ typedef list ValidPathInfos; enum BuildMode { bmNormal, bmRepair, bmCheck }; +struct BuildResult +{ + enum Status { + Built = 0, + Substituted, + AlreadyValid, + PermanentFailure, + InputRejected, + OutputRejected, + TransientFailure, // possibly transient + CachedFailure, + TimedOut, + MiscFailure, + DependencyFailed, + LogLimitExceeded, + NotDeterministic, + } status = MiscFailure; + std::string errorMsg; + //time_t startTime = 0, stopTime = 0; + bool success() { + return status == Built || status == Substituted || status == AlreadyValid; + } +}; + class StoreAPI { -- cgit v1.2.3