* nix-store, nix-instantiate: added an option `--add-root' to

  immediately add the result as a permanent GC root.  This is the only
  way to prevent a race with the garbage collector.  For instance, the
  old style

    ln -s $(nix-store -r $(nix-instantiate foo.nix)) \
      /nix/var/nix/gcroots/result

  has two time windows in which the garbage collector can interfere
  (by GC'ing the derivation and the output, respectively).  On the
  other hand,

    nix-store --add-root /nix/var/nix/gcroots/result -r \
      $(nix-instantiate --add-root /nix/var/nix/gcroots/drv \
        foo.nix)

  is safe.

* nix-build: use `--add-root' to prevent GC races.

11 files changed, 165 insertions, 17 deletions

diff --git a/scripts/nix-build.in b/scripts/nix-build.in
index 5ae591f35168..33fbc61a3d20 100644
--- a/scripts/nix-build.in
+++ b/scripts/nix-build.in
@@ -10,6 +10,9 @@ fi
 extraArgs=
 noLink=
 
+userName=$USER
+if test -z "$username"; then userName="unknown"; fi
+
 for i in "$@"; do
     case "$i" in
         --no-link)
@@ -19,11 +22,15 @@ for i in "$@"; do
             extraArgs="$extraArgs $i"
             ;;
         *)
-            storeExprs=$(@bindir@/nix-instantiate "$i")
+            storeExprs=$(@bindir@/nix-instantiate \
+                --add-root "@localstatedir@/nix/gcroots/nix-build/$userName-drv" \
+                "$i")
             for j in $storeExprs; do
                 echo "store expression is $j" >&2
             done
-            outPaths=$(@bindir@/nix-store -rv $extraArgs $storeExprs)
+            outPaths=$(@bindir@/nix-store \
+                --add-root "@localstatedir@/nix/gcroots/nix-build/$userName-out" \
+                -rv $extraArgs $storeExprs)
             for j in $outPaths; do
                 echo "$j"
                 if test -z "$noLink"; then
diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index 23fcf28916be..5c994d7b47af 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -31,6 +31,28 @@ void sigintHandler(int signo)
 }
 
 
+Path makeRootName(const Path & gcRoot, int & counter)
+{
+    counter++;
+    if (counter == 1)
+        return gcRoot;
+    else
+        return (format("%1%-%2%") % gcRoot % counter).str();
+}
+
+
+void printGCWarning()
+{
+    static bool warned = false;
+    if (!warned) {
+        printMsg(lvlInfo,
+            "warning: you did not specify `--add-root'; "
+            "the result might be removed by the garbage collector");
+        warned = true;
+    }
+}
+
+
 void setLogType(string lt)
 {
     if (lt == "pretty") logType = ltPretty;
@@ -183,7 +205,7 @@ static void initAndRun(int argc, char * * argv)
 
     /* Automatically clean up the temporary roots file when we
        exit. */
-    RemoveTempRoots removeTempRoots;
+    RemoveTempRoots removeTempRoots; /* unused variable - don't remove */
 
     run(remaining);
 }
diff --git a/src/libmain/shared.hh b/src/libmain/shared.hh
index 76b639e3785d..82da7550626f 100644
--- a/src/libmain/shared.hh
+++ b/src/libmain/shared.hh
@@ -17,6 +17,10 @@ void run(Strings args);
 /* Should print a help message to stdout and return. */
 void printHelp();
 
+/* Ugh.  No better place to put this. */
+Path makeRootName(const Path & gcRoot, int & counter);
+void printGCWarning();
+
 
 extern string programId;
 
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index cff503784520..ee9a369dc45c 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -39,6 +39,48 @@ static int openGCLock(LockType lockType)
 }
 
 
+static void createDirs(const Path & path)
+{
+    if (path == "") return;
+    createDirs(dirOf(path));
+    if (!pathExists(path))
+        if (mkdir(path.c_str(), 0777) == -1)
+            throw SysError(format("creating directory `%1%'") % path);
+}
+
+
+Path addPermRoot(const Path & _storePath, const Path & _gcRoot)
+{
+    Path storePath(canonPath(_storePath));
+    Path gcRoot(canonPath(_gcRoot));
+
+    Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % "gcroots").str());
+    
+    if (string(gcRoot, 0, rootsDir.size() + 1) != rootsDir + "/")
+        throw Error(format(
+            "path `%1%' is not a valid garbage collector root; "
+            "it's not in the `%1%' directory")
+            % gcRoot % rootsDir);
+
+    /* Grab the global GC root.  This prevents the set of permanent
+       roots from increasing while a GC is in progress. */
+    AutoCloseFD fdGCLock = openGCLock(ltRead);
+
+    /* Create directories up to `gcRoot'. */
+    createDirs(dirOf(gcRoot));
+
+    /* Remove the old symlink. */
+    unlink(gcRoot.c_str());
+
+    /* And create the new own. */
+    if (symlink(storePath.c_str(), gcRoot.c_str()) == -1)
+        throw SysError(format("symlinking `%1%' to `%2%'")
+            % gcRoot % storePath);
+
+    return gcRoot;
+}
+
+
 static string tempRootsDir = "temproots";
 
 /* The file to which we write our temporary roots. */
@@ -210,6 +252,9 @@ void collectGarbage(const PathSet & roots, GCAction action,
        b) Processes from creating new temporary root files. */
     AutoCloseFD fdGCLock = openGCLock(ltWrite);
 
+    /* !!! Find the roots here, after we've grabbed the GC lock, since
+       the set of permanent roots cannot increase now. */
+
     /* Determine the live paths which is just the closure of the
        roots under the `references' relation. */
     PathSet livePaths;
@@ -264,6 +309,7 @@ void collectGarbage(const PathSet & roots, GCAction action,
        will not work anymore because we get cycles. */
     storePaths = topoSort(storePaths2);
 
+    /* Try to delete store paths in the topologically sorted order. */
     for (Paths::iterator i = storePaths.begin(); i != storePaths.end(); ++i) {
 
         debug(format("considering deletion of `%1%'") % *i);
diff --git a/src/libstore/gc.hh b/src/libstore/gc.hh
index 91c5be91406b..03e1d769153b 100644
--- a/src/libstore/gc.hh
+++ b/src/libstore/gc.hh
@@ -26,5 +26,8 @@ void addTempRoot(const Path & path);
    as a (permanent) root. */
 void removeTempRoots();
 
+/* Register a permanent GC root. */
+Path addPermRoot(const Path & storePath, const Path & gcRoot);
+
 
 #endif /* !__GC_H */
diff --git a/src/libstore/store.cc b/src/libstore/store.cc
index c7b84e7c63fc..b915fce243a8 100644
--- a/src/libstore/store.cc
+++ b/src/libstore/store.cc
@@ -168,7 +168,7 @@ void copyPath(const Path & src, const Path & dst)
 }
 
 
-static bool isInStore(const Path & path)
+bool isStorePath(const Path & path)
 {
     return path[0] == '/'
         && path.compare(0, nixStore.size(), nixStore) == 0
@@ -180,7 +180,7 @@ static bool isInStore(const Path & path)
 
 void assertStorePath(const Path & path)
 {
-    if (!isInStore(path))
+    if (!isStorePath(path))
         throw Error(format("path `%1%' is not in the Nix store") % path);
 }
 
@@ -579,7 +579,7 @@ void verifyStore()
         if (!pathExists(path)) {
             printMsg(lvlError, format("path `%1%' disappeared") % path);
             invalidatePath(path, txn);
-        } else if (!isInStore(path)) {
+        } else if (!isStorePath(path)) {
             printMsg(lvlError, format("path `%1%' is not in the Nix store") % path);
             invalidatePath(path, txn);
         } else
diff --git a/src/libstore/store.hh b/src/libstore/store.hh
index dce4eb1d62ac..3a0b7a7131a2 100644
--- a/src/libstore/store.hh
+++ b/src/libstore/store.hh
@@ -62,6 +62,8 @@ void registerValidPath(const Transaction & txn,
 /* Throw an exception if `path' is not directly in the Nix store. */
 void assertStorePath(const Path & path);
 
+bool isStorePath(const Path & path);
+
 /* "Fix", or canonicalise, the meta-data of the files in a store path
    after it has been built.  In particular:
    - the last modification date on each file is set to 0 (i.e.,
diff --git a/src/nix-instantiate/help.txt b/src/nix-instantiate/help.txt
index 38355ae4a260..5b9d82aa993e 100644
--- a/src/nix-instantiate/help.txt
+++ b/src/nix-instantiate/help.txt
@@ -1,6 +1,6 @@
 nix-instantiate [OPTIONS...] [FILES...]
 
-`nix-instantiate' turns Nix expressions into store expressions. 
+`nix-instantiate' turns Nix expressions into store derivations. 
 
 The argument `-' may be specified to read a Nix expression from
 standard input.
@@ -14,3 +14,5 @@ Options:
 
   --eval-only: evaluate and print resulting term; do not instantiate
   --parse-only: parse and print abstract syntax tree
+
+  --add-root: add garbage collector roots for the result
diff --git a/src/nix-instantiate/main.cc b/src/nix-instantiate/main.cc
index 0deaab36d0a4..7d12c201fb14 100644
--- a/src/nix-instantiate/main.cc
+++ b/src/nix-instantiate/main.cc
@@ -3,6 +3,7 @@
 
 #include "globals.hh"
 #include "build.hh"
+#include "gc.hh"
 #include "shared.hh"
 #include "eval.hh"
 #include "parser.hh"
@@ -26,6 +27,14 @@ static Expr evalStdin(EvalState & state, bool parseOnly)
 }
 
 
+static Path gcRoot;
+static int rootNr = 0;
+
+
+/* Print out the paths of the resulting derivation(s).  If the user
+   specified the `--add-root' flag, we register the derivation as a
+   garbage collection root and print out the path of the GC root
+   symlink instead. */
 static void printDrvPaths(EvalState & state, Expr e)
 {
     ATermList es;
@@ -37,7 +46,13 @@ static void printDrvPaths(EvalState & state, Expr e)
         if (a && evalString(state, a) == "derivation") {
             a = queryAttr(e, "drvPath");
             if (a) {
-                cout << format("%1%\n") % evalPath(state, a);
+                Path drvPath = evalPath(state, a);
+                if (gcRoot == "")
+                    printGCWarning();
+                else
+                    drvPath = addPermRoot(drvPath,
+                        makeRootName(gcRoot, rootNr));
+                cout << format("%1%\n") % drvPath;
                 return;
             }
             throw Error("bad derivation");
@@ -77,10 +92,10 @@ void run(Strings args)
     bool evalOnly = false;
     bool parseOnly = false;
 
-    for (Strings::iterator it = args.begin();
-         it != args.end(); )
+    for (Strings::iterator i = args.begin();
+         i != args.end(); )
     {
-        string arg = *it++;
+        string arg = *i++;
 
         if (arg == "-")
             readStdin = true;
@@ -92,6 +107,11 @@ void run(Strings args)
             readOnlyMode = true;
             parseOnly = evalOnly = true;
         }
+        else if (arg == "--add-root") {
+            if (i == args.end())
+                throw UsageError("`--add-root requires an argument");
+            gcRoot = *i++;
+        }
         else if (arg[0] == '-')
             throw UsageError(format("unknown flag `%1%`") % arg);
         else
@@ -105,10 +125,10 @@ void run(Strings args)
         printResult(state, e, evalOnly);
     }
 
-    for (Strings::iterator it = files.begin();
-         it != files.end(); it++)
+    for (Strings::iterator i = files.begin();
+         i != files.end(); i++)
     {
-        Expr e = evalFile(state, absPath(*it));
+        Expr e = evalFile(state, absPath(*i));
         /* !!! parseOnly ignored */
         printResult(state, e, evalOnly);
     }
diff --git a/src/nix-store/help.txt b/src/nix-store/help.txt
index 35d5423cd8d2..5a5050e47b68 100644
--- a/src/nix-store/help.txt
+++ b/src/nix-store/help.txt
@@ -42,3 +42,5 @@ Options:
 
   --verbose / -v: verbose operation (may be repeated)
   --keep-failed / -K: keep temporary directories of failed builds
+
+  --add-root: add garbage collector roots for the result
diff --git a/src/nix-store/main.cc b/src/nix-store/main.cc
index 810fe94b65eb..c1fedaf48cc1 100644
--- a/src/nix-store/main.cc
+++ b/src/nix-store/main.cc
@@ -1,5 +1,9 @@
 #include <iostream>
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
 #include "globals.hh"
 #include "build.hh"
 #include "gc.hh"
@@ -18,6 +22,10 @@ void printHelp()
 }
 
 
+static Path gcRoot;
+static int rootNr = 0;
+
+
 static Path findOutput(const Derivation & drv, string id)
 {
     for (DerivationOutputs::const_iterator i = drv.outputs.begin();
@@ -27,6 +35,22 @@ static Path findOutput(const Derivation & drv, string id)
 }
 
 
+static Path followSymlinks(Path & path)
+{
+    while (!isStorePath(path)) {
+        struct stat st;
+        if (lstat(path.c_str(), &st))
+            throw SysError(format("getting status of `%1%'") % path);
+        if (!S_ISLNK(st.st_mode)) return path;
+        string target = readLink(path);
+        path = canonPath(string(target, 0, 1) == "/"
+            ? target
+            : path + "/" + target);
+    }
+    return path;
+}
+
+
 /* Realisation the given path.  For a derivation that means build it;
    for other paths it means ensure their validity. */
 static Path realisePath(const Path & path)
@@ -35,7 +59,14 @@ static Path realisePath(const Path & path)
         PathSet paths;
         paths.insert(path);
         buildDerivations(paths);
-        return findOutput(derivationFromPath(path), "out");
+        Path outPath = findOutput(derivationFromPath(path), "out");
+        
+        if (gcRoot == "")
+            printGCWarning();
+        else
+            outPath = addPermRoot(outPath, makeRootName(gcRoot, rootNr));
+        
+        return outPath;
     } else {
         ensurePath(path);
         return path;
@@ -48,6 +79,10 @@ static void opRealise(Strings opFlags, Strings opArgs)
 {
     if (!opFlags.empty()) throw UsageError("unknown flag");
 
+    for (Strings::iterator i = opArgs.begin();
+         i != opArgs.end(); i++)
+        *i = followSymlinks(*i);
+            
     if (opArgs.size() > 1) {
         PathSet drvPaths;
         for (Strings::iterator i = opArgs.begin();
@@ -374,8 +409,8 @@ void run(Strings args)
     Strings opFlags, opArgs;
     Operation op = 0;
 
-    for (Strings::iterator i = args.begin(); i != args.end(); ++i) {
-        string arg = *i;
+    for (Strings::iterator i = args.begin(); i != args.end(); ) {
+        string arg = *i++;
 
         Operation oldOp = op;
 
@@ -403,6 +438,11 @@ void run(Strings args)
             op = opInit;
         else if (arg == "--verify")
             op = opVerify;
+        else if (arg == "--add-root") {
+            if (i == args.end())
+                throw UsageError("`--add-root requires an argument");
+            gcRoot = *i++;
+        }
         else if (arg[0] == '-')
             opFlags.push_back(arg);
         else