From b23b4d394a39b60188ed74ecdf1027bc7dd5b9b3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 8 Dec 2015 22:50:18 +0100 Subject: daemon: Allow builds to be repeated. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes it easy to detect non-deterministic builds. * nix/libstore/build.cc (DerivationGoal): Remove 'InodesSeen'; add 'curRound', 'nrRound', and 'prevInfos'. (DerivationGoal::inputsRealised): Initialize 'nrRound'. (NotDeterministic): New error type. (DerivationGoal::buildDone): Check whether we need to repeat. (DerivationGoal::startBuilder): Adjust message. (DerivationGoal::registerOutputs): Check whether we get the same result. * nix/libstore/globals.cc (Settings::get(const string & name, int def)): New method. * nix/libstore/globals.hh (Settings): Add it. * nix/libstore/store-api.hh (ValidPathInfo): Add operator ==. * nix/nix-daemon/nix-daemon.cc (performOp): Allow "build-repeat" for "untrusted" users. Co-authored-by: Ludovic Courtès --- nix/libstore/build.cc | 61 ++++++++++++++++++++++++++++++++++++-------- nix/libstore/globals.cc | 7 +++++ nix/libstore/globals.hh | 2 ++ nix/libstore/store-api.hh | 13 +++++++--- nix/nix-daemon/nix-daemon.cc | 2 +- 5 files changed, 71 insertions(+), 14 deletions(-) (limited to 'nix') diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index 64678a5594..e19c32b1c9 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -785,10 +785,16 @@ private: temporary paths. */ PathSet redirectedBadOutputs; - /* Set of inodes seen during calls to canonicalisePathMetaData() - for this build's outputs. This needs to be shared between - outputs to allow hard links between outputs. */ - InodesSeen inodesSeen; + /* The current round, if we're building multiple times. */ + unsigned int curRound = 1; + + unsigned int nrRounds; + + /* 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. */ + ValidPathInfos prevInfos; public: DerivationGoal(const Path & drvPath, const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode = bmNormal); @@ -1194,8 +1200,12 @@ void DerivationGoal::inputsRealised() /* Is this a fixed-output derivation? */ fixedOutput = true; - foreach (DerivationOutputs::iterator, i, drv.outputs) - if (i->second.hash == "") fixedOutput = false; + for (auto & i : drv.outputs) + if (i.second.hash == "") fixedOutput = false; + + /* Don't repeat fixed-output derivations since they're already + verified by their output hash.*/ + nrRounds = fixedOutput ? 1 : settings.get("build-repeat", 0) + 1; /* Okay, try to build. Note that here we don't wait for a build slot to become available, since we don't need one if there is a @@ -1371,6 +1381,9 @@ void replaceValidPath(const Path & storePath, const Path tmpPath) } +MakeError(NotDeterministic, BuildError) + + void DerivationGoal::buildDone() { trace("build done"); @@ -1470,6 +1483,15 @@ void DerivationGoal::buildDone() deleteTmpDir(true); + /* Repeat the build if necessary. */ + if (curRound++ < nrRounds) { + outputLocks.unlock(); + buildUser.release(); + state = &DerivationGoal::tryToBuild; + worker.wakeUp(shared_from_this()); + return; + } + /* It is now safe to delete the lock files, since all future lockers will see that the output paths are valid; they will not create new lock files with the same names as the old @@ -1623,10 +1645,13 @@ int childEntry(void * arg) void DerivationGoal::startBuilder() { - startNest(nest, lvlInfo, format( - buildMode == bmRepair ? "repairing path(s) %1%" : - buildMode == bmCheck ? "checking path(s) %1%" : - "building path(s) %1%") % showPaths(missingPaths)); + auto f = format( + buildMode == bmRepair ? "repairing path(s) %1%" : + buildMode == bmCheck ? "checking path(s) %1%" : + nrRounds > 1 ? "building path(s) %1% (round %2%/%3%)" : + "building path(s) %1%"); + f.exceptions(boost::io::all_error_bits ^ boost::io::too_many_args_bit); + startNest(nest, lvlInfo, f % showPaths(missingPaths) % curRound % nrRounds); /* Right platform? */ if (!canBuildLocally(drv.platform)) { @@ -1638,6 +1663,7 @@ void DerivationGoal::startBuilder() } /* Construct the environment passed to the builder. */ + env.clear(); /* Most shells initialise PATH to some default (/bin:/usr/bin:...) when PATH is not set. We don't want this, so we fill it in with some dummy @@ -2267,6 +2293,11 @@ void DerivationGoal::registerOutputs() ValidPathInfos infos; + /* Set of inodes seen during calls to canonicalisePathMetaData() + for this build's outputs. This needs to be shared between + outputs to allow hard links between outputs. */ + InodesSeen inodesSeen; + /* Check whether the output paths were created, and grep each output path to determine what other paths it references. Also make all output paths read-only. */ @@ -2438,6 +2469,16 @@ void DerivationGoal::registerOutputs() if (buildMode == bmCheck) return; + if (curRound > 1 && prevInfos != infos) + throw NotDeterministic( + format("result of ‘%1%’ differs from previous round; rejecting as non-deterministic") + % drvPath); + + if (curRound < nrRounds) { + prevInfos = infos; + return; + } + /* Register each output path as valid, and register the sets of paths referenced by each of them. If there are cycles in the outputs, this will fail. */ diff --git a/nix/libstore/globals.cc b/nix/libstore/globals.cc index 07f23d469c..84fc885eba 100644 --- a/nix/libstore/globals.cc +++ b/nix/libstore/globals.cc @@ -137,6 +137,13 @@ bool Settings::get(const string & name, bool def) return res; } +int Settings::get(const string & name, int def) +{ + int res = def; + _get(res, name); + return res; +} + void Settings::update() { diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh index c17e10d7c3..8c07e360f2 100644 --- a/nix/libstore/globals.hh +++ b/nix/libstore/globals.hh @@ -27,6 +27,8 @@ struct Settings { bool get(const string & name, bool def); + int get(const string & name, int def); + void update(); string pack(); diff --git a/nix/libstore/store-api.hh b/nix/libstore/store-api.hh index 3764f3e542..9403cbee19 100644 --- a/nix/libstore/store-api.hh +++ b/nix/libstore/store-api.hh @@ -88,10 +88,17 @@ struct ValidPathInfo Path deriver; Hash hash; PathSet references; - time_t registrationTime; - unsigned long long narSize; // 0 = unknown + time_t registrationTime = 0; + unsigned long long narSize = 0; // 0 = unknown unsigned long long id; // internal use only - ValidPathInfo() : registrationTime(0), narSize(0) { } + + bool operator == (const ValidPathInfo & i) const + { + return + path == i.path + && hash == i.hash + && references == i.references; + } }; typedef list ValidPathInfos; diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc index c1e697bcac..35c284f7e1 100644 --- a/nix/nix-daemon/nix-daemon.cc +++ b/nix/nix-daemon/nix-daemon.cc @@ -565,7 +565,7 @@ static void performOp(bool trusted, unsigned int clientVersion, for (unsigned int i = 0; i < n; i++) { string name = readString(from); string value = readString(from); - if (name == "build-timeout" || name == "use-ssh-substituter") + if (name == "build-timeout" || name == "build-repeat" || name == "use-ssh-substituter") settings.set(name, value); else settings.set(trusted ? name : "untrusted-" + name, value); -- cgit 1.4.1