diff --git a/include/pup/index/format.hpp b/include/pup/index/format.hpp index a18f03b3..e2f697a6 100644 --- a/include/pup/index/format.hpp +++ b/include/pup/index/format.hpp @@ -41,7 +41,11 @@ inline constexpr auto INDEX_MAGIC = std::array { 'P', 'U', 'P', 'I' }; /// resolve to one another through the identity join (issue #167). Also /// covers OrderOnly edges, persisted since issue #166 and load-bearing for /// removal routing; v14 indexes predate them and would lose one removal. -inline constexpr auto INDEX_VERSION = std::uint32_t { 15 }; +/// 16 - Glob matches are ordered by path rather than by interning (readdir) +/// order, so %f and the identity hashing it changed for every glob-fed +/// command (issue #171); v15 identities no longer join, and a scoped build +/// would merge the stale twin back in beside the new one. +inline constexpr auto INDEX_VERSION = std::uint32_t { 16 }; /// Index file header (56 bytes) - v9 struct alignas(8) RawHeader { diff --git a/src/parser/glob.cpp b/src/parser/glob.cpp index db58617b..da0525ba 100644 --- a/src/parser/glob.cpp +++ b/src/parser/glob.cpp @@ -254,7 +254,8 @@ auto glob_expand( } } - std::ranges::sort(results); + // StringId order is interning order, i.e. readdir order, not path order. + std::ranges::sort(results, {}, [&pool](StringId id) { return pool.get(id); }); return results; } diff --git a/test/unit/test_glob.cpp b/test/unit/test_glob.cpp index 823e4f34..4286239e 100644 --- a/test/unit/test_glob.cpp +++ b/test/unit/test_glob.cpp @@ -6,12 +6,58 @@ #include "pup/core/string_pool.hpp" #include "pup/parser/glob.hpp" +#include +#include +#include +#include +#include +#include + +namespace fs = std::filesystem; + using namespace pup::parser; using pup::StringId; using pup::global_pool; namespace { auto sv(StringId id) -> std::string_view { return global_pool().get(id); } + +/// RAII helper to create a temporary directory tree for testing +class TempDir { +public: + // Test shards run as concurrent processes, so the name must be unique across them. + TempDir() + { + auto rng = std::random_device {}; + auto dist = std::uniform_int_distribution { 0, 0xFFFFFFFF }; + for (;;) { + auto candidate = fs::temp_directory_path() / ("pup_glob_" + std::to_string(dist(rng))); + if (fs::create_directory(candidate)) { + path_ = candidate; + return; + } + } + } + + ~TempDir() + { + std::error_code ec; + fs::remove_all(path_, ec); + } + + TempDir(TempDir const&) = delete; + auto operator=(TempDir const&) -> TempDir& = delete; + + [[nodiscard]] auto path() const -> fs::path const& { return path_; } + + auto create_file(std::string_view rel) -> void + { + std::ofstream { path_ / rel }; + } + +private: + fs::path path_; +}; } // namespace TEST_CASE("Glob pattern matching", "[glob]") @@ -223,3 +269,53 @@ TEST_CASE("glob_match_extract", "[glob]") REQUIRE(sv(glob_match_extract("**.c", "foo.c")) == "foo"); } } + +SCENARIO("glob expansion orders matches by path, not by interning order", "[glob]") +{ + GIVEN("a directory tree whose paths were interned in reverse-lexicographic order") + { + // Names unique to this test, so the interning below decides their handles. + auto const paths = std::array { + "gord_alpha.txt", + "gord_bravo.txt", + "gord_sub/gord_charlie.txt", + "gord_sub/gord_mike.txt", + "gord_zeta.txt", + }; + + auto tmp = TempDir {}; + fs::create_directory(tmp.path() / "gord_sub"); + for (auto path : paths) { + tmp.create_file(path); + } + for (auto i = paths.size(); i-- > 0;) { + (void)global_pool().intern(paths[i]); + } + + auto expanded = [&](std::string_view pattern) { + auto matches = glob_expand(pattern, tmp.path().string()); + REQUIRE(matches.has_value()); + auto result = std::vector {}; + for (auto id : *matches) { + result.push_back(sv(id)); + } + return result; + }; + + WHEN("a plain pattern is expanded") + { + THEN("the matches are in lexicographic path order") + { + REQUIRE(expanded("*.txt") == std::vector { paths[0], paths[1], paths[4] }); + } + } + + WHEN("a recursive pattern is expanded") + { + THEN("the matches are in lexicographic path order") + { + REQUIRE(expanded("**/*.txt") == std::vector(paths.begin(), paths.end())); + } + } + } +}