From 5388944e8d1ca61e23d42a6a0769b925f099f4e1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 25 Jan 2010 16:04:32 +0000 Subject: [PATCH] * Make the garbage collector do the right thing when `gc-keep-outputs' is enabled by not depending on the deriver. --- src/libstore/gc.cc | 87 +++++++++++++++++++++++++++++-------- src/libstore/local-store.hh | 2 + src/libstore/store-api.cc | 13 ++++++ src/libstore/store-api.hh | 6 +++ 4 files changed, 90 insertions(+), 18 deletions(-) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index fc9791023..baa6826fe 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -454,7 +454,12 @@ struct LocalStore::GCState PathSet busy; bool gcKeepOutputs; bool gcKeepDerivations; - GCState(GCResults & results_) : results(results_) + + bool drvsIndexed; + typedef std::multimap DrvsByName; + DrvsByName drvsByName; // derivation paths hashed by name attribute + + GCState(GCResults & results_) : results(results_), drvsIndexed(false) { } }; @@ -474,6 +479,42 @@ bool LocalStore::isActiveTempFile(const GCState & state, && state.tempRoots.find(string(path, 0, path.size() - suffix.size())) != state.tempRoots.end(); } + +/* Return all the derivations in the Nix store that have `path' as an + output. This function assumes that derivations have the same name + as their outputs. */ +PathSet LocalStore::findDerivers(GCState & state, const Path & path) +{ + PathSet derivers; + + Path deriver = queryDeriver(path); + if (deriver != "") derivers.insert(deriver); + + if (!state.drvsIndexed) { + Paths entries = readDirectory(nixStore); + foreach (Paths::iterator, i, entries) + if (isDerivation(*i)) + state.drvsByName.insert(std::pair( + getNameOfStorePath(*i), nixStore + "/" + *i)); + state.drvsIndexed = true; + } + + string name = getNameOfStorePath(path); + + // Urgh, I should have used Haskell... + std::pair range = + state.drvsByName.equal_range(name); + + for (GCState::DrvsByName::iterator i = range.first; i != range.second; ++i) + if (isValidPath(i->second)) { + Derivation drv = derivationFromPath(i->second); + foreach (DerivationOutputs::iterator, j, drv.outputs) + if (j->second.path == path) derivers.insert(i->second); + } + + return derivers; +} + bool LocalStore::tryToDelete(GCState & state, const Path & path) { @@ -519,24 +560,34 @@ bool LocalStore::tryToDelete(GCState & state, const Path & path) if (!pathExists(path)) return true; /* If gc-keep-outputs is set, then don't delete this path if - its deriver is not garbage. !!! This is somewhat buggy, - since there might be multiple derivers, but the database - only stores one. */ + its deriver is not garbage. !!! Nix does not reliably + store derivers, so we have to look at all derivations to + determine which of them derive `path'. Since this makes + the garbage collector very slow to start on large Nix + stores, here we just look for all derivations that have the + same name as `path' (where the name is the part of the + filename after the hash, i.e. the `name' attribute of the + derivation). This is somewhat hacky: currently, the + deriver of a path always has the same name as the output, + but this might change in the future. */ if (state.gcKeepOutputs) { - Path deriver = queryDeriver(path); - /* Break an infinite recursion if gc-keep-derivations and - gc-keep-outputs are both set by tentatively assuming - that this path is garbage. This is a safe assumption - because at this point, the only thing that can prevent - it from being garbage is the deriver. Since - tryToDelete() works "upwards" through the dependency - graph, it won't encouter this path except in the call - to tryToDelete() in the gc-keep-derivation branch. */ - state.deleted.insert(path); - if (deriver != "" && !tryToDelete(state, deriver)) { - state.deleted.erase(path); - printMsg(lvlDebug, format("cannot delete `%1%' because its deriver is alive") % path); - goto isLive; + PathSet derivers = findDerivers(state, path); + foreach (PathSet::iterator, deriver, derivers) { + /* Break an infinite recursion if gc-keep-derivations + and gc-keep-outputs are both set by tentatively + assuming that this path is garbage. This is a safe + assumption because at this point, the only thing + that can prevent it from being garbage is the + deriver. Since tryToDelete() works "upwards" + through the dependency graph, it won't encouter + this path except in the call to tryToDelete() in + the gc-keep-derivation branch. */ + state.deleted.insert(path); + if (!tryToDelete(state, *deriver)) { + state.deleted.erase(path); + printMsg(lvlDebug, format("cannot delete `%1%' because its deriver `%2%' is alive") % path % *deriver); + goto isLive; + } } } } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index e851d0509..ef585fdaa 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -178,6 +178,8 @@ private: bool tryToDelete(GCState & state, const Path & path); + PathSet findDerivers(GCState & state, const Path & path); + bool isActiveTempFile(const GCState & state, const Path & path, const string & suffix); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 01dd51621..f0abe61ad 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1,6 +1,7 @@ #include "store-api.hh" #include "globals.hh" #include "util.hh" +#include "derivations.hh" #include @@ -52,6 +53,18 @@ Path toStorePath(const Path & path) } +string getNameOfStorePath(const Path & path) +{ + Path::size_type slash = path.rfind('/'); + string p = slash == Path::npos ? path : string(path, slash + 1); + Path::size_type dash = p.find('-'); + assert(dash != Path::npos); + string p2 = string(p, dash + 1); + if (isDerivation(p2)) p2 = string(p2, 0, p2.size() - 4); + return p2; +} + + Path followLinksToStore(const Path & _path) { Path path = absPath(_path); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 101215707..8506d47e3 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -243,6 +243,12 @@ void checkStoreName(const string & name); Path toStorePath(const Path & path); +/* Get the "name" part of a store path, that is, the part after the + hash and the dash, and with any ".drv" suffix removed + (e.g. /nix/store/-foo-1.2.3.drv => foo-1.2.3). */ +string getNameOfStorePath(const Path & path); + + /* Follow symlinks until we end up with a path in the Nix store. */ Path followLinksToStore(const Path & path);