Update profiles to use std::filesystem::path

[vinayakankugoyal]

Motivation

Update profiles to use std::filesystem::path

Context

#9205

---

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[xokdvium]

I'm a lot on the fence of relying more on std::filesystem for I/O and not just path manipulation, since it doesn't have facilities for dirfd-based operations. There are a lot of symlink following TOCTOU issues lurking no doubt. I would prefer if we kept our file-system functions so that we can implement safe primitives without undoing these changes.

[Ericson2314]

@xokdvium my comments are in that direction. @Mic92 convinced also for exception we need to wrap most things too.

[Mic92]

I'm a lot on the fence of relying more on std::filesystem for I/O and not just path manipulation, since it doesn't have facilities for dirfd-based operations. There are a lot of symlink following TOCTOU issues lurking no doubt. I would prefer if we kept our file-system functions so that we can implement safe primitives without undoing these changes.

We should make a clear decision on this i.e have the std::filesystem path type but than keep all io function in one place that allows us to wrap errors and potentially have the option to implement more secure but potential platform abstraction in one place. How does this sound?

[xokdvium]

We should make a clear decision on this i.e have the std::filesystem path type but than keep all io function in one place that allows us to wrap errors and potentially have the option to implement more secure but potential platform-dependent abstraction in one place. How does this sound?

Sounds perfect. Could you file an issue outlining this? I'd prefer also if we could start leveraging SourceAccessor more when I'm done with making it use openat2. RestoreSink now provides safer facilities since I recently rewrote it to use dirfd on unix. That way we have a good default that's harder to misuse.

[Ericson2314]

Sounds good to me too, and to be clear my review comments above are already going in that direction (at least I think they are).

[Ericson2314]

diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh
index 246e0e350f..8200e40e95 100644
--- a/src/libutil/include/nix/util/file-system.hh
+++ b/src/libutil/include/nix/util/file-system.hh
@@ -56,7 +56,10 @@ inline Path absPath(const Path & path, std::optional<PathView> dir = {}, bool re
     return absPath(PathView{path}, dir, resolveSymlinks);
 }

-std::filesystem::path absPath(const std::filesystem::path & path, bool resolveSymlinks = false);
+std::filesystem::path absPath(
+    const std::filesystem::path & path,
+    std::optional<std::filesystem::path> dir = {},
+    bool resolveSymlinks = false);

 /**
  * Canonicalise a path by removing all `.` or `..` components an

let's do this too, and then in the places we were removing absPath, don't do that after all.

[Ericson2314]

This looks good to me now (after squashing). The only function not wrapped is parent_path, which according to https://en.cppreference.com/w/cpp/filesystem/path/parent_path.html is technically allowed to throw for implementation-specific reasons, but I don't think I will.

Since I am co-author here, going to weight for another approval, however.