From feeb0369ba37ea376c7bdd3860ce869621508676 Mon Sep 17 00:00:00 2001 From: Mura Li <2606021+typeless@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:43:49 +0800 Subject: [PATCH 1/2] Merge filesystem and generated glob matches into one path-ordered list (fixes #177, fixes #178) expand_glob_pattern expanded a pattern two ways and treated the second as a fallback: if the filesystem glob matched anything it returned early, so one file on disk discarded every generated match for that pattern. The generated scan also pushed matches in nodes_of_type order -- arena order, i.e. node creation order -- with no sort, while #174 had just made the filesystem branch path-ordered. Both defects are visible in one build. With putup's default in-tree layout the generated files land beside the sources, so the branch flips between the first and second build of an unchanged project: : foreach zeta.in mike.in alpha.in |> cp %f %o |> %B.gen : *.gen |> echo %f > %o |> matches.txt build 1 (clean): zeta.gen mike.gen alpha.gen graph branch, creation order build 2 (no change): alpha.gen mike.gen zeta.gen filesystem branch, path order and the command re-runs to produce it %f is hashed by compute_command_identity, so that is command identity moving under an unchanged project -- the #166/#167 failure class, reached through the input set rather than the hash. Now both sources feed one list that is sorted by path and deduped (an in-tree build sees a generated file from both sides; it is one input). This matches tup, which serves file and generated nodes from a single query over node_dir_index (dir, name) -- measured: for the fixture above, tup emits alpha.o mike.o zeta.o. request_demand_driven_parse becomes unconditional. It ran only on the fallback path, so with a merge the generated half of the match set would otherwise depend on how far the parse fixpoint had progressed. Cost: the generated-node scan now runs for every glob rather than only for patterns that match nothing on disk. It is a linear scan of the file arena per pattern; tup answers the same question from an index. Both scenarios were observed failing first: matches.txt read "zeta.gen mike.gen alpha.gen" for the ordering fixture and "kept.gen" alone for the mixed source/generated one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01TLob4Ef3qyMd3Dnp9q3XmA --- src/graph/builder.cpp | 29 ++++++---- .../fixtures/glob_generated/Tupfile.fixture | 2 + test/e2e/fixtures/glob_generated/Tupfile.ini | 0 test/e2e/fixtures/glob_generated/alpha.in | 1 + test/e2e/fixtures/glob_generated/mike.in | 1 + test/e2e/fixtures/glob_generated/zeta.in | 1 + .../glob_generated_and_source/Tupfile.fixture | 2 + .../glob_generated_and_source/Tupfile.ini | 0 .../glob_generated_and_source/alpha.in | 1 + .../glob_generated_and_source/kept.gen | 1 + .../glob_generated_and_source/zeta.in | 1 + test/unit/test_e2e.cpp | 54 +++++++++++++++++++ 12 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 test/e2e/fixtures/glob_generated/Tupfile.fixture create mode 100644 test/e2e/fixtures/glob_generated/Tupfile.ini create mode 100644 test/e2e/fixtures/glob_generated/alpha.in create mode 100644 test/e2e/fixtures/glob_generated/mike.in create mode 100644 test/e2e/fixtures/glob_generated/zeta.in create mode 100644 test/e2e/fixtures/glob_generated_and_source/Tupfile.fixture create mode 100644 test/e2e/fixtures/glob_generated_and_source/Tupfile.ini create mode 100644 test/e2e/fixtures/glob_generated_and_source/alpha.in create mode 100644 test/e2e/fixtures/glob_generated_and_source/kept.gen create mode 100644 test/e2e/fixtures/glob_generated_and_source/zeta.in diff --git a/src/graph/builder.cpp b/src/graph/builder.cpp index 883e204a..85e86f5c 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,16 @@ 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 {}; + + 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) ? p : 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 +692,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/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 1ca228e4..fa8e1fae 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -8182,3 +8182,57 @@ 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"); + } + } + } +} From d85186d2e74a6ffe4c08fdec36fc60acfd6f1eb6 Mon Sep 17 00:00:00 2001 From: Mura Li <2606021+typeless@users.noreply.github.com> Date: Mon, 27 Jul 2026 15:14:54 +0800 Subject: [PATCH 2/2] Normalize filesystem glob matches into the generated half's path space The merge in the previous commit deduped on raw strings from two different path spaces. pup::path::join does not normalize, so for a pattern spelled ../b/*.q the filesystem half pushed "z/../b/gen.q" while the generated half pushed the graph's "b/gen.q". std::unique compares StringId handles, so both survived and the rule received the same file twice: build 1: cat ../b/gen.q > out.txt (generated half only) build 2: cat ../b/gen.q ../b/gen.q > out.txt (both halves, two spellings) With gcc %f or ld %f that is a multiple-definition failure. Normalizing the filesystem half conforms to expand_inputs, which already normalizes the same way one call up (builder.cpp:955). Found by adversarial review; no fixture used a ../ or ./ glob, so CI stayed green. INDEX_VERSION 16 -> 17: the merge changes both the membership and the order of %f for any pattern a generated file matches, so v16 identities no longer join for those commands. PR #174 set this rule three commits back for the same class of change; leaving it at 16 was a silent divergence from it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01TLob4Ef3qyMd3Dnp9q3XmA --- include/pup/index/format.hpp | 7 ++++- src/graph/builder.cpp | 4 ++- test/e2e/fixtures/glob_parent_dir/Tupfile.ini | 0 .../glob_parent_dir/a/Tupfile.fixture | 1 + test/e2e/fixtures/glob_parent_dir/b/keep.txt | 1 + .../glob_parent_dir/z/Tupfile.fixture | 1 + test/unit/test_e2e.cpp | 31 +++++++++++++++++++ 7 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 test/e2e/fixtures/glob_parent_dir/Tupfile.ini create mode 100644 test/e2e/fixtures/glob_parent_dir/a/Tupfile.fixture create mode 100644 test/e2e/fixtures/glob_parent_dir/b/keep.txt create mode 100644 test/e2e/fixtures/glob_parent_dir/z/Tupfile.fixture 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 85e86f5c..66721136 100644 --- a/src/graph/builder.cpp +++ b/src/graph/builder.cpp @@ -670,9 +670,11 @@ auto expand_glob_pattern( 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) { - matches.push_back(is_empty(ctx.current_dir) ? p : pup::path::join(str(ctx.current_dir), str(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))))); } } 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 fa8e1fae..35e7c3bf 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -8236,3 +8236,34 @@ SCENARIO("A glob matches source and generated files together", "[e2e][glob]") } } } + +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"); + } + } + } +}