diff --git a/include/pup/index/format.hpp b/include/pup/index/format.hpp index e2f697a6..c05bd157 100644 --- a/include/pup/index/format.hpp +++ b/include/pup/index/format.hpp @@ -45,7 +45,12 @@ inline constexpr auto INDEX_MAGIC = std::array { 'P', 'U', 'P', 'I' }; /// 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 }; +/// 17 - Glob expansion merges filesystem and generated matches into one list +/// instead of letting a filesystem match suppress the generated ones, so +/// both the membership and the order of %f changed for any pattern a +/// generated file matches (issues #177, #178); v16 identities no longer +/// join for those commands. +inline constexpr auto INDEX_VERSION = std::uint32_t { 17 }; /// Index file header (56 bytes) - v9 struct alignas(8) RawHeader { diff --git a/src/graph/builder.cpp b/src/graph/builder.cpp index 883e204a..66721136 100644 --- a/src/graph/builder.cpp +++ b/src/graph/builder.cpp @@ -652,8 +652,12 @@ auto format_condition_expr(parser::EvalContext& eval, parser::Conditional const& // ยง6 โ€” Glob expansion // --------------------------------------------------------------------------- -/// Expand a glob pattern against filesystem and graph nodes. +/// Expand a glob pattern against the filesystem and the graph's generated nodes. /// Adds matched paths to result vector. +/// +/// Both sources feed one path-ordered list. A match on disk must not hide a +/// generated match, or the input set โ€” and the identity hashing %f โ€” would be a +/// function of filesystem state rather than of the project (issue #178). auto expand_glob_pattern( BuilderContext& ctx, std::string_view path, @@ -664,18 +668,18 @@ auto expand_glob_pattern( auto base_sv = is_empty(ctx.current_dir) ? str(ctx.options.source_root) : pool.get(pup::path::join(str(ctx.options.source_root), str(ctx.current_dir))); - auto expanded = parser::glob_expand(path, base_sv); - if (expanded && !expanded->empty()) { + auto matches = Vec {}; + + // Normalized into the generated half's path space: join() does not normalize, + // so a ../ pattern spells the same file two ways and dedup keeps both. + if (auto expanded = parser::glob_expand(path, base_sv)) { for (auto p : *expanded) { - if (!is_empty(ctx.current_dir)) { - result.push_back(pup::path::join(str(ctx.current_dir), str(p))); - } else { - result.push_back(p); - } + matches.push_back(is_empty(ctx.current_dir) ? pup::path::normalize(str(p)) : pup::path::normalize(pool.get(pup::path::join(str(ctx.current_dir), str(p))))); } - return; } + // Unconditional: the generated half of the match set would otherwise depend on + // how far the parse fixpoint has progressed. auto pattern_dir = pup::path::parent(path); auto abs_pattern_dir_sv = pool.get(pup::path::normalize(pool.get(pup::path::join(str(ctx.current_dir), pattern_dir)))); request_demand_driven_parse(*ctx.eval, abs_pattern_dir_sv); @@ -690,9 +694,16 @@ auto expand_glob_pattern( } auto match_path_sv = pool.get(pup::strip_path_prefix(node_path_sv, build_root_name)); if (glob.matches(match_path_sv)) { - result.push_back(pool.intern(node_path_sv)); + matches.push_back(pool.intern(node_path_sv)); } } + + // In-tree builds see a generated file from both sources; it is one input. + std::ranges::sort(matches, {}, [&pool](StringId id) { return pool.get(id); }); + matches.erase(std::unique(matches.begin(), matches.end()), matches.end()); + for (auto id : matches) { + result.push_back(id); + } } /// Apply exclusion patterns to filter out paths from the result. diff --git a/test/e2e/fixtures/glob_generated/Tupfile.fixture b/test/e2e/fixtures/glob_generated/Tupfile.fixture new file mode 100644 index 00000000..09857a7f --- /dev/null +++ b/test/e2e/fixtures/glob_generated/Tupfile.fixture @@ -0,0 +1,2 @@ +: foreach zeta.in mike.in alpha.in |> cp %f %o |> %B.gen +: *.gen |> echo %f > %o |> matches.txt diff --git a/test/e2e/fixtures/glob_generated/Tupfile.ini b/test/e2e/fixtures/glob_generated/Tupfile.ini new file mode 100644 index 00000000..e69de29b diff --git a/test/e2e/fixtures/glob_generated/alpha.in b/test/e2e/fixtures/glob_generated/alpha.in new file mode 100644 index 00000000..4a580070 --- /dev/null +++ b/test/e2e/fixtures/glob_generated/alpha.in @@ -0,0 +1 @@ +alpha diff --git a/test/e2e/fixtures/glob_generated/mike.in b/test/e2e/fixtures/glob_generated/mike.in new file mode 100644 index 00000000..f4692f5e --- /dev/null +++ b/test/e2e/fixtures/glob_generated/mike.in @@ -0,0 +1 @@ +mike diff --git a/test/e2e/fixtures/glob_generated/zeta.in b/test/e2e/fixtures/glob_generated/zeta.in new file mode 100644 index 00000000..fd08df0a --- /dev/null +++ b/test/e2e/fixtures/glob_generated/zeta.in @@ -0,0 +1 @@ +zeta diff --git a/test/e2e/fixtures/glob_generated_and_source/Tupfile.fixture b/test/e2e/fixtures/glob_generated_and_source/Tupfile.fixture new file mode 100644 index 00000000..16cb5796 --- /dev/null +++ b/test/e2e/fixtures/glob_generated_and_source/Tupfile.fixture @@ -0,0 +1,2 @@ +: foreach zeta.in alpha.in |> cp %f %o |> %B.gen +: *.gen |> echo %f > %o |> matches.txt diff --git a/test/e2e/fixtures/glob_generated_and_source/Tupfile.ini b/test/e2e/fixtures/glob_generated_and_source/Tupfile.ini new file mode 100644 index 00000000..e69de29b diff --git a/test/e2e/fixtures/glob_generated_and_source/alpha.in b/test/e2e/fixtures/glob_generated_and_source/alpha.in new file mode 100644 index 00000000..4a580070 --- /dev/null +++ b/test/e2e/fixtures/glob_generated_and_source/alpha.in @@ -0,0 +1 @@ +alpha diff --git a/test/e2e/fixtures/glob_generated_and_source/kept.gen b/test/e2e/fixtures/glob_generated_and_source/kept.gen new file mode 100644 index 00000000..3ffd3771 --- /dev/null +++ b/test/e2e/fixtures/glob_generated_and_source/kept.gen @@ -0,0 +1 @@ +checked-in diff --git a/test/e2e/fixtures/glob_generated_and_source/zeta.in b/test/e2e/fixtures/glob_generated_and_source/zeta.in new file mode 100644 index 00000000..fd08df0a --- /dev/null +++ b/test/e2e/fixtures/glob_generated_and_source/zeta.in @@ -0,0 +1 @@ +zeta diff --git a/test/e2e/fixtures/glob_parent_dir/Tupfile.ini b/test/e2e/fixtures/glob_parent_dir/Tupfile.ini new file mode 100644 index 00000000..e69de29b diff --git a/test/e2e/fixtures/glob_parent_dir/a/Tupfile.fixture b/test/e2e/fixtures/glob_parent_dir/a/Tupfile.fixture new file mode 100644 index 00000000..301df31a --- /dev/null +++ b/test/e2e/fixtures/glob_parent_dir/a/Tupfile.fixture @@ -0,0 +1 @@ +: |> echo x > %o |> ../b/gen.q diff --git a/test/e2e/fixtures/glob_parent_dir/b/keep.txt b/test/e2e/fixtures/glob_parent_dir/b/keep.txt new file mode 100644 index 00000000..121fc7f1 --- /dev/null +++ b/test/e2e/fixtures/glob_parent_dir/b/keep.txt @@ -0,0 +1 @@ +not a match diff --git a/test/e2e/fixtures/glob_parent_dir/z/Tupfile.fixture b/test/e2e/fixtures/glob_parent_dir/z/Tupfile.fixture new file mode 100644 index 00000000..3af85260 --- /dev/null +++ b/test/e2e/fixtures/glob_parent_dir/z/Tupfile.fixture @@ -0,0 +1 @@ +: ../b/*.q |> cat %f > %o |> out.txt diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 1ca228e4..35e7c3bf 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -8182,3 +8182,88 @@ SCENARIO("A group reference composes a nested project into the build", "[e2e][ex } } } + +SCENARIO("A glob over generated files is path-ordered and stable across builds", "[e2e][glob]") +{ + GIVEN("a rule globbing files its neighbours generate, declared out of alphabetical order") + { + auto f = E2EFixture { "glob_generated" }; + REQUIRE(f.init().success()); + REQUIRE(f.build().success()); + + THEN("the matches are in path order, not rule order") + { + REQUIRE(f.read_file("matches.txt") == "alpha.gen mike.gen zeta.gen\n"); + } + + // The generated files exist on disk from here on, so a filesystem glob can + // now see what only the graph could see during the first build. + WHEN("the unchanged project is rebuilt") + { + auto second = f.build(); + + THEN("nothing re-runs") + { + INFO("stdout: " << second.stdout_output); + REQUIRE(second.is_noop()); + } + + THEN("the matches are unchanged") + { + REQUIRE(f.read_file("matches.txt") == "alpha.gen mike.gen zeta.gen\n"); + } + } + } +} + +SCENARIO("A glob matches source and generated files together", "[e2e][glob]") +{ + GIVEN("a pattern matching both a checked-in file and two generated ones") + { + auto f = E2EFixture { "glob_generated_and_source" }; + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("every match is an input, in path order") + { + INFO("stdout: " << result.stdout_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("matches.txt") == "alpha.gen kept.gen zeta.gen\n"); + } + } + } +} + +SCENARIO("A glob reaching into a sibling directory counts each match once", "[e2e][glob]") +{ + GIVEN("a pattern spelled ../b/*.q whose match is generated by a third directory") + { + auto f = E2EFixture { "glob_parent_dir" }; + REQUIRE(f.init().success()); + REQUIRE(f.build().success()); + + THEN("the generated file is one input, not one per spelling") + { + REQUIRE(f.read_file("z/out.txt") == "x\n"); + } + + WHEN("the unchanged project is rebuilt") + { + auto second = f.build(); + + THEN("nothing re-runs") + { + INFO("stdout: " << second.stdout_output); + REQUIRE(second.is_noop()); + } + + THEN("the input is still counted once") + { + REQUIRE(f.read_file("z/out.txt") == "x\n"); + } + } + } +}