From 479c356510dcdcb43f16c7483a48ff3308c6dd2b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 12 Jun 2025 11:35:28 +0200 Subject: [PATCH] Drop guessOrInventPathFromFD() No need to do hacky stuff like that when we already know the original path. --- src/libstore/unix/build/derivation-builder.cc | 14 +++---- src/libutil/file-descriptor.cc | 26 ------------- src/libutil/file-system.cc | 39 +++++++++---------- .../include/nix/util/file-descriptor.hh | 17 -------- src/libutil/include/nix/util/file-system.hh | 4 +- src/libutil/meson.build | 2 - 6 files changed, 28 insertions(+), 74 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index a125676e5..36405a8a2 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -314,7 +314,7 @@ protected: /** * Make a file owned by the builder addressed by its file descriptor. */ - void chownToBuilder(const AutoCloseFD & fd); + void chownToBuilder(int fd, const Path & path); /** * Run the builder's process. @@ -730,7 +730,7 @@ void DerivationBuilderImpl::startBuilder() if (!tmpDirFd) throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); - chownToBuilder(tmpDirFd); + chownToBuilder(tmpDirFd.get(), tmpDir); for (auto & [outputName, status] : initialOutputs) { /* Set scratch path we'll actually use during the build. @@ -1073,8 +1073,8 @@ void DerivationBuilderImpl::initEnv() AutoCloseFD passAsFileFd{openat(tmpDirFd.get(), fn.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; if (!passAsFileFd) throw SysError("opening `passAsFile` file in the sandbox '%1%'", p); - writeFile(passAsFileFd, rewriteStrings(i.second, inputRewrites)); - chownToBuilder(passAsFileFd); + writeFile(passAsFileFd, p, rewriteStrings(i.second, inputRewrites)); + chownToBuilder(passAsFileFd.get(), p); env[i.first + "Path"] = tmpDirInSandbox() + "/" + fn; } } @@ -1278,11 +1278,11 @@ void DerivationBuilderImpl::chownToBuilder(const Path & path) throw SysError("cannot change ownership of '%1%'", path); } -void DerivationBuilderImpl::chownToBuilder(const AutoCloseFD & fd) +void DerivationBuilderImpl::chownToBuilder(int fd, const Path & path) { if (!buildUser) return; - if (fchown(fd.get(), buildUser->getUID(), buildUser->getGID()) == -1) - throw SysError("cannot change ownership of file '%1%'", fd.guessOrInventPath()); + if (fchown(fd, buildUser->getUID(), buildUser->getGID()) == -1) + throw SysError("cannot change ownership of file '%1%'", path); } void DerivationBuilderImpl::runChild() diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index 9193a30bb..9e0827442 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -1,8 +1,5 @@ #include "nix/util/serialise.hh" #include "nix/util/util.hh" -#include "nix/util/file-system.hh" - -#include "util-config-private.hh" #include #include @@ -77,29 +74,6 @@ Descriptor AutoCloseFD::get() const return fd; } -std::string guessOrInventPathFromFD(Descriptor fd) - { - assert(fd >= 0); - /* On Linux, there's no F_GETPATH available. - * But we can read /proc/ */ - #if defined(__linux__) - try { - return readLink(fmt("/proc/self/fd/%1%", fd)); - } catch (...) { - } - #elif defined (HAVE_F_GETPATH) && HAVE_F_GETPATH - std::string fdName(PATH_MAX, '\0'); - if (fcntl(fd, F_GETPATH, fdName.data()) != -1) { - fdName.resize(strlen(fdName.c_str())); - return fdName; - } - #else - #error "No implementation for retrieving file descriptors path." - #endif - - return fmt("", fd); - } - void AutoCloseFD::close() { diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 12ada6d5c..3fd5ae669 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -315,19 +315,19 @@ void writeFile(const Path & path, std::string_view s, mode_t mode) if (!fd) throw SysError("opening file '%1%'", path); - writeFile(fd, s, mode); + writeFile(fd, path, s, mode); /* Close explicitly to propagate the exceptions. */ fd.close(); } -void writeFile(AutoCloseFD & fd, std::string_view s, mode_t mode) +void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode) { assert(fd); try { writeFull(fd.get(), s); } catch (Error & e) { - e.addTrace({}, "writing file '%1%'", fd.guessOrInventPath()); + e.addTrace({}, "writing file '%1%'", origPath); throw; } } @@ -339,7 +339,7 @@ void writeFileAndSync(const Path & path, std::string_view s, mode_t mode) if (!fd) throw SysError("opening file '%1%'", path); - writeFile(fd, s, mode); + writeFile(fd, path, s, mode); fd.fsync(); /* Close explicitly to ensure that exceptions are propagated. */ fd.close(); @@ -444,8 +444,6 @@ void recursiveSync(const Path & path) static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, uint64_t & bytesFreed, std::exception_ptr & ex MOUNTEDPATHS_PARAM) { #ifndef _WIN32 - /* This ensures that `name` is an immediate child of `parentfd`. */ - assert(!path.empty() && path.string().find('/') == std::string::npos && "`name` is an immediate child to `parentfd`"); checkInterrupt(); #ifdef __FreeBSD__ @@ -456,13 +454,14 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, } #endif - std::string name(path.native()); + std::string name(path.filename()); + assert(name != "." && name != ".." && !name.empty()); struct stat st; if (fstatat(parentfd, name.c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1) { if (errno == ENOENT) return; - throw SysError("getting status of '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + throw SysError("getting status of %1%", path); } if (!S_ISDIR(st.st_mode)) { @@ -493,24 +492,23 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, /* Make the directory accessible. */ const auto PERM_MASK = S_IRUSR | S_IWUSR | S_IXUSR; if ((st.st_mode & PERM_MASK) != PERM_MASK) { - if (fchmodat(parentfd, name.c_str(), st.st_mode | PERM_MASK, 0) == -1) { - throw SysError("chmod '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); - } + if (fchmodat(parentfd, name.c_str(), st.st_mode | PERM_MASK, 0) == -1) + throw SysError("chmod %1%", path); } int fd = openat(parentfd, name.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW); if (fd == -1) - throw SysError("opening directory '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + throw SysError("opening directory %1%", path); AutoCloseDir dir(fdopendir(fd)); if (!dir) - throw SysError("opening directory '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + throw SysError("opening directory %1%", path); struct dirent * dirent; while (errno = 0, dirent = readdir(dir.get())) { /* sic */ checkInterrupt(); std::string childName = dirent->d_name; if (childName == "." || childName == "..") continue; - _deletePath(dirfd(dir.get()), childName, bytesFreed, ex MOUNTEDPATHS_ARG); + _deletePath(dirfd(dir.get()), path / childName, bytesFreed, ex MOUNTEDPATHS_ARG); } if (errno) throw SysError("reading directory %1%", path); } @@ -519,7 +517,7 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, if (unlinkat(parentfd, name.c_str(), flags) == -1) { if (errno == ENOENT) return; try { - throw SysError("cannot unlink '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + throw SysError("cannot unlink %1%", path); } catch (...) { if (!ex) ex = std::current_exception(); @@ -535,19 +533,18 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, static void _deletePath(const std::filesystem::path & path, uint64_t & bytesFreed MOUNTEDPATHS_PARAM) { - Path dir = dirOf(path.string()); - if (dir == "") - dir = "/"; + assert(path.is_absolute()); + assert(path.parent_path() != path); - AutoCloseFD dirfd = toDescriptor(open(dir.c_str(), O_RDONLY)); + AutoCloseFD dirfd = toDescriptor(open(path.parent_path().string().c_str(), O_RDONLY)); if (!dirfd) { if (errno == ENOENT) return; - throw SysError("opening directory '%1%'", path); + throw SysError("opening directory %s", path.parent_path()); } std::exception_ptr ex; - _deletePath(dirfd.get(), path.filename(), bytesFreed, ex MOUNTEDPATHS_ARG); + _deletePath(dirfd.get(), path, bytesFreed, ex MOUNTEDPATHS_ARG); if (ex) std::rethrow_exception(ex); diff --git a/src/libutil/include/nix/util/file-descriptor.hh b/src/libutil/include/nix/util/file-descriptor.hh index 35b359fb5..e2bcce2a2 100644 --- a/src/libutil/include/nix/util/file-descriptor.hh +++ b/src/libutil/include/nix/util/file-descriptor.hh @@ -106,14 +106,6 @@ void drainFD( #endif ); - /* - * Will attempt to guess *A* path associated that might lead to the same file as used by this - * file descriptor. - * - * The returned string should NEVER be used as a valid path. - */ - std::string guessOrInventPathFromFD(Descriptor fd); - /** * Get [Standard Input](https://en.wikipedia.org/wiki/Standard_streams#Standard_input_(stdin)) */ @@ -168,15 +160,6 @@ public: AutoCloseFD& operator =(const AutoCloseFD & fd) = delete; AutoCloseFD& operator =(AutoCloseFD&& fd); Descriptor get() const; - - /** - * Will attempt to guess *A* path associated that might lead to the same file as used by this - * file descriptor. - * - * The returned string should NEVER be used as a valid path. - */ - std::string guessOrInventPath() const { return guessOrInventPathFromFD(fd); } - explicit operator bool() const; Descriptor release(); void close(); diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh index 77d5f2aa1..5f062412d 100644 --- a/src/libutil/include/nix/util/file-system.hh +++ b/src/libutil/include/nix/util/file-system.hh @@ -173,18 +173,20 @@ void readFile(const Path & path, Sink & sink, bool memory_map = true); * Write a string to a file. */ void writeFile(const Path & path, std::string_view s, mode_t mode = 0666); + static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666) { return writeFile(path.string(), s, mode); } void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); + static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, bool sync = false) { return writeFile(path.string(), source, mode, sync); } -void writeFile(AutoCloseFD & fd, std::string_view s, mode_t mode = 0666 ); +void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666); /** * Write a string to a file and flush the file and its parents direcotry to disk. diff --git a/src/libutil/meson.build b/src/libutil/meson.build index 2203e2294..f5ad2b1f6 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -37,8 +37,6 @@ foreach funcspec : check_funcs configdata.set(define_name, define_value, description: funcspec[1]) endforeach -configdata.set('HAVE_F_GETPATH', cxx.has_header_symbol('fcntl.h', 'F_GETPATH').to_int()) - subdir('nix-meson-build-support/libatomic') if host_machine.system() == 'windows'