diff --git a/include/pup/platform/file_io.hpp b/include/pup/platform/file_io.hpp index 1ed57fa2..a994cf99 100644 --- a/include/pup/platform/file_io.hpp +++ b/include/pup/platform/file_io.hpp @@ -102,6 +102,8 @@ auto write_file(std::string_view path, std::string_view data) -> Result; // Directory traversal. // +// Entries come back in name order — readdir order is a filesystem artifact. +// // A DirEntry's `name` is a view into the `names` arena of the DirEntries that // produced it — valid until the next read_directory on that DirEntries, or its // destruction. In a walk, the `rel_path` handed to the visitor is likewise a diff --git a/src/platform/file_io-posix.cpp b/src/platform/file_io-posix.cpp index d052fab1..5509907a 100644 --- a/src/platform/file_io-posix.cpp +++ b/src/platform/file_io-posix.cpp @@ -9,6 +9,7 @@ #include "pup/platform/file_io.hpp" #include "pup/platform/sys.hpp" +#include #include #include #include @@ -630,6 +631,7 @@ auto read_directory(std::string_view path, DirEntries& out) -> Result for (auto i = std::size_t { 0 }; i < out.entries.size(); ++i) { out.entries[i].name = base.substr(offsets[i], offsets[i + 1U] - offsets[i]); } + std::ranges::sort(out.entries, {}, &DirEntry::name); return {}; } diff --git a/src/platform/file_io-win32.cpp b/src/platform/file_io-win32.cpp index 29cab68c..e9faf01c 100644 --- a/src/platform/file_io-win32.cpp +++ b/src/platform/file_io-win32.cpp @@ -8,6 +8,8 @@ #include "pup/core/string_pool.hpp" #include "pup/platform/file_io.hpp" +#include + #include namespace pup::platform { @@ -640,6 +642,7 @@ auto read_directory(std::string_view path, DirEntries& out) -> Result for (auto i = std::size_t { 0 }; i < out.entries.size(); ++i) { out.entries[i].name = base.substr(offsets[i], offsets[i + 1] - offsets[i]); } + std::ranges::sort(out.entries, {}, &DirEntry::name); return {}; } diff --git a/test/unit/test_platform_file_io.cpp b/test/unit/test_platform_file_io.cpp index ca71ab01..2ce2148e 100644 --- a/test/unit/test_platform_file_io.cpp +++ b/test/unit/test_platform_file_io.cpp @@ -7,12 +7,14 @@ #include "pup/core/string_pool.hpp" #include "pup/platform/file_io.hpp" +#include #include #include #include #include #include #include +#include #ifndef _WIN32 #include @@ -417,3 +419,62 @@ TEST_CASE("walk_directory descends deep trees without a per-frame stack blowup", REQUIRE(args.saw_leaf); } #endif + +SCENARIO("read_directory returns entries in name order", "[platform][file_io][walk]") +{ + GIVEN("a directory whose entries were created in reverse-lexicographic order") + { + // Mixed case on purpose: NTFS enumerates case-insensitively, so an + // all-lowercase set would already arrive sorted and pin nothing there. + auto const names = std::array { + "Bravo", "Zulu", "alpha", "mike" + }; + auto dir = TempDir { "order" }; + for (auto i = names.size(); i-- > 0;) { + dir.write(names[i], "x"); + } + + WHEN("the directory is read") + { + auto listing = pup::platform::DirEntries {}; + REQUIRE(pup::platform::read_directory(dir.path().string(), listing).has_value()); + + THEN("the entries are in name order") + { + auto seen = std::vector {}; + for (auto const& entry : listing.entries) { + seen.push_back(entry.name); + } + REQUIRE(seen == std::vector(names.begin(), names.end())); + } + } + } +} + +SCENARIO("walk_directory visits every level in name order", "[platform][file_io][walk]") +{ + GIVEN("a nested tree created in reverse-lexicographic order") + { + auto dir = TempDir { "walk_order" }; + for (auto name : std::array { "alpha.txt", "sub/bravo.txt", "sub/Yankee.txt", "Zulu.txt" }) { + dir.write(name, "x"); + } + + WHEN("the tree is walked") + { + auto seen = std::vector {}; + auto r = pup::platform::walk_directory( + dir.path().string(), + [&](pup::platform::DirEntry const& /*entry*/, std::string_view rel_path) -> bool { + seen.emplace_back(rel_path); + return true; + }); + + THEN("each directory is enumerated in name order, depth first") + { + REQUIRE(r.has_value()); + REQUIRE(seen == std::vector { "Zulu.txt", "alpha.txt", "sub", "sub/Yankee.txt", "sub/bravo.txt" }); + } + } + } +}