diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 50d4991be..2da22a773 100644 --- a/src/libmain/shared.cc +++ b/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); - - } diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 63dca3083..418a65f75 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/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 diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index d29734a3e..c8fec5244 100644 --- a/src/libstore/ssh.cc +++ b/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(createTempDir("", "nix", true, true, 0700)); + state->tmpDir = std::make_unique(createTempDir("", "nix", 0700)); } void SSHMaster::addCommonSSHOpts(Strings & args) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index f3847e604..81e0a2584 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/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 diff --git a/src/libstore/unix/user-lock.cc b/src/libstore/unix/user-lock.cc index 2bee277f9..ef806af79 100644 --- a/src/libstore/unix/user-lock.cc +++ b/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 { diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 90ec5eda5..ad8cef38c 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -8,12 +8,12 @@ #include "nix/util/util.hh" #include +#include #include #include #include #include #include -#include #include #include @@ -27,10 +27,6 @@ # include #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 & 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 globalCounter = 0; - std::atomic 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 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 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 { diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh index b8fa4cfa0..b4cc1567d 100644 --- a/src/libutil/include/nix/util/file-system.hh +++ b/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 #endif -#include -#include #include -#include -#include #include /** @@ -316,8 +310,8 @@ typedef std::unique_ptr 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 `--`. 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. */ diff --git a/src/libutil/source-accessor.cc b/src/libutil/source-accessor.cc index b9ebc82b6..fc9752456 100644 --- a/src/libutil/source-accessor.cc +++ b/src/libutil/source-accessor.cc @@ -1,5 +1,5 @@ +#include #include "nix/util/source-accessor.hh" -#include "nix/util/archive.hh" namespace nix {