Merge pull request #9948 from obsidiansystems/no-canon-path-from-cwd

Get rid of `CanonPath::fromCwd`

src/libutil/archive.cc

@@ -110,8 +110,8 @@ void SourceAccessor::dumpPath(
 
 time_t dumpPathAndGetMtime(const Path & path, Sink & sink, PathFilter & filter)
 {
-    PosixSourceAccessor accessor;
-    accessor.dumpPath(CanonPath::fromCwd(path), sink, filter);
+    auto [accessor, canonPath] = PosixSourceAccessor::createAtRoot(path);
+    accessor.dumpPath(canonPath, sink, filter);
     return accessor.mtime;
 }
 

src/libutil/canon-path.cc

@@ -20,11 +20,6 @@ CanonPath::CanonPath(const std::vector<std::string> & elems)
         push(s);
 }
 
-CanonPath CanonPath::fromCwd(std::string_view path)
-{
-    return CanonPath(unchecked_t(), absPath(path));
-}
-
 std::optional<CanonPath> CanonPath::parent() const
 {
     if (isRoot()) return std::nullopt;

src/libutil/canon-path.hh

@@ -52,8 +52,6 @@ public:
      */
     CanonPath(const std::vector<std::string> & elems);
 
-    static CanonPath fromCwd(std::string_view path = ".");
-
     static CanonPath root;
 
     /**

src/libutil/posix-source-accessor.cc

@@ -6,6 +6,33 @@
 
 namespace nix {
 
+PosixSourceAccessor::PosixSourceAccessor(std::filesystem::path && root)
+    : root(std::move(root))
+{
+    assert(root.empty() || root.is_absolute());
+    displayPrefix = root;
+}
+
+PosixSourceAccessor::PosixSourceAccessor()
+    : PosixSourceAccessor(std::filesystem::path {})
+{ }
+
+std::pair<PosixSourceAccessor, CanonPath> PosixSourceAccessor::createAtRoot(const std::filesystem::path & path)
+{
+    std::filesystem::path path2 = absPath(path.native());
+    return {
+        PosixSourceAccessor { path2.root_path() },
+        CanonPath { static_cast<std::string>(path2.relative_path()) },
+    };
+}
+
+std::filesystem::path PosixSourceAccessor::makeAbsPath(const CanonPath & path)
+{
+    return root.empty()
+        ? (std::filesystem::path { path.abs() })
+        : root / path.rel();
+}
+
 void PosixSourceAccessor::readFile(
     const CanonPath & path,
     Sink & sink,
@@ -13,9 +40,11 @@ void PosixSourceAccessor::readFile(
 {
     assertNoSymlinks(path);
 
-    AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
+    auto ap = makeAbsPath(path);
+
+    AutoCloseFD fd = open(ap.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
     if (!fd)
-        throw SysError("opening file '%1%'", path);
+        throw SysError("opening file '%1%'", ap.native());
 
     struct stat st;
     if (fstat(fd.get(), &st) == -1)
@@ -46,7 +75,7 @@ void PosixSourceAccessor::readFile(
 bool PosixSourceAccessor::pathExists(const CanonPath & path)
 {
     if (auto parent = path.parent()) assertNoSymlinks(*parent);
-    return nix::pathExists(path.abs());
+    return nix::pathExists(makeAbsPath(path));
 }
 
 std::optional<struct stat> PosixSourceAccessor::cachedLstat(const CanonPath & path)
@@ -60,7 +89,7 @@ std::optional<struct stat> PosixSourceAccessor::cachedLstat(const CanonPath & pa
     }
 
     std::optional<struct stat> st{std::in_place};
-    if (::lstat(path.c_str(), &*st)) {
+    if (::lstat(makeAbsPath(path).c_str(), &*st)) {
         if (errno == ENOENT || errno == ENOTDIR)
             st.reset();
         else
@@ -95,7 +124,7 @@ SourceAccessor::DirEntries PosixSourceAccessor::readDirectory(const CanonPath &
 {
     assertNoSymlinks(path);
     DirEntries res;
-    for (auto & entry : nix::readDirectory(path.abs())) {
+    for (auto & entry : nix::readDirectory(makeAbsPath(path))) {
         std::optional<Type> type;
         switch (entry.type) {
         case DT_REG: type = Type::tRegular; break;
@@ -110,12 +139,12 @@ SourceAccessor::DirEntries PosixSourceAccessor::readDirectory(const CanonPath &
 std::string PosixSourceAccessor::readLink(const CanonPath & path)
 {
     if (auto parent = path.parent()) assertNoSymlinks(*parent);
-    return nix::readLink(path.abs());
+    return nix::readLink(makeAbsPath(path));
 }
 
-std::optional<CanonPath> PosixSourceAccessor::getPhysicalPath(const CanonPath & path)
+std::optional<std::filesystem::path> PosixSourceAccessor::getPhysicalPath(const CanonPath & path)
 {
-    return path;
+    return makeAbsPath(path);
 }
 
 void PosixSourceAccessor::assertNoSymlinks(CanonPath path)

src/libutil/posix-source-accessor.hh

@@ -9,6 +9,16 @@ namespace nix {
  */
 struct PosixSourceAccessor : virtual SourceAccessor
 {
+    /**
+     * Optional root path to prefix all operations into the native file
+     * system. This allows prepending funny things like `C:\` that
+     * `CanonPath` intentionally doesn't support.
+     */
+    const std::filesystem::path root;
+
+    PosixSourceAccessor();
+    PosixSourceAccessor(std::filesystem::path && root);
+
     /**
      * The most recent mtime seen by lstat(). This is a hack to
      * support dumpPathAndGetMtime(). Should remove this eventually.
@@ -28,7 +38,22 @@ struct PosixSourceAccessor : virtual SourceAccessor
 
     std::string readLink(const CanonPath & path) override;
 
-    std::optional<CanonPath> getPhysicalPath(const CanonPath & path) override;
+    std::optional<std::filesystem::path> getPhysicalPath(const CanonPath & path) override;
+
+    /**
+     * Create a `PosixSourceAccessor` and `CanonPath` corresponding to
+     * some native path.
+     *
+     * The `PosixSourceAccessor` is rooted as far up the tree as
+     * possible, (e.g. on Windows it could scoped to a drive like
+     * `C:\`). This allows more `..` parent accessing to work.
+     *
+     * See
+     * [`std::filesystem::path::root_path`](https://en.cppreference.com/w/cpp/filesystem/path/root_path)
+     * and
+     * [`std::filesystem::path::relative_path`](https://en.cppreference.com/w/cpp/filesystem/path/relative_path).
+     */
+    static std::pair<PosixSourceAccessor, CanonPath> createAtRoot(const std::filesystem::path & path);
 
 private:
 
@@ -38,6 +63,8 @@ private:
     void assertNoSymlinks(CanonPath path);
 
     std::optional<struct stat> cachedLstat(const CanonPath & path);
+
+    std::filesystem::path makeAbsPath(const CanonPath & path);
 };
 
 }

src/libutil/source-accessor.hh

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <filesystem>
+
 #include "canon-path.hh"
 #include "hash.hh"
 
@@ -119,7 +121,7 @@ struct SourceAccessor
      * possible. This is only possible for filesystems that are
      * materialized in the root filesystem.
      */
-    virtual std::optional<CanonPath> getPhysicalPath(const CanonPath & path)
+    virtual std::optional<std::filesystem::path> getPhysicalPath(const CanonPath & path)
     { return std::nullopt; }
 
     bool operator == (const SourceAccessor & x) const

src/libutil/source-path.cc

@@ -35,7 +35,7 @@ void SourcePath::dumpPath(
     PathFilter & filter) const
 { return accessor->dumpPath(path, sink, filter); }
 
-std::optional<CanonPath> SourcePath::getPhysicalPath() const
+std::optional<std::filesystem::path> SourcePath::getPhysicalPath() const
 { return accessor->getPhysicalPath(path); }
 
 std::string SourcePath::to_string() const

src/libutil/source-path.hh

@@ -82,7 +82,7 @@ struct SourcePath
      * Return the location of this path in the "real" filesystem, if
      * it has a physical location.
      */
-    std::optional<CanonPath> getPhysicalPath() const;
+    std::optional<std::filesystem::path> getPhysicalPath() const;
 
     std::string to_string() const;