From d23bf515a703f35a3680912a1adde8478937f7e1 Mon Sep 17 00:00:00 2001 From: Emily Date: Sun, 3 Aug 2025 19:06:02 +0100 Subject: [PATCH] libstore: use `makeTemp{,Sibling}Path` more Change-Id: I6a6a6964c885be6dea0a69ee3162fbf4b812471f --- lix/libstore/local-binary-cache-store.cc | 5 +- lix/libstore/local-store.cc | 4 +- lix/libutil/file-system.cc | 76 +++++++++++++++--------- lix/libutil/file-system.hh | 5 ++ 4 files changed, 58 insertions(+), 32 deletions(-) diff --git a/lix/libstore/local-binary-cache-store.cc b/lix/libstore/local-binary-cache-store.cc index f61593937..d735d098f 100644 --- a/lix/libstore/local-binary-cache-store.cc +++ b/lix/libstore/local-binary-cache-store.cc @@ -69,11 +69,10 @@ protected: ) override try { auto path2 = binaryCacheDir + "/" + path; - static std::atomic counter{0}; - Path tmp = fmt("%s.tmp.%d.%d", path2, getpid(), ++counter); + Path tmp = makeTempPath(path2); AutoDelete del(tmp, false); StreamToSourceAdapter source(istream); - writeFile(tmp, source); + writeFileExcl(tmp, source); renameFile(tmp, path2); del.cancel(); return {result::success()}; diff --git a/lix/libstore/local-store.cc b/lix/libstore/local-store.cc index d7cfe6874..2f0259961 100644 --- a/lix/libstore/local-store.cc +++ b/lix/libstore/local-store.cc @@ -1749,9 +1749,9 @@ try { createDirs(dirOf(logPath)); - auto tmpFile = fmt("%s.tmp.%d", logPath, getpid()); + auto tmpFile = makeTempSiblingPath(logPath); - writeFile(tmpFile, compress("bzip2", log)); + writeFileExcl(tmpFile, compress("bzip2", log)); renameFile(tmpFile, logPath); co_return result::success(); diff --git a/lix/libutil/file-system.cc b/lix/libutil/file-system.cc index 8686714bd..e53cc3ff5 100644 --- a/lix/libutil/file-system.cc +++ b/lix/libutil/file-system.cc @@ -363,11 +363,25 @@ Generator readFileSource(const Path & path) }(std::move(fd)); } -void writeFile(const Path & path, std::string_view s, mode_t mode, bool allowInterrupts) +static AutoCloseFD openForWrite(const Path & path, mode_t mode) { AutoCloseFD fd{sys::open(path, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode)}; if (!fd) throw SysError("opening file '%1%'", path); + return fd; +} + +static AutoCloseFD openForWriteExcl(const Path & path, mode_t mode) +{ + AutoCloseFD fd{sys::open(path, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL, mode)}; + if (!fd) + throw SysError("opening file '%1%'", path); + return fd; +} + +void writeFile(const Path & path, std::string_view s, mode_t mode, bool allowInterrupts) +{ + AutoCloseFD fd = openForWrite(path, mode); writeFile(fd, s, mode, allowInterrupts); @@ -375,6 +389,16 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, bool allowInt fd.close(); } +void writeFileExcl(const Path & path, std::string_view s, mode_t mode, bool allowInterrupts) +{ + AutoCloseFD fd = openForWriteExcl(path, mode); + + writeFile(fd, s, mode, allowInterrupts); + + // Close explicitly to propagate the exceptions. + fd.close(); +} + void writeFile(AutoCloseFD & fd, std::string_view s, mode_t mode, bool allowInterrupts) { assert(fd); @@ -394,10 +418,7 @@ void writeFileUninterruptible(const Path & path, std::string_view s, mode_t mode void writeFileAndSync(const Path & path, std::string_view s, mode_t mode) { { - AutoCloseFD fd{sys::open(path, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode)}; - if (!fd) { - throw SysError("opening file '%1%'", path); - } + AutoCloseFD fd = openForWrite(path, mode); writeFile(fd, s, mode); fd.fsync(); @@ -408,14 +429,6 @@ void writeFileAndSync(const Path & path, std::string_view s, mode_t mode) syncParent(path); } -static AutoCloseFD openForWrite(const Path & path, mode_t mode) -{ - AutoCloseFD fd{sys::open(path, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode)}; - if (!fd) - throw SysError("opening file '%1%'", path); - return fd; -} - static void closeForWrite(const Path & path, AutoCloseFD & fd, bool sync) { if (sync) @@ -446,6 +459,26 @@ void writeFile(const Path & path, Source & source, mode_t mode) closeForWrite(path, fd, false); } +void writeFileExcl(const Path & path, Source & source, mode_t mode) +{ + AutoCloseFD fd = openForWriteExcl(path, mode); + + std::vector buf(64 * 1024); + + try { + while (true) { + try { + auto n = source.read(buf.data(), buf.size()); + writeFull(fd.get(), {buf.data(), n}); + } catch (EndOfFile &) { break; } + } + } catch (Error & e) { + e.addTrace({}, "writing file '%1%'", path); + throw; + } + closeForWrite(path, fd, false); +} + kj::Promise> writeFile(const Path & path, AsyncInputStream & source, mode_t mode) try { AutoCloseFD fd = openForWrite(path, mode); @@ -724,20 +757,9 @@ void createSymlink(const Path & target, const Path & link) void replaceSymlink(const Path & target, const Path & link) { - for (unsigned int n = 0; true; n++) { - Path tmp = canonPath(fmt("%s/.%d_%s", dirOf(link), n, baseNameOf(link))); - - try { - createSymlink(target, tmp); - } catch (SysError & e) { - if (e.errNo == EEXIST) continue; - throw; - } - - renameFile(tmp, link); - - break; - } + Path tmp = canonPath(makeTempSiblingPath(link)); + createSymlink(target, tmp); + renameFile(tmp, link); } void setWriteTime(const fs::path & p, const struct stat & st) diff --git a/lix/libutil/file-system.hh b/lix/libutil/file-system.hh index 2c224aa4f..faf96b2f7 100644 --- a/lix/libutil/file-system.hh +++ b/lix/libutil/file-system.hh @@ -192,8 +192,13 @@ Generator readFileSource(const Path & path); void writeFile( const Path & path, std::string_view s, mode_t mode = 0666, bool allowInterrupts = true ); +/** Write a string to an exclusively-opened file. */ +void writeFileExcl( + const Path & path, std::string_view s, mode_t mode = 0666, bool allowInterrupts = true +); void writeFileUninterruptible(const Path & path, std::string_view s, mode_t mode = 0666); void writeFile(const Path & path, Source & source, mode_t mode = 0666); +void writeFileExcl(const Path & path, Source & source, mode_t mode = 0666); void writeFile( AutoCloseFD & fd, std::string_view s, mode_t mode = 0666, bool allowInterrupts = true