* normaliseFState() now locks all output paths prior to building, thus

  ensuring that simultaneous invocations of Nix don't clobber
  each other's  builds.

* Fixed a bug in `make install'.

5 files changed, 151 insertions, 30 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 847f3eb19fa9..85bf502825ed 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -22,7 +22,7 @@ noinst_LIBRARIES = libnix.a libshared.a
 
 libnix_a_SOURCES = util.cc hash.cc archive.cc md5.c \
  store.cc fstate.cc normalise.cc exec.cc \
- globals.cc db.cc references.cc
+ globals.cc db.cc references.cc pathlocks.cc
 
 libshared_a_SOURCES = shared.cc
 
@@ -44,6 +44,7 @@ install-data-local:
 	$(INSTALL) -d $(localstatedir)/nix
 	$(INSTALL) -d $(localstatedir)/nix/db
 	$(INSTALL) -d $(localstatedir)/nix/links
+	rm -f $(prefix)/current
 	ln -sf $(localstatedir)/nix/links/current $(prefix)/current
 	$(INSTALL) -d $(localstatedir)/log/nix
 	$(INSTALL) -d $(prefix)/store
diff --git a/src/normalise.cc b/src/normalise.cc
index e8fc6fc55078..cc84b6541f39 100644
--- a/src/normalise.cc
+++ b/src/normalise.cc
@@ -4,6 +4,7 @@
 #include "references.hh"
 #include "db.hh"
 #include "exec.hh"
+#include "pathlocks.hh"
 #include "globals.hh"
 
 
@@ -23,7 +24,29 @@ static FSId storeSuccessor(const FSId & id1, ATerm sc)
 }
 
 
-typedef set<FSId> FSIdSet;
+static FSId useSuccessor(const FSId & id)
+{
+    string idSucc;
+    if (nixDB.queryString(noTxn, dbSuccessors, id, idSucc)) {
+        debug(format("successor %1% -> %2%") % (string) id % idSucc);
+        return parseHash(idSucc);
+    } else
+        return id;
+}
+
+
+typedef map<string, FSId> OutPaths;
+typedef map<string, SliceElem> ElemMap;
+
+
+Strings pathsFromOutPaths(const OutPaths & ps)
+{
+    Strings ss;
+    for (OutPaths::const_iterator i = ps.begin();
+         i != ps.end(); i++)
+        ss.push_back(i->first);
+    return ss;
+}
 
 
 FSId normaliseFState(FSId id, FSIdSet pending)
@@ -32,19 +55,66 @@ FSId normaliseFState(FSId id, FSIdSet pending)
 
     /* Try to substitute $id$ by any known successors in order to
        speed up the rewrite process. */
-    string idSucc;
-    while (nixDB.queryString(noTxn, dbSuccessors, id, idSucc)) {
-        debug(format("successor %1% -> %2%") % (string) id % idSucc);
-        id = parseHash(idSucc);
-    }
+    id = useSuccessor(id);
 
     /* Get the fstate expression. */
     FState fs = parseFState(termFromId(id));
 
-    /* It this is a normal form (i.e., a slice) we are done. */
+    /* If this is a normal form (i.e., a slice) we are done. */
     if (fs.type == FState::fsSlice) return id;
+    if (fs.type != FState::fsDerive) abort();
     
-    /* Otherwise, it's a derivation. */
+
+    /* Otherwise, it's a derive expression, and we have to build it to
+       determine its normal form. */
+
+
+    /* Some variables. */
+
+    /* Output paths, with their ids. */
+    OutPaths outPaths;
+
+    /* Input paths, with their slice elements. */
+    ElemMap inMap; 
+
+    /* Referencable paths (i.e., input and output paths). */
+    Strings refPaths;
+
+    /* The environment to be passed to the builder. */
+    Environment env; 
+
+
+    /* Parse the outputs. */
+    for (DeriveOutputs::iterator i = fs.derive.outputs.begin();
+         i != fs.derive.outputs.end(); i++)
+    {
+        debug(format("building %1% in `%2%'") % (string) i->second % i->first);
+        outPaths[i->first] = i->second;
+        refPaths.push_back(i->first);
+    }
+
+    /* Obtain locks on all output paths.  The locks are automatically
+       released when we exit this function or Nix crashes. */
+    PathLocks outputLock(pathsFromOutPaths(outPaths));
+
+    /* Now check again whether there is a successor.  This is because
+       another process may have started building in parallel.  After
+       it has finished and released the locks, we can (and should)
+       reuse its results.  (Strictly speaking the first successor
+       check above can be omitted, but that would be less efficient.)
+       Note that since we now hold the locks on the output paths, no
+       other process can build this expression, so no further checks
+       are necessary. */
+    {
+        FSId id2 = useSuccessor(id);
+        if (id2 != id) {
+            FState fs = parseFState(termFromId(id2));
+            debug(format("skipping build of %1%, someone beat us to it")
+                % (string) id);
+            if (fs.type != FState::fsSlice) abort();
+            return id2;
+        }
+    }
 
     /* Right platform? */
     if (fs.derive.platform != thisSystem)
@@ -52,14 +122,12 @@ FSId normaliseFState(FSId id, FSIdSet pending)
             % fs.derive.platform % thisSystem);
         
     /* Realise inputs (and remember all input paths). */
-    typedef map<string, SliceElem> ElemMap;
-
-    ElemMap inMap;
-
     for (FSIds::iterator i = fs.derive.inputs.begin();
          i != fs.derive.inputs.end(); i++) {
         FSId nf = normaliseFState(*i, pending);
         realiseSlice(nf, pending);
+        /* !!! nf should be a root of the garbage collector while we
+           are building */
         FState fs = parseFState(termFromId(nf));
         if (fs.type != FState::fsSlice) abort();
         for (SliceElems::iterator j = fs.slice.elems.begin();
@@ -67,31 +135,18 @@ FSId normaliseFState(FSId id, FSIdSet pending)
             inMap[j->path] = *j;
     }
 
-    Strings inPaths;
     for (ElemMap::iterator i = inMap.begin(); i != inMap.end(); i++)
-        inPaths.push_back(i->second.path);
+        refPaths.push_back(i->second.path);
 
     /* Build the environment. */
-    Environment env;
     for (StringPairs::iterator i = fs.derive.env.begin();
          i != fs.derive.env.end(); i++)
         env[i->first] = i->second;
 
-    /* Parse the outputs. */
-    typedef map<string, FSId> OutPaths;
-    OutPaths outPaths;
-    for (DeriveOutputs::iterator i = fs.derive.outputs.begin();
-         i != fs.derive.outputs.end(); i++)
-    {
-        debug(format("building %1% in `%2%'") % (string) i->second % i->first);
-        outPaths[i->first] = i->second;
-        inPaths.push_back(i->first);
-    }
-
     /* We can skip running the builder if we can expand all output
        paths from their ids. */
     bool fastBuild = true;
-    for (OutPaths::iterator i = outPaths.begin(); 
+    for (OutPaths::iterator i = outPaths.begin();
          i != outPaths.end(); i++)
     {
         try {
@@ -132,7 +187,7 @@ FSId normaliseFState(FSId id, FSIdSet pending)
         registerPath(path, i->second);
         fs.slice.roots.push_back(i->second);
 
-        Strings refs = filterReferences(path, inPaths);
+        Strings refs = filterReferences(path, refPaths);
 
         SliceElem elem;
         elem.path = path;
diff --git a/src/pathlocks.cc b/src/pathlocks.cc
new file mode 100644
index 000000000000..ac53dc6437d0
--- /dev/null
+++ b/src/pathlocks.cc
@@ -0,0 +1,48 @@
+#include <fcntl.h>
+
+#include "pathlocks.hh"
+
+
+PathLocks::PathLocks(const Strings & _paths)
+{
+    /* Note that `fds' is built incrementally so that the destructor
+       will only release those locks that we have already acquired. */
+
+    /* Sort the paths.  This assures that locks are always acquired in
+       the same order, thus preventing deadlocks. */
+    Strings paths(_paths);
+    paths.sort();
+    
+    /* Acquire the lock for each path. */
+    for (Strings::iterator i = paths.begin(); i != paths.end(); i++) {
+        string path = *i;
+        string lockPath = path + ".lock";
+
+        debug(format("locking path `%1%'") % path);
+        
+        /* Open/create the lock file. */
+        int fd = open(lockPath.c_str(), O_WRONLY | O_CREAT, 0666);
+        if (fd == -1)
+            throw SysError(format("opening lock file `%1%'") % lockPath);
+
+        fds.push_back(fd);
+
+        /* Lock it. */
+        struct flock lock;
+        lock.l_type = F_WRLCK; /* exclusive lock */
+        lock.l_whence = SEEK_SET;
+        lock.l_start = 0;
+        lock.l_len = 0; /* entire file */
+
+        while (fcntl(fd, F_SETLKW, &lock) == -1)
+            if (errno != EINTR)
+                throw SysError(format("acquiring lock on `%1%'") % lockPath);
+    }
+}
+
+
+PathLocks::~PathLocks()
+{
+    for (list<int>::iterator i = fds.begin(); i != fds.end(); i++)
+        close(*i);
+}
diff --git a/src/pathlocks.hh b/src/pathlocks.hh
new file mode 100644
index 000000000000..ae3a37cdb214
--- /dev/null
+++ b/src/pathlocks.hh
@@ -0,0 +1,18 @@
+#ifndef __PATHLOCKS_H
+#define __PATHLOCKS_H
+
+#include "util.hh"
+
+
+class PathLocks 
+{
+private:
+    list<int> fds;
+
+public:
+    PathLocks(const Strings & _paths);
+    ~PathLocks();
+};
+
+
+#endif /* !__PATHLOCKS_H */
diff --git a/src/store.cc b/src/store.cc
index 2facb4cb1a20..9f8e76998285 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -260,7 +260,6 @@ void addToStore(string srcPath, string & dstPath, FSId & id,
     dstPath = canonPath(nixStore + "/" + (string) id + "-" + baseName);
 
     try {
-        /* !!! should not use the substitutes! */
         dstPath = expandId(id, deterministicName ? dstPath : "", 
             nixStore, FSIdSet(), true);
         return;