From c3fb2aa1f9d1fa756dac38d3588c836c5a5395dc 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. --- src/libstore/globals.cc | 2 +- src/libutil/file-system.cc | 8 ++++++-- src/libutil/file-system.hh | 4 ++++ src/nix-build/nix-build.cc | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index d22ae4ca0..fa0938d7b 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -415,7 +415,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.cc b/src/libutil/file-system.cc index 9dd6a5133..9f81ee452 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -494,10 +494,14 @@ void AutoDelete::reset(const Path & p, bool recursive) { ////////////////////////////////////////////////////////////////////// +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 @@ -537,7 +541,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/libutil/file-system.hh b/src/libutil/file-system.hh index 963265e34..9d565c881 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -234,6 +234,10 @@ Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix", */ std::pair createTempFile(const Path & prefix = "nix"); +/** + * Return `TMPDIR`, or the default temporary directory if unset or empty. + */ +Path defaultTempDir(); /** * Used in various places. diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index a372e4b1c..418ee0ddd 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -476,7 +476,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 b9e7f5aa2df3f0e223f5c44b8089cbf9b81be691 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. --- 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 418ee0ddd..35eef5b83 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -475,8 +475,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 fd31945742710984de22805ee8d97fbd83c3f8eb 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() --- 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 a9b6a8dbf..f65afdf07 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2090,7 +2090,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 dd26f413791b7885558afcc628623648b7fa6396 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 --- 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 f65afdf07..612434e4d 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2093,7 +2093,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 850c9a6cafb74a82b8111dd6aeb4c0d434aba414 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 --- 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 85c5eed4c..5da87e935 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -49,7 +49,7 @@ public: , BinaryCacheStore(params) , cacheUri(scheme + "://" + _cacheUri) { - if (cacheUri.back() == '/') + while (!cacheUri.empty() && cacheUri.back() == '/') cacheUri.pop_back(); diskCache = getNarInfoDiskCache(); From 3b7f2bf99751bb51a9e9c4dab0fe2db1a6ff07ca Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 24 Mar 2024 00:54:44 +0100 Subject: [PATCH 6/6] git/dumpTree: Assert name not empty before back() --- src/libutil/git.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libutil/git.cc b/src/libutil/git.cc index 0b6e35222..a60589baa 100644 --- a/src/libutil/git.cc +++ b/src/libutil/git.cc @@ -251,6 +251,7 @@ void dumpTree(const Tree & entries, Sink & sink, for (auto & [name, entry] : entries) { auto name2 = name; if (entry.mode == Mode::Directory) { + assert(!name2.empty()); assert(name2.back() == '/'); name2.pop_back(); }