From 545145cd582cd80b857760ec11bb5a91b6271506 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 1 Aug 2003 14:11:19 +0000 Subject: [PATCH] * 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'. --- src/Makefile.am | 3 +- src/normalise.cc | 111 +++++++++++++++++++++++++++++++++++------------ src/pathlocks.cc | 48 ++++++++++++++++++++ src/pathlocks.hh | 18 ++++++++ src/store.cc | 1 - 5 files changed, 151 insertions(+), 30 deletions(-) create mode 100644 src/pathlocks.cc create mode 100644 src/pathlocks.hh diff --git a/src/Makefile.am b/src/Makefile.am index 847f3eb19..85bf50282 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 e8fc6fc55..cc84b6541 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 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 OutPaths; +typedef map 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 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 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 000000000..ac53dc643 --- /dev/null +++ b/src/pathlocks.cc @@ -0,0 +1,48 @@ +#include + +#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::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 000000000..ae3a37cdb --- /dev/null +++ b/src/pathlocks.hh @@ -0,0 +1,18 @@ +#ifndef __PATHLOCKS_H +#define __PATHLOCKS_H + +#include "util.hh" + + +class PathLocks +{ +private: + list fds; + +public: + PathLocks(const Strings & _paths); + ~PathLocks(); +}; + + +#endif /* !__PATHLOCKS_H */ diff --git a/src/store.cc b/src/store.cc index 2facb4cb1..9f8e76998 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;