From b8caabe25f3565d74ef75e2ed8163ac1d5377cd8 Mon Sep 17 00:00:00 2001 From: Amaan Qureshi Date: Wed, 25 Feb 2026 15:11:50 -0500 Subject: [PATCH] libstore: replace unlink() with std::filesystem::remove() --- src/libstore/builtins/buildenv.cc | 14 ++++++++++---- src/libstore/gc.cc | 22 ++++++++++++++++------ src/libstore/local-store.cc | 4 +++- src/libstore/unix/pathlocks.cc | 8 +++++--- src/libutil/file-system.cc | 3 ++- src/nix/nix-channel/nix-channel.cc | 7 +++++-- 6 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index 514a3f3a3..9ca3f3125 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -79,8 +79,11 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, auto target = canonPath(dstFile, true).string(); if (!S_ISDIR(lstat(target).st_mode)) throw Error("collision between %1% and non-directory %2%", PathFmt(srcFile), PathFmt(target)); - if (unlink(dstFile.c_str()) == -1) - throw SysError("unlinking %1%", PathFmt(dstFile)); + try { + std::filesystem::remove(dstFile); + } catch (std::filesystem::filesystem_error & e) { + throw SystemError(e.code(), "unlinking %s", PathFmt(dstFile)); + } if (mkdir( dstFile.c_str() #ifndef _WIN32 // TODO abstract mkdir perms for Windows @@ -107,8 +110,11 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, throw BuildEnvFileConflictError(readLink(dstFile).string(), srcFile, priority); if (prevPriority < priority) continue; - if (unlink(dstFile.c_str()) == -1) - throw SysError("unlinking %1%", PathFmt(dstFile)); + try { + std::filesystem::remove(dstFile); + } catch (std::filesystem::filesystem_error & e) { + throw SystemError(e.code(), "unlinking %s", PathFmt(dstFile)); + } } else if (S_ISDIR(dstSt.st_mode)) throw Error("collision between non-directory '%1%' and directory '%2%'", srcFile, dstFile); } diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index d644b3c60..e41a8fc3a 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -54,10 +54,13 @@ void LocalStore::createTempRootsFile() return; while (1) { - if (pathExists(fnTempRoots)) + if (pathExists(fnTempRoots)) { /* It *must* be stale, since there can be no two processes with the same pid. */ - unlink(fnTempRoots.string().c_str()); + /* The error code of std::filesystem::remove() is intentionally ignored. */ + std::error_code ec; + std::filesystem::remove(fnTempRoots, ec); + } *fdTempRoots = openLockFile(fnTempRoots, true); @@ -190,7 +193,9 @@ void LocalStore::findTempRoots(Roots & tempRoots, bool censor) we don't care about its temporary roots. */ if (lockFile(fd.get(), ltWrite, false)) { printInfo("removing stale temporary roots file '%1%'", path); - unlink(path.c_str()); + /* The error code of std::filesystem::remove() is intentionally ignored. */ + std::error_code ec; + std::filesystem::remove(path, ec); writeFull(fd.get(), "d"); continue; } @@ -247,7 +252,9 @@ void LocalStore::findRoots(const Path & path, std::filesystem::file_type type, R if (!pathExists(target)) { if (isInDir(path, std::filesystem::path{config->stateDir.get()} / gcRootsDir / "auto")) { printInfo("removing stale link from '%1%' to '%2%'", path, target); - unlink(path.c_str()); + /* The error code of std::filesystem::remove() is intentionally ignored. */ + std::error_code ec; + std::filesystem::remove(path, ec); } } else { if (!std::filesystem::is_symlink(target)) @@ -782,8 +789,11 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) printMsg(lvlTalkative, "deleting unused link '%1%'", path); - if (unlink(path.c_str()) == -1) - throw SysError("deleting '%1%'", path); + try { + std::filesystem::remove(path); + } catch (std::filesystem::filesystem_error & e) { + throw SystemError(e.code(), "deleting %s", PathFmt(path)); + } /* Do not account for deleted file here. Rely on deletePath() accounting. */ diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ada70f3b6..323cb01a4 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -450,7 +450,9 @@ LocalStore::~LocalStore() auto fdTempRoots(_fdTempRoots.lock()); if (*fdTempRoots) { fdTempRoots->close(); - unlink(fnTempRoots.string().c_str()); + /* The error code of std::filesystem::remove() is intentionally ignored. */ + std::error_code ec; + std::filesystem::remove(fnTempRoots, ec); } } catch (...) { ignoreExceptionInDestructor(); diff --git a/src/libstore/unix/pathlocks.cc b/src/libstore/unix/pathlocks.cc index 26410827c..71e61b8cd 100644 --- a/src/libstore/unix/pathlocks.cc +++ b/src/libstore/unix/pathlocks.cc @@ -31,10 +31,12 @@ void deleteLockFile(const std::filesystem::path & path, Descriptor desc) races. Write a (meaningless) token to the file to indicate to other processes waiting on this lock that the lock is stale (deleted). */ - unlink(path.c_str()); + std::error_code ec; + std::filesystem::remove(path, ec); writeFull(desc, "d"); - /* Note that the result of unlink() is ignored; removing the lock - file is an optimisation, not a necessity. */ + /* Note that the error code of std::filesystem::remove() is + intentionally ignored; removing the lock file is an optimisation, + not a necessity. */ } bool lockFile(Descriptor desc, LockType lockType, bool wait) diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 23cfb7aba..0b7b164fc 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -587,7 +587,8 @@ AutoCloseFD createAnonymousTempFile() if (!fd2) throw SysError("creating temporary file %s", PathFmt(path)); fd = std::move(fd2); - unlink(requireCString(path)); /* We only care about the file descriptor. */ + std::error_code ec; + std::filesystem::remove(path, ec); /* We only care about the file descriptor. */ #endif return fd; diff --git a/src/nix/nix-channel/nix-channel.cc b/src/nix/nix-channel/nix-channel.cc index 953ff7a7f..209f84d0a 100644 --- a/src/nix/nix-channel/nix-channel.cc +++ b/src/nix/nix-channel/nix-channel.cc @@ -192,8 +192,11 @@ static void update(const StringSet & channelNames) if (lstat(nixDefExpr.c_str(), &st) == 0) { if (S_ISLNK(st.st_mode)) // old-skool ~/.nix-defexpr - if (unlink(nixDefExpr.c_str()) == -1) - throw SysError("unlinking %1%", PathFmt(nixDefExpr)); + try { + std::filesystem::remove(nixDefExpr); + } catch (std::filesystem::filesystem_error & e) { + throw SystemError(e.code(), "unlinking %s", PathFmt(nixDefExpr)); + } } else if (errno != ENOENT) { throw SysError("getting status of %1%", PathFmt(nixDefExpr)); }