From 8be347afcac8006967921e4738215d98405f6d75 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 29 Mar 2024 15:21:10 -0400 Subject: [PATCH 1/2] Factor out `nix::maybeLstat` This function is nice for more than `PosixSourceAccessor`. We can make a few things simpler with it. Note that the error logic slightly changes in some of the call sites, in that we also count `ENOTDIR` and not just `ENOENT` as not having the file, but that should be fine. --- src/libstore/build/local-derivation-goal.cc | 24 ++++++++++----------- src/libstore/builtins/buildenv.cc | 18 +++++++--------- src/libutil/file-system.cc | 22 +++++++++++++------ src/libutil/file-system.hh | 1 + src/libutil/posix-source-accessor.cc | 8 +------ 5 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index d52ebf06421..914fffd160e 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2053,13 +2053,13 @@ void LocalDerivationGoal::runChild() i.first, i.second.source); std::string path = i.first; - struct stat st; - if (lstat(path.c_str(), &st)) { - if (i.second.optional && errno == ENOENT) + auto optSt = maybeLstat(path.c_str()); + if (!optSt) { + if (i.second.optional) continue; - throw SysError("getting attributes of path '%s", path); + throw SysError("getting attributes of required path '%s", path); } - if (S_ISDIR(st.st_mode)) + if (S_ISDIR(optSt->st_mode)) sandboxProfile += fmt("\t(subpath \"%s\")\n", path); else sandboxProfile += fmt("\t(literal \"%s\")\n", path); @@ -2271,14 +2271,12 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() continue; } - struct stat st; - if (lstat(actualPath.c_str(), &st) == -1) { - if (errno == ENOENT) - throw BuildError( - "builder for '%s' failed to produce output path for output '%s' at '%s'", - worker.store.printStorePath(drvPath), outputName, actualPath); - throw SysError("getting attributes of path '%s'", actualPath); - } + auto optSt = maybeLstat(actualPath.c_str()); + if (!optSt) + throw BuildError( + "builder for '%s' failed to produce output path for output '%s' at '%s'", + worker.store.printStorePath(drvPath), outputName, actualPath); + struct stat & st = *optSt; #ifndef __CYGWIN__ /* Check that the output is not group or world writable, as diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index 1ed7b39cc58..31a6b32f19c 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -64,9 +64,9 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, continue; else if (S_ISDIR(srcSt.st_mode)) { - struct stat dstSt; - auto res = lstat(dstFile.c_str(), &dstSt); - if (res == 0) { + auto dstStOpt = maybeLstat(dstFile.c_str()); + if (dstStOpt) { + auto & dstSt = *dstStOpt; if (S_ISDIR(dstSt.st_mode)) { createLinks(state, srcFile, dstFile, priority); continue; @@ -82,14 +82,13 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, createLinks(state, srcFile, dstFile, priority); continue; } - } else if (errno != ENOENT) - throw SysError("getting status of '%1%'", dstFile); + } } else { - struct stat dstSt; - auto res = lstat(dstFile.c_str(), &dstSt); - if (res == 0) { + auto dstStOpt = maybeLstat(dstFile.c_str()); + if (dstStOpt) { + auto & dstSt = *dstStOpt; if (S_ISLNK(dstSt.st_mode)) { auto prevPriority = state.priorities[dstFile]; if (prevPriority == priority) @@ -104,8 +103,7 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, throw SysError("unlinking '%1%'", dstFile); } else if (S_ISDIR(dstSt.st_mode)) throw Error("collision between non-directory '%1%' and directory '%2%'", srcFile, dstFile); - } else if (errno != ENOENT) - throw SysError("getting status of '%1%'", dstFile); + } } createSymlink(srcFile, dstFile); diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index c0e268e9d2e..89d3097317b 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -174,15 +174,23 @@ struct stat lstat(const Path & path) } +std::optional maybeLstat(const Path & path) +{ + std::optional st{std::in_place}; + if (lstat(path.c_str(), &*st)) + { + if (errno == ENOENT || errno == ENOTDIR) + st.reset(); + else + throw SysError("getting status of '%s'", path); + } + return st; +} + + bool pathExists(const Path & path) { - int res; - struct stat st; - res = lstat(path.c_str(), &st); - if (!res) return true; - if (errno != ENOENT && errno != ENOTDIR) - throw SysError("getting status of %1%", path); - return false; + return maybeLstat(path).has_value(); } bool pathAccessible(const Path & path) diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 9d565c881c7..dd071e1af79 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -84,6 +84,7 @@ bool isDirOrInDir(std::string_view path, std::string_view dir); */ struct stat stat(const Path & path); struct stat lstat(const Path & path); +std::optional maybeLstat(const Path & path); /** * @return true iff the given path exists. diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index 41c2db59a14..8039d4b80fc 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -97,13 +97,7 @@ std::optional PosixSourceAccessor::cachedLstat(const CanonPath & pa if (i != cache->end()) return i->second; } - std::optional st{std::in_place}; - if (::lstat(absPath.c_str(), &*st)) { - if (errno == ENOENT || errno == ENOTDIR) - st.reset(); - else - throw SysError("getting status of '%s'", showPath(path)); - } + auto st = nix::maybeLstat(absPath.c_str()); auto cache(_cache.lock()); if (cache->size() >= 16384) cache->clear(); From 3752bbef28899bc05e2e144fae5dcf37d99f86b5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 30 Mar 2024 10:39:25 -0400 Subject: [PATCH 2/2] Document `maybeLstat` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com> --- src/libutil/file-system.hh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index dd071e1af79..06a993829c0 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -84,6 +84,10 @@ bool isDirOrInDir(std::string_view path, std::string_view dir); */ struct stat stat(const Path & path); struct stat lstat(const Path & path); +/** + * `lstat` the given path if it exists. + * @return std::nullopt if the path doesn't exist, or an optional containing the result of `lstat` otherwise + */ std::optional maybeLstat(const Path & path); /**