From 0a21619ae23f04ea1d6c555b9ac8f094ee7a0290 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 05:08:41 +0000 Subject: [PATCH 1/3] build: list the files bundle-modules and generate-classes read outside their source globs bundle-modules.ts reads src/jsc/modules/NativeModuleList.h (native module ids), src/js/builtins/BunBuiltinNames.h (BunBuiltinNames+extras.h) and src/jsc/bindings/js_classes.ts ($inherits indices); generate-classes.ts reads js_classes.ts as well. None of them matched the globs those edges were built from, so editing one of them alone did not re-run the step and the generated ids stayed stale until a globbed file was touched. Add them to the edges' inputs. --- scripts/build/CLAUDE.md | 2 +- scripts/build/codegen.ts | 50 +++- .../build-codegen-extra-inputs.test.ts | 237 ++++++++++++++++++ 3 files changed, 278 insertions(+), 11 deletions(-) create mode 100644 test/internal/build-codegen-extra-inputs.test.ts diff --git a/scripts/build/CLAUDE.md b/scripts/build/CLAUDE.md index b823afe32340..9d5641df18d5 100644 --- a/scripts/build/CLAUDE.md +++ b/scripts/build/CLAUDE.md @@ -125,7 +125,7 @@ Tables: `cpuTargetFlags` (`-march`/`-mcpu`/`-mtune` — also forwarded to local **Iterate on a dependency from a local checkout** — `bun bd --local-deps=mimalloc=~/code/mimalloc …` builds that dep from the clone instead of the pinned tarball (no fetch, no patches; edits rebuild incrementally). Any `github-archive` dep the graph compiles (not lolhtml — cargo reads that via `Cargo.toml`); details in `deps/README.md`. -**Add a codegen step** — add a function in `codegen.ts` following the shape of `emitErrorCode` (simple) or `emitCppBind` (needs file-list input). Call it from `emitCodegen()` and add outputs to the right `CodegenOutputs` group (`rustInputs` if the Rust build reads it (the `include!`d generated `.rs` files) — `cppSources` if it's a `.cpp` to compile, `cppAll` if it's a header). +**Add a codegen step** — add a function in `codegen.ts` following the shape of `emitErrorCode` (simple) or `emitCppBind` (needs file-list input). Call it from `emitCodegen()` and add outputs to the right `CodegenOutputs` group (`rustInputs` if the Rust build reads it (the `include!`d generated `.rs` files) — `cppSources` if it's a `.cpp` to compile, `cppAll` if it's a header). The edge's inputs must cover everything the script reads, not just the globbed source list: ninja re-runs the step only for files on the edge, so a header or table the script opens from elsewhere (see `extraInputs` in `emitJsModules`) has to be listed too, or edits to it silently leave the outputs stale. Same when teaching an existing script to read a new file. **Add a Config field** — add to `Config` interface and `PartialConfig` in `config.ts`, resolve in `resolveConfig()`. If it needs a CLI flag, `build.ts`'s arg parser already handles `--anyfield=value` generically. diff --git a/scripts/build/codegen.ts b/scripts/build/codegen.ts index 7aa6c2238db7..aa34f41e4479 100644 --- a/scripts/build/codegen.ts +++ b/scripts/build/codegen.ts @@ -578,7 +578,20 @@ function emitErrorCode({ n, cfg, o, dirStamp }: Ctx): void { o.cppHeaders.push(...cppOutputs); } -function emitGeneratedClasses({ n, cfg, sources, o, dirStamp }: Ctx): void { +/** + * The `$inherits()` table. Its indices are baked into two outputs of two + * different steps: replacements.ts rewrites `$inheritsBlob(x)` in every bundled + * module to `$inherits(, x)` (emitJsModules), and generate-classes.ts + * emits the matching `switch (id)` into ZigGeneratedClasses.cpp + * (emitGeneratedClasses). It matches neither step's source glob, so both edges + * list it explicitly; an edit to it has to regenerate both sides together. + */ +function jsClassesTable(cfg: Config): string { + return resolve(cfg.cwd, "src", "jsc", "bindings", "js_classes.ts"); +} + +/** Exported (with emitJsModules) for test/internal/build-codegen-extra-inputs.test.ts. */ +export function emitGeneratedClasses({ n, cfg, sources, o, dirStamp }: Ctx): void { const script = resolve(cfg.cwd, "src", "codegen", "generate-classes.ts"); const outputs = [ @@ -598,7 +611,7 @@ function emitGeneratedClasses({ n, cfg, sources, o, dirStamp }: Ctx): void { n.build({ outputs, rule: "codegen", - inputs: [script, ...sources.zigGeneratedClasses], + inputs: [script, ...sources.zigGeneratedClasses, jsClassesTable(cfg)], orderOnlyInputs: [dirStamp], vars: { cwd: cfg.cwd, @@ -701,15 +714,32 @@ function emitCppBind({ n, cfg, sources, o, dirStamp }: Ctx): void { o.rustInputs.push(outputRs); } -function emitJsModules({ n, cfg, sources, o, dirStamp }: Ctx): void { +export function emitJsModules({ n, cfg, sources, o, dirStamp }: Ctx): void { const script = resolve(cfg.cwd, "src", "codegen", "bundle-modules.ts"); - // InternalModuleRegistry.cpp is read by the script (for a sanity check). - const extraInput = resolve(cfg.cwd, "src", "jsc", "bindings", "InternalModuleRegistry.cpp"); - // replacements.ts bakes ErrorCode.ts indices into every bundled module - // ($makeErrorWithCode(N, ...)); without this dep an ErrorCode.ts edit leaves - // stale error numbers in the JS bundles while the C++ enum regenerates. - const errorCodeInput = resolve(cfg.cwd, "src", "jsc", "bindings", "ErrorCode.ts"); + // Inputs from outside sources.js (src/js/**/*.{js,ts}) and sources.jsCodegen + // (src/codegen/*.ts). The script bakes each file it reads from elsewhere into + // its outputs, so an edit to one of them alone has to re-run the step, and + // ninja only does that for files listed here. + const extraInputs = [ + // internal-module-registry-scanner.ts numbers the native modules from this + // list. The numbers end up in InternalModuleRegistry+*.h, + // SyntheticModuleType.h, NativeModuleImpl.h, generated_resolved_source_tag.rs + // and in the require() rewrites inside the bundled modules. + resolve(cfg.cwd, "src", "jsc", "modules", "NativeModuleList.h"), + // bundle-functions.ts writes BunBuiltinNames+extras.h: the private names + // the builtins use minus the ones this header already declares. + resolve(cfg.cwd, "src", "js", "builtins", "BunBuiltinNames.h"), + // replacements.ts bakes each class's index into $inherits(N, ...). + jsClassesTable(cfg), + // replacements.ts bakes ErrorCode.ts indices into every bundled module + // ($makeErrorWithCode(N, ...)); without this dep an ErrorCode.ts edit leaves + // stale error numbers in the JS bundles while the C++ enum regenerates. + resolve(cfg.cwd, "src", "jsc", "bindings", "ErrorCode.ts"), + // Not read by the script; inherited from the CMake build's input list for + // this step. + resolve(cfg.cwd, "src", "jsc", "bindings", "InternalModuleRegistry.cpp"), + ]; const outputs = [ resolve(cfg.codegenDir, "WebCoreJSBuiltins.cpp"), @@ -736,7 +766,7 @@ function emitJsModules({ n, cfg, sources, o, dirStamp }: Ctx): void { n.build({ outputs, rule: "codegen", - inputs: [script, ...sources.js, ...sources.jsCodegen, extraInput, errorCodeInput], + inputs: [script, ...sources.js, ...sources.jsCodegen, ...extraInputs], orderOnlyInputs: [dirStamp], vars: { cwd: cfg.cwd, diff --git a/test/internal/build-codegen-extra-inputs.test.ts b/test/internal/build-codegen-extra-inputs.test.ts new file mode 100644 index 000000000000..8edbad06c43a --- /dev/null +++ b/test/internal/build-codegen-extra-inputs.test.ts @@ -0,0 +1,237 @@ +/** + * Two codegen steps in scripts/build/codegen.ts read files their source lists + * do not cover and bake them into their outputs: + * + * bundle-modules (sources: src/js, src/codegen) also reads + * src/jsc/modules/NativeModuleList.h native module ids: InternalModuleRegistry+*.h, + * SyntheticModuleType.h, the require() rewrites, ... + * src/js/builtins/BunBuiltinNames.h what BunBuiltinNames+extras.h has to add + * src/jsc/bindings/js_classes.ts $inherits(, ...) in every bundled module + * src/jsc/bindings/ErrorCode.ts $makeErrorWithCode(, ...) likewise + * generate-classes (sources: the .classes.ts files) also reads + * src/jsc/bindings/js_classes.ts the switch over those same indices in ZigGeneratedClasses.cpp + * + * Ninja re-runs a step only when an input listed on its edge is newer than the + * outputs, so each of these files has to be on the edge. Before they were, + * editing one of them alone (adding a native module, say) left the generated + * ids stale until some globbed file happened to be touched. js_classes.ts is + * on both edges because its indices have to change on the JS side and the C++ + * side in the same build. + * + * Emits the two steps into a scratch build dir with made-up source lists and + * reads the edges back out of the ninja text. No ninja, compiler or subprocess + * is involved, so this runs on every host. + */ +import { describe, expect, test } from "bun:test"; +import { tempDir } from "harness"; +import { existsSync } from "node:fs"; +import { resolve } from "node:path"; + +import { + emitGeneratedClasses, + emitJsModules, + registerCodegenRules, + type CodegenOutputs, +} from "../../scripts/build/codegen.ts"; +import { registerDirStamps } from "../../scripts/build/compile.ts"; +import { resolveConfig, type Config, type Toolchain } from "../../scripts/build/config.ts"; +import { Ninja } from "../../scripts/build/ninja.ts"; +import { quoteArgs } from "../../scripts/build/shell.ts"; +import type { Sources } from "../../scripts/glob-sources.ts"; + +/** A fully-populated fake toolchain; emitting these two steps runs none of it. */ +function mockToolchain(): Toolchain { + return { + cc: "/fake/llvm/bin/clang", + cxx: "/fake/llvm/bin/clang++", + hostCc: undefined, + hostCxx: undefined, + clangVersion: "21.1.8", + clangResourceDir: "/fake/llvm/lib/clang/21", + ar: "/fake/llvm/bin/llvm-ar", + ranlib: "/fake/llvm/bin/llvm-ranlib", + ld: "/fake/llvm/bin/ld.lld", + ld64Lld: "/fake/llvm/bin/ld64.lld", + rustLld: undefined, + rustLlvmVersion: "22.1.4", + rustSysroot: undefined, + rustHostTriple: undefined, + strip: "/fake/bin/strip", + llvmStrip: "/fake/llvm/bin/llvm-strip", + dsymutil: "/fake/llvm/bin/dsymutil", + bun: "/fake/bin/bun", + jsRuntime: "/fake/bin/bun", + esbuild: "/fake/bin/esbuild", + ccache: undefined, + cmake: "/fake/bin/cmake", + cargo: undefined, + cargoHome: undefined, + rustupHome: undefined, + msvcLinker: undefined, + rc: undefined, + mt: undefined, + nasm: undefined, + }; +} + +interface Configured { + cfg: Config; + n: Ninja; + sources: Sources; + o: CodegenOutputs; + dirStamp: string; +} + +/** + * A linux-x64 debug target in `buildDir` (resolves on every host once told + * where its sysroot is; the path is only recorded), with the rules the two + * emitters reference registered and the empty output groups emitCodegen() + * hands them. The source lists hold only what these two emitters read, and + * the files in them are never opened: the emitters just put the paths on + * the edges. + */ +function configure(buildDir: string): Configured { + const cfg = resolveConfig( + { os: "linux", arch: "x64", abi: "gnu", buildType: "Debug", buildDir, linuxSysroot: buildDir }, + mockToolchain(), + ); + const n = new Ninja({ buildDir }); + registerDirStamps(n, cfg); + registerCodegenRules(n, cfg); + const src = (p: string) => resolve(cfg.cwd, "src", p); + const sources = { + js: [src("js/internal/a.ts"), src("js/node/fs.ts")], + jsCodegen: [src("codegen/bundle-modules.ts"), src("codegen/replacements.ts")], + zigGeneratedClasses: [src("jsc/resolve_message.classes.ts"), src("runtime/api/bun/subprocess.classes.ts")], + } as Sources; + const o: CodegenOutputs = { + all: [], + rustInputs: [], + rustOrderOnly: [], + cppSources: [], + cppHeaders: [], + cppAll: [], + bindgenV2Cpp: [], + internalModulesAsm: resolve(cfg.codegenDir, "InternalModuleRegistryConstants.S"), + internalModulesBin: resolve(cfg.codegenDir, "InternalModuleRegistryConstants.bin"), + rootInstall: resolve(buildDir, "stamps", "install.stamp"), + }; + return { cfg, n, sources, o, dirStamp: resolve(cfg.codegenDir, ".dir") }; +} + +interface Edge { + /** Explicit and implicit inputs (either re-runs the edge when newer than the outputs), absolute. */ + inputs: string[]; + /** The edge's variable bindings (`args`, `desc`, ...). */ + vars: Record; +} + +/** Undo ninja.ts's build-line escaping (`$ `, `$:`, `$$`). */ +function unescapePath(token: string): string { + return token.replace(/\$([ :$])/g, "$1"); +} + +/** The edge in `n` that produces `output` (an absolute path). */ +function edgeProducing(n: Ninja, output: string): Edge { + // ninja.ts wraps long build lines with `$` continuations; the edge's + // ` name = value` bindings follow the build line up to a blank line. + const lines = n + .toString() + .replace(/ \$\n {4}/g, " ") + .split("\n"); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]!; + if (!line.startsWith("build ")) continue; + // `build out1 out2: rule in1 in2 | implicit1 || orderonly1`; the first + // unescaped `: ` ends the outputs. + const ruleAt = line.search(/(? token !== "|") + .map(unescapePath); + if (!outputs.includes(n.rel(output))) continue; + + const [, ...dependencies] = line + .slice(ruleAt + 2) + .split(/(?<=[^$]) /) + .filter(token => token !== "|"); + const orderOnlyAt = dependencies.indexOf("||"); + const inputs = (orderOnlyAt === -1 ? dependencies : dependencies.slice(0, orderOnlyAt)).map(token => + resolve(n.buildDir, unescapePath(token)), + ); + + const vars: Record = {}; + for (let j = i + 1; j < lines.length && lines[j] !== ""; j++) { + const binding = lines[j]!.match(/^ {2}(\w+) = (.*)$/); + if (binding) vars[binding[1]!] = binding[2]!.replace(/\$\$/g, "$"); + } + return { inputs, vars }; + } + throw new Error(`no build edge produces ${output}`); +} + +interface Tracked { + /** The file is where the edge points (a rename has to update codegen.ts too). */ + exists: boolean; + /** The edge lists it. */ + listed: boolean; +} + +/** One entry per repo-relative path in `files`. */ +function tracking(cfg: Config, edge: Edge, files: string[]): Record { + return Object.fromEntries( + files.map(file => { + const abs = resolve(cfg.cwd, file); + return [file, { exists: existsSync(abs), listed: edge.inputs.includes(abs) }]; + }), + ); +} + +function tracked(files: string[]): Record { + return Object.fromEntries(files.map(file => [file, { exists: true, listed: true }])); +} + +describe("codegen edges list the files their scripts read from outside the source lists", () => { + test("bundle-modules: NativeModuleList.h, BunBuiltinNames.h, js_classes.ts, ErrorCode.ts", () => { + using dir = tempDir("build-codegen-extra-inputs", {}); + const { cfg, n, sources, o, dirStamp } = configure(String(dir)); + + emitJsModules({ n, cfg, sources, o, dirStamp }); + + const edge = edgeProducing(n, resolve(cfg.codegenDir, "InternalModuleRegistry+enum.h")); + const reads = [ + "src/jsc/modules/NativeModuleList.h", + "src/js/builtins/BunBuiltinNames.h", + "src/jsc/bindings/js_classes.ts", + "src/jsc/bindings/ErrorCode.ts", + ]; + expect(tracking(cfg, edge, reads)).toEqual(tracked(reads)); + // Listed in addition to the source lists, not instead of them. + expect(edge.inputs).toEqual( + expect.arrayContaining([ + resolve(cfg.cwd, "src", "codegen", "bundle-modules.ts"), + ...sources.js, + ...sources.jsCodegen, + ]), + ); + }); + + test("generate-classes: js_classes.ts, as a dependency rather than a command-line argument", () => { + using dir = tempDir("build-codegen-extra-inputs", {}); + const { cfg, n, sources, o, dirStamp } = configure(String(dir)); + + emitGeneratedClasses({ n, cfg, sources, o, dirStamp }); + + const script = resolve(cfg.cwd, "src", "codegen", "generate-classes.ts"); + const edge = edgeProducing(n, resolve(cfg.codegenDir, "ZigGeneratedClasses.cpp")); + const reads = ["src/jsc/bindings/js_classes.ts"]; + expect(tracking(cfg, edge, reads)).toEqual(tracked(reads)); + expect(edge.inputs).toEqual(expect.arrayContaining([script, ...sources.zigGeneratedClasses])); + // generate-classes.ts loads every path on its command line as a list of + // class definitions, so the command line has to stay the .classes.ts files. + expect(edge.vars.args).toBe( + quoteArgs(["run", script, ...sources.zigGeneratedClasses, cfg.codegenDir], cfg.host.os === "windows"), + ); + }); +}); From 07aed7fbc30466a6238274fba87a8d519ef0efdc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:59:23 +0000 Subject: [PATCH 2/3] build: derive the codegen edges' extra inputs from the scripts' imports in the test The test now walks each script's static import closure and checks the edge in both directions: everything imported or opened has to be listed, and every hand-listed input has to be imported or opened. That turned up InternalModuleRegistry.cpp on the bundle-modules edge, which no script reads (it came over from the CMake input list), so it is dropped, and class-definitions.ts / helpers.ts missing from the generate-classes edge, which now lists the src/codegen sources like bundle-modules and cppbind do. glob-sources.ts gains globSourceList(field) so the test can use the real src/codegen list without globbing every pattern. --- scripts/build/CLAUDE.md | 2 +- scripts/build/codegen.ts | 20 +- scripts/glob-sources.ts | 58 ++--- .../build-codegen-extra-inputs.test.ts | 212 +++++++++++------- 4 files changed, 175 insertions(+), 117 deletions(-) diff --git a/scripts/build/CLAUDE.md b/scripts/build/CLAUDE.md index 9d5641df18d5..df13e328ead0 100644 --- a/scripts/build/CLAUDE.md +++ b/scripts/build/CLAUDE.md @@ -125,7 +125,7 @@ Tables: `cpuTargetFlags` (`-march`/`-mcpu`/`-mtune` — also forwarded to local **Iterate on a dependency from a local checkout** — `bun bd --local-deps=mimalloc=~/code/mimalloc …` builds that dep from the clone instead of the pinned tarball (no fetch, no patches; edits rebuild incrementally). Any `github-archive` dep the graph compiles (not lolhtml — cargo reads that via `Cargo.toml`); details in `deps/README.md`. -**Add a codegen step** — add a function in `codegen.ts` following the shape of `emitErrorCode` (simple) or `emitCppBind` (needs file-list input). Call it from `emitCodegen()` and add outputs to the right `CodegenOutputs` group (`rustInputs` if the Rust build reads it (the `include!`d generated `.rs` files) — `cppSources` if it's a `.cpp` to compile, `cppAll` if it's a header). The edge's inputs must cover everything the script reads, not just the globbed source list: ninja re-runs the step only for files on the edge, so a header or table the script opens from elsewhere (see `extraInputs` in `emitJsModules`) has to be listed too, or edits to it silently leave the outputs stale. Same when teaching an existing script to read a new file. +**Add a codegen step** — add a function in `codegen.ts` following the shape of `emitErrorCode` (simple) or `emitCppBind` (needs file-list input). Call it from `emitCodegen()` and add outputs to the right `CodegenOutputs` group (`rustInputs` if the Rust build reads it (the `include!`d generated `.rs` files) — `cppSources` if it's a `.cpp` to compile, `cppAll` if it's a header). The edge's inputs must cover everything the script imports or opens, not just the globbed source list: ninja re-runs the step only for files on the edge, so a module or header the script pulls in from elsewhere (see `extraInputs` in `emitJsModules`) has to be listed too, or edits to it silently leave the outputs stale. Same when teaching an existing script to read a new file. `test/internal/build-codegen-extra-inputs.test.ts` checks the listed edges against their scripts' import closures; add the new step to its table. **Add a Config field** — add to `Config` interface and `PartialConfig` in `config.ts`, resolve in `resolveConfig()`. If it needs a CLI flag, `build.ts`'s arg parser already handles `--anyfield=value` generically. diff --git a/scripts/build/codegen.ts b/scripts/build/codegen.ts index aa34f41e4479..fff2dcaba5db 100644 --- a/scripts/build/codegen.ts +++ b/scripts/build/codegen.ts @@ -611,7 +611,10 @@ export function emitGeneratedClasses({ n, cfg, sources, o, dirStamp }: Ctx): voi n.build({ outputs, rule: "codegen", - inputs: [script, ...sources.zigGeneratedClasses, jsClassesTable(cfg)], + // jsCodegen covers the script's imports (class-definitions.ts, which the + // .classes.ts files import as well, and helpers.ts). Only the .classes.ts + // files go on the command line. + inputs: [script, ...sources.jsCodegen, ...sources.zigGeneratedClasses, jsClassesTable(cfg)], orderOnlyInputs: [dirStamp], vars: { cwd: cfg.cwd, @@ -717,10 +720,11 @@ function emitCppBind({ n, cfg, sources, o, dirStamp }: Ctx): void { export function emitJsModules({ n, cfg, sources, o, dirStamp }: Ctx): void { const script = resolve(cfg.cwd, "src", "codegen", "bundle-modules.ts"); - // Inputs from outside sources.js (src/js/**/*.{js,ts}) and sources.jsCodegen - // (src/codegen/*.ts). The script bakes each file it reads from elsewhere into - // its outputs, so an edit to one of them alone has to re-run the step, and - // ninja only does that for files listed here. + // Everything the script reads from outside sources.js (src/js/**/*.{js,ts}) + // and sources.jsCodegen (src/codegen/*.ts). Each is baked into the outputs, + // so an edit to one of them alone has to re-run the step, and ninja only + // does that for files listed here. test/internal/build-codegen-extra-inputs + // checks this list against the script's imports in both directions. const extraInputs = [ // internal-module-registry-scanner.ts numbers the native modules from this // list. The numbers end up in InternalModuleRegistry+*.h, @@ -736,9 +740,6 @@ export function emitJsModules({ n, cfg, sources, o, dirStamp }: Ctx): void { // ($makeErrorWithCode(N, ...)); without this dep an ErrorCode.ts edit leaves // stale error numbers in the JS bundles while the C++ enum regenerates. resolve(cfg.cwd, "src", "jsc", "bindings", "ErrorCode.ts"), - // Not read by the script; inherited from the CMake build's input list for - // this step. - resolve(cfg.cwd, "src", "jsc", "bindings", "InternalModuleRegistry.cpp"), ]; const outputs = [ @@ -786,9 +787,6 @@ export function emitJsModules({ n, cfg, sources, o, dirStamp }: Ctx): void { function emitBakeCodegen({ n, cfg, sources, o, dirStamp }: Ctx): void { const script = resolve(cfg.cwd, "src", "codegen", "bake-codegen.ts"); - // InternalModuleRegistry.cpp is listed as a dep in CMake for this step too. - // The script doesn't read it; CMake copy-paste. We skip it. - // CMake only declares bake.client.js and bake.server.js as outputs. The // script also emits bake.error.js (the runtime embeds it). We declare // all three. diff --git a/scripts/glob-sources.ts b/scripts/glob-sources.ts index 804d38c72f51..81576a00c4d5 100644 --- a/scripts/glob-sources.ts +++ b/scripts/glob-sources.ts @@ -140,35 +140,43 @@ export type Sources = { [K in keyof typeof patterns]: string[] }; */ export function globAllSources(): Sources { const result = {} as Sources; + for (const field of Object.keys(patterns) as (keyof Sources)[]) { + result[field] = globSourceList(field); + } + return result; +} - for (const [field, spec] of Object.entries(patterns) as [keyof Sources, SourcePattern][]) { - const excludeExact = new Set(); - const excludePrefix: string[] = []; - for (const ex of (spec.exclude ?? []).map(normalize)) { - if (ex.endsWith("/**")) - excludePrefix.push(ex.slice(0, -2)); // keep trailing '/' - else excludeExact.add(ex); - } - const files: string[] = []; - for (const pattern of spec.paths) { - for (const rel of globSync(pattern, { cwd: root })) { - const normalized = normalize(rel); - if (excludeExact.has(normalized)) continue; - if (excludePrefix.some(p => normalized.startsWith(p))) continue; - files.push(resolve(root, normalized)); - } +/** + * Glob one source list. Tests of the build scripts use this to feed an + * emitter the real list for the one field it reads, since globbing every + * field (`src/**` walks) is slow under a debug build. + */ +export function globSourceList(field: keyof Sources): string[] { + const spec: SourcePattern = patterns[field]; + const excludeExact = new Set(); + const excludePrefix: string[] = []; + for (const ex of (spec.exclude ?? []).map(normalize)) { + if (ex.endsWith("/**")) + excludePrefix.push(ex.slice(0, -2)); // keep trailing '/' + else excludeExact.add(ex); + } + const files: string[] = []; + for (const pattern of spec.paths) { + for (const rel of globSync(pattern, { cwd: root })) { + const normalized = normalize(rel); + if (excludeExact.has(normalized)) continue; + if (excludePrefix.some(p => normalized.startsWith(p))) continue; + files.push(resolve(root, normalized)); } - - files.sort((a, b) => a.localeCompare(b)); - assert(files.length > 0, `Source list '${field}' matched nothing`, { - file: import.meta.url, - hint: `Patterns: ${spec.paths.join(", ")}`, - }); - - result[field] = files; } - return result; + files.sort((a, b) => a.localeCompare(b)); + assert(files.length > 0, `Source list '${field}' matched nothing`, { + file: import.meta.url, + hint: `Patterns: ${spec.paths.join(", ")}`, + }); + + return files; } /** Forward slashes, no leading ./ — for exclude-set comparisons. */ diff --git a/test/internal/build-codegen-extra-inputs.test.ts b/test/internal/build-codegen-extra-inputs.test.ts index 8edbad06c43a..c8d601324ea5 100644 --- a/test/internal/build-codegen-extra-inputs.test.ts +++ b/test/internal/build-codegen-extra-inputs.test.ts @@ -1,31 +1,38 @@ /** - * Two codegen steps in scripts/build/codegen.ts read files their source lists - * do not cover and bake them into their outputs: + * A codegen edge in scripts/build/codegen.ts has to list every file its script + * bakes into the outputs: ninja re-runs the step only when an input listed on + * the edge is newer than the outputs, and the codegen scripts emit no depfiles, + * so the input list is all ninja knows. The globbed source lists cover most of + * it; whatever a script imports or opens from elsewhere has to be listed by + * hand, and that is where the gaps were: * - * bundle-modules (sources: src/js, src/codegen) also reads - * src/jsc/modules/NativeModuleList.h native module ids: InternalModuleRegistry+*.h, - * SyntheticModuleType.h, the require() rewrites, ... - * src/js/builtins/BunBuiltinNames.h what BunBuiltinNames+extras.h has to add - * src/jsc/bindings/js_classes.ts $inherits(, ...) in every bundled module - * src/jsc/bindings/ErrorCode.ts $makeErrorWithCode(, ...) likewise - * generate-classes (sources: the .classes.ts files) also reads + * bundle-modules (source lists: src/js, src/codegen) also uses + * src/jsc/modules/NativeModuleList.h opened by internal-module-registry-scanner.ts; numbers the + * native modules (InternalModuleRegistry+*.h, SyntheticModuleType.h, + * the require() rewrites in every bundled module, ...) + * src/js/builtins/BunBuiltinNames.h opened by bundle-functions.ts; decides what BunBuiltinNames+extras.h adds + * src/jsc/bindings/js_classes.ts imported by replacements.ts; $inherits(, ...) in every module + * src/jsc/bindings/ErrorCode.ts imported by replacements.ts; $makeErrorWithCode(, ...) likewise + * generate-classes (source lists: the .classes.ts files, src/codegen) also uses * src/jsc/bindings/js_classes.ts the switch over those same indices in ZigGeneratedClasses.cpp * - * Ninja re-runs a step only when an input listed on its edge is newer than the - * outputs, so each of these files has to be on the edge. Before they were, - * editing one of them alone (adding a native module, say) left the generated - * ids stale until some globbed file happened to be touched. js_classes.ts is - * on both edges because its indices have to change on the JS side and the C++ - * side in the same build. + * Until the edges listed them, editing one of these files alone (adding a + * native module, say) left the generated ids stale until some globbed file + * happened to be touched. js_classes.ts is on both edges because its indices + * have to change on the JS side and the C++ side in the same build. * - * Emits the two steps into a scratch build dir with made-up source lists and - * reads the edges back out of the ninja text. No ninja, compiler or subprocess - * is involved, so this runs on every host. + * Rather than pin those file names, each step below is checked against what + * its script actually imports: the edge has to carry the script's static + * import closure plus the files it opens at run time (`reads`, which imports + * cannot reveal), and every hand-listed input has to be one of those two, so a + * dead input shows up as well. The steps are emitted into a scratch build dir; + * only the src/codegen list is the real one (the closures live there), the + * other lists are made up and never opened. No ninja or subprocess involved. */ import { describe, expect, test } from "bun:test"; import { tempDir } from "harness"; -import { existsSync } from "node:fs"; -import { resolve } from "node:path"; +import { existsSync, readFileSync } from "node:fs"; +import { dirname, extname, relative, resolve, sep } from "node:path"; import { emitGeneratedClasses, @@ -37,9 +44,9 @@ import { registerDirStamps } from "../../scripts/build/compile.ts"; import { resolveConfig, type Config, type Toolchain } from "../../scripts/build/config.ts"; import { Ninja } from "../../scripts/build/ninja.ts"; import { quoteArgs } from "../../scripts/build/shell.ts"; -import type { Sources } from "../../scripts/glob-sources.ts"; +import { globSourceList, type Sources } from "../../scripts/glob-sources.ts"; -/** A fully-populated fake toolchain; emitting these two steps runs none of it. */ +/** A fully-populated fake toolchain; emitting these steps runs none of it. */ function mockToolchain(): Toolchain { return { cc: "/fake/llvm/bin/clang", @@ -74,23 +81,15 @@ function mockToolchain(): Toolchain { }; } -interface Configured { - cfg: Config; - n: Ninja; - sources: Sources; - o: CodegenOutputs; - dirStamp: string; -} +type Ctx = Parameters[0]; /** - * A linux-x64 debug target in `buildDir` (resolves on every host once told - * where its sysroot is; the path is only recorded), with the rules the two + * A linux-x64 debug target in `buildDir` (it resolves on every host once told + * where its sysroot is; the path is only recorded), with the rules the * emitters reference registered and the empty output groups emitCodegen() - * hands them. The source lists hold only what these two emitters read, and - * the files in them are never opened: the emitters just put the paths on - * the edges. + * hands them. Only the source lists the steps under test read are filled in. */ -function configure(buildDir: string): Configured { +function configure(buildDir: string): Ctx { const cfg = resolveConfig( { os: "linux", arch: "x64", abi: "gnu", buildType: "Debug", buildDir, linuxSysroot: buildDir }, mockToolchain(), @@ -100,8 +99,8 @@ function configure(buildDir: string): Configured { registerCodegenRules(n, cfg); const src = (p: string) => resolve(cfg.cwd, "src", p); const sources = { + jsCodegen: globSourceList("jsCodegen"), js: [src("js/internal/a.ts"), src("js/node/fs.ts")], - jsCodegen: [src("codegen/bundle-modules.ts"), src("codegen/replacements.ts")], zigGeneratedClasses: [src("jsc/resolve_message.classes.ts"), src("runtime/api/bun/subprocess.classes.ts")], } as Sources; const o: CodegenOutputs = { @@ -171,65 +170,118 @@ function edgeProducing(n: Ninja, output: string): Edge { throw new Error(`no build edge produces ${output}`); } -interface Tracked { - /** The file is where the edge points (a rename has to update codegen.ts too). */ - exists: boolean; - /** The edge lists it. */ - listed: boolean; -} - -/** One entry per repo-relative path in `files`. */ -function tracking(cfg: Config, edge: Edge, files: string[]): Record { - return Object.fromEntries( - files.map(file => { - const abs = resolve(cfg.cwd, file); - return [file, { exists: existsSync(abs), listed: edge.inputs.includes(abs) }]; - }), - ); +/** + * Every file under src/ that `script` reaches through import, require() and + * re-export specifiers, not counting the script itself. Builtins resolve to + * `node:`/`bun:` names and packages into node_modules, so the src/ filter drops + * both; a specifier that does not resolve throws, as loading the script would. + * Files the scripts load by path at run time (.classes.ts and such) are the + * globbed inputs, not imports, and do not show up here. + */ +function importClosure(cfg: Config, script: string): string[] { + const srcDir = resolve(cfg.cwd, "src") + sep; + const seen = new Set(); + const pending = [script]; + while (pending.length > 0) { + const file = pending.pop()!; + if (seen.has(file)) continue; + seen.add(file); + const loader = extname(file).slice(1) as "ts" | "tsx" | "js" | "jsx"; + for (const { path: specifier } of new Bun.Transpiler({ loader }).scanImports(readFileSync(file, "utf8"))) { + const resolved = Bun.resolveSync(specifier, dirname(file)); + if (resolved.startsWith(srcDir) && !resolved.includes(`${sep}node_modules${sep}`)) pending.push(resolved); + } + } + seen.delete(script); + return [...seen].sort(); } -function tracked(files: string[]): Record { - return Object.fromEntries(files.map(file => [file, { exists: true, listed: true }])); +interface Step { + name: string; + emit: (ctx: Ctx) => void; + /** The script the edge runs, repo-relative. */ + script: string; + /** One of the edge's outputs, relative to the codegen dir; identifies the edge. */ + output: string; + /** Files the script (or a module it imports) opens at run time from outside the source lists, repo-relative. */ + reads: string[]; + /** A few of the script's imports, repo-relative: pins that the closure walk sees them at all. */ + importsInclude: string[]; } -describe("codegen edges list the files their scripts read from outside the source lists", () => { - test("bundle-modules: NativeModuleList.h, BunBuiltinNames.h, js_classes.ts, ErrorCode.ts", () => { - using dir = tempDir("build-codegen-extra-inputs", {}); - const { cfg, n, sources, o, dirStamp } = configure(String(dir)); - - emitJsModules({ n, cfg, sources, o, dirStamp }); - - const edge = edgeProducing(n, resolve(cfg.codegenDir, "InternalModuleRegistry+enum.h")); - const reads = [ +const steps: Step[] = [ + { + name: "bundle-modules", + emit: emitJsModules, + script: "src/codegen/bundle-modules.ts", + output: "InternalModuleRegistry+enum.h", + reads: [ + // internal-module-registry-scanner.ts, createInternalModuleRegistry() "src/jsc/modules/NativeModuleList.h", + // bundle-functions.ts, the BunBuiltinNames+extras.h block "src/js/builtins/BunBuiltinNames.h", + ], + importsInclude: [ "src/jsc/bindings/js_classes.ts", "src/jsc/bindings/ErrorCode.ts", - ]; - expect(tracking(cfg, edge, reads)).toEqual(tracked(reads)); - // Listed in addition to the source lists, not instead of them. - expect(edge.inputs).toEqual( - expect.arrayContaining([ - resolve(cfg.cwd, "src", "codegen", "bundle-modules.ts"), - ...sources.js, - ...sources.jsCodegen, - ]), - ); + // Loaded with require(), not import. + "src/codegen/bundle-functions.ts", + "src/codegen/internal-module-registry-scanner.ts", + ], + }, + { + name: "generate-classes", + emit: emitGeneratedClasses, + script: "src/codegen/generate-classes.ts", + output: "ZigGeneratedClasses.cpp", + reads: [], + importsInclude: ["src/jsc/bindings/js_classes.ts", "src/codegen/class-definitions.ts"], + }, +]; + +describe("codegen edges carry what their scripts import or open, and nothing else by hand", () => { + test.each(steps)("$name", ({ emit, script, output, reads, importsInclude }) => { + using dir = tempDir("build-codegen-extra-inputs", {}); + const ctx = configure(String(dir)); + const { cfg, n, sources } = ctx; + const repoRelative = (file: string) => relative(cfg.cwd, file).replaceAll(sep, "/"); + const scriptPath = resolve(cfg.cwd, script); + const readPaths = reads.map(file => resolve(cfg.cwd, file)); + + emit(ctx); + + const imports = importClosure(cfg, scriptPath); + expect(imports.map(repoRelative)).toEqual(expect.arrayContaining(importsInclude)); + + const edge = edgeProducing(n, resolve(cfg.codegenDir, output)); + const inputs = new Set(edge.inputs); + const fromSourceLists = new Set(Object.values(sources).flat()); + const used = new Set([...imports, ...readPaths]); + expect({ + script: inputs.has(scriptPath), + // Imported or opened by the script, but editing it would not re-run the step. + untracked: [...used].filter(file => !inputs.has(file)).map(repoRelative), + // Listed by hand, but the script neither imports nor opens it: a stale + // entry whose edits re-run the step for nothing. + unexplained: edge.inputs + .filter(file => file !== scriptPath && !fromSourceLists.has(file) && !used.has(file)) + .map(repoRelative), + // A `reads` entry that was moved has to be updated in codegen.ts as well. + readsMissingOnDisk: readPaths.filter(file => !existsSync(file)).map(repoRelative), + }).toEqual({ script: true, untracked: [], unexplained: [], readsMissingOnDisk: [] }); }); - test("generate-classes: js_classes.ts, as a dependency rather than a command-line argument", () => { + test("generate-classes passes only the .classes.ts files on the command line", () => { using dir = tempDir("build-codegen-extra-inputs", {}); - const { cfg, n, sources, o, dirStamp } = configure(String(dir)); + const ctx = configure(String(dir)); + const { cfg, n, sources } = ctx; - emitGeneratedClasses({ n, cfg, sources, o, dirStamp }); + emitGeneratedClasses(ctx); + // generate-classes.ts loads every path on its command line as a list of + // class definitions, so the dependency-only inputs must not be passed there. const script = resolve(cfg.cwd, "src", "codegen", "generate-classes.ts"); const edge = edgeProducing(n, resolve(cfg.codegenDir, "ZigGeneratedClasses.cpp")); - const reads = ["src/jsc/bindings/js_classes.ts"]; - expect(tracking(cfg, edge, reads)).toEqual(tracked(reads)); - expect(edge.inputs).toEqual(expect.arrayContaining([script, ...sources.zigGeneratedClasses])); - // generate-classes.ts loads every path on its command line as a list of - // class definitions, so the command line has to stay the .classes.ts files. expect(edge.vars.args).toBe( quoteArgs(["run", script, ...sources.zigGeneratedClasses, cfg.codegenDir], cfg.host.os === "windows"), ); From dde4235ba69b4c41abde7c11a05eafea857d81d0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 07:59:29 +0000 Subject: [PATCH 3/3] ci: retrigger