* Make the garbage collector do the right thing when `gc-keep-outputs'

  is enabled by not depending on the deriver.

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<string, Path> 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<string, Path>(
+                        getNameOfStorePath(*i), nixStore + "/" + *i));
+        state.drvsIndexed = true;
+    }
+    
+    string name = getNameOfStorePath(path);
+
+    // Urgh, I should have used Haskell...
+    std::pair<GCState::DrvsByName::iterator, GCState::DrvsByName::iterator> 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 <limits.h>
 
@@ -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/<hash>-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);