From 1a336bf865402a779720de2770a3a46a1e3b5e87 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 22 Mar 2024 22:41:50 +0100 Subject: [PATCH 1/6] fix: Treat empty TMPDIR as unset Fixes an instance of nix: src/libutil/util.cc:139: nix::Path nix::canonPath(PathView, bool): Assertion `path != ""' failed. ... which I've been getting in one of my shells for some reason. I have yet to find out why TMPDIR was empty, but it's no reason for Nix to break. (cherry picked from commit c3fb2aa1f9d1fa756dac38d3588c836c5a5395dc) --- src/libstore/globals.cc | 2 +- src/libutil/file-system.hh | 17 +++++++++++++++++ src/libutil/filesystem.cc | 8 ++++++-- src/nix-build/nix-build.cc | 2 +- 4 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 src/libutil/file-system.hh diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 9c25d986880..cdf9813db22 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -408,7 +408,7 @@ void initLibStore() { sshd). This breaks build users because they don't have access to the TMPDIR, in particular in ‘nix-store --serve’. */ #if __APPLE__ - if (hasPrefix(getEnv("TMPDIR").value_or("/tmp"), "/var/folders/")) + if (hasPrefix(defaultTempDir(), "/var/folders/")) unsetenv("TMPDIR"); #endif diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh new file mode 100644 index 00000000000..1d404028b7c --- /dev/null +++ b/src/libutil/file-system.hh @@ -0,0 +1,17 @@ +#pragma once +/** + * @file + * + * Utiltities for working with the file sytem and file paths. + */ + +#include "types.hh" + +namespace nix { + +/** + * Return `TMPDIR`, or the default temporary directory if unset or empty. + */ +Path defaultTempDir(); + +} diff --git a/src/libutil/filesystem.cc b/src/libutil/filesystem.cc index 2a7787c0e01..ebe71fe7c00 100644 --- a/src/libutil/filesystem.cc +++ b/src/libutil/filesystem.cc @@ -10,10 +10,14 @@ namespace fs = std::filesystem; namespace nix { +std::string defaultTempDir() { + return getEnvNonEmpty("TMPDIR").value_or("/tmp"); +} + static Path tempName(Path tmpRoot, const Path & prefix, bool includePid, std::atomic & counter) { - tmpRoot = canonPath(tmpRoot.empty() ? getEnv("TMPDIR").value_or("/tmp") : tmpRoot, true); + tmpRoot = canonPath(tmpRoot.empty() ? defaultTempDir() : tmpRoot, true); if (includePid) return fmt("%1%/%2%-%3%-%4%", tmpRoot, prefix, getpid(), counter++); else @@ -53,7 +57,7 @@ Path createTempDir(const Path & tmpRoot, const Path & prefix, std::pair createTempFile(const Path & prefix) { - Path tmpl(getEnv("TMPDIR").value_or("/tmp") + "/" + prefix + ".XXXXXX"); + Path tmpl(defaultTempDir() + "/" + prefix + ".XXXXXX"); // Strictly speaking, this is UB, but who cares... // FIXME: use O_TMPFILE. AutoCloseFD fd(mkstemp((char *) tmpl.c_str())); diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index e2189fc669c..40400e3a934 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -458,7 +458,7 @@ static void main_nix_build(int argc, char * * argv) auto env = getEnv(); auto tmp = getEnv("TMPDIR"); - if (!tmp) tmp = getEnv("XDG_RUNTIME_DIR").value_or("/tmp"); + if (!tmp || tmp->empty()) tmp = getEnv("XDG_RUNTIME_DIR").value_or("/tmp"); if (pure) { decltype(env) newEnv; From c1f69dfc5ed9a9e3ac0ad3299537e36a10bffc49 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 24 Mar 2024 00:34:21 +0100 Subject: [PATCH 2/6] fix: Treat empty XDG_RUNTIME_DIR as unset See preceding commit. Not observed in the wild, but is sensible and consistent with TMPDIR behavior. (cherry picked from commit b9e7f5aa2df3f0e223f5c44b8089cbf9b81be691) --- src/nix-build/nix-build.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 40400e3a934..6c378b244cf 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -457,8 +457,9 @@ static void main_nix_build(int argc, char * * argv) // Set the environment. auto env = getEnv(); - auto tmp = getEnv("TMPDIR"); - if (!tmp || tmp->empty()) tmp = getEnv("XDG_RUNTIME_DIR").value_or("/tmp"); + auto tmp = getEnvNonEmpty("TMPDIR"); + if (!tmp) + tmp = getEnvNonEmpty("XDG_RUNTIME_DIR").value_or("/tmp"); if (pure) { decltype(env) newEnv; From 28fc0e4f58d040675949a8b7ad8cbe07deedd9c0 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 24 Mar 2024 00:45:15 +0100 Subject: [PATCH 3/6] local-derivation-goal.cc: Reuse defaultTempDir() (cherry picked from commit fd31945742710984de22805ee8d97fbd83c3f8eb) --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index f7f3204a44f..6e583c33313 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2116,7 +2116,7 @@ void LocalDerivationGoal::runChild() /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms to find temporary directories, so we want to open up a broader place for them to dump their files, if needed. */ - Path globalTmpDir = canonPath(getEnvNonEmpty("TMPDIR").value_or("/tmp"), true); + Path globalTmpDir = canonPath(defaultTempDir(), true); /* They don't like trailing slashes on subpath directives */ if (globalTmpDir.back() == '/') globalTmpDir.pop_back(); From d1848506f8b07797a8bfbb6a0c2b36b1a34f6458 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 24 Mar 2024 00:45:57 +0100 Subject: [PATCH 4/6] local-derivation-goal.cc: Remove *all* trailing slashes (cherry picked from commit dd26f413791b7885558afcc628623648b7fa6396) --- src/libstore/build/local-derivation-goal.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 6e583c33313..cc401f57fd0 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2119,7 +2119,8 @@ void LocalDerivationGoal::runChild() Path globalTmpDir = canonPath(defaultTempDir(), true); /* They don't like trailing slashes on subpath directives */ - if (globalTmpDir.back() == '/') globalTmpDir.pop_back(); + while (!globalTmpDir.empty() && globalTmpDir.back() == '/') + globalTmpDir.pop_back(); if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") { builder = "/usr/bin/sandbox-exec"; From 45d900f5c279b4382e197ea411e4c12f1ce91a8c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 24 Mar 2024 00:46:15 +0100 Subject: [PATCH 5/6] HttpBinaryCacheStore: Remove *all* trailing slashes (cherry picked from commit 850c9a6cafb74a82b8111dd6aeb4c0d434aba414) --- src/libstore/http-binary-cache-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 85c5eed4c01..5da87e935db 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -49,7 +49,7 @@ class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public v , BinaryCacheStore(params) , cacheUri(scheme + "://" + _cacheUri) { - if (cacheUri.back() == '/') + while (!cacheUri.empty() && cacheUri.back() == '/') cacheUri.pop_back(); diskCache = getNarInfoDiskCache(); From 20a46e17a82750e5d86e0c5042f5f81a3d55929f Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 10 Jun 2024 16:22:11 +0200 Subject: [PATCH 6/6] Fix includes for cherry-picks --- src/libstore/build/local-derivation-goal.cc | 1 + src/libstore/globals.cc | 1 + src/libutil/filesystem.cc | 1 + 3 files changed, 3 insertions(+) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index cc401f57fd0..e45f5af81bd 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -16,6 +16,7 @@ #include "cgroup.hh" #include "personality.hh" #include "namespaces.hh" +#include "file-system.hh" #include #include diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index cdf9813db22..f6a3fefca91 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -4,6 +4,7 @@ #include "args.hh" #include "abstract-setting-to-json.hh" #include "compute-levels.hh" +#include "file-system.hh" #include #include diff --git a/src/libutil/filesystem.cc b/src/libutil/filesystem.cc index ebe71fe7c00..17c3cb8bfba 100644 --- a/src/libutil/filesystem.cc +++ b/src/libutil/filesystem.cc @@ -5,6 +5,7 @@ #include "finally.hh" #include "util.hh" #include "types.hh" +#include "file-system.hh" namespace fs = std::filesystem;