From 2001895f3d2668549feb60a182aa624a7b6292eb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 2 Oct 2012 17:13:46 -0400 Subject: [PATCH] =?UTF-8?q?Add=20a=20--repair=20flag=20to=20=E2=80=98nix-s?= =?UTF-8?q?tore=20-r=E2=80=99=20to=20repair=20derivation=20outputs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this flag, if any valid derivation output is missing or corrupt, it will be recreated by using a substitute if available, or by rebuilding the derivation. The latter may use hash rewriting if chroots are not available. --- src/libstore/build.cc | 153 +++++++++++++++++++++-------------- src/libstore/local-store.cc | 10 +++ src/libstore/local-store.hh | 6 +- src/libstore/remote-store.cc | 3 +- src/libstore/remote-store.hh | 2 +- src/libstore/store-api.hh | 2 +- src/nix-store/nix-store.cc | 4 +- 7 files changed, 116 insertions(+), 64 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 3097b55ef..ddcd45a61 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -241,7 +241,7 @@ public: ~Worker(); /* Make a goal (with caching). */ - GoalPtr makeDerivationGoal(const Path & drvPath); + GoalPtr makeDerivationGoal(const Path & drvPath, bool repair = false); GoalPtr makeSubstitutionGoal(const Path & storePath, bool repair = false); /* Remove a dead goal. */ @@ -825,13 +825,18 @@ private: HashRewrites rewritesToTmp, rewritesFromTmp; PathSet redirectedOutputs; + /* Whether we're repairing. If set, a derivation will be rebuilt + if its outputs are valid but corrupt or missing. */ + bool repair; + map redirectedBadOutputs; + /* Magic exit code denoting that setting up the child environment failed. (It's possible that the child actually returns the exit code, but ah well.) */ const static int childSetupFailed = 189; public: - DerivationGoal(const Path & drvPath, Worker & worker); + DerivationGoal(const Path & drvPath, Worker & worker, bool repair = false); ~DerivationGoal(); void cancel(); @@ -882,21 +887,24 @@ private: void handleEOF(int fd); /* Return the set of (in)valid paths. */ - PathSet checkPathValidity(bool returnValid); + PathSet checkPathValidity(bool returnValid, bool checkHash); /* Abort the goal if `path' failed to build. */ bool pathFailed(const Path & path); /* Forcibly kill the child process, if any. */ void killChild(); + + Path addHashRewrite(const Path & path); }; -DerivationGoal::DerivationGoal(const Path & drvPath, Worker & worker) +DerivationGoal::DerivationGoal(const Path & drvPath, Worker & worker, bool repair) : Goal(worker) , fLogFile(0) , bzLogFile(0) , useChroot(false) + , repair(repair) { this->drvPath = drvPath; state = &DerivationGoal::init; @@ -1001,10 +1009,10 @@ void DerivationGoal::haveDerivation() worker.store.addTempRoot(i->second.path); /* Check what outputs paths are not already valid. */ - PathSet invalidOutputs = checkPathValidity(false); + PathSet invalidOutputs = checkPathValidity(false, repair); /* If they are all valid, then we're done. */ - if (invalidOutputs.size() == 0) { + if (invalidOutputs.size() == 0 && !repair) { amDone(ecSuccess); return; } @@ -1019,7 +1027,7 @@ void DerivationGoal::haveDerivation() them. */ if (settings.useSubstitutes) foreach (PathSet::iterator, i, invalidOutputs) - addWaitee(worker.makeSubstitutionGoal(*i)); + addWaitee(worker.makeSubstitutionGoal(*i, repair)); if (waitees.empty()) /* to prevent hang (no wake-up event) */ outputsSubstituted(); @@ -1037,7 +1045,7 @@ void DerivationGoal::outputsSubstituted() nrFailed = nrNoSubstituters = 0; - if (checkPathValidity(false).size() == 0) { + if (checkPathValidity(false, repair).size() == 0) { amDone(ecSuccess); return; } @@ -1173,7 +1181,7 @@ void DerivationGoal::tryToBuild() omitted, but that would be less efficient.) Note that since we now hold the locks on the output paths, no other process can build this derivation, so no further checks are necessary. */ - validPaths = checkPathValidity(true); + validPaths = checkPathValidity(true, repair); if (validPaths.size() == drv.outputs.size()) { debug(format("skipping build of derivation `%1%', someone beat us to it") % drvPath); @@ -1256,6 +1264,24 @@ void DerivationGoal::tryToBuild() } +void replaceValidPath(const Path & storePath, const Path tmpPath) +{ + /* We can't atomically replace storePath (the original) with + 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() % rand()).str(); + if (pathExists(storePath)) { + makeMutable(storePath); + rename(storePath.c_str(), oldPath.c_str()); + } + if (rename(tmpPath.c_str(), storePath.c_str()) == -1) + throw SysError(format("moving `%1%' to `%2%'") % tmpPath % storePath); + if (pathExists(oldPath)) + deletePathWrapped(oldPath); +} + + void DerivationGoal::buildDone() { trace("build done"); @@ -1309,10 +1335,18 @@ void DerivationGoal::buildDone() /* If the output was already valid, just skip (discard) it. */ if (validPaths.find(path) != validPaths.end()) continue; - if (useChroot && pathExists(chrootRootDir + path)) + if (useChroot && pathExists(chrootRootDir + path)) { /* Move output paths from the chroot to the Nix store. */ - if (rename((chrootRootDir + path).c_str(), path.c_str()) == -1) - throw SysError(format("moving build output `%1%' from the chroot to the Nix store") % path); + if (repair) + replaceValidPath(path, chrootRootDir + path); + else + if (rename((chrootRootDir + path).c_str(), path.c_str()) == -1) + throw SysError(format("moving build output `%1%' from the chroot to the Nix store") % path); + } + + Path redirected; + if (repair && (redirected = redirectedBadOutputs[path]) != "" && pathExists(redirected)) + replaceValidPath(path, redirected); if (!pathExists(path)) continue; @@ -1786,25 +1820,28 @@ void DerivationGoal::startBuilder() #endif } - /* We're not doing a chroot build, but we have some valid output - paths. Since we can't just overwrite or delete them, we have - to do hash rewriting: i.e. in the environment/arguments passed - to the build, we replace the hashes of the valid outputs with - unique dummy strings; after the build, we discard the - redirected outputs corresponding to the valid outputs, and - rewrite the contents of the new outputs to replace the dummy - strings with the actual hashes. */ - else if (validPaths.size() > 0) { - //throw Error(format("derivation `%1%' is blocked by its output path(s) %2%") % drvPath % showPaths(validPaths)); - foreach (PathSet::iterator, i, validPaths) { - string h1 = string(*i, settings.nixStore.size() + 1, 32); - string h2 = string(printHash32(hashString(htSHA256, "rewrite:" + drvPath + ":" + *i)), 0, 32); - Path p = settings.nixStore + "/" + h2 + string(*i, settings.nixStore.size() + 33); - assert(i->size() == p.size()); - rewritesToTmp[h1] = h2; - rewritesFromTmp[h2] = h1; - redirectedOutputs.insert(p); - } + else { + + /* We're not doing a chroot build, but we have some valid + output paths. Since we can't just overwrite or delete + them, we have to do hash rewriting: i.e. in the + environment/arguments passed to the build, we replace the + hashes of the valid outputs with unique dummy strings; + after the build, we discard the redirected outputs + corresponding to the valid outputs, and rewrite the + contents of the new outputs to replace the dummy strings + with the actual hashes. */ + if (validPaths.size() > 0) + //throw Error(format("derivation `%1%' is blocked by its output path(s) %2%") % drvPath % showPaths(validPaths)); + foreach (PathSet::iterator, i, validPaths) + redirectedOutputs.insert(addHashRewrite(*i)); + + /* If we're repairing, then we don't want to delete the + corrupt outputs in advance. So rewrite them as well. */ + if (repair) + foreach (PathSet::iterator, i, missing) + if (worker.store.isValidPath(*i) && pathExists(*i)) + redirectedBadOutputs[*i] = addHashRewrite(*i); } @@ -2278,15 +2315,15 @@ void DerivationGoal::handleEOF(int fd) } -PathSet DerivationGoal::checkPathValidity(bool returnValid) +PathSet DerivationGoal::checkPathValidity(bool returnValid, bool checkHash) { PathSet result; - foreach (DerivationOutputs::iterator, i, drv.outputs) - if (worker.store.isValidPath(i->second.path)) { - if (returnValid) result.insert(i->second.path); - } else { - if (!returnValid) result.insert(i->second.path); - } + foreach (DerivationOutputs::iterator, i, drv.outputs) { + bool good = + worker.store.isValidPath(i->second.path) && + (!checkHash || worker.store.pathContentsGood(i->second.path)); + if (good == returnValid) result.insert(i->second.path); + } return result; } @@ -2309,6 +2346,19 @@ bool DerivationGoal::pathFailed(const Path & path) } +Path DerivationGoal::addHashRewrite(const Path & path) +{ + string h1 = string(path, settings.nixStore.size() + 1, 32); + string h2 = string(printHash32(hashString(htSHA256, "rewrite:" + drvPath + ":" + path)), 0, 32); + Path p = settings.nixStore + "/" + h2 + string(path, settings.nixStore.size() + 33); + if (pathExists(p)) deletePathWrapped(p); + assert(path.size() == p.size()); + rewritesToTmp[h1] = h2; + rewritesFromTmp[h2] = h1; + return p; +} + + ////////////////////////////////////////////////////////////////////// @@ -2667,22 +2717,7 @@ void SubstitutionGoal::finished() worker.store.optimisePath(destPath); // FIXME: combine with hashPath() - if (repair) { - /* We can't atomically replace storePath (the original) with - destPath (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() % rand()).str(); - if (pathExists(storePath)) { - makeMutable(storePath); - rename(storePath.c_str(), oldPath.c_str()); - } - if (rename(destPath.c_str(), storePath.c_str()) == -1) - throw SysError(format("moving `%1%' to `%2%'") % destPath % storePath); - if (pathExists(oldPath)) - deletePathWrapped(oldPath); - } + if (repair) replaceValidPath(storePath, destPath); ValidPathInfo info2; info2.path = storePath; @@ -2752,11 +2787,11 @@ Worker::~Worker() } -GoalPtr Worker::makeDerivationGoal(const Path & path) +GoalPtr Worker::makeDerivationGoal(const Path & path, bool repair) { GoalPtr goal = derivationGoals[path].lock(); if (!goal) { - goal = GoalPtr(new DerivationGoal(path, *this)); + goal = GoalPtr(new DerivationGoal(path, *this, repair)); derivationGoals[path] = goal; wakeUp(goal); } @@ -3090,7 +3125,7 @@ unsigned int Worker::exitStatus() ////////////////////////////////////////////////////////////////////// -void LocalStore::buildPaths(const PathSet & drvPaths) +void LocalStore::buildPaths(const PathSet & drvPaths, bool repair) { startNest(nest, lvlDebug, format("building %1%") % showPaths(drvPaths)); @@ -3100,9 +3135,9 @@ void LocalStore::buildPaths(const PathSet & drvPaths) Goals goals; foreach (PathSet::const_iterator, i, drvPaths) if (isDerivation(*i)) - goals.insert(worker.makeDerivationGoal(*i)); + goals.insert(worker.makeDerivationGoal(*i, repair)); else - goals.insert(worker.makeSubstitutionGoal(*i)); + goals.insert(worker.makeSubstitutionGoal(*i, repair)); worker.run(goals); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index e038cd4b2..a4ad97331 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1671,6 +1671,16 @@ void LocalStore::verifyPath(const Path & path, const PathSet & store, } +bool LocalStore::pathContentsGood(const Path & path) +{ + ValidPathInfo info = queryPathInfo(path); + if (!pathExists(path)) return false; + HashResult current = hashPath(info.hash.type, path); + Hash nullHash(htSHA256); + return info.hash == nullHash || info.hash == current.first; +} + + /* Functions for upgrading from the pre-SQLite database. */ PathSet LocalStore::queryValidPathsOld() diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 9f1c8280a..00356bf9d 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -150,7 +150,7 @@ public: Paths importPaths(bool requireSignature, Source & source); - void buildPaths(const PathSet & paths); + void buildPaths(const PathSet & paths, bool repair = false); void ensurePath(const Path & path); @@ -202,6 +202,10 @@ public: a substituter (if available). */ void repairPath(const Path & path); + /* Check whether the given valid path exists and has the right + contents. */ + bool pathContentsGood(const Path & path); + private: Path schemaPath; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 11712b487..2a895593b 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -474,8 +474,9 @@ Paths RemoteStore::importPaths(bool requireSignature, Source & source) } -void RemoteStore::buildPaths(const PathSet & drvPaths) +void RemoteStore::buildPaths(const PathSet & drvPaths, bool repair) { + if (repair) throw Error("`--repair' is not supported when building through the Nix daemon"); openConnection(); writeInt(wopBuildPaths, to); writeStrings(drvPaths, to); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index ae4c48dad..fe60ccb39 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -63,7 +63,7 @@ public: Paths importPaths(bool requireSignature, Source & source); - void buildPaths(const PathSet & paths); + void buildPaths(const PathSet & paths, bool repair = false); void ensurePath(const Path & path); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a562360ce..800645fa4 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -184,7 +184,7 @@ public: output paths can be created by running the builder, after recursively building any sub-derivations. For inputs that are not derivations, substitute them. */ - virtual void buildPaths(const PathSet & paths) = 0; + virtual void buildPaths(const PathSet & paths, bool repair = false) = 0; /* Ensure that a path is valid. If it is not currently valid, it may be made valid by running a substitute (if defined for the diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index b3f4ebdf5..104e0b241 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -93,9 +93,11 @@ static PathSet realisePath(const Path & path, bool build = true) static void opRealise(Strings opFlags, Strings opArgs) { bool dryRun = false; + bool repair = false; foreach (Strings::iterator, i, opFlags) if (*i == "--dry-run") dryRun = true; + else if (*i == "--repair") repair = true; else throw UsageError(format("unknown flag `%1%'") % *i); foreach (Strings::iterator, i, opArgs) @@ -107,7 +109,7 @@ static void opRealise(Strings opFlags, Strings opArgs) /* Build all paths at the same time to exploit parallelism. */ PathSet paths(opArgs.begin(), opArgs.end()); - store->buildPaths(paths); + store->buildPaths(paths, repair); foreach (Paths::iterator, i, opArgs) { PathSet paths = realisePath(*i, false);