From bde4a50740e145b6ffb90b5e1218633befec0de4 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Thu, 21 Aug 2025 14:36:10 +0000 Subject: [PATCH] Revert "libutil: use `makeTempPath` in `createTempSubdir`" Revert submission 3850 Reason for revert: caused multiple regressions noticed in https://git.lix.systems/lix-project/lix/issues/975 and https://git.lix.systems/lix-project/lix/issues/966 (suspected). Root cause analysis has not been done yet and this breaks Lix on Darwin on HEAD. Reverted changes: /q/submissionid:3850 Change-Id: I22d2c315bd8b1eed536c08f2d5b368331907f994 --- lix/libstore/build/local-derivation-goal.cc | 6 +-- lix/libstore/ssh.cc | 2 +- lix/libstore/temporary-dir.cc | 5 +- lix/libstore/temporary-dir.hh | 3 +- lix/libutil/file-system.cc | 52 ++++++++++++++------- lix/libutil/file-system.hh | 2 +- 6 files changed, 44 insertions(+), 26 deletions(-) diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index c4947504c..a0adb1541 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -447,7 +447,7 @@ try { /* Create a temporary directory where the build will take place. */ tmpDirRoot = - createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), 0700); + createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), false, false, 0700); } catch (SysError & e) { /* * Fallback to the global tmpdir and create a safe space there @@ -466,7 +466,7 @@ try { constexpr int toplevelDirMode = 0700; #endif auto nixBuildsTmp = createTempDir( - "", fmt("nix-builds-%s", geteuid()), toplevelDirMode + "", fmt("nix-builds-%s", geteuid()), false, false, toplevelDirMode ); warn( "Failed to use the system-wide build directory '%s', falling back to a temporary " @@ -475,7 +475,7 @@ try { nixBuildsTmp ); tmpDirRoot = createTempDir( - nixBuildsTmp, "nix-build-" + std::string(drvPath.name()), 0700 + nixBuildsTmp, "nix-build-" + std::string(drvPath.name()), false, false, 0700 ); worker.buildDirOverride = nixBuildsTmp; } diff --git a/lix/libstore/ssh.cc b/lix/libstore/ssh.cc index a7b48a330..ab3ec5a20 100644 --- a/lix/libstore/ssh.cc +++ b/lix/libstore/ssh.cc @@ -25,7 +25,7 @@ SSH::SSH(const std::string & host, const std::optional port, const std throw Error("invalid SSH host name '%s'", host); auto state(state_.lock()); - state->tmpDir = std::make_unique(createTempDir("", "nix", 0700)); + state->tmpDir = std::make_unique(createTempDir("", "nix", true, true, 0700)); } void SSH::addCommonSSHOpts(Strings & args) diff --git a/lix/libstore/temporary-dir.cc b/lix/libstore/temporary-dir.cc index 2e601e34c..685d5aee7 100644 --- a/lix/libstore/temporary-dir.cc +++ b/lix/libstore/temporary-dir.cc @@ -5,9 +5,10 @@ namespace nix { -Path createTempDir(const Path & tmpRoot, const Path & prefix, mode_t mode) +Path createTempDir(const Path & tmpRoot, const Path & prefix, + bool includePid, bool useGlobalCounter, mode_t mode) { - return createTempSubdir(tmpRoot.empty() ? defaultTempDir() : tmpRoot, prefix, mode); + return createTempSubdir(tmpRoot.empty() ? defaultTempDir() : tmpRoot, prefix, includePid, useGlobalCounter, mode); } std::pair createTempFile(const Path & prefix) diff --git a/lix/libstore/temporary-dir.hh b/lix/libstore/temporary-dir.hh index 9bc4df9c4..f109868c9 100644 --- a/lix/libstore/temporary-dir.hh +++ b/lix/libstore/temporary-dir.hh @@ -8,7 +8,8 @@ namespace nix { /** * Create a temporary directory. */ -Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix", mode_t mode = 0755); +Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix", + bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755); /** * Create a temporary file, returning a file handle and its path. diff --git a/lix/libutil/file-system.cc b/lix/libutil/file-system.cc index 26ac4fa94..3b1d3f676 100644 --- a/lix/libutil/file-system.cc +++ b/lix/libutil/file-system.cc @@ -646,28 +646,44 @@ void AutoDelete::reset(const Path & p, bool recursive) { ////////////////////////////////////////////////////////////////////// -Path createTempSubdir(const Path & parent, const Path & prefix, - mode_t mode) +static Path tempName(PathView parent, const Path & prefix, bool includePid, + std::atomic & counter) { - checkInterrupt(); - Path tmpDir = makeTempPath(parent + "/", prefix); - if (mkdir(tmpDir.c_str(), mode) == 0) { + auto tmpRoot = canonPath(parent, true); + if (includePid) + return fmt("%1%/%2%-%3%-%4%", tmpRoot, prefix, getpid(), counter++); + else + return fmt("%1%/%2%-%3%", tmpRoot, prefix, counter++); +} + +Path createTempSubdir(const Path & parent, 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(parent, prefix, includePid, counter); + if (mkdir(tmpDir.c_str(), mode) == 0) { #if __FreeBSD__ - /* Explicitly set the group of the directory. This is to - work around around problems caused by BSD's group - ownership semantics (directories inherit the group of - the parent). For instance, the group of /tmp on - FreeBSD is "wheel", so all directories created in /tmp - will be owned by "wheel"; but if the user is not in - "wheel", then "tar" will fail to unpack archives that - have the setgid bit set on directories. */ - if (chown(tmpDir.c_str(), (uid_t) -1, getegid()) != 0) { - throw SysError("setting group of directory '%1%'", tmpDir); - } + /* Explicitly set the group of the directory. This is to + work around around problems caused by BSD's group + ownership semantics (directories inherit the group of + the parent). For instance, the group of /tmp on + FreeBSD is "wheel", so all directories created in /tmp + will be owned by "wheel"; but if the user is not in + "wheel", then "tar" will fail to unpack archives that + have the setgid bit set on directories. */ + if (chown(tmpDir.c_str(), (uid_t) -1, getegid()) != 0) + throw SysError("setting group of directory '%1%'", tmpDir); #endif - return tmpDir; + return tmpDir; + } + if (errno != EEXIST) + throw SysError("creating directory '%1%'", tmpDir); } - throw SysError("creating directory '%1%'", tmpDir); } Path makeTempPath(const Path & root, const Path & prefix) diff --git a/lix/libutil/file-system.hh b/lix/libutil/file-system.hh index 85af08fdb..fb8b93a80 100644 --- a/lix/libutil/file-system.hh +++ b/lix/libutil/file-system.hh @@ -308,7 +308,7 @@ typedef std::unique_ptr AutoCloseDir; * Create a temporary directory in a given parent directory. */ Path createTempSubdir(const Path & parent, const Path & prefix = "nix", - mode_t mode = 0755); + bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755); /** * Return temporary path constructed by appending to a root path.