* In normaliseFState(), wrap registration of the output paths and the

  normal form in a single transaction to ensure that if we crash,
  either everything is registered or nothing is.  This is for
  recoverability: unregistered paths in the store can be deleted
  arbitrarily, while registered paths can only be deleted by running
  the garbage collector.

6 files changed, 44 insertions, 28 deletions

diff --git a/src/fstate.cc b/src/fstate.cc
index 5da3d8358f74..2e2857ffc9f6 100644
--- a/src/fstate.cc
+++ b/src/fstate.cc
@@ -44,7 +44,10 @@ FSId writeTerm(ATerm t, const string & suffix, FSId id)
 //     debug(format("written term %1% = %2%") % (string) id %
 //         printTerm(t));
 
-    registerPath(path, id);
+    Transaction txn(nixDB);
+    registerPath(txn, path, id);
+    txn.commit();
+
     return id;
 }
 
diff --git a/src/nix.cc b/src/nix.cc
index 42cc4a87c228..4beeb5da8943 100644
--- a/src/nix.cc
+++ b/src/nix.cc
@@ -239,14 +239,16 @@ static void opSuccessor(Strings opFlags, Strings opArgs)
 {
     if (!opFlags.empty()) throw UsageError("unknown flag");
     if (opArgs.size() % 2) throw UsageError("expecting even number of arguments");
-    
+
+    Transaction txn(nixDB); /* !!! this could be a big transaction */ 
     for (Strings::iterator i = opArgs.begin();
          i != opArgs.end(); )
     {
         FSId id1 = parseHash(*i++);
         FSId id2 = parseHash(*i++);
-        registerSuccessor(id1, id2);
+        registerSuccessor(txn, id1, id2);
     }
+    txn.commit();
 }
 
 
diff --git a/src/normalise.cc b/src/normalise.cc
index 2eb34d4ee2da..074eda296008 100644
--- a/src/normalise.cc
+++ b/src/normalise.cc
@@ -8,19 +8,10 @@
 #include "globals.hh"
 
 
-void registerSuccessor(const FSId & id1, const FSId & id2)
+void registerSuccessor(const Transaction & txn,
+    const FSId & id1, const FSId & id2)
 {
-    Transaction txn(nixDB);
     nixDB.setString(txn, dbSuccessors, id1, id2);
-    txn.commit();
-}
-
-
-static FSId storeSuccessor(const FSId & id1, ATerm sc)
-{
-    FSId id2 = writeTerm(sc, "-s-" + (string) id1);
-    registerSuccessor(id1, id2);
-    return id2;
 }
 
 
@@ -153,7 +144,7 @@ FSId normaliseFState(FSId id, FSIdSet pending)
             expandId(i->second, i->first, "/", pending);
         } catch (Error & e) {
             debug(format("fast build failed for `%1%': %2%")
-		  % i->first % e.what());
+                % i->first % e.what());
             fastBuild = false;
             break;
         }
@@ -175,8 +166,8 @@ FSId normaliseFState(FSId id, FSIdSet pending)
     } else
         msg(lvlChatty, format("fast build succesful"));
 
-    /* Check whether the output paths were created, and register each
-       one. */
+    /* Check whether the output paths were created, and grep each
+       output path to determine what other paths it references. */
     FSIdSet used;
     for (OutPaths::iterator i = outPaths.begin(); 
          i != outPaths.end(); i++)
@@ -184,7 +175,6 @@ FSId normaliseFState(FSId id, FSIdSet pending)
         string path = i->first;
         if (!pathExists(path))
             throw Error(format("path `%1%' does not exist") % path);
-        registerPath(path, i->second);
         fs.slice.roots.push_back(i->second);
 
         Strings refs = filterReferences(path, refPaths);
@@ -224,10 +214,27 @@ FSId normaliseFState(FSId id, FSIdSet pending)
         }
     }
 
+    /* Write the normal form.  This does not have to occur in the
+       transaction below because writing terms is idem-potent. */
     fs.type = FState::fsSlice;
     ATerm nf = unparseFState(fs);
     msg(lvlVomit, format("normal form: %1%") % printTerm(nf));
-    return storeSuccessor(id, nf);
+    FSId idNF = writeTerm(nf, "-s-" + (string) id);
+
+    /* Register each outpat path, and register the normal form.  This
+       is wrapped in one database transaction to ensure that if we
+       crash, either everything is registered or nothing is.  This is
+       for recoverability: unregistered paths in the store can be
+       deleted arbitrarily, while registered paths can only be deleted
+       by running the garbage collector. */
+    Transaction txn(nixDB);
+    for (OutPaths::iterator i = outPaths.begin(); 
+         i != outPaths.end(); i++)
+        registerPath(txn, i->first, i->second);
+    registerSuccessor(txn, id, idNF);
+    txn.commit();
+
+    return idNF;
 }
 
 
diff --git a/src/normalise.hh b/src/normalise.hh
index 98f58783e5b8..59ab325739e4 100644
--- a/src/normalise.hh
+++ b/src/normalise.hh
@@ -29,7 +29,8 @@ Strings fstateRequisites(const FSId & id,
 FSIds findGenerators(const FSIds & ids);
 
 /* Register a successor. */
-void registerSuccessor(const FSId & id1, const FSId & id2);
+void registerSuccessor(const Transaction & txn,
+    const FSId & id1, const FSId & id2);
 
 
 #endif /* !__NORMALISE_H */
diff --git a/src/store.cc b/src/store.cc
index 9f8e76998285..2411a737fab7 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -105,17 +105,16 @@ void registerSubstitute(const FSId & srcId, const FSId & subId)
 }
 
 
-void registerPath(const string & _path, const FSId & id)
+void registerPath(const Transaction & txn,
+    const string & _path, const FSId & id)
 {
     string path(canonPath(_path));
-    Transaction txn(nixDB);
 
     debug(format("registering path `%1%' with id %2%")
         % path % (string) id);
 
     string oldId;
     if (nixDB.queryString(txn, dbPath2Id, path, oldId)) {
-        txn.abort();
         if (id != parseHash(oldId))
             throw Error(format("path `%1%' already contains id %2%")
                 % path % oldId);
@@ -130,8 +129,6 @@ void registerPath(const string & _path, const FSId & id)
     paths.push_back(path);
     
     nixDB.setStrings(txn, dbId2Paths, id, paths);
-
-    txn.commit();
 }
 
 
@@ -215,7 +212,9 @@ string expandId(const FSId & id, const string & target,
                 return path;
             else {
                 copyPath(path, target);
-                registerPath(target, id);
+                Transaction txn(nixDB);
+                registerPath(txn, target, id);
+                txn.commit();
                 return target;
             }
         }
@@ -267,7 +266,9 @@ void addToStore(string srcPath, string & dstPath, FSId & id,
     }
     
     copyPath(srcPath, dstPath);
-    registerPath(dstPath, id);
+    Transaction txn(nixDB);
+    registerPath(txn, dstPath, id);
+    txn.commit();
 }
 
 
diff --git a/src/store.hh b/src/store.hh
index d1f1be6689ed..7f6b24569038 100644
--- a/src/store.hh
+++ b/src/store.hh
@@ -4,6 +4,7 @@
 #include <string>
 
 #include "hash.hh"
+#include "db.hh"
 
 using namespace std;
 
@@ -20,7 +21,8 @@ void copyPath(string src, string dst);
 void registerSubstitute(const FSId & srcId, const FSId & subId);
 
 /* Register a path keyed on its id. */
-void registerPath(const string & path, const FSId & id);
+void registerPath(const Transaction & txn,
+    const string & path, const FSId & id);
 
 /* Query the id of a path. */
 bool queryPathId(const string & path, FSId & id);