Merge pull request #11018 from siddhantk232/canonpath-fs-sink

Use `CanonPath` in `fs-sink.hh`

src/libfetchers/git-utils.cc

@@ -851,10 +851,10 @@ struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink
     }
 
     void createRegularFile(
-        const Path & path,
+        const CanonPath & path,
         std::function<void(CreateRegularFileSink &)> func) override
     {
-        auto pathComponents = tokenizeString<std::vector<std::string>>(path, "/");
+        auto pathComponents = tokenizeString<std::vector<std::string>>(path.rel(), "/");
         if (!prepareDirs(pathComponents, false)) return;
 
         git_writestream * stream = nullptr;
@@ -862,11 +862,11 @@ struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink
             throw Error("creating a blob stream object: %s", git_error_last()->message);
 
         struct CRF : CreateRegularFileSink {
-            const Path & path;
+            const CanonPath & path;
             GitFileSystemObjectSinkImpl & back;
             git_writestream * stream;
             bool executable = false;
-            CRF(const Path & path, GitFileSystemObjectSinkImpl & back, git_writestream * stream)
+            CRF(const CanonPath & path, GitFileSystemObjectSinkImpl & back, git_writestream * stream)
                 : path(path), back(back), stream(stream)
             {}
             void operator () (std::string_view data) override
@@ -891,15 +891,15 @@ struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink
             : GIT_FILEMODE_BLOB);
     }
 
-    void createDirectory(const Path & path) override
+    void createDirectory(const CanonPath & path) override
     {
-        auto pathComponents = tokenizeString<std::vector<std::string>>(path, "/");
+        auto pathComponents = tokenizeString<std::vector<std::string>>(path.rel(), "/");
         (void) prepareDirs(pathComponents, true);
     }
 
-    void createSymlink(const Path & path, const std::string & target) override
+    void createSymlink(const CanonPath & path, const std::string & target) override
     {
-        auto pathComponents = tokenizeString<std::vector<std::string>>(path, "/");
+        auto pathComponents = tokenizeString<std::vector<std::string>>(path.rel(), "/");
         if (!prepareDirs(pathComponents, false)) return;
 
         git_oid oid;

src/libstore/nar-accessor.cc

@@ -71,9 +71,11 @@ struct NarAccessor : public SourceAccessor
             : acc(acc), source(source)
         { }
 
-        NarMember & createMember(const Path & path, NarMember member)
+        NarMember & createMember(const CanonPath & path, NarMember member)
         {
-            size_t level = std::count(path.begin(), path.end(), '/');
+            size_t level = 0;
+            for (auto _ : path) ++level;
+
             while (parents.size() > level) parents.pop();
 
             if (parents.empty()) {
@@ -83,14 +85,14 @@ struct NarAccessor : public SourceAccessor
             } else {
                 if (parents.top()->stat.type != Type::tDirectory)
                     throw Error("NAR file missing parent directory of path '%s'", path);
-                auto result = parents.top()->children.emplace(baseNameOf(path), std::move(member));
+                auto result = parents.top()->children.emplace(*path.baseName(), std::move(member));
                 auto & ref = result.first->second;
                 parents.push(&ref);
                 return ref;
             }
         }
 
-        void createDirectory(const Path & path) override
+        void createDirectory(const CanonPath & path) override
         {
             createMember(path, NarMember{ .stat = {
                 .type = Type::tDirectory,
@@ -100,7 +102,7 @@ struct NarAccessor : public SourceAccessor
             } });
         }
 
-        void createRegularFile(const Path & path, std::function<void(CreateRegularFileSink &)> func) override
+        void createRegularFile(const CanonPath & path, std::function<void(CreateRegularFileSink &)> func) override
         {
             auto & nm = createMember(path, NarMember{ .stat = {
                 .type = Type::tRegular,
@@ -112,7 +114,7 @@ struct NarAccessor : public SourceAccessor
             func(nmc);
         }
 
-        void createSymlink(const Path & path, const std::string & target) override
+        void createSymlink(const CanonPath & path, const std::string & target) override
         {
             createMember(path,
                 NarMember{

src/libutil/archive.cc

@@ -165,7 +165,7 @@ struct CaseInsensitiveCompare
 };
 
 
-static void parse(FileSystemObjectSink & sink, Source & source, const Path & path)
+static void parse(FileSystemObjectSink & sink, Source & source, const CanonPath & path)
 {
     std::string s;
 
@@ -246,7 +246,7 @@ static void parse(FileSystemObjectSink & sink, Source & source, const Path & pat
                                 }
                             } else if (s == "node") {
                                 if (name.empty()) throw badArchive("entry name missing");
-                                parse(sink, source, path + "/" + name);
+                                parse(sink, source, path / name);
                             } else
                                 throw badArchive("unknown field " + s);
                         }
@@ -290,11 +290,11 @@ void parseDump(FileSystemObjectSink & sink, Source & source)
     }
     if (version != narVersionMagic1)
         throw badArchive("input doesn't look like a Nix archive");
-    parse(sink, source, "");
+    parse(sink, source, CanonPath::root);
 }
 
 
-void restorePath(const Path & path, Source & source)
+void restorePath(const std::filesystem::path & path, Source & source)
 {
     RestoreSink sink;
     sink.dstPath = path;

src/libutil/archive.hh

@@ -75,7 +75,7 @@ void dumpString(std::string_view s, Sink & sink);
 
 void parseDump(FileSystemObjectSink & sink, Source & source);
 
-void restorePath(const Path & path, Source & source);
+void restorePath(const std::filesystem::path & path, Source & source);
 
 /**
  * Read a NAR from 'source' and write it to 'sink'.

src/libutil/fs-sink.cc

@@ -14,7 +14,7 @@ namespace nix {
 
 void copyRecursive(
     SourceAccessor & accessor, const CanonPath & from,
-    FileSystemObjectSink & sink, const Path & to)
+    FileSystemObjectSink & sink, const CanonPath & to)
 {
     auto stat = accessor.lstat(from);
 
@@ -43,7 +43,7 @@ void copyRecursive(
         for (auto & [name, _] : accessor.readDirectory(from)) {
             copyRecursive(
                 accessor, from / name,
-                sink, to + "/" + name);
+                sink, to / name);
             break;
         }
         break;
@@ -69,17 +69,9 @@ static RestoreSinkSettings restoreSinkSettings;
 static GlobalConfig::Register r1(&restoreSinkSettings);
 
 
-void RestoreSink::createDirectory(const Path & path)
+void RestoreSink::createDirectory(const CanonPath & path)
 {
-    Path p = dstPath + path;
+    std::filesystem::create_directory(dstPath / path.rel());
-    if (
-#ifndef _WIN32 // TODO abstract mkdir perms for Windows
-        mkdir(p.c_str(), 0777) == -1
-#else
-        !CreateDirectoryW(pathNG(p).c_str(), NULL)
-#endif
-        )
-        throw NativeSysError("creating directory '%1%'", p);
 };
 
 struct RestoreRegularFile : CreateRegularFileSink {
@@ -90,13 +82,18 @@ struct RestoreRegularFile : CreateRegularFileSink {
     void preallocateContents(uint64_t size) override;
 };
 
-void RestoreSink::createRegularFile(const Path & path, std::function<void(CreateRegularFileSink &)> func)
+void RestoreSink::createRegularFile(const CanonPath & path, std::function<void(CreateRegularFileSink &)> func)
 {
-    Path p = dstPath + path;
+    auto p = dstPath;
+
+    if (!path.rel().empty()) {
+        p = p / path.rel();
+    }
+
     RestoreRegularFile crf;
     crf.fd =
 #ifdef _WIN32
-        CreateFileW(pathNG(path).c_str(), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL)
+        CreateFileW(path.c_str(), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL)
 #else
         open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)
 #endif
@@ -141,14 +138,16 @@ void RestoreRegularFile::operator () (std::string_view data)
     writeFull(fd.get(), data);
 }
 
-void RestoreSink::createSymlink(const Path & path, const std::string & target)
+void RestoreSink::createSymlink(const CanonPath & path, const std::string & target)
 {
-    Path p = dstPath + path;
+    auto p = dstPath;
+    if (!path.rel().empty())
+        p = dstPath / path.rel();
     nix::createSymlink(target, p);
 }
 
 
-void RegularFileSink::createRegularFile(const Path & path, std::function<void(CreateRegularFileSink &)> func)
+void RegularFileSink::createRegularFile(const CanonPath & path, std::function<void(CreateRegularFileSink &)> func)
 {
     struct CRF : CreateRegularFileSink {
         RegularFileSink & back;
@@ -163,7 +162,7 @@ void RegularFileSink::createRegularFile(const Path & path, std::function<void(Cr
 }
 
 
-void NullFileSystemObjectSink::createRegularFile(const Path & path, std::function<void(CreateRegularFileSink &)> func)
+void NullFileSystemObjectSink::createRegularFile(const CanonPath & path, std::function<void(CreateRegularFileSink &)> func)
 {
     struct : CreateRegularFileSink {
         void operator () (std::string_view data) override {}

src/libutil/fs-sink.hh

@@ -28,17 +28,17 @@ struct FileSystemObjectSink
 {
     virtual ~FileSystemObjectSink() = default;
 
-    virtual void createDirectory(const Path & path) = 0;
+    virtual void createDirectory(const CanonPath & path) = 0;
 
     /**
      * This function in general is no re-entrant. Only one file can be
      * written at a time.
      */
     virtual void createRegularFile(
-        const Path & path,
+        const CanonPath & path,
         std::function<void(CreateRegularFileSink &)>) = 0;
 
-    virtual void createSymlink(const Path & path, const std::string & target) = 0;
+    virtual void createSymlink(const CanonPath & path, const std::string & target) = 0;
 };
 
 /**
@@ -46,17 +46,17 @@ struct FileSystemObjectSink
  */
 void copyRecursive(
     SourceAccessor & accessor, const CanonPath & sourcePath,
-    FileSystemObjectSink & sink, const Path & destPath);
+    FileSystemObjectSink & sink, const CanonPath & destPath);
 
 /**
  * Ignore everything and do nothing
  */
 struct NullFileSystemObjectSink : FileSystemObjectSink
 {
-    void createDirectory(const Path & path) override { }
+    void createDirectory(const CanonPath & path) override { }
-    void createSymlink(const Path & path, const std::string & target) override { }
+    void createSymlink(const CanonPath & path, const std::string & target) override { }
     void createRegularFile(
-        const Path & path,
+        const CanonPath & path,
         std::function<void(CreateRegularFileSink &)>) override;
 };
 
@@ -65,15 +65,15 @@ struct NullFileSystemObjectSink : FileSystemObjectSink
  */
 struct RestoreSink : FileSystemObjectSink
 {
-    Path dstPath;
+    std::filesystem::path dstPath;
 
-    void createDirectory(const Path & path) override;
+    void createDirectory(const CanonPath & path) override;
 
     void createRegularFile(
-        const Path & path,
+        const CanonPath & path,
         std::function<void(CreateRegularFileSink &)>) override;
 
-    void createSymlink(const Path & path, const std::string & target) override;
+    void createSymlink(const CanonPath & path, const std::string & target) override;
 };
 
 /**
@@ -88,18 +88,18 @@ struct RegularFileSink : FileSystemObjectSink
 
     RegularFileSink(Sink & sink) : sink(sink) { }
 
-    void createDirectory(const Path & path) override
+    void createDirectory(const CanonPath & path) override
     {
         regular = false;
     }
 
-    void createSymlink(const Path & path, const std::string & target) override
+    void createSymlink(const CanonPath & path, const std::string & target) override
     {
         regular = false;
     }
 
     void createRegularFile(
-        const Path & path,
+        const CanonPath & path,
         std::function<void(CreateRegularFileSink &)>) override;
 };
 

src/libutil/git.cc

@@ -53,7 +53,7 @@ static std::string getString(Source & source, int n)
 
 void parseBlob(
     FileSystemObjectSink & sink,
-    const Path & sinkPath,
+    const CanonPath & sinkPath,
     Source & source,
     BlobMode blobMode,
     const ExperimentalFeatureSettings & xpSettings)
@@ -116,7 +116,7 @@ void parseBlob(
 
 void parseTree(
     FileSystemObjectSink & sink,
-    const Path & sinkPath,
+    const CanonPath & sinkPath,
     Source & source,
     std::function<SinkHook> hook,
     const ExperimentalFeatureSettings & xpSettings)
@@ -147,7 +147,7 @@ void parseTree(
         Hash hash(HashAlgorithm::SHA1);
         std::copy(hashs.begin(), hashs.end(), hash.hash);
 
-        hook(name, TreeEntry {
+        hook(CanonPath{name}, TreeEntry {
             .mode = mode,
             .hash = hash,
         });
@@ -171,7 +171,7 @@ ObjectType parseObjectType(
 
 void parse(
     FileSystemObjectSink & sink,
-    const Path & sinkPath,
+    const CanonPath & sinkPath,
     Source & source,
     BlobMode rootModeIfBlob,
     std::function<SinkHook> hook,
@@ -208,7 +208,7 @@ std::optional<Mode> convertMode(SourceAccessor::Type type)
 
 void restore(FileSystemObjectSink & sink, Source & source, std::function<RestoreHook> hook)
 {
-    parse(sink, "", source, BlobMode::Regular, [&](Path name, TreeEntry entry) {
+    parse(sink, CanonPath::root, source, BlobMode::Regular, [&](CanonPath name, TreeEntry entry) {
         auto [accessor, from] = hook(entry.hash);
         auto stat = accessor->lstat(from);
         auto gotOpt = convertMode(stat.type);

src/libutil/git.hh

@@ -64,7 +64,7 @@ using Tree = std::map<std::string, TreeEntry>;
  * Implementations may seek to memoize resources (bandwidth, storage,
  * etc.) for the same Git hash.
  */
-using SinkHook = void(const Path & name, TreeEntry entry);
+using SinkHook = void(const CanonPath & name, TreeEntry entry);
 
 /**
  * Parse the "blob " or "tree " prefix.
@@ -89,13 +89,13 @@ enum struct BlobMode : RawMode
 };
 
 void parseBlob(
-    FileSystemObjectSink & sink, const Path & sinkPath,
+    FileSystemObjectSink & sink, const CanonPath & sinkPath,
     Source & source,
     BlobMode blobMode,
     const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
 
 void parseTree(
-    FileSystemObjectSink & sink, const Path & sinkPath,
+    FileSystemObjectSink & sink, const CanonPath & sinkPath,
     Source & source,
     std::function<SinkHook> hook,
     const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
@@ -108,7 +108,7 @@ void parseTree(
  * a blob, this is ignored.
  */
 void parse(
-    FileSystemObjectSink & sink, const Path & sinkPath,
+    FileSystemObjectSink & sink, const CanonPath & sinkPath,
     Source & source,
     BlobMode rootModeIfBlob,
     std::function<SinkHook> hook,

src/libutil/memory-source-accessor.cc

@@ -124,9 +124,9 @@ SourcePath MemorySourceAccessor::addFile(CanonPath path, std::string && contents
 
 using File = MemorySourceAccessor::File;
 
-void MemorySink::createDirectory(const Path & path)
+void MemorySink::createDirectory(const CanonPath & path)
 {
-    auto * f = dst.open(CanonPath{path}, File { File::Directory { } });
+    auto * f = dst.open(path, File { File::Directory { } });
     if (!f)
         throw Error("file '%s' cannot be made because some parent file is not a directory", path);
 
@@ -146,9 +146,9 @@ struct CreateMemoryRegularFile : CreateRegularFileSink {
     void preallocateContents(uint64_t size) override;
 };
 
-void MemorySink::createRegularFile(const Path & path, std::function<void(CreateRegularFileSink &)> func)
+void MemorySink::createRegularFile(const CanonPath & path, std::function<void(CreateRegularFileSink &)> func)
 {
-    auto * f = dst.open(CanonPath{path}, File { File::Regular {} });
+    auto * f = dst.open(path, File { File::Regular {} });
     if (!f)
         throw Error("file '%s' cannot be made because some parent file is not a directory", path);
     if (auto * rp = std::get_if<File::Regular>(&f->raw)) {
@@ -173,9 +173,9 @@ void CreateMemoryRegularFile::operator () (std::string_view data)
     regularFile.contents += data;
 }
 
-void MemorySink::createSymlink(const Path & path, const std::string & target)
+void MemorySink::createSymlink(const CanonPath & path, const std::string & target)
 {
-    auto * f = dst.open(CanonPath{path}, File { File::Symlink { } });
+    auto * f = dst.open(path, File { File::Symlink { } });
     if (!f)
         throw Error("file '%s' cannot be made because some parent file is not a directory", path);
     if (auto * s = std::get_if<File::Symlink>(&f->raw))

src/libutil/memory-source-accessor.hh

@@ -81,13 +81,13 @@ struct MemorySink : FileSystemObjectSink
 
     MemorySink(MemorySourceAccessor & dst) : dst(dst) { }
 
-    void createDirectory(const Path & path) override;
+    void createDirectory(const CanonPath & path) override;
 
     void createRegularFile(
-        const Path & path,
+        const CanonPath & path,
         std::function<void(CreateRegularFileSink &)>) override;
 
-    void createSymlink(const Path & path, const std::string & target) override;
+    void createSymlink(const CanonPath & path, const std::string & target) override;
 };
 
 }

src/libutil/tarfile.cc

@@ -178,6 +178,7 @@ time_t unpackTarfileToSink(TarArchive & archive, FileSystemObjectSink & parseSin
         auto path = archive_entry_pathname(entry);
         if (!path)
             throw Error("cannot get archive member name: %s", archive_error_string(archive.archive));
+        auto cpath = CanonPath{path};
         if (r == ARCHIVE_WARN)
             warn(archive_error_string(archive.archive));
         else
@@ -188,11 +189,11 @@ time_t unpackTarfileToSink(TarArchive & archive, FileSystemObjectSink & parseSin
         switch (archive_entry_filetype(entry)) {
 
         case AE_IFDIR:
-            parseSink.createDirectory(path);
+            parseSink.createDirectory(cpath);
             break;
 
         case AE_IFREG: {
-            parseSink.createRegularFile(path, [&](auto & crf) {
+            parseSink.createRegularFile(cpath, [&](auto & crf) {
                 if (archive_entry_mode(entry) & S_IXUSR)
                     crf.isExecutable();
 
@@ -216,7 +217,7 @@ time_t unpackTarfileToSink(TarArchive & archive, FileSystemObjectSink & parseSin
         case AE_IFLNK: {
             auto target = archive_entry_symlink(entry);
 
-            parseSink.createSymlink(path, target);
+            parseSink.createSymlink(cpath, target);
 
             break;
         }

tests/unit/libutil/git.cc

@@ -67,7 +67,7 @@ TEST_F(GitTest, blob_read) {
         StringSink out;
         RegularFileSink out2 { out };
         ASSERT_EQ(parseObjectType(in, mockXpSettings), ObjectType::Blob);
-        parseBlob(out2, "", in, BlobMode::Regular, mockXpSettings);
+        parseBlob(out2, CanonPath::root, in, BlobMode::Regular, mockXpSettings);
 
         auto expected = readFile(goldenMaster("hello-world.bin"));
 
@@ -132,8 +132,8 @@ TEST_F(GitTest, tree_read) {
         NullFileSystemObjectSink out;
         Tree got;
         ASSERT_EQ(parseObjectType(in, mockXpSettings), ObjectType::Tree);
-        parseTree(out, "", in, [&](auto & name, auto entry) {
+        parseTree(out, CanonPath::root, in, [&](auto & name, auto entry) {
-            auto name2 = name;
+            auto name2 = std::string{name.rel()};
             if (entry.mode == Mode::Directory)
                 name2 += '/';
             got.insert_or_assign(name2, std::move(entry));
@@ -210,14 +210,14 @@ TEST_F(GitTest, both_roundrip) {
 
     MemorySink sinkFiles2 { *files2 };
 
-    std::function<void(const Path, const Hash &, BlobMode)> mkSinkHook;
+    std::function<void(const CanonPath, const Hash &, BlobMode)> mkSinkHook;
     mkSinkHook = [&](auto prefix, auto & hash, auto blobMode) {
         StringSource in { cas[hash] };
         parse(
             sinkFiles2, prefix, in, blobMode,
-            [&](const Path & name, const auto & entry) {
+            [&](const CanonPath & name, const auto & entry) {
                 mkSinkHook(
-                    prefix + "/" + name,
+                    prefix / name,
                     entry.hash,
                     // N.B. this cast would not be acceptable in real
                     // code, because it would make an assert reachable,
@@ -227,7 +227,7 @@ TEST_F(GitTest, both_roundrip) {
             mockXpSettings);
     };
 
-    mkSinkHook("", root.hash, BlobMode::Regular);
+    mkSinkHook(CanonPath::root, root.hash, BlobMode::Regular);
 
     ASSERT_EQ(*files, *files2);
 }