Merge remote-tracking branch 'origin/master' into flakes

7 files changed, 155 insertions, 144 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index dabd447f5..db7300c58 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1629,6 +1629,61 @@ void DerivationGoal::buildDone()
            being valid. */
         registerOutputs();
 
+        if (settings.postBuildHook != "") {
+            Activity act(*logger, lvlInfo, actPostBuildHook,
+                fmt("running post-build-hook '%s'", settings.postBuildHook),
+                Logger::Fields{drvPath});
+            PushActivity pact(act.id);
+            auto outputPaths = drv->outputPaths();
+            std::map<std::string, std::string> hookEnvironment = getEnv();
+
+            hookEnvironment.emplace("DRV_PATH", drvPath);
+            hookEnvironment.emplace("OUT_PATHS", chomp(concatStringsSep(" ", outputPaths)));
+
+            RunOptions opts(settings.postBuildHook, {});
+            opts.environment = hookEnvironment;
+
+            struct LogSink : Sink {
+                Activity & act;
+                std::string currentLine;
+
+                LogSink(Activity & act) : act(act) { }
+
+                void operator() (const unsigned char * data, size_t len) override {
+                    for (size_t i = 0; i < len; i++) {
+                        auto c = data[i];
+
+                        if (c == '\n') {
+                            flushLine();
+                        } else {
+                            currentLine += c;
+                        }
+                    }
+                }
+
+                void flushLine() {
+                    if (settings.verboseBuild) {
+                        printError("post-build-hook: " + currentLine);
+                    } else {
+                        act.result(resPostBuildLogLine, currentLine);
+                    }
+                    currentLine.clear();
+                }
+
+                ~LogSink() {
+                    if (currentLine != "") {
+                        currentLine += '\n';
+                        flushLine();
+                    }
+                }
+            };
+            LogSink sink(act);
+
+            opts.standardOut = &sink;
+            opts.mergeStderrToStdout = true;
+            runProgram2(opts);
+        }
+
         if (buildMode == bmCheck) {
             done(BuildResult::Built);
             return;
@@ -2734,7 +2789,13 @@ void DerivationGoal::runChild()
                on. */
             if (fixedOutput) {
                 ss.push_back("/etc/resolv.conf");
-                ss.push_back("/etc/nsswitch.conf");
+
+                // Only use nss functions to resolve hosts and
+                // services. Don’t use it for anything else that may
+                // be configured for this system. This limits the
+                // potential impurities introduced in fixed outputs.
+                writeFile(chrootRootDir + "/etc/nsswitch.conf", "hosts: files dns\nservices: files\n");
+
                 ss.push_back("/etc/services");
                 ss.push_back("/etc/hosts");
                 if (pathExists("/var/run/nscd/socket"))
@@ -3978,17 +4039,6 @@ void SubstitutionGoal::tryToRun()
         return;
     }
 
-    /* If the store path is already locked (probably by a
-       DerivationGoal), then put this goal to sleep. Note: we don't
-       acquire a lock here since that breaks addToStore(), so below we
-       handle an AlreadyLocked exception from addToStore(). The check
-       here is just an optimisation to prevent having to redo a
-       download due to a locked path. */
-    if (pathIsLockedByMe(worker.store.toRealPath(storePath))) {
-        worker.waitForAWhile(shared_from_this());
-        return;
-    }
-
     maintainRunningSubstitutions = std::make_unique<MaintainCount<uint64_t>>(worker.runningSubstitutions);
     worker.updateProgress();
 
@@ -4028,12 +4078,6 @@ void SubstitutionGoal::finished()
 
     try {
         promise.get_future().get();
-    } catch (AlreadyLocked & e) {
-        /* Probably a DerivationGoal is already building this store
-           path. Sleep for a while and try again. */
-        state = &SubstitutionGoal::init;
-        worker.waitForAWhile(shared_from_this());
-        return;
     } catch (std::exception & e) {
         printError(e.what());
 
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 26e2b0dca..366dbfb0a 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -29,7 +29,7 @@ static string gcRootsDir = "gcroots";
    read.  To be precise: when they try to create a new temporary root
    file, they will block until the garbage collector has finished /
    yielded the GC lock. */
-int LocalStore::openGCLock(LockType lockType)
+AutoCloseFD LocalStore::openGCLock(LockType lockType)
 {
     Path fnGCLock = (format("%1%/%2%")
         % stateDir % gcLockName).str();
@@ -49,7 +49,7 @@ int LocalStore::openGCLock(LockType lockType)
        process that can open the file for reading can DoS the
        collector. */
 
-    return fdGCLock.release();
+    return fdGCLock;
 }
 
 
@@ -221,26 +221,22 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor)
         //FDPtr fd(new AutoCloseFD(openLockFile(path, false)));
         //if (*fd == -1) continue;
 
-        if (path != fnTempRoots) {
-
-            /* Try to acquire a write lock without blocking.  This can
-               only succeed if the owning process has died.  In that case
-               we don't care about its temporary roots. */
-            if (lockFile(fd->get(), ltWrite, false)) {
-                printError(format("removing stale temporary roots file '%1%'") % path);
-                unlink(path.c_str());
-                writeFull(fd->get(), "d");
-                continue;
-            }
-
-            /* Acquire a read lock.  This will prevent the owning process
-               from upgrading to a write lock, therefore it will block in
-               addTempRoot(). */
-            debug(format("waiting for read lock on '%1%'") % path);
-            lockFile(fd->get(), ltRead, true);
-
+        /* Try to acquire a write lock without blocking.  This can
+           only succeed if the owning process has died.  In that case
+           we don't care about its temporary roots. */
+        if (lockFile(fd->get(), ltWrite, false)) {
+            printError(format("removing stale temporary roots file '%1%'") % path);
+            unlink(path.c_str());
+            writeFull(fd->get(), "d");
+            continue;
         }
 
+        /* Acquire a read lock.  This will prevent the owning process
+           from upgrading to a write lock, therefore it will block in
+           addTempRoot(). */
+        debug(format("waiting for read lock on '%1%'") % path);
+        lockFile(fd->get(), ltRead, true);
+
         /* Read the entire file. */
         string contents = readFile(fd->get());
 
@@ -444,17 +440,22 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor)
     }
 
 #if !defined(__linux__)
-    try {
-        std::regex lsofRegex(R"(^n(/.*)$)");
-        auto lsofLines =
-            tokenizeString<std::vector<string>>(runProgram(LSOF, true, { "-n", "-w", "-F", "n" }), "\n");
-        for (const auto & line : lsofLines) {
-            std::smatch match;
-            if (std::regex_match(line, match, lsofRegex))
-                unchecked[match[1]].emplace("{lsof}");
+    // lsof is really slow on OS X. This actually causes the gc-concurrent.sh test to fail.
+    // See: https://github.com/NixOS/nix/issues/3011
+    // Because of this we disable lsof when running the tests.
+    if (getEnv("_NIX_TEST_NO_LSOF") == "") {
+        try {
+            std::regex lsofRegex(R"(^n(/.*)$)");
+            auto lsofLines =
+                tokenizeString<std::vector<string>>(runProgram(LSOF, true, { "-n", "-w", "-F", "n" }), "\n");
+            for (const auto & line : lsofLines) {
+                std::smatch match;
+                if (std::regex_match(line, match, lsofRegex))
+                    unchecked[match[1]].emplace("{lsof}");
+            }
+        } catch (ExecError & e) {
+            /* lsof not installed, lsof failed */
         }
-    } catch (ExecError & e) {
-        /* lsof not installed, lsof failed */
     }
 #endif
 
@@ -866,7 +867,12 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
 
 void LocalStore::autoGC(bool sync)
 {
-    auto getAvail = [this]() {
+    static auto fakeFreeSpaceFile = getEnv("_NIX_TEST_FREE_SPACE_FILE", "");
+
+    auto getAvail = [this]() -> uint64_t {
+        if (!fakeFreeSpaceFile.empty())
+            return std::stoll(readFile(fakeFreeSpaceFile));
+
         struct statvfs st;
         if (statvfs(realStoreDir.c_str(), &st))
             throw SysError("getting filesystem info about '%s'", realStoreDir);
@@ -887,7 +893,7 @@ void LocalStore::autoGC(bool sync)
 
         auto now = std::chrono::steady_clock::now();
 
-        if (now < state->lastGCCheck + std::chrono::seconds(5)) return;
+        if (now < state->lastGCCheck + std::chrono::seconds(settings.minFreeCheckInterval)) return;
 
         auto avail = getAvail();
 
diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh
index 2aecebe3d..e845c29b0 100644
--- a/src/libstore/globals.hh
+++ b/src/libstore/globals.hh
@@ -315,6 +315,9 @@ public:
         "pre-build-hook",
         "A program to run just before a build to set derivation-specific build settings."};
 
+    Setting<std::string> postBuildHook{this, "", "post-build-hook",
+        "A program to run just after each succesful build."};
+
     Setting<std::string> netrcFile{this, fmt("%s/%s", nixConfDir, "netrc"), "netrc-file",
         "Path to the netrc file used to obtain usernames/passwords for downloads."};
 
@@ -342,6 +345,9 @@ public:
     Setting<uint64_t> maxFree{this, std::numeric_limits<uint64_t>::max(), "max-free",
         "Stop deleting garbage when free disk space is above the specified amount."};
 
+    Setting<uint64_t> minFreeCheckInterval{this, 5, "min-free-check-interval",
+        "Number of seconds between checking free disk space."};
+
     Setting<Paths> pluginFiles{this, {}, "plugin-files",
         "Plugins to dynamically load at nix initialization time."};
 
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index f39c73b23..a9399130c 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1203,7 +1203,8 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 
     bool errors = false;
 
-    /* Acquire the global GC lock to prevent a garbage collection. */
+    /* Acquire the global GC lock to get a consistent snapshot of
+       existing and valid paths. */
     AutoCloseFD fdGCLock = openGCLock(ltWrite);
 
     PathSet store;
@@ -1214,13 +1215,11 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 
     PathSet validPaths2 = queryAllValidPaths(), validPaths, done;
 
+    fdGCLock = -1;
+
     for (auto & i : validPaths2)
         verifyPath(i, store, done, validPaths, repair, errors);
 
-    /* Release the GC lock so that checking content hashes (which can
-       take ages) doesn't block the GC or builds. */
-    fdGCLock = -1;
-
     /* Optionally, check the content hashes (slow). */
     if (checkContents) {
         printInfo("checking hashes...");
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 6b655647b..af8b84bf5 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -263,7 +263,7 @@ private:
     bool isActiveTempFile(const GCState & state,
         const Path & path, const string & suffix);
 
-    int openGCLock(LockType lockType);
+    AutoCloseFD openGCLock(LockType lockType);
 
     void findRoots(const Path & path, unsigned char type, Roots & roots);
 
diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc
index 08d1efdbe..2635e3940 100644
--- a/src/libstore/pathlocks.cc
+++ b/src/libstore/pathlocks.cc
@@ -5,9 +5,10 @@
 #include <cerrno>
 #include <cstdlib>
 
+#include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <fcntl.h>
+#include <sys/file.h>
 
 
 namespace nix {
@@ -40,17 +41,14 @@ void deleteLockFile(const Path & path, int fd)
 
 bool lockFile(int fd, LockType lockType, bool wait)
 {
-    struct flock lock;
-    if (lockType == ltRead) lock.l_type = F_RDLCK;
-    else if (lockType == ltWrite) lock.l_type = F_WRLCK;
-    else if (lockType == ltNone) lock.l_type = F_UNLCK;
+    int type;
+    if (lockType == ltRead) type = LOCK_SH;
+    else if (lockType == ltWrite) type = LOCK_EX;
+    else if (lockType == ltNone) type = LOCK_UN;
     else abort();
-    lock.l_whence = SEEK_SET;
-    lock.l_start = 0;
-    lock.l_len = 0; /* entire file */
 
     if (wait) {
-        while (fcntl(fd, F_SETLKW, &lock) != 0) {
+        while (flock(fd, type) != 0) {
             checkInterrupt();
             if (errno != EINTR)
                 throw SysError(format("acquiring/releasing lock"));
@@ -58,9 +56,9 @@ bool lockFile(int fd, LockType lockType, bool wait)
                 return false;
         }
     } else {
-        while (fcntl(fd, F_SETLK, &lock) != 0) {
+        while (flock(fd, type | LOCK_NB) != 0) {
             checkInterrupt();
-            if (errno == EACCES || errno == EAGAIN) return false;
+            if (errno == EWOULDBLOCK) return false;
             if (errno != EINTR)
                 throw SysError(format("acquiring/releasing lock"));
         }
@@ -70,14 +68,6 @@ bool lockFile(int fd, LockType lockType, bool wait)
 }
 
 
-/* This enables us to check whether are not already holding a lock on
-   a file ourselves.  POSIX locks (fcntl) suck in this respect: if we
-   close a descriptor, the previous lock will be closed as well.  And
-   there is no way to query whether we already have a lock (F_GETLK
-   only works on locks held by other processes). */
-static Sync<StringSet> lockedPaths_;
-
-
 PathLocks::PathLocks()
     : deletePaths(false)
 {
@@ -91,7 +81,7 @@ PathLocks::PathLocks(const PathSet & paths, const string & waitMsg)
 }
 
 
-bool PathLocks::lockPaths(const PathSet & _paths,
+bool PathLocks::lockPaths(const PathSet & paths,
     const string & waitMsg, bool wait)
 {
     assert(fds.empty());
@@ -99,75 +89,54 @@ bool PathLocks::lockPaths(const PathSet & _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. */
-    Paths paths(_paths.begin(), _paths.end());
-    paths.sort();
-
-    /* Acquire the lock for each path. */
+    /* Acquire the lock for each path in sorted order. This ensures
+       that locks are always acquired in the same order, thus
+       preventing deadlocks. */
     for (auto & path : paths) {
         checkInterrupt();
         Path lockPath = path + ".lock";
 
         debug(format("locking path '%1%'") % path);
 
-        {
-            auto lockedPaths(lockedPaths_.lock());
-            if (lockedPaths->count(lockPath)) {
-                if (!wait) return false;
-                throw AlreadyLocked("deadlock: trying to re-acquire self-held lock '%s'", lockPath);
-            }
-            lockedPaths->insert(lockPath);
-        }
-
-        try {
-
-            AutoCloseFD fd;
+        AutoCloseFD fd;
 
-            while (1) {
+        while (1) {
 
-                /* Open/create the lock file. */
-                fd = openLockFile(lockPath, true);
+            /* Open/create the lock file. */
+            fd = openLockFile(lockPath, true);
 
-                /* Acquire an exclusive lock. */
-                if (!lockFile(fd.get(), ltWrite, false)) {
-                    if (wait) {
-                        if (waitMsg != "") printError(waitMsg);
-                        lockFile(fd.get(), ltWrite, true);
-                    } else {
-                        /* Failed to lock this path; release all other
-                           locks. */
-                        unlock();
-                        lockedPaths_.lock()->erase(lockPath);
-                        return false;
-                    }
+            /* Acquire an exclusive lock. */
+            if (!lockFile(fd.get(), ltWrite, false)) {
+                if (wait) {
+                    if (waitMsg != "") printError(waitMsg);
+                    lockFile(fd.get(), ltWrite, true);
+                } else {
+                    /* Failed to lock this path; release all other
+                       locks. */
+                    unlock();
+                    return false;
                 }
-
-                debug(format("lock acquired on '%1%'") % lockPath);
-
-                /* Check that the lock file hasn't become stale (i.e.,
-                   hasn't been unlinked). */
-                struct stat st;
-                if (fstat(fd.get(), &st) == -1)
-                    throw SysError(format("statting lock file '%1%'") % lockPath);
-                if (st.st_size != 0)
-                    /* This lock file has been unlinked, so we're holding
-                       a lock on a deleted file.  This means that other
-                       processes may create and acquire a lock on
-                       `lockPath', and proceed.  So we must retry. */
-                    debug(format("open lock file '%1%' has become stale") % lockPath);
-                else
-                    break;
             }
 
-            /* Use borrow so that the descriptor isn't closed. */
-            fds.push_back(FDPair(fd.release(), lockPath));
-
-        } catch (...) {
-            lockedPaths_.lock()->erase(lockPath);
-            throw;
+            debug(format("lock acquired on '%1%'") % lockPath);
+
+            /* Check that the lock file hasn't become stale (i.e.,
+               hasn't been unlinked). */
+            struct stat st;
+            if (fstat(fd.get(), &st) == -1)
+                throw SysError(format("statting lock file '%1%'") % lockPath);
+            if (st.st_size != 0)
+                /* This lock file has been unlinked, so we're holding
+                   a lock on a deleted file.  This means that other
+                   processes may create and acquire a lock on
+                   `lockPath', and proceed.  So we must retry. */
+                debug(format("open lock file '%1%' has become stale") % lockPath);
+            else
+                break;
         }
 
+        /* Use borrow so that the descriptor isn't closed. */
+        fds.push_back(FDPair(fd.release(), lockPath));
     }
 
     return true;
@@ -189,8 +158,6 @@ void PathLocks::unlock()
     for (auto & i : fds) {
         if (deletePaths) deleteLockFile(i.second, i.first);
 
-        lockedPaths_.lock()->erase(i.second);
-
         if (close(i.first) == -1)
             printError(
                 format("error (ignored): cannot close lock file on '%1%'") % i.second);
@@ -208,11 +175,4 @@ void PathLocks::setDeletion(bool deletePaths)
 }
 
 
-bool pathIsLockedByMe(const Path & path)
-{
-    Path lockPath = path + ".lock";
-    return lockedPaths_.lock()->count(lockPath);
-}
-
-
 }
diff --git a/src/libstore/pathlocks.hh b/src/libstore/pathlocks.hh
index db51f950a..411da0222 100644
--- a/src/libstore/pathlocks.hh
+++ b/src/libstore/pathlocks.hh
@@ -16,8 +16,6 @@ enum LockType { ltRead, ltWrite, ltNone };
 
 bool lockFile(int fd, LockType lockType, bool wait);
 
-MakeError(AlreadyLocked, Error);
-
 class PathLocks
 {
 private:
@@ -37,6 +35,4 @@ public:
     void setDeletion(bool deletePaths);
 };
 
-bool pathIsLockedByMe(const Path & path);
-
 }