* Revert r19650 (implement gc-keep-outputs by looking for derivations

  with the same name as the output) and instead use the
  DerivationOutputs table in the database, which is the correct way to
  to do things.

5 files changed, 34 insertions, 74 deletions

diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index f58f691c99dd..659c636e3d19 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -416,12 +416,7 @@ struct LocalStore::GCState
     PathSet busy;
     bool gcKeepOutputs;
     bool gcKeepDerivations;
-
-    bool drvsIndexed;
-    typedef std::multimap<string, Path> DrvsByName;
-    DrvsByName drvsByName; // derivation paths hashed by name attribute
-
-    GCState(GCResults & results_) : results(results_), drvsIndexed(false)
+    GCState(GCResults & results_) : results(results_)
     {
     }
 };
@@ -441,42 +436,6 @@ 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)
 {
@@ -522,18 +481,9 @@ 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.  !!! 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. */
+           there are derivers of this path that are not garbage. */
         if (state.gcKeepOutputs) {
-            PathSet derivers = findDerivers(state, path);
+            PathSet derivers = queryValidDerivers(path);
             foreach (PathSet::iterator, deriver, derivers) {
                 /* Break an infinite recursion if gc-keep-derivations
                    and gc-keep-outputs are both set by tentatively
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 2f12256db02e..0590b294baed 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -297,6 +297,8 @@ void LocalStore::prepareStatements()
         "select time from FailedPaths where path = ?;");
     stmtAddDerivationOutput.create(db,
         "insert or replace into DerivationOutputs (drv, id, path) values (?, ?, ?);");
+    stmtQueryValidDerivers.create(db,
+        "select v.id, v.path from DerivationOutputs d join ValidPaths v on d.drv = v.id where d.path = ?;");
 }
 
 
@@ -599,6 +601,28 @@ Path LocalStore::queryDeriver(const Path & path)
 }
 
 
+PathSet LocalStore::queryValidDerivers(const Path & path)
+{
+    assertStorePath(path);
+
+    SQLiteStmtUse use(stmtQueryValidDerivers);
+    stmtQueryValidDerivers.bind(path);
+
+    PathSet derivers;
+    int r;
+    while ((r = sqlite3_step(stmtQueryValidDerivers)) == SQLITE_ROW) {
+        const char * s = (const char *) sqlite3_column_text(stmtQueryValidDerivers, 1);
+        assert(s);
+        derivers.insert(s);
+    }
+    
+    if (r != SQLITE_DONE)
+        throw SQLiteError(db, format("error getting valid derivers of `%1%'") % path);
+    
+    return derivers;
+}
+
+
 void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter & run)
 {
     if (run.pid != -1) return;
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index f10ba18d5a71..6bd47b3055f7 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -103,6 +103,12 @@ public:
     void queryReferrers(const Path & path, PathSet & referrers);
 
     Path queryDeriver(const Path & path);
+
+    /* Return all currently valid derivations that have `path' as an
+       output.  (Note that the result of `queryDeriver()' is the
+       derivation that was actually used to produce `path', which may
+       not exist anymore.) */
+    PathSet queryValidDerivers(const Path & path);
     
     PathSet querySubstitutablePaths();
     
@@ -199,6 +205,7 @@ private:
     SQLiteStmt stmtRegisterFailedPath;
     SQLiteStmt stmtHasPathFailed;
     SQLiteStmt stmtAddDerivationOutput;
+    SQLiteStmt stmtQueryValidDerivers;
 
     int getSchema();
 
@@ -228,8 +235,6 @@ 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 f0abe61ad1e1..01dd51621625 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -1,7 +1,6 @@
 #include "store-api.hh"
 #include "globals.hh"
 #include "util.hh"
-#include "derivations.hh"
 
 #include <limits.h>
 
@@ -53,18 +52,6 @@ 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 23a3ab25e470..d85ae0c9a10a 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -243,12 +243,6 @@ 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);