Skip to content

Merge filesystem and generated glob matches into one path-ordered list (fixes #177, fixes #178) - #180

Merged
typeless merged 2 commits into
mainfrom
fix/177-178-glob-merge
Jul 27, 2026
Merged

Merge filesystem and generated glob matches into one path-ordered list (fixes #177, fixes #178)#180
typeless merged 2 commits into
mainfrom
fix/177-178-glob-merge

Conversation

@typeless

Copy link
Copy Markdown
Owner

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 (#178). 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 (#177).

Both defects 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
before  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
        build 3:             up to date

after   build 1 (clean):     alpha.gen mike.gen zeta.gen
        build 2 (no change): Nothing to do

%f is hashed by compute_command_identity, so the before-column is command identity moving under an unchanged project — the #166/#167 failure class, reached through the input set rather than through the hash.

Fix

Both sources feed one list, 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 against real tup, which emits alpha.o mike.o zeta.o for the same fixture.

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, not only for patterns matching nothing on disk — a linear scan of the file arena per pattern. tup answers the same question from an index; if this shows up in profiles, a per-directory index of generated nodes is the fix.

Tests

Two fixtures, both observed failing first: matches.txt read zeta.gen mike.gen alpha.gen for the ordering scenario, and kept.gen alone for the mixed source/generated one.

make test green (142321 assertions, 618 cases, 32 e2e shards); make tidy and make iwyu clean; bootstrap scripts unchanged.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TLob4Ef3qyMd3Dnp9q3XmA

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TLob4Ef3qyMd3Dnp9q3XmA
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

PR metrics

Performance (gcc example, Linux)

Workload Instructions CPU time Page faults D1 miss LL miss Wall Peak RSS
parse 1074 M (+0.1%) 0.29 s 14.1 k 0.6% (-0.2pp) 0% (-0.1pp) 0.292 s 30.7 MB (-0.2MB)
dry-run 1792 M (+0.3%) 0.34 s 16.6 k 0.7% (-0.2pp) 0% (-0.1pp) 0.356 s 40.1 MB (+0.3MB)

Deterministic signals: instructions (cachegrind-simulated instruction reads — exact across runs, no PMU needed), page faults, peak RSS, and the cachegrind D1/LL miss rates. CPU time is user+sys from time(1).

Internal statistics (gcc example, up-to-date dry run)

Metric Value
Tupfiles parsed 24
Commands 3545
Commands scheduled 0
Files checked 5818
Files changed 0
Files in index 6087
Graph edges 367913
Index size (bytes) 7264970
Implicit deps 330624
Hash computations 0
Hashes skipped (stat cache) 5818
Stat calls 5869
Parse time (ms) 299.9
Total time (ms) 376.3
Runner CPU INTEL(R) XEON(R) PLATINUM 8573C

Counters from putup -n --stat on the fully-built gcc example (up-to-date dry run): deterministic work measures — a jump in commands scheduled, hash computations, or stat calls is a real behavior change, not noise. Timings are the minimum over repeated runs, compared only against a baseline from the same CPU model; the counters are the regression signal.
Timing deltas suppressed: baseline ran on different hardware (AMD EPYC 9V74 80-Core Processor).

Binary size (Linux)

Binary .text .data .bss File
putup 497.8 KB (+0.8%) 1.7 KB 98.7 KB 588.3 KB (+0.9%)

Test coverage (lines)

Overall Median file Min file Max file
86.6% 94.9% 0.0% src/platform/path-posix.cpp 100.0% include/pup/core/arena.hpp

103 files · 14988/17315 lines covered

Deltas vs main@1738aa715.

Updated for d85186d

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TLob4Ef3qyMd3Dnp9q3XmA
@typeless

Copy link
Copy Markdown
Owner Author

Correction to this PR's description, verified on a branch with all four PRs merged.

The description says making request_demand_driven_parse unconditional removes the dependence of the generated half on parse-fixpoint progress. It does not. It parses the pattern's directory, not the directory that produces the match. When the consuming directory sorts before the producing one, the generated node does not exist yet and the glob still resolves against a partially-built graph:

project: z/ globs ../b/*.q ; a/ generates ../b/gen.q   -> works (producer parses first)
project: m/ globs ../b/*.q ; n/ generates ../b/gen.q   -> does not

  build 1:  cat  > out.txt              <- %f empty, generated node not yet in the graph
  build 2:  cat ../b/gen.q > out.txt    <- filesystem half sees it now; identity changed
  build 3:  up to date

So for cross-directory generated matches, %f and command identity still differ between the first and second build of an unchanged project. What this PR does fix is real — the shadowing (#178), the ordering (#177), and the duplicate-spelling regression — but the fixpoint claim is too strong, and the residue belongs to a root cause none of these PRs touch: identity is hashed from the rendered command text, so it inherits whatever impurity the parse-time resolution has.

Filing that as a design issue rather than leaving the claim standing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant