Merge branch 'master' into no-manifests

7 files changed, 103 insertions, 64 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 0972d6e193..1840fb7b21 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -45,7 +45,7 @@
 #include <sched.h>
 #endif
 
-#define CHROOT_ENABLED HAVE_CHROOT && HAVE_UNSHARE && HAVE_SYS_MOUNT_H && defined(MS_BIND) && defined(CLONE_NEWNS)
+#define CHROOT_ENABLED HAVE_CHROOT && HAVE_UNSHARE && HAVE_SYS_MOUNT_H && defined(MS_BIND) && defined(MS_PRIVATE) && defined(CLONE_NEWNS)
 
 #if CHROOT_ENABLED
 #include <sys/socket.h>
@@ -604,18 +604,17 @@ void getOwnership(const Path & path)
 }
 
 
-void deletePathWrapped(const Path & path,
-    unsigned long long & bytesFreed, unsigned long long & blocksFreed)
+void deletePathWrapped(const Path & path, unsigned long long & bytesFreed)
 {
     try {
         /* First try to delete it ourselves. */
-        deletePath(path, bytesFreed, blocksFreed);
+        deletePath(path, bytesFreed);
     } catch (SysError & e) {
         /* If this failed due to a permission error, then try it with
            the setuid helper. */
         if (settings.buildUsersGroup != "" && !amPrivileged()) {
             getOwnership(path);
-            deletePath(path, bytesFreed, blocksFreed);
+            deletePath(path, bytesFreed);
         } else
             throw;
     }
@@ -624,8 +623,8 @@ void deletePathWrapped(const Path & path,
 
 void deletePathWrapped(const Path & path)
 {
-    unsigned long long dummy1, dummy2;
-    deletePathWrapped(path, dummy1, dummy2);
+    unsigned long long dummy1;
+    deletePathWrapped(path, dummy1);
 }
 
 
@@ -1470,9 +1469,9 @@ HookReply DerivationGoal::tryBuildHook()
 }
 
 
-void chmod(const Path & path, mode_t mode)
+void chmod_(const Path & path, mode_t mode)
 {
-    if (::chmod(path.c_str(), 01777) == -1)
+    if (chmod(path.c_str(), mode) == -1)
         throw SysError(format("setting permissions on `%1%'") % path);
 }
 
@@ -1674,7 +1673,7 @@ void DerivationGoal::startBuilder()
            instead.) */
         Path chrootTmpDir = chrootRootDir + "/tmp";
         createDirs(chrootTmpDir);
-        chmod(chrootTmpDir, 01777);
+        chmod_(chrootTmpDir, 01777);
 
         /* Create a /etc/passwd with entries for the build user and the
            nobody account.  The latter is kind of a hack to support
@@ -1710,7 +1709,7 @@ void DerivationGoal::startBuilder()
            precaution, make the fake Nix store only writable by the
            build user. */
         createDirs(chrootRootDir + settings.nixStore);
-        chmod(chrootRootDir + settings.nixStore, 01777);
+        chmod_(chrootRootDir + settings.nixStore, 01777);
 
         foreach (PathSet::iterator, i, inputPaths) {
             struct stat st;
@@ -1844,22 +1843,40 @@ void DerivationGoal::initChild()
             char domainname[] = "(none)"; // kernel default
             setdomainname(domainname, sizeof(domainname));
 
+            /* Make all filesystems private.  This is necessary
+               because subtrees may have been mounted as "shared"
+               (MS_SHARED).  (Systemd does this, for instance.)  Even
+               though we have a private mount namespace, mounting
+               filesystems on top of a shared subtree still propagates
+               outside of the namespace.  Making a subtree private is
+               local to the namespace, though, so setting MS_PRIVATE
+               does not affect the outside world. */
+            Strings mounts = tokenizeString(readFile("/proc/self/mountinfo", true), "\n");
+            foreach (Strings::iterator, i, mounts) {
+                Strings fields = tokenizeString(*i, " ");
+                assert(fields.size() >= 5);
+                Strings::iterator j = fields.begin();
+                std::advance(j, 4);
+                if (mount(0, j->c_str(), 0, MS_PRIVATE, 0) == -1)
+                    throw SysError(format("unable to make filesystem `%1%' private") % *j);
+            }
+
             /* Bind-mount all the directories from the "host"
                filesystem that we want in the chroot
                environment. */
             foreach (PathSet::iterator, i, dirsInChroot) {
                 Path source = *i;
                 Path target = chrootRootDir + source;
+                if (source == "/proc") continue; // backwards compatibility
                 debug(format("bind mounting `%1%' to `%2%'") % source % target);
-
                 createDirs(target);
-
                 if (mount(source.c_str(), target.c_str(), "", MS_BIND, 0) == -1)
                     throw SysError(format("bind mount from `%1%' to `%2%' failed") % source % target);
             }
 
             /* Bind a new instance of procfs on /proc to reflect our
                private PID namespace. */
+            createDirs(chrootRootDir + "/proc");
             if (mount("none", (chrootRootDir + "/proc").c_str(), "proc", 0, 0) == -1)
                 throw SysError("mounting /proc");
 
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 1355702f87..88b7bec326 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -425,10 +425,9 @@ bool LocalStore::isActiveTempFile(const GCState & state,
 void LocalStore::deleteGarbage(GCState & state, const Path & path)
 {
     printMsg(lvlInfo, format("deleting `%1%'") % path);
-    unsigned long long bytesFreed, blocksFreed;
-    deletePathWrapped(path, bytesFreed, blocksFreed);
+    unsigned long long bytesFreed;
+    deletePathWrapped(path, bytesFreed);
     state.results.bytesFreed += bytesFreed;
-    state.results.blocksFreed += blocksFreed;
 }
 
 
@@ -550,7 +549,7 @@ bool LocalStore::tryToDelete(GCState & state, const Path & path)
         } else
             deleteGarbage(state, path);
 
-        if (state.options.maxFreed && state.results.bytesFreed + state.bytesInvalidated > state.options.maxFreed) {
+        if (state.results.bytesFreed + state.bytesInvalidated > state.options.maxFreed) {
             printMsg(lvlInfo, format("deleted or invalidated more than %1% bytes; stopping") % state.options.maxFreed);
             throw GCLimitReached();
         }
@@ -576,11 +575,13 @@ bool LocalStore::tryToDelete(GCState & state, const Path & path)
    safely deleted.  FIXME: race condition with optimisePath(): we
    might see a link count of 1 just before optimisePath() increases
    the link count. */
-void LocalStore::removeUnusedLinks()
+void LocalStore::removeUnusedLinks(const GCState & state)
 {
     AutoCloseDir dir = opendir(linksDir.c_str());
     if (!dir) throw SysError(format("opening directory `%1%'") % linksDir);
 
+    long long actualSize = 0, unsharedSize = 0;
+
     struct dirent * dirent;
     while (errno = 0, dirent = readdir(dir)) {
         checkInterrupt();
@@ -592,13 +593,28 @@ void LocalStore::removeUnusedLinks()
         if (lstat(path.c_str(), &st) == -1)
             throw SysError(format("statting `%1%'") % path);
 
-        if (st.st_nlink != 1) continue;
+        if (st.st_nlink != 1) {
+            unsigned long long size = st.st_blocks * 512ULL;
+            actualSize += size;
+            unsharedSize += (st.st_nlink - 1) * size;
+            continue;
+        }
 
         printMsg(lvlTalkative, format("deleting unused link `%1%'") % path);
 
         if (unlink(path.c_str()) == -1)
             throw SysError(format("deleting `%1%'") % path);
+
+        state.results.bytesFreed += st.st_blocks * 512;
     }
+
+    struct stat st;
+    if (stat(linksDir.c_str(), &st) == -1)
+        throw SysError(format("statting `%1%'") % linksDir);
+    long long overhead = st.st_blocks * 512ULL;
+
+    printMsg(lvlInfo, format("note: currently hard linking saves %.2f MiB")
+        % ((unsharedSize - actualSize - overhead) / (1024.0 * 1024.0)));
 }
 
 
@@ -660,7 +676,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
                 throw Error(format("cannot delete path `%1%' since it is still alive") % *i);
         }
 
-    } else {
+    } else if (options.maxFreed > 0) {
 
         if (shouldDelete(state.options.action))
             printMsg(lvlError, format("deleting garbage..."));
@@ -718,7 +734,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
 
     /* Clean up the links directory. */
     printMsg(lvlError, format("deleting unused links..."));
-    removeUnusedLinks();
+    removeUnusedLinks(state);
 }
 
 
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 3cb016e9ca..ba05829221 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -261,7 +261,7 @@ private:
 
     int openGCLock(LockType lockType);
 
-    void removeUnusedLinks();
+    void removeUnusedLinks(const GCState & state);
 
     void startSubstituter(const Path & substituter,
         RunningSubstituter & runningSubstituter);
@@ -298,8 +298,7 @@ void getOwnership(const Path & path);
 
 /* Like deletePath(), but changes the ownership of `path' using the
    setuid wrapper if necessary (and possible). */
-void deletePathWrapped(const Path & path,
-    unsigned long long & bytesFreed, unsigned long long & blocksFreed);
+void deletePathWrapped(const Path & path, unsigned long long & bytesFreed);
 
 void deletePathWrapped(const Path & path);
 
diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc
index 334f4f355f..9d0242bbc8 100644
--- a/src/libstore/optimise-store.cc
+++ b/src/libstore/optimise-store.cc
@@ -102,11 +102,11 @@ void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path)
         /* Nope, create a hard link in the links directory. */
         makeMutable(path);
         MakeImmutable mk1(path);
-
-        if (link(path.c_str(), linkPath.c_str()) == -1)
+        if (link(path.c_str(), linkPath.c_str()) == 0) return;
+        if (errno != EEXIST)
             throw SysError(format("cannot link `%1%' to `%2%'") % linkPath % path);
-
-        return;
+        /* Fall through if another process created ‘linkPath’ before
+           we did. */
     }
 
     /* Yes!  We've seen a file with the same contents.  Replace the
@@ -123,9 +123,6 @@ void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path)
 
     printMsg(lvlTalkative, format("linking `%1%' to `%2%'") % path % linkPath);
 
-    Path tempLink = (format("%1%/.tmp-link-%2%-%3%")
-        % settings.nixStore % getpid() % rand()).str();
-
     /* Make the containing directory writable, but only if it's not
        the store itself (we don't want or need to mess with its
        permissions). */
@@ -140,40 +137,55 @@ void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path)
        so make it mutable first (and make it immutable again when
        we're done).  We also have to make ‘path’ mutable, otherwise
        rename() will fail to delete it. */
-    makeMutable(linkPath);
-    MakeImmutable mk1(linkPath);
-
     makeMutable(path);
     MakeImmutable mk2(path);
 
-    if (link(linkPath.c_str(), tempLink.c_str()) == -1) {
-        if (errno == EMLINK) {
-            /* Too many links to the same file (>= 32000 on most file
-               systems).  This is likely to happen with empty files.
-               Just shrug and ignore. */
-            printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
-            return;
+    /* Another process might be doing the same thing (creating a new
+       link to ‘linkPath’) and make ‘linkPath’ immutable before we're
+       done.  In that case, just retry. */
+    unsigned int retries = 1024;
+    while (--retries > 0) {
+        makeMutable(linkPath);
+        MakeImmutable mk1(linkPath);
+
+        Path tempLink = (format("%1%/.tmp-link-%2%-%3%")
+            % settings.nixStore % getpid() % rand()).str();
+
+        if (link(linkPath.c_str(), tempLink.c_str()) == -1) {
+            if (errno == EMLINK) {
+                /* Too many links to the same file (>= 32000 on most
+                   file systems).  This is likely to happen with empty
+                   files.  Just shrug and ignore. */
+                if (st.st_size)
+                    printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
+                return;
+            }
+            if (errno == EPERM) continue;
+            throw SysError(format("cannot link `%1%' to `%2%'") % tempLink % linkPath);
         }
-        throw SysError(format("cannot link `%1%' to `%2%'") % tempLink % linkPath);
-    }
-
-    /* Atomically replace the old file with the new hard link. */
-    if (rename(tempLink.c_str(), path.c_str()) == -1) {
-        if (errno == EMLINK) {
-            /* Some filesystems generate too many links on the rename,
-               rather than on the original link.  (Probably it
-               temporarily increases the st_nlink field before
-               decreasing it again.) */
-            printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
 
-            /* Unlink the temp link. */
+        /* Atomically replace the old file with the new hard link. */
+        if (rename(tempLink.c_str(), path.c_str()) == -1) {
             if (unlink(tempLink.c_str()) == -1)
                 printMsg(lvlError, format("unable to unlink `%1%'") % tempLink);
-            return;
+            if (errno == EMLINK) {
+                /* Some filesystems generate too many links on the
+                   rename, rather than on the original link.
+                   (Probably it temporarily increases the st_nlink
+                   field before decreasing it again.) */
+                if (st.st_size)
+                    printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
+                return;
+            }
+            if (errno == EPERM) continue;
+            throw SysError(format("cannot rename `%1%' to `%2%'") % tempLink % path);
         }
-        throw SysError(format("cannot rename `%1%' to `%2%'") % tempLink % path);
+
+        break;
     }
 
+    if (retries == 0) throw Error(format("cannot link `%1%' to `%2%'") % path % linkPath);
+
     stats.filesLinked++;
     stats.bytesFreed += st.st_size;
     stats.blocksFreed += st.st_blocks;
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 56396541ad..d3c05f0df4 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -558,7 +558,7 @@ void RemoteStore::collectGarbage(const GCOptions & options, GCResults & results)
 
     results.paths = readStrings<PathSet>(from);
     results.bytesFreed = readLongLong(from);
-    results.blocksFreed = readLongLong(from);
+    readLongLong(from); // obsolete
 }
 
 
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 6f81a9aab0..32aaca6be0 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -2,7 +2,7 @@
 #include "globals.hh"
 #include "util.hh"
 
-#include <limits.h>
+#include <climits>
 
 
 namespace nix {
@@ -12,7 +12,7 @@ GCOptions::GCOptions()
 {
     action = gcDeleteDead;
     ignoreLiveness = false;
-    maxFreed = 0;
+    maxFreed = ULLONG_MAX;
 }
 
 
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 324d802dc4..a562360ce3 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -48,8 +48,7 @@ struct GCOptions
     /* For `gcDeleteSpecific', the paths to delete. */
     PathSet pathsToDelete;
 
-    /* Stop after at least `maxFreed' bytes have been freed.  0 means
-       no limit. */
+    /* Stop after at least `maxFreed' bytes have been freed. */
     unsigned long long maxFreed;
 
     GCOptions();
@@ -66,13 +65,9 @@ struct GCResults
        number of bytes that would be or was freed. */
     unsigned long long bytesFreed;
 
-    /* The number of file system blocks that would be or was freed. */
-    unsigned long long blocksFreed;
-
     GCResults()
     {
         bytesFreed = 0;
-        blocksFreed = 0;
     }
 };