diff --git a/CHANGELOG.md b/CHANGELOG.md index f7624e1e6..b4a66b16e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Copied filesystem walkers keep independent traversal positions on disk, + matching walkers over archives and memory filesystems. + - Stream copies report failed reads and writes, and file writes check their final flush. Saving a text file over its source reads the text before truncation. diff --git a/src/odr/internal/common/filesystem.cpp b/src/odr/internal/common/filesystem.cpp index 350d0839f..f4beb11f1 100644 --- a/src/odr/internal/common/filesystem.cpp +++ b/src/odr/internal/common/filesystem.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include namespace odr::internal { @@ -20,18 +21,34 @@ namespace { class SystemFileWalker final : public abstract::FileWalker { public: SystemFileWalker(AbsPath root, const Path &path) - : m_root{std::move(root)}, - m_iterator{std::filesystem::recursive_directory_iterator(path.path())} { + : m_root{std::move(root)}, m_start{path.path()}, m_iterator{m_start} {} + + SystemFileWalker(const SystemFileWalker &other) + : m_root{other.m_root}, m_start{other.m_start} { + if (other.end()) { + return; + } + // Directory iterators are single-pass; copying one shares its traversal. + m_iterator = std::filesystem::recursive_directory_iterator(m_start); + while (!end() && m_iterator->path() != other.m_iterator->path()) { + ++m_iterator; + } + if (end()) { + throw std::runtime_error("walked directory changed while copying"); + } } [[nodiscard]] std::unique_ptr clone() const override { return std::make_unique(*this); } - /// TODO always false: two walkers at the same place do not compare equal. - /// Nothing calls this yet, so nothing depends on the answer. - [[nodiscard]] bool equals(const FileWalker & /*rhs*/) const override { - return false; + [[nodiscard]] bool equals(const FileWalker &other) const override { + const auto *rhs = dynamic_cast(&other); + if (rhs == nullptr || m_root != rhs->m_root || m_start != rhs->m_start) { + return false; + } + return end() || rhs->end() ? end() && rhs->end() + : m_iterator->path() == rhs->m_iterator->path(); } [[nodiscard]] bool end() const override { @@ -65,6 +82,7 @@ class SystemFileWalker final : public abstract::FileWalker { private: AbsPath m_root; + std::filesystem::path m_start; std::filesystem::recursive_directory_iterator m_iterator; }; } // namespace diff --git a/test/src/internal/common/filesystem_test.cpp b/test/src/internal/common/filesystem_test.cpp index 9e910f996..bd9259d95 100644 --- a/test/src/internal/common/filesystem_test.cpp +++ b/test/src/internal/common/filesystem_test.cpp @@ -3,11 +3,16 @@ #include #include #include +#include +#include #include +#include +#include #include #include +#include #include using namespace odr::internal; @@ -40,6 +45,42 @@ filesystem_of(const std::vector &files, } // namespace +TEST(Filesystem, copied_walkers_advance_independently) { + struct Directory final { + std::filesystem::path path = std::filesystem::temp_directory_path() / + ("odr-walker-" + random_string(12)); + ~Directory() { + std::error_code error; + std::filesystem::remove_all(path, error); + } + } directory; + std::filesystem::create_directories(directory.path / "nested"); + util::file::write("one", (directory.path / "nested/one").string()); + util::file::write("two", (directory.path / "nested/two").string()); + util::file::write("three", (directory.path / "three").string()); + const SystemFilesystem disk{AbsPath(directory.path)}; + const VirtualFilesystem memory = + filesystem_of({"/nested/one", "/nested/two", "/three"}, {"/nested"}); + + for (const abstract::ReadableFilesystem *filesystem : + std::array{&disk, &memory}) { + const auto walker = filesystem->file_walker(AbsPath("/")); + while (!walker->end()) { + const auto copy = walker->clone(); + const AbsPath path = walker->path(); + const auto depth = walker->depth(); + EXPECT_TRUE(copy->equals(*walker)); + walker->next(); + ASSERT_FALSE(copy->end()); + EXPECT_EQ(copy->path(), path); + EXPECT_EQ(copy->depth(), depth); + copy->next(); + EXPECT_TRUE(copy->equals(*walker)); + } + EXPECT_TRUE(walker->clone()->end()); + } +} + TEST(VirtualFilesystem, walker_reports_the_depth_of_every_entry) { const VirtualFilesystem filesystem = filesystem_of({"/mimetype", "/a/b.txt", "/a/c/d.txt"});