Glob matches are compared across two path spaces, so out-of-tree %f order, dedup and exclusions are all wrong

[typeless]

Summary

expand_glob_pattern (src/graph/builder.cpp:661) merges two sources of matches into one list: a filesystem scan, whose paths are source-relative, and a scan of Generated graph nodes, whose paths carry the build-root prefix (:697). Out-of-tree those are different path spaces, and everything downstream compares the merged list as plain strings:

• the sort (:702)
• the dedup (:703)
• apply_exclusions (:711)

In-tree the two spaces coincide, which is why the entire E2E suite misses this: every fixture builds in-tree (test/e2e/e2e_fixture.cpp:158 runs a bare configure).

PR #180 introduced the merge and a follow-up commit normalized the ../ component lexically. That handled one axis; the build-root prefix is the other, and it was left.

Three consequences, all reproduced on a47fe41f5

1. %f order depends on the name of the build directory.

a/Tupfile:  : src.in |> cp %f %o |> gen.dat
            : *.dat  |> cat %f > %o |> all.txt        # a/keep.dat is checked in

putup -B AAA   ->  cat ../AAA/a/gen.dat keep.dat > ../AAA/a/all.txt     all.txt: GEN\nKEEP
putup -B zz    ->  cat keep.dat ../zz/a/gen.dat > ../zz/a/all.txt       all.txt: KEEP\nGEN

"../AAA/a/gen.dat" sorts before "keep.dat"; "../zz/a/gen.dat" sorts after. Same project, different artifact bytes, decided by a directory name. Link order, cat concatenation and archive member order all shift between an in-tree dev build and an out-of-tree CI build.

2. A generated file that shadows a same-named source appears twice.

a/Tupfile:  : gen.src |> cp %f %o |> x.dat
            : *.dat   |> cat %f > %o |> all.txt       # a/x.dat is also checked in

putup -B build  ->  cat ../build/a/x.dat ../build/a/x.dat > ../build/a/all.txt
all.txt: FROMRULE\nFROMRULE

The filesystem half yields a/x.dat, the graph half build/a/x.dat; the string dedup keeps both, and resolve_input_node maps both to the same Generated node. The checked-in source is silently never consumed. Second build is a no-op, so it stays wrong.

3. Input exclusions silently stop applying.

: keep.in |> cp %f %o |> keep.gen
: skip.in |> cp %f %o |> skip.gen
: *.gen !skip.gen |> echo %f > %o |> out.txt

in-tree:      out.txt = keep.gen                        exclusion honoured
out-of-tree:  out.txt = build/keep.gen build/skip.gen   exclusion ignored

Exclusion strings are normalized source-relative and compared for equality, so they never match a prefixed generated path. This one predates #180 — the old fallback branch pushed the same prefixed paths — but #180 made the generated half contribute to every pattern rather than only to patterns the filesystem missed, so the exposure went from narrow to universal.

One fix

Emit the graph half in the stripped logical (source-relative) space at the merge point — match_path_sv is already computed at :695 for the match itself — and let resolve_input_node's BuildRoot-first lookup (:376) carry it back to the Generated node. One path space downstream makes the existing sort, dedup and exclusion comparison correct, and restores the ordering that PR #180 measured against tup, which serves both node kinds from one table in one space.

Test-suite gap

Worth fixing alongside: no E2E fixture builds out-of-tree, so no test in the suite can observe any of this. The three cases above are cheap to add as out-of-tree scenarios.

Provenance

Found by an adversarial review of the merged PRs in the incremental-correctness campaign; all three reproduced independently before filing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TLob4Ef3qyMd3Dnp9q3XmA

[typeless]

Correction to the "Test-suite gap" section above: I wrote that no E2E fixture builds out-of-tree. That is wrong — test/unit/test_e2e.cpp uses -B build in 265 places, so out-of-tree coverage is extensive. I took the claim from a review report and repeated it without checking; the defects themselves were all verified independently and stand.

The actual gap is narrower and worth stating precisely, because it is what a regression test has to cover: no scenario exercises a glob that matches a generated file and a source file in the same directory, built out-of-tree. That is the only shape where the two path spaces meet and disagree — the sort compares ../build/a/gen.dat against keep.dat, the dedup compares a/x.dat against build/a/x.dat, and an exclusion normalized source-relative is compared against a build-prefixed path.

So the three regression tests should each be that shape, out-of-tree:

1. a glob matching one generated and one source file, asserting %f order is identical under two differently-named build directories;
2. a generated file whose name shadows a checked-in source file matched by the same glob, asserting it appears once;
3. an exclusion naming a generated file, asserting it is honoured.