Merge pull request #13241 from fzakaria/lix-2100

cherry-pick https://gerrit.lix.systems/c/lix/+/2100

src/libmain/shared.cc

@@ -176,16 +176,6 @@ void initNix(bool loadConfig)
        now.  In particular, store objects should be readable by
        everybody. */
     umask(0022);
-
-    /* Initialise the PRNG. */
-    struct timeval tv;
-    gettimeofday(&tv, 0);
-#ifndef _WIN32
-    srandom(tv.tv_usec);
-#endif
-    srand(tv.tv_usec);
-
-
 }
 
 

src/libstore/build/derivation-goal.cc

@@ -10,7 +10,7 @@
 #include "nix/util/util.hh"
 #include "nix/util/compression.hh"
 #include "nix/store/common-protocol.hh"
-#include "nix/store/common-protocol-impl.hh"
+#include "nix/store/common-protocol-impl.hh" // Don't remove is actually needed
 #include "nix/store/local-store.hh" // TODO remove, along with remaining downcasts
 
 #include <fstream>

src/libstore/ssh.cc

@@ -34,7 +34,7 @@ SSHMaster::SSHMaster(
         throw Error("invalid SSH host name '%s'", host);
 
     auto state(state_.lock());
-    state->tmpDir = std::make_unique<AutoDelete>(createTempDir("", "nix", true, true, 0700));
+    state->tmpDir = std::make_unique<AutoDelete>(createTempDir("", "nix", 0700));
 }
 
 void SSHMaster::addCommonSSHOpts(Strings & args)

src/libstore/unix/build/derivation-builder.cc

@@ -1,22 +1,15 @@
 #include "nix/store/build/derivation-builder.hh"
+#include "nix/util/file-system.hh"
 #include "nix/store/local-store.hh"
 #include "nix/util/processes.hh"
-#include "nix/store/indirect-root-store.hh"
-#include "nix/store/build/hook-instance.hh"
-#include "nix/store/build/worker.hh"
 #include "nix/store/builtins.hh"
-#include "nix/store/builtins/buildenv.hh"
 #include "nix/store/path-references.hh"
 #include "nix/util/finally.hh"
 #include "nix/util/util.hh"
 #include "nix/util/archive.hh"
 #include "nix/util/git.hh"
-#include "nix/util/compression.hh"
 #include "nix/store/daemon.hh"
 #include "nix/util/topo-sort.hh"
-#include "nix/util/callback.hh"
-#include "nix/util/json-utils.hh"
-#include "nix/util/current-process.hh"
 #include "nix/store/build/child.hh"
 #include "nix/util/unix-domain-socket.hh"
 #include "nix/store/posix-fs-canonicalise.hh"
@@ -671,23 +664,33 @@ static void replaceValidPath(const Path & storePath, const Path & tmpPath)
        tmpPath (the replacement), so we have to move it out of the
        way first.  We'd better not be interrupted here, because if
        we're repairing (say) Glibc, we end up with a broken system. */
-    Path oldPath = fmt("%1%.old-%2%-%3%", storePath, getpid(), rand());
-    if (pathExists(storePath))
+    Path oldPath;
+    
+    if (pathExists(storePath)) {
+        // why do we loop here?
+        // although makeTempPath should be unique, we can't 
+        // guarantee that.
+        do {
+            oldPath = makeTempPath(storePath, ".old");
+            // store paths are often directories so we can't just unlink() it
+            // let's make sure the path doesn't exist before we try to use it
+        } while (pathExists(oldPath));
         movePath(storePath, oldPath);
-
+    }
     try {
         movePath(tmpPath, storePath);
     } catch (...) {
         try {
             // attempt to recover
-            movePath(oldPath, storePath);
+            if (!oldPath.empty())
+                movePath(oldPath, storePath);
         } catch (...) {
             ignoreExceptionExceptInterrupt();
         }
         throw;
     }
-
-    deletePath(oldPath);
+    if (!oldPath.empty())
+        deletePath(oldPath);
 }
 
 
@@ -877,7 +880,7 @@ void DerivationBuilderImpl::startBuilder()
 
     /* Create a temporary directory where the build will take
        place. */
-    topTmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), false, false, 0700);
+    topTmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), 0700);
 #ifdef __APPLE__
     if (false) {
 #else

src/libstore/unix/user-lock.cc

@@ -7,6 +7,7 @@
 #include "nix/store/globals.hh"
 #include "nix/store/pathlocks.hh"
 #include "nix/util/users.hh"
+#include "nix/util/logging.hh"
 
 namespace nix {
 

src/libutil/file-system.cc

@@ -8,12 +8,12 @@
 #include "nix/util/util.hh"
 
 #include <atomic>
+#include <random>
 #include <cerrno>
 #include <climits>
 #include <cstdio>
 #include <cstdlib>
 #include <deque>
-#include <sstream>
 #include <filesystem>
 
 #include <fcntl.h>
@@ -27,10 +27,6 @@
 # include <io.h>
 #endif
 
-#include "nix/util/strings-inline.hh"
-
-#include "util-config-private.hh"
-
 namespace nix {
 
 DirectoryIterator::DirectoryIterator(const std::filesystem::path& p) {
@@ -582,26 +578,11 @@ std::string defaultTempDir() {
     return getEnvNonEmpty("TMPDIR").value_or("/tmp");
 }
 
-static Path tempName(Path tmpRoot, const Path & prefix, bool includePid,
-    std::atomic<unsigned int> & counter)
+Path createTempDir(const Path & tmpRoot, const Path & prefix, mode_t mode)
 {
-    tmpRoot = canonPath(tmpRoot.empty() ? defaultTempDir() : tmpRoot, true);
-    if (includePid)
-        return fmt("%1%/%2%-%3%-%4%", tmpRoot, prefix, getpid(), counter++);
-    else
-        return fmt("%1%/%2%-%3%", tmpRoot, prefix, counter++);
-}
-
-Path createTempDir(const Path & tmpRoot, const Path & prefix,
-    bool includePid, bool useGlobalCounter, mode_t mode)
-{
-    static std::atomic<unsigned int> globalCounter = 0;
-    std::atomic<unsigned int> localCounter = 0;
-    auto & counter(useGlobalCounter ? globalCounter : localCounter);
-
     while (1) {
         checkInterrupt();
-        Path tmpDir = tempName(tmpRoot, prefix, includePid, counter);
+        Path tmpDir = makeTempPath(tmpRoot, prefix);
         if (mkdir(tmpDir.c_str()
 #ifndef _WIN32 // TODO abstract mkdir perms for Windows
                     , mode
@@ -641,6 +622,14 @@ std::pair<AutoCloseFD, Path> createTempFile(const Path & prefix)
     return {std::move(fd), tmpl};
 }
 
+Path makeTempPath(const Path & root, const Path & suffix)
+{
+    // start the counter at a random value to minimize issues with preexisting temp paths
+    static std::atomic<uint32_t> counter(std::random_device{}());
+    auto tmpRoot = canonPath(root.empty() ? defaultTempDir() : root, true);
+    return fmt("%1%/%2%-%3%-%4%", tmpRoot, suffix, getpid(), counter.fetch_add(1, std::memory_order_relaxed));
+}
+
 void createSymlink(const Path & target, const Path & link)
 {
     try {

src/libutil/include/nix/util/file-system.hh

@@ -6,8 +6,6 @@
  */
 
 #include "nix/util/types.hh"
-#include "nix/util/error.hh"
-#include "nix/util/logging.hh"
 #include "nix/util/file-descriptor.hh"
 #include "nix/util/file-path.hh"
 
@@ -18,12 +16,8 @@
 #ifdef _WIN32
 # include <windef.h>
 #endif
-#include <signal.h>
 
-#include <atomic>
 #include <functional>
-#include <map>
-#include <sstream>
 #include <optional>
 
 /**
@@ -316,8 +310,8 @@ typedef std::unique_ptr<DIR, DIRDeleter> AutoCloseDir;
 /**
  * Create a temporary directory.
  */
-Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix",
-    bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755);
+Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix", 
+                   mode_t mode = 0755);
 
 /**
  * Create a temporary file, returning a file handle and its path.
@@ -335,6 +329,14 @@ Path defaultTempDir();
  */
 bool isExecutableFileAmbient(const std::filesystem::path & exe);
 
+/**
+ * Return temporary path constructed by appending a suffix to a root path.
+ *
+ * The constructed path looks like `<root><suffix>-<pid>-<unique>`. To create a
+ * path nested in a directory, provide a suffix starting with `/`.
+ */
+Path makeTempPath(const Path & root, const Path & suffix = ".tmp");
+
 /**
  * Used in various places.
  */

src/libutil/source-accessor.cc

@@ -1,5 +1,5 @@
+#include <atomic>
 #include "nix/util/source-accessor.hh"
-#include "nix/util/archive.hh"
 
 namespace nix {