From 9958bd6992e2b3e7bacb493a372d17d5a5b95d90 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 2 Oct 2012 14:08:59 -0400 Subject: [PATCH] =?UTF-8?q?Add=20operation=20=E2=80=98nix-store=20--repair?= =?UTF-8?q?-path=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This operation allows fixing corrupted or accidentally deleted store paths by redownloading them using substituters, if available. Since the corrupted path cannot be replaced atomically, there is a very small time window (one system call) during which neither the old (corrupted) nor the new (repaired) contents are available. So repairing should be used with some care on critical packages like Glibc. --- doc/manual/nix-store.xml | 46 ++++++++++++ doc/manual/release-notes.xml | 6 ++ scripts/copy-from-other-stores.pl.in | 5 +- scripts/download-from-binary-cache.pl.in | 7 +- scripts/download-using-manifests.pl.in | 7 +- src/libstore/build.cc | 95 +++++++++++++++++------- src/libstore/local-store.hh | 4 + src/nix-store/nix-store.cc | 17 +++++ 8 files changed, 151 insertions(+), 36 deletions(-) diff --git a/doc/manual/nix-store.xml b/doc/manual/nix-store.xml index cb77b3147..633dcd871 100644 --- a/doc/manual/nix-store.xml +++ b/doc/manual/nix-store.xml @@ -848,6 +848,52 @@ $ nix-store --verify-path $(nix-store -qR $(which svn)) + + +Operation <option>--repair-path</option> + + + Synopsis + + nix-store + + paths + + + +Description + +The operation attempts to +“repair” the specified paths by redownloading them using the available +substituters. If no substitutes are available, then repair is not +possible. + +During repair, there is a very small time window during +which the old path (if it exists) is moved out of the way and replaced +with the new path. If repair is interrupted in between, then the +system may be left in a broken state (e.g., if the path contains a +critical system component like the GNU C Library). + + + +Example + + +$ nix-store --verify-path /nix/store/cj7a81wsm1ijwwpkks3725661h3263p5-glibc-2.13 +path `/nix/store/cj7a81wsm1ijwwpkks3725661h3263p5-glibc-2.13' was modified! + expected hash `2db57715ae90b7e31ff1f2ecb8c12ec1cc43da920efcbe3b22763f36a1861588', + got `481c5aa5483ebc97c20457bb8bca24deea56550d3985cda0027f67fe54b808e4' + +$ nix-store --repair-path /nix/store/cj7a81wsm1ijwwpkks3725661h3263p5-glibc-2.13 +fetching path `/nix/store/cj7a81wsm1ijwwpkks3725661h3263p5-glibc-2.13'... +… + + + + + + + Operation <option>--dump</option> diff --git a/doc/manual/release-notes.xml b/doc/manual/release-notes.xml index 6fbd7c269..af196344c 100644 --- a/doc/manual/release-notes.xml +++ b/doc/manual/release-notes.xml @@ -19,6 +19,12 @@ names. + + The new operation nix-store --repair-path + allows corrupted or deleted store paths to be repaired by + redownloading them. + + Nix no longer sets the immutable bit on files in the Nix store. Instead, the recommended way to guard the Nix store diff --git a/scripts/copy-from-other-stores.pl.in b/scripts/copy-from-other-stores.pl.in index 3ee6f075b..9ed7e4cc2 100755 --- a/scripts/copy-from-other-stores.pl.in +++ b/scripts/copy-from-other-stores.pl.in @@ -83,12 +83,13 @@ if ($ARGV[0] eq "--query") { elsif ($ARGV[0] eq "--substitute") { - die unless scalar @ARGV == 2; + die unless scalar @ARGV == 3; my $storePath = $ARGV[1]; + my $destPath = $ARGV[2]; my ($store, $sourcePath) = findStorePath $storePath; die unless $store; print STDERR "\n*** Copying `$storePath' from `$sourcePath'\n\n"; - system("$binDir/nix-store --dump $sourcePath | $binDir/nix-store --restore $storePath") == 0 + system("$binDir/nix-store --dump $sourcePath | $binDir/nix-store --restore $destPath") == 0 or die "cannot copy `$sourcePath' to `$storePath'"; print "\n"; # no hash to verify } diff --git a/scripts/download-from-binary-cache.pl.in b/scripts/download-from-binary-cache.pl.in index 751623eeb..317989e40 100644 --- a/scripts/download-from-binary-cache.pl.in +++ b/scripts/download-from-binary-cache.pl.in @@ -486,7 +486,7 @@ sub printSubstitutablePaths { sub downloadBinary { - my ($storePath) = @_; + my ($storePath, $destPath) = @_; foreach my $cache (@caches) { my $info = getCachedInfoFrom($storePath, $cache); @@ -510,7 +510,7 @@ sub downloadBinary { my $url = "$cache->{url}/$info->{url}"; # FIXME: handle non-relative URLs print STDERR "\n*** Downloading ‘$url’ to ‘$storePath’...\n"; Nix::Utils::checkURL $url; - if (system("$Nix::Config::curl --fail --location --insecure '$url' | $decompressor | $Nix::Config::binDir/nix-store --restore $storePath") != 0) { + if (system("$Nix::Config::curl --fail --location --insecure '$url' | $decompressor | $Nix::Config::binDir/nix-store --restore $destPath") != 0) { die "download of `$info->{url}' failed" . ($! ? ": $!" : "") . "\n" unless $? == 0; next; } @@ -557,8 +557,9 @@ if ($ARGV[0] eq "--query") { elsif ($ARGV[0] eq "--substitute") { my $storePath = $ARGV[1] or die; + my $destPath = $ARGV[2] or die; getAvailableCaches; - downloadBinary($storePath); + downloadBinary($storePath, $destPath); } else { diff --git a/scripts/download-using-manifests.pl.in b/scripts/download-using-manifests.pl.in index 8f66a292e..c73511f85 100755 --- a/scripts/download-using-manifests.pl.in +++ b/scripts/download-using-manifests.pl.in @@ -238,8 +238,9 @@ elsif ($ARGV[0] ne "--substitute") { } -die unless scalar @ARGV == 2; +die unless scalar @ARGV == 3; my $targetPath = $ARGV[1]; +my $destPath = $ARGV[2]; $fast = 0; @@ -324,7 +325,7 @@ while (scalar @path > 0) { # This was the last patch. Unpack the final NAR archive # into the target path. print STDERR " unpacking patched archive...\n"; - system("$Nix::Config::binDir/nix-store --restore $v < $tmpNar2") == 0 + system("$Nix::Config::binDir/nix-store --restore $destPath < $tmpNar2") == 0 or die "cannot unpack $tmpNar2 to `$v'\n"; } @@ -351,7 +352,7 @@ while (scalar @path > 0) { or die "cannot download and unpack `$narFile->{url}' to `$v'\n"; } else { # Unpack the archive to the target path. - system("$curl '$narFile->{url}' | $decompressor | $Nix::Config::binDir/nix-store --restore '$v'") == 0 + system("$curl '$narFile->{url}' | $decompressor | $Nix::Config::binDir/nix-store --restore '$destPath'") == 0 or die "cannot download and unpack `$narFile->{url}' to `$v'\n"; } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index fecee04d5..3097b55ef 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -242,7 +242,7 @@ public: /* Make a goal (with caching). */ GoalPtr makeDerivationGoal(const Path & drvPath); - GoalPtr makeSubstitutionGoal(const Path & storePath); + GoalPtr makeSubstitutionGoal(const Path & storePath, bool repair = false); /* Remove a dead goal. */ void removeGoal(GoalPtr goal); @@ -2344,11 +2344,18 @@ private: /* Lock on the store path. */ boost::shared_ptr outputLock; + /* Whether to try to repair a valid path. */ + bool repair; + + /* Location where we're downloading the substitute. Differs from + storePath when doing a repair. */ + Path destPath; + typedef void (SubstitutionGoal::*GoalState)(); GoalState state; public: - SubstitutionGoal(const Path & storePath, Worker & worker); + SubstitutionGoal(const Path & storePath, Worker & worker, bool repair = false); ~SubstitutionGoal(); void cancel(); @@ -2371,9 +2378,10 @@ public: }; -SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker) +SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker, bool repair) : Goal(worker) , hasSubstitute(false) + , repair(repair) { this->storePath = storePath; state = &SubstitutionGoal::init; @@ -2415,7 +2423,7 @@ void SubstitutionGoal::init() worker.store.addTempRoot(storePath); /* If the path already exists we're done. */ - if (worker.store.isValidPath(storePath)) { + if (!repair && worker.store.isValidPath(storePath)) { amDone(ecSuccess); return; } @@ -2519,7 +2527,7 @@ void SubstitutionGoal::tryToRun() } /* Check again whether the path is invalid. */ - if (worker.store.isValidPath(storePath)) { + if (!repair && worker.store.isValidPath(storePath)) { debug(format("store path `%1%' has become valid") % storePath); outputLock->setDeletion(true); amDone(ecSuccess); @@ -2531,9 +2539,11 @@ void SubstitutionGoal::tryToRun() outPipe.create(); logPipe.create(); + destPath = repair ? storePath + ".tmp" : storePath; + /* Remove the (stale) output path if it exists. */ - if (pathExists(storePath)) - deletePathWrapped(storePath); + if (pathExists(destPath)) + deletePathWrapped(destPath); /* Fork the substitute program. */ pid = fork(); @@ -2561,6 +2571,7 @@ void SubstitutionGoal::tryToRun() args.push_back(baseNameOf(sub)); args.push_back("--substitute"); args.push_back(storePath); + args.push_back(destPath); const char * * argArr = strings2CharPtrs(args); execv(sub.c_str(), (char * *) argArr); @@ -2617,10 +2628,10 @@ void SubstitutionGoal::finished() throw SubstError(format("fetching path `%1%' %2%") % storePath % statusToString(status)); - if (!pathExists(storePath)) - throw SubstError(format("substitute did not produce path `%1%'") % storePath); + if (!pathExists(destPath)) + throw SubstError(format("substitute did not produce path `%1%'") % destPath); - hash = hashPath(htSHA256, storePath); + hash = hashPath(htSHA256, destPath); /* Verify the expected hash we got from the substituer. */ if (expectedHashStr != "") { @@ -2631,7 +2642,7 @@ void SubstitutionGoal::finished() if (hashType == htUnknown) throw Error(format("unknown hash algorithm in `%1%'") % expectedHashStr); Hash expectedHash = parseHash16or32(hashType, string(expectedHashStr, n + 1)); - Hash actualHash = hashType == htSHA256 ? hash.first : hashPath(hashType, storePath).first; + Hash actualHash = hashType == htSHA256 ? hash.first : hashPath(hashType, destPath).first; if (expectedHash != actualHash) throw SubstError(format("hash mismatch in downloaded path `%1%': expected %2%, got %3%") % storePath % printHash(expectedHash) % printHash(actualHash)); @@ -2652,9 +2663,26 @@ void SubstitutionGoal::finished() return; } - canonicalisePathMetaData(storePath); + canonicalisePathMetaData(destPath); - worker.store.optimisePath(storePath); // FIXME: combine with hashPath() + 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); + } ValidPathInfo info2; info2.path = storePath; @@ -2724,29 +2752,27 @@ Worker::~Worker() } -template -static GoalPtr addGoal(const Path & path, - Worker & worker, WeakGoalMap & goalMap) +GoalPtr Worker::makeDerivationGoal(const Path & path) { - GoalPtr goal = goalMap[path].lock(); + GoalPtr goal = derivationGoals[path].lock(); if (!goal) { - goal = GoalPtr(new T(path, worker)); - goalMap[path] = goal; - worker.wakeUp(goal); + goal = GoalPtr(new DerivationGoal(path, *this)); + derivationGoals[path] = goal; + wakeUp(goal); } return goal; } -GoalPtr Worker::makeDerivationGoal(const Path & nePath) +GoalPtr Worker::makeSubstitutionGoal(const Path & path, bool repair) { - return addGoal(nePath, *this, derivationGoals); -} - - -GoalPtr Worker::makeSubstitutionGoal(const Path & storePath) -{ - return addGoal(storePath, *this, substitutionGoals); + GoalPtr goal = substitutionGoals[path].lock(); + if (!goal) { + goal = GoalPtr(new SubstitutionGoal(path, *this, repair)); + substitutionGoals[path] = goal; + wakeUp(goal); + } + return goal; } @@ -3109,4 +3135,17 @@ void LocalStore::ensurePath(const Path & path) } +void LocalStore::repairPath(const Path & path) +{ + Worker worker(*this); + GoalPtr goal = worker.makeSubstitutionGoal(path, true); + Goals goals = singleton(goal); + + worker.run(goals); + + if (goal->getExitCode() != Goal::ecSuccess) + throw Error(format("cannot repair path `%1%'") % path, worker.exitStatus()); +} + + } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 8899873a7..80db10de1 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -197,6 +197,10 @@ public: void vacuumDB(); + /* Repair the contents of the given path by redownloading it using + a substituter (if available). */ + void repairPath(const Path & path); + private: Path schemaPath; diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index d3a707f0d..ce415ce4a 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -734,6 +734,21 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) } +/* Repair the contents of the given path by redownloading it using a + substituter (if available). */ +static void opRepairPath(Strings opFlags, Strings opArgs) +{ + if (!opFlags.empty()) + throw UsageError("no flags expected"); + + foreach (Strings::iterator, i, opArgs) { + Path path = followLinksToStorePath(*i); + printMsg(lvlTalkative, format("repairing path `%1%'...") % path); + ensureLocalStore().repairPath(path); + } +} + + static void showOptimiseStats(OptimiseStats & stats) { printMsg(lvlError, @@ -834,6 +849,8 @@ void run(Strings args) op = opVerify; else if (arg == "--verify-path") op = opVerifyPath; + else if (arg == "--repair-path") + op = opRepairPath; else if (arg == "--optimise") op = opOptimise; else if (arg == "--query-failed-paths")