Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughChangesBuild header dependency tracking
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 10:02 AM PT - Aug 13th, 2026
✅ @robobun, your commit 900fb0a84a3fec112338a593cb9fd9a4af29318d passed in 🧪 To try this PR locally: bunx bun-pr 37992That installs a local version of the PR into your bun-37992 --bun |
|
Status: fixed, waiting on CI (current head 900fb0a).
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — focused build-system fix that makes depfile entries string-match declared codegen outputs so the PCH rebuilds in the same run.
What was reviewed:
includeFlags()incompile.ts: verifiedn.rel()semantics (returns""for buildDir, native-sep relative otherwise); theisAbsolute(rel) || rel === ".." || rel.startsWith(".."+sep)guard correctly keeps out-of-tree/cache dirs absolute and handles the..odd-named-subdir edge case the test pins.BunBuiltinNames+extras.hdeclaration: confirmedbundle-functions.tswrites it unconditionally withwriteIfNotChanged, so declaring it as an output is safe for restat and won't fail on a fresh build.compile_commands.jsonstill resolves: entries usedirectory: cfg.buildDir, so relative-Icodegenis interpreted correctly by clangd.- Test follows the existing
test/internal/build-*.test.tspattern (pure ninja-emission, no compiler/subprocess).
Extended reasoning...
Overview
This PR fixes a two-build-convergence bug in the ninja build graph: on a warm tree, regenerating a codegen header that root-pch.h includes did not rebuild the PCH in the same run, because the PCH's depfile recorded the header under an absolute path (-I/abs/build/debug/codegen) while the codegen edge declares it buildDir-relative (codegen/X.h), and ninja matches the two by string. The fix introduces includeFlags() in compile.ts to spell buildDir-internal -I dirs the way ninja.ts spells declared outputs, routes bunCompileFlags() in bun.ts through it, and declares the previously-undeclared BunBuiltinNames+extras.h output in emitJsModules. Comments in bun.ts/codegen.ts/compile.ts/CLAUDE.md are updated to state the actual invariant. A pure ninja-emission test pins all three properties.
Files touched: scripts/build/{compile.ts, bun.ts, codegen.ts, CLAUDE.md} and a new test/internal/build-codegen-header-tracking.test.ts. No runtime or user-facing code.
Security risks
None. This is entirely build-graph/flag-assembly logic in scripts/build/; it touches no runtime, network, auth, or user-input code paths.
Level of scrutiny
Medium — build-system correctness, not shipped runtime code. The blast radius is developer/CI incremental-rebuild behavior. The worst realistic failure mode is a mis-spelled -I breaking a compile, which CI catches immediately; the more subtle risk (over- or under-invalidation) is exactly what the PR is fixing, and the description shows ninja -t deps/-t query transcripts demonstrating one-build convergence and restat pruning both still hold. Windows uses deps: msvc (/showIncludes) rather than gcc-style depfiles, so the string-match mechanism differs there — but relative -I paths are still valid for clang-cl running from buildDir, so the change is at worst a no-op on that platform.
Other factors
- The refactor is behavior-preserving apart from the
-Ispelling:bunCompileFlags()produces the same include set (bunIncludes(cfg) + buildDir + depIncludes) and defines as before, only routed throughincludeFlags().ComputedFlagswas already exported fromflags.ts. - Verified
n.rel()inninja.tsreturnsrelative(buildDir, path)(""for buildDir itself), matching the-I.special case; and thatbundle-functions.ts:841writesBunBuiltinNames+extras.hinside an unconditional block, so declaring it as an output cannot leave ninja waiting on a file that isn't produced. compile_commands.jsonentries carrydirectory: cfg.buildDir, so clangd resolves the now-relative-Icodegen/-I.correctly.- The new test follows the four existing
test/internal/build-*.test.tsfiles' pattern (mockToolchain,resolveConfigwith a fake sysroot, directNinjaconstruction, no subprocess/compiler), and the PR description states it fails with each half of the fix reverted independently. - Documented cost (one full C++ recompile on warm trees due to the flag-line change) is unavoidable and clearly called out.
…codegen edges (#38035) ### Problem - On a warm tree, editing a `.bind.ts` or `.bindv2.ts` file so that its generated header changes leaves every hand-written TU that includes the header unbuilt for one `bun bd`; the second `bun bd` compiles it. Probe on `build/debug` (full transcript below): append a `fn` to `src/runtime/api/BunObject.bind.ts`, build `obj/src/jsc/bindings/BunObject.cpp.o`, and the run is `[1/1] gen .bind.ts -> GeneratedBindings.cpp` while `ninja -n` afterwards still lists `cxx obj/src/jsc/bindings/BunObject.cpp.o`. The first build therefore links a `BunObject.cpp.o` compiled against the old `GeneratedBunObject.h` (old struct layouts, old function set) with a fresh `GeneratedBindings.cpp.o`. - Cause: none of the `Generated*.h` files is a declared output of any ninja edge. `emitBindgen` (`scripts/build/codegen.ts`) declares only `GeneratedBindings.cpp`, while `bindgen.ts` also writes one `Generated<Name>.h` per `.bind.ts` (`src/codegen/bindgen.ts:1665`); bindgenv2's `list-outputs` (`src/codegen/bindgenv2/script.ts:56`) reports only `cppSourcePath`, while `generate()` also writes `cppHeaderPath` for every type, and `emitBindgenV2` asserted that nothing but `.cpp` comes back. On a warm `build/debug`, `ninja -t query codegen/<name>` shows no producing edge for all 7 bindgen headers and all 9 bindgenv2 headers, and `ninja -t deps obj/src/jsc/bindings/BunObject.cpp.o` lists `codegen/GeneratedBunObject.h`. - Why that makes the TU lag: compiles are order-only on codegen and rely on the depfile to be re-dirtied. Ninja re-checks a depfile entry after the edge that produces it ran only when the entry names a declared output; an entry no edge declares is a source file to ninja, stat'd once at startup, so the header rewritten later in the same run is only seen by the next run (the invariant `bun.ts` states for `cppAll`, "those headers ARE declared ninja outputs with restat, so depfile tracking is exact", did not hold for these). - Affected includers: `BunObject.cpp` and `ZigGlobalObject.cpp` (`GeneratedBunObject.h`), `NodeModuleModule.cpp` (`GeneratedNodeModuleModule.h`), `GeneratedBindings.cpp` (`GeneratedBunObject.h`, `GeneratedNodeOs.h`, `GeneratedFmtJsc.h`), and each generated bindgenv2 `.cpp`, which includes its own header and, through it, the header-only union types (`GeneratedSSLConfig.cpp` -> `GeneratedSSLConfigFile.h`, `GeneratedALPNProtocols.h`); a union has no `.cpp`, so until now it had no declared output at all. ### Fix - `src/codegen/bindgenv2/script.ts`: `list-outputs` reports `cppHeaderPath` for every type with a header, mirroring what `generate()` writes (`hasCppHeader` is the cheap twin of `cppHeader` that `base.ts` defines for this purpose). - `scripts/build/codegen.ts`, `emitBindgenV2`: accepts `.h` entries from `list-outputs` (still rejecting anything else), declares them on the edge with the `.cpp` files, and pushes them into `cppHeaders`, so they reach `cppAll` like the other generated headers. - `scripts/build/codegen.ts`, `emitBindgen`: declares `Generated<Stem>.h` for every file in `sources.bindgen` next to `GeneratedBindings.cpp` and pushes them into `cppHeaders`. The stem transform is the one `bindgen.ts` uses (`pascal()` in `bindgen-lib-internal.ts`, `node_os` -> `NodeOs`), derived at configure time like the node-fallbacks and string-map outputs are; the header set is a function of the file list the build already globs, so a configure-time spawn is not needed (bindgenv2 needs one because its set depends on what the files export). The test below pins the two sides to each other against the real `bindgen.ts`. - The file's "Undeclared outputs" comment now states the rule (a file that gets compiled or included has to be declared) and what is still legitimately undeclared. `emitBindgen`/`emitBindgenV2` are exported for the test. - Why this is the right fix: the build's documented design for generated headers is declared output + `restat` + order-only + depfile; these headers were the ones outside it, and declaring them makes them follow that design. Both scripts write with `writeIfNotChanged`, so `restat` still prunes: a rerun that leaves the headers unchanged recompiles nothing (checked below). No compile command changes, so existing trees do not recompile anything; each of the two gen steps runs once more (their new outputs have no build-log entry yet) and is pruned from there. If a future `.bind.ts` were named so that its header collided with another step's output, configure would now fail with ninja.ts's duplicate-output error instead of two steps silently writing the same file. - Verified: - `test/internal/build-codegen-declared-outputs.test.ts`: `emitBindgenV2` run on a probe `.bindv2.ts` (one union, one enumeration) declares exactly `{GeneratedProbeEnum.cpp, GeneratedProbeEnum.h, GeneratedProbeUnion.h}` on one edge with the headers in `cppHeaders`; `list-outputs` on the probe names exactly what `generate` writes; `emitBindgen` declares `GeneratedNodeOs.h`/`GeneratedBunObject.h` next to `GeneratedBindings.cpp`; and for the repo's real `.bind.ts` files its declared set equals exactly the files `bindgen.ts` writes into a scratch dir. With `src/` stashed the two bindgenv2 tests fail (2 headers missing); with `scripts/` stashed configure fails on the old `.cpp`-only assertion; removing either `cppHeaders.push` fails the corresponding emission test. - Probe above on the unfixed tree: gen only, cxx deferred to the next run. Fixed tree: same edit runs gen + cxx in one invocation, `ninja -n` is then empty, reverting the edit does the same, and a `touch` of the `.bind.ts` runs gen alone (restat prunes the cxx). Transcript below. - After the change `ninja -t query` shows a producing edge for every `codegen/Generated*.h`; what remains undeclared in `build/debug/codegen` is `.d.ts` files, `JSSink.lut.txt`, `bake_empty_file`, the `eval/` dir, the configure-time writes, and `BunBuiltinNames+extras.h` (the PCH case #37992 handles; the two changes are independent). - `build.ninja` before/after differs only in the two gen edges, the `codegen` phony and the order-only lists that carry `cppAll`; `test/internal/build-post-link-ordering.test.ts` and `build-debug-info-flags.test.ts` still pass; `tsc -p scripts/build` reports the same pre-existing diagnostics as main. ### Background - **Order-only input** (`||` in ninja): has to exist before the edge runs, but its mtime never dirties the edge. The build uses it for the codegen headers so that a compile is only rebuilt for the headers it really includes. - **Depfile** (`deps = gcc`): the list of files clang read while compiling a TU; ninja stores it and treats each entry as an implicit input of that compile on later runs. Whether an entry can re-dirty the compile within the run that rewrites it depends on whether some edge declares the entry as an output: declared outputs are re-checked after their edge runs, anything else is stat'd once at startup. - **restat**: after an edge runs, ninja re-stats its outputs and drops downstream work for outputs whose mtime did not move; `writeIfNotChanged` in the codegen scripts is what keeps the mtimes still. This is why declaring more outputs does not cause extra recompiles. - **bindgen** (`src/codegen/bindgen.ts`, `*.bind.ts`): generates the C++ side of native functions; one `GeneratedBindings.cpp` plus a `Generated<Name>.h` per file holding the function pointers and struct definitions the hand-written C++ uses. **bindgenv2** (`src/codegen/bindgenv2/`, `*.bindv2.ts`): generates a conversion header per named type, plus a `.cpp` for dictionaries and enumerations; unions are header-only. Its output set depends on what the files export, so configure asks the script with `--command=list-outputs`. <details> <summary>Probe transcript (linux-x64, warm build/debug; the edit appends an exported <code>fn</code> to BunObject.bind.ts, which adds a declaration to GeneratedBunObject.h)</summary> Unfixed: ``` $ bun scripts/build.ts --profile=debug --target=obj/src/jsc/bindings/BunObject.cpp.o # baseline ninja: no work to do. $ printf '\nexport const undeclaredHeaderProbe = fn({ args: {}, ret: t.u64 });\n' >> src/runtime/api/BunObject.bind.ts $ bun scripts/build.ts --profile=debug --target=obj/src/jsc/bindings/BunObject.cpp.o [1/1] gen .bind.ts -> GeneratedBindings.cpp $ grep -c jsUndeclaredHeaderProbe build/debug/codegen/GeneratedBunObject.h 2 <- header rewritten... $ ninja -C build/debug -n obj/src/jsc/bindings/BunObject.cpp.o [1/1] cxx obj/src/jsc/bindings/BunObject.cpp.o <- ...but its includer only recompiles next time $ ninja -C build/debug -t query codegen/GeneratedBunObject.h codegen/GeneratedBunObject.h: outputs: <- no "input:" edge produces it ``` Fixed (tree converged first, `ninja: no work to do`): ``` $ printf '...' >> src/runtime/api/BunObject.bind.ts # same edit $ bun scripts/build.ts --profile=debug --target=obj/src/jsc/bindings/BunObject.cpp.o [1/2] gen .bind.ts -> GeneratedBindings.cpp [2/2] cxx obj/src/jsc/bindings/BunObject.cpp.o $ ninja -C build/debug -n obj/src/jsc/bindings/BunObject.cpp.o ninja: no work to do. $ git checkout src/runtime/api/BunObject.bind.ts $ bun scripts/build.ts --profile=debug --target=obj/src/jsc/bindings/BunObject.cpp.o [1/2] gen .bind.ts -> GeneratedBindings.cpp [2/2] cxx obj/src/jsc/bindings/BunObject.cpp.o $ touch src/runtime/api/BunObject.bind.ts $ bun scripts/build.ts --profile=debug --target=obj/src/jsc/bindings/BunObject.cpp.o [1/2] gen .bind.ts -> GeneratedBindings.cpp <- headers unchanged, cxx pruned by restat ``` The two edges as now written to build.ninja: ``` build codegen/GeneratedBindings.cpp codegen/GeneratedBindgenTest.h codegen/GeneratedFmtJsc.h codegen/GeneratedNodeModuleModule.h codegen/GeneratedBunObject.h codegen/GeneratedBake.h codegen/GeneratedDevServer.h codegen/GeneratedNodeOs.h: codegen ../../src/codegen/bindgen.ts ... build codegen/GeneratedSocketConfigBinaryType.h codegen/GeneratedSocketConfigBinaryType.cpp codegen/GeneratedSocketConfigHandlers.h codegen/GeneratedSocketConfigHandlers.cpp codegen/GeneratedSocketConfig.h codegen/GeneratedSocketConfig.cpp codegen/GeneratedSocketConfigTLS.h codegen/GeneratedALPNProtocols.h codegen/GeneratedSSLConfig.h codegen/GeneratedSSLConfig.cpp codegen/GeneratedSSLConfigFile.h codegen/GeneratedSSLConfigSingleFile.h codegen/GeneratedFakeTimersConfig.h codegen/GeneratedFakeTimersConfig.cpp: codegen ... ``` </details>
…ts in them Codegen headers are order-only inputs of the PCH, cxx and cc edges; the compiler's depfile is what re-dirties a compile when one of them changes. Ninja matches depfile entries to declared outputs by string, and the compiler records a header as "<-I dir>/<name>", so with -I/abs/build/codegen the PCH depended on /abs/build/codegen/X.h, a node no edge produces, which ninja stats once at startup. A codegen rerun in the same run therefore never reached the PCH: the first bun bd after adding a src/js module compiled every TU against a PCH built from the old InternalModuleRegistry+enum.h and failed, and the second bun bd rebuilt the PCH. The pch rule is the one compile rule that does not go through ccache, whose CCACHE_BASEDIR rewriting had been relativizing the flags of every other rule; a plain ninja invocation or a host without ccache had the same one-build lag on every TU. includeFlags() now spells include dirs inside buildDir buildDir-relative (-Icodegen, -Ideps/zlib, -I.), matching ninja.ts, and bunCompileFlags() assembles bun's compile flags through it. Directories outside buildDir stay absolute so ccache entries remain shareable between worktrees. BunBuiltinNames+extras.h, which root-pch.h reaches through BunBuiltinNames.h, is written by bundle-functions.ts but was not a declared output of the JS modules step, so it is declared there; the depfile can only follow a declared output.
…utputs implicit The direct deps that include another dep's generated headers (libarchive, libspng and lsquic all include zlib's zlib.h/zconf.h out of deps/zlib/) had the same shape as the PCH: an absolute -I into the build dir and the headers reachable only as order-only inputs, so a zlib rebuild reached their objects one build late whenever ccache was not rewriting the flags. emitDirect now spells DirectBuild.includes through includeFlags() (libspng moves its hand-written -I from cflags into includes) and takes the fetchDeps producers' outputs as implicit inputs of every object, the way emitNestedCmake and bun.ts's depHeaderSignal already do; only the dep's own source stamp stays order-only. cc()/cxx()/pch() reject an absolute -I into the build dir at configure time, so the spelling is enforced where the depfile is consumed instead of by convention; the full graph passes. The codegen.ts header comment is merged with the one from the bindgen headers change, which landed in the meantime.
d81933d to
6745ed8
Compare
There was a problem hiding this comment.
Beyond the inline nit, I checked the other DirectBuild deps for raw -I<buildDir>/... in cflags that would trip the new assertIncludesSpelledAsDeclared — libspng was the only one (fixed here); boringssl's hand-written -I points at depSourceDir (vendor/), not the build dir. lsquic already routes depBuildDir(cfg, "zlib") through includes:, so it picks up the relative spelling via includeFlags() without changes.
Extended reasoning...
Verified the new configure-time assertion in compile()/pch() won't break any existing dep beyond the libspng case this PR already migrates: grepped scripts/build/deps/ for -I${...} in cflags and for depBuildDir — boringssl's raw -I targets depSourceDir (outside buildDir, passes the prefix filter), and lsquic/libarchive already list their zlib build dir under includes: where emitDirect now runs it through includeFlags(). Also confirmed the nasm-branch nit is latent: the two .asm-using deps (boringssl, libjpeg-turbo) declare no fetchDeps.
…ir .c/.cpp objects Splitting fetchDepStamps out of emitDirect's order-only list had left the nasm arm with no dependency on the cross deps at all. nasm() now takes implicitInputs like cc()/cxx(), emitDirect passes the same list to all three, and the emitDirect test compiles one source of each kind.
There was a problem hiding this comment.
LGTM — the .asm branch concern from my earlier pass is addressed in 900fb0a (nasm() now takes implicitInputs and emitDirect passes implicit to it, with the emitDirect test covering all three arms).
What was reviewed:
includeFlags()/buildDirSpelling()correctly relativize only paths inside buildDir (.for buildDir itself,..-prefix and cross-drive absolutes stay untouched);n.rel()semantics in ninja.ts confirmed.assertIncludesSpelledAsDeclared()— the cheapstartsWith(n.buildDir)prefix filter can over-match (/foo/barvs/foo/barbaz), but the follow-upbuildDirSpelling()call returns undefined for those, so no false positive; quoted-flag unwrapping matches what emitDirect emits.emitDirect:fetchDepStampsmoved from order-only to implicit now applies to cc/cxx/nasm alike;genIncno longer needsq()since the relativizeddeps/<name>path is space-free.emitJsModules:BunBuiltinNames+extras.hlands incppHeadersvia the.hfilter, so it flows intocppAlland the PCH's order-only set.
Extended reasoning...
Overview
This PR fixes a one-build-lag in ninja's depfile tracking of generated headers: the PCH (and, without ccache, every C/C++ compile) recorded codegen headers under absolute paths in its depfile, while the codegen edges declare them buildDir-relative — ninja matches by string, so a codegen rerun mid-run went unnoticed until the next build. The fix introduces includeFlags() in compile.ts to spell build-dir -I directories the same way ninja.ts spells outputs, threads it through bunCompileFlags() (bun.ts) and emitDirect (source.ts), enforces it with a configure-time assertion in cc()/cxx()/pch(), declares the previously-undeclared BunBuiltinNames+extras.h output in emitJsModules, and promotes direct deps' fetchDepStamps from order-only to implicit inputs (matching emitNestedCmake). libspng's hand-written -I moves from cflags to includes so it goes through the same path. Eight files touched, all under scripts/build/ plus docs and a new test file.
Security risks
None. This is build-graph emission logic only — no runtime code, no user-facing surface, no parsing of untrusted input. The one assert() added is a configure-time developer guard.
Level of scrutiny
Medium. It's build-system-only, so the blast radius is bounded to build correctness (worst case: a build fails or over-rebuilds, not a shipped runtime bug). But it changes the -I spelling on every compile edge (~1300 edges), which forces a one-time full recompile on existing trees, and it changes dependency semantics for direct deps' cross-dep inputs. The PR description is unusually thorough — root-causes the ninja string-matching behavior, shows ninja -t deps/-t query transcripts before/after, verifies restat pruning still works, and notes CI built on every platform including clang-cl.
Other factors
- My earlier inline comment on the
.asmarm losing itsfetchDepStampsdependency was addressed by commit 900fb0a:nasm()now acceptsimplicitInputs(typed viaPick<CompileOpts, ...>),emitDirectpassesimplicitto it, and the newemitDirecttest asserts implicit inputs on cc/cxx/nasm edges alike. - I checked
buildDirSpelling()againstNinja.rel()in ninja.ts:rel(buildDir)→""→., paths outside buildDir →..-prefixed or absolute (Windows cross-drive) →undefined, and the..oddedge case in the test is handled correctly by the=== ".."/startsWith(".." + sep)guards. - The configure-time assertion's cheap prefix filter (
dir.startsWith(n.buildDir)) can over-match a sibling path with a shared prefix, but the subsequentbuildDirSpelling()call is the exact check and returns undefined for those — so the assert only fires on genuine build-dir absolutes. - The test file exercises each part in isolation (includeFlags, bunCompileFlags, the compile-constructor assertion including the quoted form, emitDirect with a synthetic producer/consumer through resolveDep, emitJsModules) with no compiler/ninja/subprocess, and each is designed to fail when only its part is reverted.
This branch has not been deployed
1 participant
Problem
src/js/) makes the firstbun bdfail and the second one succeed. First run, while compiling the unified TU that holdsInternalModuleRegistry.cpp:InternalModuleRegistry+enum.hbut did not rebuild the PCH that includes it; the second run does ([1/2] pch,[2/2] cxx).scripts/build/bun.ts, PCH step), and the depfile is what is supposed to re-dirty the PCH when one changes. The depfile does not do that, because the flags say-I/abs/build/debug/codegen(bun.ts:257-258before this change), so clang records/abs/build/debug/codegen/InternalModuleRegistry+enum.h, whileninja.tsdeclares the codegen edge's output ascodegen/InternalModuleRegistry+enum.h. Ninja matches the two by string: the absolute entry is a node no edge produces, stat'd once at startup, so the PCH keeps its startup verdict while codegen rewrites the file later in the same run.ninja -t deps pch/root-pch.h.hxx.pchon the unfixed tree shows the absolute entries;ninja -t queryon the absolute path shows a node with no producing edge.pchrule is the only compile rule that bypasses ccache (deliberately,compile.ts), and ccache'sCCACHE_BASEDIRrewriting is what had been relativizing the-Iflags of every other rule, so underbun bdonly the PCH lagged. Runningninjadirectly (no ccache env), or building on a host without ccache, gives every TU the same one-build lag: reproduced below by compiling one TU through plainninjaand then regenerating its headers, after which ninja reportsno work to dofor both the PCH and the TU.BunBuiltinNames+extras.h(written bybundle-functions.ts, included byBunBuiltinNames.h, whichroot-pch.hreaches) was not a declared output of any edge at all, so even a correctly spelled depfile entry could not follow a change to it. It is one of the sevencodegen/headers in the PCH's depfile; the other six are declared.zlib.h/zconf.hout ofdeps/zlib/) compiled with-I/abs/build/debug/deps/zliband had those headers only as order-only inputs (source.ts,emitDirect), so a zlib rebuild reached their 192 objects one build late under the same conditions.emitNestedCmakeand bun's own compiles already take cross-dep outputs as implicit inputs;emitDirectwas the odd one out.Fix
compile.ts: newincludeFlags()spells include dirs inside buildDir buildDir-relative (-Icodegen,-Ideps/zlib,-I.), via the samen.rel()ninja.ts uses for the outputs, so a depfile entry for a generated header is the declared output's string. Dirs outside buildDir stay absolute: nothing found through them is a declared output (dep headers are tracked as implicit inputs, see below), and a relative spelling of the shared cache dir would vary with the checkout's depth and stop ccache entries from being shared across worktrees.compile.ts:cc()/cxx()/pch()reject an absolute-Iinto the build dir at configure time, so the spelling is enforced where the depfile is consumed rather than by convention; the whole current graph (1221 objects + PCH) passes, and the check is not measurable in configure time (theemitBunphase stays at ~120 ms either way).source.ts(emitDirect):DirectBuild.includesgoes throughincludeFlags()(libspng moves its hand-written-Ifromcflagsintoincludesso it takes the same path), and thefetchDepsproducers' outputs (their generated headers + source stamp) become implicit inputs of every object (nasm()gained theimplicitInputsoption so the.asmobjects get the same list as the.c/.cppones), as inemitNestedCmake; only the dep's own source stamp stays order-only. With that, a cross-dep header is tracked both ways:touch build/debug/deps/zlib/zlib.hnow schedules exactly zlib's 49 objects plus libarchive's 123, lsquic's 69 and libspng's 1 (ninja -n).bun.ts: the flag assembly moves intobunCompileFlags()(used for the PCH, cxx and cc edges alike) and goes throughincludeFlags(); the comments that claimed codegen outputs "don't change mid-build" now state the actual invariant.codegen.ts:emitJsModulesdeclaresBunBuiltinNames+extras.h(it lands incppHeaders/cppAlllike the other headers of that step;bundle-functions.tsalready writes it withwriteIfNotChanged, so restat prunes it as usual). The "undeclared outputs" comment now says what undeclared means for tracking..classes.tsor builtin edit) and without a hand-maintained list of the headersroot-pch.hhappens to reach. Restat still prunes: touching a JS module that changes no header runs bundle-modules and nothing else (checked below).-Ispelling changes the command line of every C/C++ edge (bun's and the direct deps'), so existing trees recompile once (the same as any flag change). ccache'd compiles already saw these flags in relative form, so the hashing of source paths is unchanged; the dep objects came back from ccache here.test/internal/build-codegen-header-tracking.test.ts(passes withbun bd test; fails withscripts/stashed). Each part has its own test that fails when just that part is reverted:bunCompileFlags(bun's-Ispelling), the compile constructors (the configure-time check, including the quoted formemitDirectemits),emitDirect(a synthetic producer/consumer pair resolved throughresolveDepinside a scratch build dir: the generated header is an implicit input of the consumer's object and its-Iis the declared spelling), andemitJsModules(BunBuiltinNames+extras.hdeclared). No compiler, ninja or subprocess involved; each test takes milliseconds (what a debug build spends on the file is loading the build scripts).ninja <registry TU>plans codegen + cxx only and fails as above; the second invocation rebuilds the PCH. Fixed: one invocation runs codegen, pch, cxx, for adding the module and for removing it. Transcripts in the details block.ninja -t depsfor the PCH and for a TU compiled through plainninja(no ccache env) both listcodegen/...entries; all seven headers the PCH reaches are now declared outputs.touchof a JS module with no header impact: bundle-modules runs, pch/cxx are restat-pruned.bun bdon the changed flags links and passes its smoke test, and a second one is a no-op;build.ninjadiffers from before in the-Ispellings and in the direct-dep objects' implicit inputs (e.g.spng.c.o: cc ... | deps/zlib/zlib.h deps/zlib/zconf.h ../../vendor/zlib/.ref || ...). Existingtest/internal/build-*tests still pass, including the bindgen declared-outputs tests that landed in the meantime (build: declare the bindgen and bindgenv2 headers as outputs of their codegen edges #38035, whose codegen.ts comment this merges with).-I.Background
||in ninja): must exist before the edge runs, but its mtime never dirties the edge. Implicit input (|): also dirties the edge. The build uses order-only for the ~50 codegen headers so a compile is only rebuilt for the headers it really includes, as recorded by the depfile.deps = gcc): clang's-MD/-MMDoutput listing every file the compile read; ninja stores it and adds each entry as an implicit input of the edge on later runs. Entries are plain path strings, spelled exactly as clang constructed them from the-Idirectory and the#includename; ninja does not resolve two spellings of one file to one node. (Windows usesdeps = msvcinstead, where ninja itself normalizes/showIncludespaths to buildDir-relative, so the lag described here is a linux/darwin problem and the relative-Iis simply equivalent there.)restatcan un-dirty it); a file that no edge declares keeps its startup stat for the whole run, so a change made to it mid-run is seen by the next run.pch/root-pch.h.hxx.pch):root-pch.hprecompiled once; every C++ TU has an implicit dep on it and gets the PCH's copy of every header it contains. When a header inside it has changed on disk since, a TU either fails clang's PCH validation (the "has been modified since the precompiled header was built" error in the report) or, as in the runs below, is compiled with the PCH's stale copy and fails on the first inconsistency with the freshly generated headers it includes itself; a change that stayed consistent would go into the binary stale. Sevencodegen/headers are inside it, so a change to any of them must rebuild the PCH before any TU compiles.fetchDeps(source.ts): most vendored libraries are compiled straight into bun's ninja graph (emitDirect, oneccedge per file; WebKit is the one nested cmake build). A dep'sfetchDepsnames the deps whose headers it includes at compile time;resolveDepturns them into the producers'outputs, which for a direct producer are its generated headers (zlib'szlib.h/zconf.hare substituted from.intemplates bydep_substedges intodeps/zlib/) plus its source stamp (the.refwritten by the fetch, or the source dir for in-tree deps).depHeaderSignalinbun.tsis the same set used as implicit inputs of bun's own compiles.configure.ts): ccache rewrites absolute paths under the repo root to cwd-relative ones before hashing and before invoking the compiler, which is why depfiles of ccache'dbun bdcompiles already containedcodegen/X.h. Thepchrule skips ccache because a cached.pchcarries another worktree's absolute header paths.Probe transcripts (linux-x64, warm build/debug, TU = obj/unified/UnifiedSource-src_jsc_bindings-7.cpp.o, the bundle holding InternalModuleRegistry.cpp)
Unfixed,
echo 'export default {};' > src/js/internal/zz_probe.ts, reconfigure:Unfixed, after that TU had been compiled by plain
ninja(no ccache env; its deps log now holds absolutecodegen/paths), regenerate the headers by removing the probe and touching a surviving module:Unfixed, deps recorded for the PCH (the TU compiled through
bun bd, i.e. through ccache, recordedcodegen/...instead):Fixed (same plain
ninja, no ccache env). Baseline rebuild from the flag change, then add the probe, then remove it (+ touch a surviving module), then touch a module with no header impact:build.ninjadiff from the change, on the PCH edge and every compile edge:-I/workspace/bun/build/debug/codegen->-Icodegen,-I/workspace/bun/build/debug->-I.,-I/workspace/bun/build/debug/deps/{zlib,libjpeg-turbo,cares}->-Ideps/...; all other-Iflags unchanged.One neighbouring gap found while reproducing is filed separately and not changed here: deleting a globbed codegen input does not re-run its step at all (the edge's input set is not tracked). The other one, the undeclared bindgen/bindgenv2
Generated*.hheaders, has since landed as #38035.[stamp-90s] gate passed · iteration 1 · 8 files touched
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 1 rejected · iteration 1
evidence per changed file