* The determination of the root set should be made by the privileged

  process, so forward the operation.
* Spam the user about GC misconfigurations (NIX-71).
* findRoots: skip all roots that are unreadable - the warnings with
  which we spam the user should be enough.

6 files changed, 124 insertions, 60 deletions

diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index c9e51f447a..e5217c9b86 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -91,12 +91,6 @@ void LocalStore::addIndirectRoot(const Path & path)
 }
 
 
-typedef std::map<Path, Path> Roots;
-
-
-static void findRoots(Roots & roots, bool ignoreUnreadable);
-
-
 Path addPermRoot(const Path & _storePath, const Path & _gcRoot,
     bool indirect, bool allowOutsideRootsDir)
 {
@@ -122,17 +116,17 @@ Path addPermRoot(const Path & _storePath, const Path & _gcRoot,
             
         createSymlink(gcRoot, storePath, false);
 
-        /* Check that the root can be found by the garbage collector. */
-        Roots roots;
-        findRoots(roots, true);
-        if (roots.find(gcRoot) == roots.end())
-            printMsg(lvlError, 
-                format(
-                    "warning: the garbage collector does not find `%1%' as a root; "
-                    "therefore, `%2%' might be removed by the garbage collector")
-                % gcRoot % storePath);
     }
 
+    /* Check that the root can be found by the garbage collector. */
+    Roots roots = store->findRoots();
+    if (roots.find(gcRoot) == roots.end())
+        printMsg(lvlError, 
+            format(
+                "warning: the garbage collector does not find `%1%' as a root; "
+                "therefore, `%2%' might be removed by the garbage collector")
+            % gcRoot % storePath);
+        
     /* Grab the global GC root, causing us to block while a GC is in
        progress.  This prevents the set of permanent roots from
        increasing while a GC is in progress. */
@@ -308,58 +302,73 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
 
 
 static void findRoots(const Path & path, bool recurseSymlinks,
-    bool ignoreUnreadable, Roots & roots)
+    bool deleteStale, Roots & roots)
 {
-    struct stat st;
-    if (lstat(path.c_str(), &st) == -1)
-        throw SysError(format("statting `%1%'") % path);
-
-    printMsg(lvlVomit, format("looking at `%1%'") % path);
+    try {
+        
+        struct stat st;
+        if (lstat(path.c_str(), &st) == -1)
+            throw SysError(format("statting `%1%'") % path);
 
-    if (S_ISDIR(st.st_mode)) {
-	Strings names = readDirectory(path);
-	for (Strings::iterator i = names.begin(); i != names.end(); ++i)
-            findRoots(path + "/" + *i, recurseSymlinks, ignoreUnreadable, roots);
-    }
+        printMsg(lvlVomit, format("looking at `%1%'") % path);
 
-    else if (S_ISLNK(st.st_mode)) {
-        Path target = absPath(readLink(path), dirOf(path));
-
-        if (isInStore(target)) {
-            debug(format("found root `%1%' in `%2%'")
-                % target % path);
-            Path storePath = toStorePath(target);
-            if (store->isValidPath(storePath)) 
-                roots[path] = storePath;
-            else
-                printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'")
-                    % path % storePath);
+        if (S_ISDIR(st.st_mode)) {
+            Strings names = readDirectory(path);
+            for (Strings::iterator i = names.begin(); i != names.end(); ++i)
+                findRoots(path + "/" + *i, recurseSymlinks, deleteStale, roots);
         }
 
-        else if (recurseSymlinks) {
-            struct stat st2;
-            if (lstat(target.c_str(), &st2) == 0)
-                findRoots(target, false, ignoreUnreadable, roots);
-            else if (ignoreUnreadable && errno == EACCES)
-                /* ignore */ ;
-            else if (errno == ENOENT || errno == ENOTDIR) {
-                printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target);
-                /* Note that we only delete when recursing, i.e., when
-                   we are still in the `gcroots' tree.  We never
-                   delete stuff outside that tree. */
-                unlink(path.c_str());
+        else if (S_ISLNK(st.st_mode)) {
+            Path target = absPath(readLink(path), dirOf(path));
+
+            if (isInStore(target)) {
+                debug(format("found root `%1%' in `%2%'")
+                    % target % path);
+                Path storePath = toStorePath(target);
+                if (store->isValidPath(storePath)) 
+                    roots[path] = storePath;
+                else
+                    printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'")
+                        % path % storePath);
+            }
+
+            else if (recurseSymlinks) {
+                if (pathExists(target))
+                    findRoots(target, false, deleteStale, roots);
+                else if (deleteStale) {
+                    printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target);
+                    /* Note that we only delete when recursing, i.e.,
+                       when we are still in the `gcroots' tree.  We
+                       never delete stuff outside that tree. */
+                    unlink(path.c_str());
+                }
             }
-            else 
-                throw SysError(format("statting `%1%'") % target);
         }
+
+    }
+
+    catch (SysError & e) {
+        /* We only ignore permanent failures. */
+        if (e.errNo == EACCES || e.errNo == ENOENT || e.errNo == ENOTDIR)
+            printMsg(lvlInfo, format("cannot read potential root `%1%'") % path);
+        else
+            throw;
     }
 }
 
 
-static void findRoots(Roots & roots, bool ignoreUnreadable)
+static Roots findRoots(bool deleteStale)
 {
+    Roots roots;
     Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % gcRootsDir).str());
-    findRoots(rootsDir, true, ignoreUnreadable, roots);
+    findRoots(rootsDir, true, deleteStale, roots);
+    return roots;
+}
+
+
+Roots LocalStore::findRoots()
+{
+    return nix::findRoots(false);
 }
 
 
@@ -437,8 +446,7 @@ void collectGarbage(GCAction action, const PathSet & pathsToDelete,
 
     /* Find the roots.  Since we've grabbed the GC lock, the set of
        permanent roots cannot increase now. */
-    Roots rootMap;
-    findRoots(rootMap, false);
+    Roots rootMap = ignoreLiveness ? Roots() : findRoots(true);
 
     PathSet roots;
     for (Roots::iterator i = rootMap.begin(); i != rootMap.end(); ++i)
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 251f8922fa..a7b3926cc7 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -64,6 +64,8 @@ public:
     void addIndirectRoot(const Path & path);
     
     void syncWithGC();
+
+    Roots findRoots();
 };
 
 
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 8dd87d046c..6ddbb3e2a2 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -18,6 +18,23 @@
 namespace nix {
 
 
+Path readStorePath(Source & from)
+{
+    Path path = readString(from);
+    assertStorePath(path);
+    return path;
+}
+
+
+PathSet readStorePaths(Source & from)
+{
+    PathSet paths = readStringSet(from);
+    for (PathSet::iterator i = paths.begin(); i != paths.end(); ++i)
+        assertStorePath(*i);
+    return paths;
+}
+
+
 RemoteStore::RemoteStore()
 {
     string remoteMode = getEnv("NIX_REMOTE");
@@ -179,7 +196,7 @@ void RemoteStore::queryReferences(const Path & path,
     writeInt(wopQueryReferences, to);
     writeString(path, to);
     processStderr();
-    PathSet references2 = readStringSet(from);
+    PathSet references2 = readStorePaths(from);
     references.insert(references2.begin(), references2.end());
 }
 
@@ -190,7 +207,7 @@ void RemoteStore::queryReferrers(const Path & path,
     writeInt(wopQueryReferrers, to);
     writeString(path, to);
     processStderr();
-    PathSet referrers2 = readStringSet(from);
+    PathSet referrers2 = readStorePaths(from);
     referrers.insert(referrers2.begin(), referrers2.end());
 }
 
@@ -207,7 +224,7 @@ Path RemoteStore::addToStore(const Path & _srcPath, bool fixed,
     writeString(hashAlgo, to);
     dumpPath(srcPath, to);
     processStderr();
-    Path path = readString(from);
+    Path path = readStorePath(from);
     return path;
 }
 
@@ -221,7 +238,7 @@ Path RemoteStore::addTextToStore(const string & suffix, const string & s,
     writeStringSet(references, to);
     
     processStderr();
-    Path path = readString(from);
+    Path path = readStorePath(from);
     return path;
 }
 
@@ -270,6 +287,21 @@ void RemoteStore::syncWithGC()
 }
 
 
+Roots RemoteStore::findRoots()
+{
+    writeInt(wopFindRoots, to);
+    processStderr();
+    unsigned int count = readInt(from);
+    Roots result;
+    while (count--) {
+        Path link = readString(from);
+        Path target = readStorePath(from);
+        result[link] = target;
+    }
+    return result;
+}
+
+
 void RemoteStore::processStderr()
 {
     unsigned int msg;
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index 4b69830456..11e7e328be 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -53,6 +53,8 @@ public:
     
     void syncWithGC();
     
+    Roots findRoots();
+    
 private:
     AutoCloseFD fdSocket;
     FdSink to;
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 795488d17f..bda232d3d2 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -2,6 +2,7 @@
 #define __STOREAPI_H
 
 #include <string>
+#include <map>
 
 #include <boost/shared_ptr.hpp>
 
@@ -33,6 +34,9 @@ struct Substitute
 typedef list<Substitute> Substitutes;
 
 
+typedef std::map<Path, Path> Roots;
+
+
 class StoreAPI 
 {
 public:
@@ -118,6 +122,11 @@ public:
        In either case the permanent root is seen by the collector. */
     virtual void syncWithGC() = 0;
 
+    /* Find the roots of the garbage collector.  Each root is a pair
+       (link, storepath) where `link' is the path of the symlink
+       outside of the Nix store that point to `storePath'.  */
+    virtual Roots findRoots() = 0;
+
 };
 
 
diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh
index ff47fdd67c..bb5d9d7b34 100644
--- a/src/libstore/worker-protocol.hh
+++ b/src/libstore/worker-protocol.hh
@@ -2,6 +2,9 @@
 #define __WORKER_PROTOCOL_H
 
 
+namespace nix {
+
+
 #define WORKER_MAGIC_1 0x6e697864
 #define WORKER_MAGIC_2 0x6478696e
 
@@ -21,6 +24,7 @@ typedef enum {
     wopAddTempRoot,
     wopAddIndirectRoot,
     wopSyncWithGC,
+    wopFindRoots,
 } WorkerOp;
 
 
@@ -34,4 +38,11 @@ typedef enum {
 #define DEFAULT_SOCKET_PATH "/daemon.socket"
 
 
+Path readStorePath(Source & from);
+PathSet readStorePaths(Source & from);
+
+    
+}
+
+
 #endif /* !__WORKER_PROTOCOL_H */