Garbage collector: Don't follow symlinks arbitrarily

Only indirect roots (symlinks to symlinks to the Nix store) are now
supported.

Makefile.am

@@ -27,11 +27,9 @@ init-state:
 	$(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/profiles
 	$(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/gcroots
 	$(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/temproots
-	ln -sfn $(localstatedir)/nix/profiles $(DESTDIR)$(localstatedir)/nix/gcroots/profiles
 	$(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/userpool
 	-$(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(storedir)
 	$(INSTALL) $(INIT_FLAGS) $(GROUP_WRITABLE) -d $(DESTDIR)$(localstatedir)/nix/manifests
-	ln -sfn $(localstatedir)/nix/manifests $(DESTDIR)$(localstatedir)/nix/gcroots/manifests
 
 else
 

src/libstore/gc.cc

@@ -258,8 +258,7 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
            only succeed if the owning process has died.  In that case
            we don't care about its temporary roots. */
         if (lockFile(*fd, ltWrite, false)) {
-            printMsg(lvlError, format("removing stale temporary roots file `%1%'")
+            printMsg(lvlError, format("removing stale temporary roots file `%1%'") % path);
-                % path);
             unlink(path.c_str());
             writeFull(*fd, (const unsigned char *) "d", 1);
             continue;
@@ -290,47 +289,48 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
 }
 
 
-static void findRoots(StoreAPI & store, const Path & path,
+static void foundRoot(StoreAPI & store,
-    bool recurseSymlinks, bool deleteStale, Roots & roots)
+    const Path & path, const Path & target, Roots & roots)
 {
-    try {
-
-        struct stat st;
-        if (lstat(path.c_str(), &st) == -1)
-            throw SysError(format("statting `%1%'") % path);
-
-        printMsg(lvlVomit, format("looking at `%1%'") % path);
-
-        if (S_ISDIR(st.st_mode)) {
-            Strings names = readDirectory(path);
-            foreach (Strings::iterator, i, names)
-                findRoots(store, path + "/" + *i, recurseSymlinks, deleteStale, roots);
-        }
-
-        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%'")
+        printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") % path % storePath);
-                        % path % storePath);
+}
+
+
+static void findRoots(StoreAPI & store, const Path & path, Roots & roots)
+{
+    try {
+
+        struct stat st = lstat(path);
+
+        if (S_ISDIR(st.st_mode)) {
+            Strings names = readDirectory(path);
+            foreach (Strings::iterator, i, names)
+                findRoots(store, path + "/" + *i, roots);
         }
 
-            else if (recurseSymlinks) {
+        else if (S_ISLNK(st.st_mode)) {
-                if (pathExists(target))
+            Path target = readLink(path);
-                    findRoots(store, target, false, deleteStale, roots);
+            if (isInStore(target))
-                else if (deleteStale) {
+                foundRoot(store, path, target, roots);
+
+            /* Handle indirect roots. */
+            else {
+                target = absPath(target, dirOf(path));
+                if (!pathExists(target)) {
+                    if (isInDir(path, settings.nixStateDir + "/" + gcRootsDir + "/auto")) {
                         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 {
+                    struct stat st2 = lstat(target);
+                    if (!S_ISLNK(st2.st_mode)) return;
+                    Path target2 = readLink(target);
+                    if (isInStore(target2)) foundRoot(store, path, target2, roots);
+                }
             }
         }
 
@@ -346,18 +346,16 @@ static void findRoots(StoreAPI & store, const Path & path,
 }
 
 
-static Roots findRoots(StoreAPI & store, bool deleteStale)
-{
-    Roots roots;
-    Path rootsDir = canonPath((format("%1%/%2%") % settings.nixStateDir % gcRootsDir).str());
-    findRoots(store, rootsDir, true, deleteStale, roots);
-    return roots;
-}
-
-
 Roots LocalStore::findRoots()
 {
-    return nix::findRoots(*this, false);
+    Roots roots;
+
+    /* Process direct roots in {gcroots,manifests,profiles}. */
+    nix::findRoots(*this, settings.nixStateDir + "/" + gcRootsDir, roots);
+    nix::findRoots(*this, settings.nixStateDir + "/manifests", roots);
+    nix::findRoots(*this, settings.nixStateDir + "/profiles", roots);
+
+    return roots;
 }
 
 
@@ -637,7 +635,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
     /* Find the roots.  Since we've grabbed the GC lock, the set of
        permanent roots cannot increase now. */
     printMsg(lvlError, format("finding garbage collector roots..."));
-    Roots rootMap = options.ignoreLiveness ? Roots() : nix::findRoots(*this, true);
+    Roots rootMap = options.ignoreLiveness ? Roots() : findRoots();
 
     foreach (Roots::iterator, i, rootMap) state.roots.insert(i->second);
 

src/libstore/store-api.cc

@@ -18,10 +18,7 @@ GCOptions::GCOptions()
 
 bool isInStore(const Path & path)
 {
-    return path[0] == '/'
+    return isInDir(path, settings.nixStore);
-        && string(path, 0, settings.nixStore.size()) == settings.nixStore
-        && path.size() >= settings.nixStore.size() + 2
-        && path[settings.nixStore.size()] == '/';
 }
 
 

src/libutil/util.cc

@@ -148,6 +148,15 @@ string baseNameOf(const Path & path)
 }
 
 
+bool isInDir(const Path & path, const Path & dir)
+{
+    return path[0] == '/'
+        && string(path, 0, dir.size()) == dir
+        && path.size() >= dir.size() + 2
+        && path[dir.size()] == '/';
+}
+
+
 struct stat lstat(const Path & path)
 {
     struct stat st;

src/libutil/util.hh

@@ -45,6 +45,10 @@ Path dirOf(const Path & path);
    following the final `/'. */
 string baseNameOf(const Path & path);
 
+/* Check whether a given path is a descendant of the given
+   directory. */
+bool isInDir(const Path & path, const Path & dir);
+
 /* Get status of `path'. */
 struct stat lstat(const Path & path);