Merge pull request #11378 from Mic92/nix-dir-errors

builtins.readDir: fix nix error trace on filesystem errors

doc/manual/rl-next/filesystem-errors.md

@@ -0,0 +1,14 @@
+---
+synopsis: wrap filesystem exceptions more correctly
+issues: []
+prs: [11378]
+---
+
+
+With the switch to `std::filesystem` in different places, Nix started to throw `std::filesystem::filesystem_error` in many places instead of its own exceptions.
+
+This lead to no longer generating error traces, for example when listing a non-existing directory, and can also lead to crashes inside the Nix REPL.
+
+This version catches these types of exception correctly and wrap them into Nix's own exeception type.
+
+Author: [**@Mic92**](https://github.com/Mic92)

src/libcmd/repl.cc

@@ -2,6 +2,7 @@
 #include <cstdlib>
 #include <cstring>
 
+#include "error.hh"
 #include "repl-interacter.hh"
 #include "repl.hh"
 
@@ -720,7 +721,14 @@ void NixRepl::loadFlake(const std::string & flakeRefS)
     if (flakeRefS.empty())
         throw Error("cannot use ':load-flake' without a path specified. (Use '.' for the current working directory.)");
 
-    auto flakeRef = parseFlakeRef(fetchSettings, flakeRefS, std::filesystem::current_path().string(), true);
+    std::filesystem::path cwd;
+    try {
+        cwd = std::filesystem::current_path();
+    } catch (std::filesystem::filesystem_error & e) {
+        throw SysError("cannot determine current working directory");
+    }
+
+    auto flakeRef = parseFlakeRef(fetchSettings, flakeRefS, cwd.string(), true);
     if (evalSettings.pureEval && !flakeRef.input.isLocked())
         throw Error("cannot use ':load-flake' on locked flake reference '%s' (use --impure to override)", flakeRefS);
 

src/libstore/builtins/unpack-channel.cc

@@ -3,31 +3,53 @@
 
 namespace nix {
 
+namespace fs { using namespace std::filesystem; }
+
 void builtinUnpackChannel(
     const BasicDerivation & drv,
     const std::map<std::string, Path> & outputs)
 {
-    auto getAttr = [&](const std::string & name) {
+    auto getAttr = [&](const std::string & name) -> const std::string & {
         auto i = drv.env.find(name);
         if (i == drv.env.end()) throw Error("attribute '%s' missing", name);
         return i->second;
     };
 
-    auto out = outputs.at("out");
+    fs::path out{outputs.at("out")};
-    auto channelName = getAttr("channelName");
+    auto & channelName = getAttr("channelName");
-    auto src = getAttr("src");
+    auto & src = getAttr("src");
 
-    createDirs(out);
+    if (fs::path{channelName}.filename().string() != channelName) {
+        throw Error("channelName is not allowed to contain filesystem seperators, got %1%", channelName);
+    }
+
+    try {
+        fs::create_directories(out);
+    } catch (fs::filesystem_error &) {
+        throw SysError("creating directory '%1%'", out.string());
+    }
 
     unpackTarfile(src, out);
 
-    auto entries = std::filesystem::directory_iterator{out};
+    size_t fileCount;
-    auto fileName = entries->path().string();
+    std::string fileName;
-    auto fileCount = std::distance(std::filesystem::begin(entries), std::filesystem::end(entries));
+    try {
+        auto entries = fs::directory_iterator{out};
+        fileName = entries->path().string();
+        fileCount = std::distance(fs::begin(entries), fs::end(entries));
+    } catch (fs::filesystem_error &) {
+        throw SysError("failed to read directory %1%", out.string());
+    }
 
     if (fileCount != 1)
         throw Error("channel tarball '%s' contains more than one file", src);
-    std::filesystem::rename(fileName, (out + "/" + channelName));
+
+    auto target = out / channelName;
+    try {
+        fs::rename(fileName, target);
+    } catch (fs::filesystem_error &) {
+        throw SysError("failed to rename %1% to %2%", fileName, target.string());
+    }
 }
 
 }

src/libutil/posix-source-accessor.cc

@@ -132,23 +132,24 @@ SourceAccessor::DirEntries PosixSourceAccessor::readDirectory(const CanonPath &
 {
     assertNoSymlinks(path);
     DirEntries res;
-    for (auto & entry : std::filesystem::directory_iterator{makeAbsPath(path)}) {
+    try {
-        checkInterrupt();
+        for (auto & entry : std::filesystem::directory_iterator{makeAbsPath(path)}) {
-        auto type = [&]() -> std::optional<Type> {
+            checkInterrupt();
-            std::filesystem::file_type nativeType;
+            auto type = [&]() -> std::optional<Type> {
-            try {
+                std::filesystem::file_type nativeType;
-                nativeType = entry.symlink_status().type();
+                try {
-            } catch (std::filesystem::filesystem_error & e) {
+                    nativeType = entry.symlink_status().type();
-                // We cannot always stat the child. (Ideally there is no
+                } catch (std::filesystem::filesystem_error & e) {
-                // stat because the native directory entry has the type
+                    // We cannot always stat the child. (Ideally there is no
-                // already, but this isn't always the case.)
+                    // stat because the native directory entry has the type
-                if (e.code() == std::errc::permission_denied || e.code() == std::errc::operation_not_permitted)
+                    // already, but this isn't always the case.)
-                    return std::nullopt;
+                    if (e.code() == std::errc::permission_denied || e.code() == std::errc::operation_not_permitted)
-                else throw;
+                        return std::nullopt;
-            }
+                    else throw;
+                }
 
-            // cannot exhaustively enumerate because implementation-specific
+                // cannot exhaustively enumerate because implementation-specific
-            // additional file types are allowed.
+                // additional file types are allowed.
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wswitch-enum"
             switch (nativeType) {
@@ -158,8 +159,11 @@ SourceAccessor::DirEntries PosixSourceAccessor::readDirectory(const CanonPath &
             default: return tMisc;
             }
 #pragma GCC diagnostic pop
-        }();
+            }();
-        res.emplace(entry.path().filename().string(), type);
+            res.emplace(entry.path().filename().string(), type);
+        }
+    } catch (std::filesystem::filesystem_error & e) {
+        throw SysError("reading directory %1%", showPath(path));
     }
     return res;
 }

src/libutil/tarfile.cc

@@ -8,6 +8,10 @@
 
 namespace nix {
 
+namespace fs {
+using namespace std::filesystem;
+}
+
 namespace {
 
 int callback_open(struct archive *, void * self)
@@ -102,14 +106,14 @@ TarArchive::TarArchive(Source & source, bool raw, std::optional<std::string> com
         "Failed to open archive (%s)");
 }
 
-TarArchive::TarArchive(const Path & path)
+TarArchive::TarArchive(const fs::path & path)
     : archive{archive_read_new()}
     , buffer(defaultBufferSize)
 {
     archive_read_support_filter_all(archive);
     enableSupportedFormats(archive);
     archive_read_set_option(archive, NULL, "mac-ext", NULL);
-    check(archive_read_open_filename(archive, path.c_str(), 16384), "failed to open archive: %s");
+    check(archive_read_open_filename(archive, path.string().c_str(), 16384), "failed to open archive: %s");
 }
 
 void TarArchive::close()
@@ -123,7 +127,7 @@ TarArchive::~TarArchive()
         archive_read_free(this->archive);
 }
 
-static void extract_archive(TarArchive & archive, const Path & destDir)
+static void extract_archive(TarArchive & archive, const fs::path & destDir)
 {
     int flags = ARCHIVE_EXTRACT_TIME | ARCHIVE_EXTRACT_SECURE_SYMLINKS | ARCHIVE_EXTRACT_SECURE_NODOTDOT;
 
@@ -140,7 +144,7 @@ static void extract_archive(TarArchive & archive, const Path & destDir)
         else
             archive.check(r);
 
-        archive_entry_copy_pathname(entry, (destDir + "/" + name).c_str());
+        archive_entry_copy_pathname(entry, (destDir / name).string().c_str());
 
         // sources can and do contain dirs with no rx bits
         if (archive_entry_filetype(entry) == AE_IFDIR && (archive_entry_mode(entry) & 0500) != 0500)
@@ -149,7 +153,7 @@ static void extract_archive(TarArchive & archive, const Path & destDir)
         // Patch hardlink path
         const char * original_hardlink = archive_entry_hardlink(entry);
         if (original_hardlink) {
-            archive_entry_copy_hardlink(entry, (destDir + "/" + original_hardlink).c_str());
+            archive_entry_copy_hardlink(entry, (destDir / original_hardlink).string().c_str());
         }
 
         archive.check(archive_read_extract(archive.archive, entry, flags));
@@ -158,19 +162,19 @@ static void extract_archive(TarArchive & archive, const Path & destDir)
     archive.close();
 }
 
-void unpackTarfile(Source & source, const Path & destDir)
+void unpackTarfile(Source & source, const fs::path & destDir)
 {
     auto archive = TarArchive(source);
 
-    createDirs(destDir);
+    fs::create_directories(destDir);
     extract_archive(archive, destDir);
 }
 
-void unpackTarfile(const Path & tarFile, const Path & destDir)
+void unpackTarfile(const fs::path & tarFile, const fs::path & destDir)
 {
     auto archive = TarArchive(tarFile);
 
-    createDirs(destDir);
+    fs::create_directories(destDir);
     extract_archive(archive, destDir);
 }
 

src/libutil/tarfile.hh

@@ -15,7 +15,7 @@ struct TarArchive
 
     void check(int err, const std::string & reason = "failed to extract archive (%s)");
 
-    explicit TarArchive(const Path & path);
+    explicit TarArchive(const std::filesystem::path & path);
 
     /// @brief Create a generic archive from source.
     /// @param source - Input byte stream.
@@ -37,9 +37,9 @@ struct TarArchive
 
 int getArchiveFilterCodeByName(const std::string & method);
 
-void unpackTarfile(Source & source, const Path & destDir);
+void unpackTarfile(Source & source, const std::filesystem::path & destDir);
 
-void unpackTarfile(const Path & tarFile, const Path & destDir);
+void unpackTarfile(const std::filesystem::path & tarFile, const std::filesystem::path & destDir);
 
 time_t unpackTarfileToSink(TarArchive & archive, ExtendedFileSystemObjectSink & parseSink);