From 1354816f0ce45bb4ce3741247e8948ef59239c4a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 4 Oct 2026 22:26:55 +0200 Subject: [PATCH 1/2] fix: copy filesystem walkers without shared traversal --- CHANGELOG.md | 3 ++ src/odr/internal/common/filesystem.cpp | 29 ++++++++++++--- test/src/internal/common/filesystem_test.cpp | 39 ++++++++++++++++++++ 3 files changed, 65 insertions(+), 6 deletions(-) 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..1a460f72a 100644 --- a/src/odr/internal/common/filesystem.cpp +++ b/src/odr/internal/common/filesystem.cpp @@ -20,18 +20,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 +81,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..e617aebf7 100644 --- a/test/src/internal/common/filesystem_test.cpp +++ b/test/src/internal/common/filesystem_test.cpp @@ -3,9 +3,12 @@ #include #include #include +#include +#include #include +#include #include #include #include @@ -40,6 +43,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"}); From bed4ee25b73af534b60c6e9e345d48f1df26b26b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 4 Oct 2026 22:50:54 +0200 Subject: [PATCH 2/2] chore: include what the walker copy and its test use filesystem.cpp throws std::runtime_error, and the test uses std::filesystem and std::error_code. They compiled only through other headers. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- src/odr/internal/common/filesystem.cpp | 1 + test/src/internal/common/filesystem_test.cpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/odr/internal/common/filesystem.cpp b/src/odr/internal/common/filesystem.cpp index 1a460f72a..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 { diff --git a/test/src/internal/common/filesystem_test.cpp b/test/src/internal/common/filesystem_test.cpp index e617aebf7..bd9259d95 100644 --- a/test/src/internal/common/filesystem_test.cpp +++ b/test/src/internal/common/filesystem_test.cpp @@ -9,8 +9,10 @@ #include #include +#include #include #include +#include #include using namespace odr::internal;