diff --git a/scripts/build/CLAUDE.md b/scripts/build/CLAUDE.md index 274ea8c6d97b..553eed05605a 100644 --- a/scripts/build/CLAUDE.md +++ b/scripts/build/CLAUDE.md @@ -123,7 +123,7 @@ Tables: `cpuTargetFlags` (`-march`/`-mcpu`/`-mtune` — also forwarded to local **Bump a dependency** — edit the `commit` in `scripts/build/deps/.ts`. See `deps/README.md` for adding/removing deps. -**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 `emitBindgen` (reads a globbed source list). 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). If the step finds its input files itself (readdir/scan/bundle) rather than taking the list on its command line, give the edge a `sourceListFile()` implicit input as well: listing the files only tracks edits, and ninja does not treat a file vanishing from an edge's input list as a change, so without the manifest a deleted input never re-runs the step (`test/internal/source-lints/build-codegen-source-lists.test.ts` pins the existing ones). **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 a31d3c3cfa40..4383f1d127a3 100644 --- a/scripts/build/codegen.ts +++ b/scripts/build/codegen.ts @@ -9,6 +9,9 @@ * Source lists come from the patterns in glob-sources.ts, globbed once at * configure time via `globAllSources()`. The expanded * paths are baked into build.ninja; adding a file picks up on next configure. + * Removing one only picks up if the step's edge also tracks the SET of files + * (see `sourceListFile`): to ninja, an input that vanished from the list is + * not a change. * * bindgenv2 is special: its output set is dynamic (depends on which types * the .bindv2.ts files export). We invoke it with `--command=list-outputs` @@ -46,9 +49,14 @@ import type { Ninja } from "./ninja.ts"; import { quote, quoteArgs } from "./shell.ts"; import { generateXmlByteClass } from "./xmlByteClass.ts"; -// The individual emit functions take these four params. Bundled to keep +// The individual emit functions take these params. Bundled to keep // signatures short. -interface Ctx { +// +// Ctx and the emitters that take a globbed source list are exported for +// test/internal/source-lints/build-codegen-source-lists.test.ts, which emits +// those steps on their own: emitCodegen() as a whole spawns bun (bindgenv2 +// list-outputs). +export interface Ctx { n: Ninja; cfg: Config; sources: Sources; @@ -382,11 +390,44 @@ function shJoin(cfg: Config, args: string[]): string { return quoteArgs(args, cfg.host.os === "windows"); } +/** + * Write a globbed source list to `/-sources.txt` and return + * the path, for use as an implicit input of the step that reads those files. + * + * Listing the files themselves as inputs only tracks edits to them. Ninja + * re-runs an edge when an input is newer than its outputs or its command line + * changed; a file that disappeared from the input list is neither, so after + * `rm src/js/internal/foo.ts` the reconfigured bundle-modules edge is up to + * date and the deleted module stays in the binary until some surviving input + * is touched. The manifest turns the set into an input: writeIfChanged moves + * its mtime only when the list changes, so a removal re-runs the step and an + * unchanged list keeps the no-op reconfigure a no-op. + * + * Needed by every step whose output depends on which files exist without the + * command line saying so: bundle-modules, bindgen and generate-host-exports + * readdir the tree themselves, cppbind reads this very file, bun-error and + * bake are bundles (which files exist decides how imports resolve). Steps + * that pass the list on their command line (generate-classes, bindgenv2, + * build-fallbacks) get this from ninja's command-line tracking instead. + * + * Written at CONFIGURE time, not by a ninja rule: it's a derived manifest of + * our glob, like build_options.rs. One repo-relative forward-slash path per + * line, which is also the format cppbind reads its manifest in. codegenDir + * may not exist yet on a first configure. + */ +function sourceListFile(cfg: Config, name: string, files: string[]): string { + mkdirSync(cfg.codegenDir, { recursive: true }); + const file = resolve(cfg.codegenDir, `${name}-sources.txt`); + const lines = files.map(p => relative(cfg.cwd, p).replace(/\\/g, "/")); + writeIfChanged(file, lines.join("\n") + "\n"); + return file; +} + // ─────────────────────────────────────────────────────────────────────────── // Individual step emitters // ─────────────────────────────────────────────────────────────────────────── -function emitBunError({ n, cfg, sources, o, dirStamp }: Ctx): void { +export function emitBunError({ n, cfg, sources, o, dirStamp }: Ctx): void { const sourceDir = resolve(cfg.cwd, "packages", "bun-error"); const installStamp = emitBunInstall(n, cfg, sourceDir); @@ -399,7 +440,7 @@ function emitBunError({ n, cfg, sources, o, dirStamp }: Ctx): void { inputs: sources.bunError, // Install stamp as implicit — changing preact version re-bundles. // Root install as well (esbuild tool lives there). - implicitInputs: [installStamp, o.rootInstall], + implicitInputs: [sourceListFile(cfg, "bun-error", sources.bunError), installStamp, o.rootInstall], orderOnlyInputs: [dirStamp], vars: { cwd: sourceDir, @@ -614,7 +655,7 @@ function emitGeneratedClasses({ n, cfg, sources, o, dirStamp }: Ctx): void { // .lut.txt is consumed by emitObjectLuts below } -function emitHostExports({ n, cfg, sources, o, dirStamp }: Ctx): void { +export function emitHostExports({ n, cfg, sources, o, dirStamp }: Ctx): void { const script = resolve(cfg.cwd, "src", "codegen", "generate-host-exports.ts"); const output = resolve(cfg.codegenDir, "generated_host_exports.rs"); @@ -634,7 +675,7 @@ function emitHostExports({ n, cfg, sources, o, dirStamp }: Ctx): void { outputs: [output], rule: "codegen", inputs: [script], - implicitInputs: rsInputs, + implicitInputs: [sourceListFile(cfg, "host-exports", rsInputs), ...rsInputs], orderOnlyInputs: [dirStamp], vars: { cwd: cfg.cwd, @@ -650,24 +691,15 @@ function emitHostExports({ n, cfg, sources, o, dirStamp }: Ctx): void { o.rustInputs.push(output); } -function emitCppBind({ n, cfg, sources, o, dirStamp }: Ctx): void { +export function emitCppBind({ n, cfg, sources, o, dirStamp }: Ctx): void { const script = resolve(cfg.cwd, "src", "codegen", "cppbind.ts"); const outputRs = resolve(cfg.codegenDir, "cpp.rs"); - // Write the .cpp file list for cppbind to scan. Build system owns the - // glob (glob-sources.ts); we hand the result to cppbind - // as an explicit input instead of it reading a magic hardcoded path. - // Relative paths, forward slashes — same format cppbind expects. - // - // Written at CONFIGURE time (not via a ninja rule): it's a derived - // manifest from our glob, and we want writeIfChanged semantics so a - // stable .cpp set → unchanged mtime → ninja doesn't re-run cppbind. - // codegenDir may not exist yet on first configure — mkdir it. - mkdirSync(cfg.codegenDir, { recursive: true }); - const cxxSourcesFile = resolve(cfg.codegenDir, "cxx-sources.txt"); - const cxxSourcesLines = sources.cxx.map(p => relative(cfg.cwd, p).replace(/\\/g, "/")); - writeIfChanged(cxxSourcesFile, cxxSourcesLines.join("\n") + "\n"); + // The .cpp file list for cppbind to scan. Build system owns the glob + // (glob-sources.ts); we hand the result to cppbind as an argument instead + // of it reading a magic hardcoded path. + const cxxSourcesFile = sourceListFile(cfg, "cxx", sources.cxx); n.build({ outputs: [outputRs], @@ -702,7 +734,7 @@ 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). @@ -738,6 +770,9 @@ function emitJsModules({ n, cfg, sources, o, dirStamp }: Ctx): void { outputs, rule: "codegen", inputs: [script, ...sources.js, ...sources.jsCodegen, extraInput, errorCodeInput], + // The script readdirs src/js itself; the source list is what re-runs it + // when a module is deleted (see sourceListFile). + implicitInputs: [sourceListFile(cfg, "js", sources.js)], orderOnlyInputs: [dirStamp], vars: { cwd: cfg.cwd, @@ -754,7 +789,7 @@ function emitJsModules({ n, cfg, sources, o, dirStamp }: Ctx): void { o.cppHeaders.push(...outputs.filter(p => p.endsWith(".h"))); } -function emitBakeCodegen({ n, cfg, sources, o, dirStamp }: Ctx): void { +export 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. @@ -777,6 +812,7 @@ function emitBakeCodegen({ n, cfg, sources, o, dirStamp }: Ctx): void { outputs, rule: "codegen", inputs: [script, ...sources.bakeRuntime], + implicitInputs: [sourceListFile(cfg, "bake", sources.bakeRuntime)], orderOnlyInputs: [dirStamp], vars: { cwd: cfg.cwd, @@ -852,18 +888,20 @@ function emitBindgenV2({ n, cfg, sources, o, dirStamp }: Ctx): void { o.bindgenV2Cpp.push(...cppOutputs); } -function emitBindgen({ n, cfg, sources, o, dirStamp }: Ctx): void { +export function emitBindgen({ n, cfg, sources, o, dirStamp }: Ctx): void { const script = resolve(cfg.cwd, "src", "codegen", "bindgen.ts"); const cppOut = resolve(cfg.codegenDir, "GeneratedBindings.cpp"); // bindgen.ts scans src/ for .bind.ts files itself — this list is only for - // ninja dependency tracking. New .bind.ts files need a reconfigure to be - // picked up (next glob gets them). + // ninja dependency tracking. Added or deleted .bind.ts files need a + // reconfigure to be picked up (next glob gets them; the file list is what + // makes a deletion re-run the step). n.build({ outputs: [cppOut], rule: "codegen", inputs: [script, ...sources.bindgen], + implicitInputs: [sourceListFile(cfg, "bindgen", sources.bindgen)], orderOnlyInputs: [dirStamp], vars: { cwd: cfg.cwd, diff --git a/test/internal/source-lints/build-codegen-source-lists.test.ts b/test/internal/source-lints/build-codegen-source-lists.test.ts new file mode 100644 index 000000000000..96f0b63b6e62 --- /dev/null +++ b/test/internal/source-lints/build-codegen-source-lists.test.ts @@ -0,0 +1,279 @@ +/** + * Pins that the codegen steps which discover or bundle their globbed inputs + * also track the SET of those inputs (`sourceListFile` in + * scripts/build/codegen.ts). + * + * Ninja re-runs an edge when an input is newer than its outputs or when the + * command line changed. Deleting a file the step had globbed is neither: the + * reconfigured edge merely has a shorter input list and counts as up to date. + * So after `rm src/js/internal/foo.ts`, bundle-modules did not re-run and the + * module stayed in the binary until some surviving input was edited; the same + * held for bindgen, generate-host-exports, bake and bun-error. Each of those + * edges now has an implicit input `codegen/-sources.txt`, written at + * configure time with writeIfChanged: its mtime moves exactly when the list + * changes, so a deletion re-runs the step and an unchanged list stays a no-op. + * (cppbind has worked this way all along; it is the pattern being generalized.) + * + * Emits the six steps into a temporary build dir and reads back the ninja text + * and the manifests. The source lists are made up: nothing reads the files at + * configure time. No ninja, compiler or subprocess; runs on every host. + */ +import { describe, expect, test } from "bun:test"; +import { tempDir } from "harness"; +import { readFileSync, statSync } from "node:fs"; +import { relative, resolve } from "node:path"; + +import { + emitBakeCodegen, + emitBindgen, + emitBunError, + emitCppBind, + emitHostExports, + emitJsModules, + registerCodegenRules, + type CodegenOutputs, + type Ctx, +} 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 type { Sources } from "../../../scripts/glob-sources.ts"; + +/** A fully-populated fake toolchain; configure-time emission never runs any 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, + }; +} + +/** + * A linux-x64 debug target resolves on every host once it is told where its + * sysroot is (the path is only recorded, never opened). + */ +function linuxDebugConfig(buildDir: string): Config { + return resolveConfig( + { os: "linux", arch: "x64", abi: "gnu", buildType: "Debug", buildDir, linuxSysroot: buildDir }, + mockToolchain(), + ); +} + +/** + * Source lists shaped like globAllSources() output (absolute paths under the + * repo root), plus the subset of `rust` that generate-host-exports scrapes. + */ +function fixture(cfg: Config): { sources: Sources; hostExportsScrape: string[] } { + const src = (p: string) => resolve(cfg.cwd, "src", p); + const hostExportsScrape = [src("jsc/lib.rs"), src("runtime/api/BunObject.rs")]; + const sources: Sources = { + bunError: [resolve(cfg.cwd, "packages/bun-error/bun-error.css"), resolve(cfg.cwd, "packages/bun-error/index.tsx")], + js: [src("js/internal/a.ts"), src("js/internal/b.ts"), src("js/node/fs.ts")], + jsCodegen: [src("codegen/bundle-modules.ts"), src("codegen/replacements.ts")], + bakeRuntime: [src("runtime/bake/dev_server/mod.rs"), src("runtime/bake/hmr-runtime-client.ts")], + bindgen: [src("jsc/fmt_jsc.bind.ts"), src("runtime/node/node_os.bind.ts")], + cxx: [src("jsc/bindings/BunObject.cpp"), src("jsc/bindings/ZigGlobalObject.cpp")], + // A crate outside the scrape scope and a manifest inside it are fed to + // the cargo step but not to generate-host-exports. + rust: [resolve(cfg.cwd, "Cargo.toml"), src("bundler/lib.rs"), ...hostExportsScrape, src("runtime/Cargo.toml")], + // Read only by steps this test doesn't emit. + stringMaps: [], + nodeFallbacks: [], + zigGeneratedClasses: [], + bindgenV2: [], + bindgenV2Internal: [], + c: [], + }; + return { sources, hostExportsScrape }; +} + +interface Edge { + outputs: string[]; + inputs: string[]; + implicitInputs: string[]; +} + +/** Undo ninja.ts's build-line path escaping. */ +function unescapePath(token: string): string { + return token.replace(/\$([ :$])/g, "$1"); +} + +/** + * Parse every `build` line of the generated ninja text into its output and + * input sections (paths buildDir-relative, as ninja.ts writes them). + */ +function parseEdges(ninja: string): Edge[] { + const edges: Edge[] = []; + for (const line of ninja.replace(/ \$\n +/g, " ").split("\n")) { + if (!line.startsWith("build ")) continue; + const tokens = line.slice("build ".length).split(/(?<=[^$]) /); + // `build out | implout: rule in | implin || orderonly`: the rule name + // follows the first token ending in an unescaped colon. + const colon = tokens.findIndex(t => t.endsWith(":") && !t.endsWith("$:")); + if (colon === -1) throw new Error(`unparseable build line: ${line}`); + const outputs = [...tokens.slice(0, colon), tokens[colon]!.slice(0, -1)].filter(t => t !== "|").map(unescapePath); + const rest = tokens.slice(colon + 2); + const implicitStart = rest.indexOf("|"); + const orderOnlyStart = rest.indexOf("||"); + const end = orderOnlyStart === -1 ? rest.length : orderOnlyStart; + const inputs = rest.slice(0, implicitStart === -1 ? end : implicitStart).map(unescapePath); + const implicitInputs = implicitStart === -1 ? [] : rest.slice(implicitStart + 1, end).map(unescapePath); + edges.push({ outputs, inputs, implicitInputs }); + } + return edges; +} + +/** One configure's worth of the glob-driven steps, the way emitCodegen() drives them. */ +function emit(cfg: Config, sources: Sources): Edge[] { + const n = new Ninja({ buildDir: cfg.buildDir }); + registerDirStamps(n, cfg); + registerCodegenRules(n, cfg); + const o: CodegenOutputs = { + all: [], + rustInputs: [], + rustOrderOnly: [], + cppSources: [], + cppHeaders: [], + cppAll: [], + bindgenV2Cpp: [], + internalModulesAsm: resolve(cfg.codegenDir, "InternalModuleRegistryConstants.S"), + internalModulesBin: resolve(cfg.codegenDir, "InternalModuleRegistryConstants.bin"), + rootInstall: resolve(cfg.buildDir, "stamps", "install_root.stamp"), + }; + const ctx: Ctx = { n, cfg, sources, o, dirStamp: resolve(cfg.codegenDir, ".dir") }; + for (const step of [emitBunError, emitHostExports, emitCppBind, emitJsModules, emitBakeCodegen, emitBindgen]) { + step(ctx); + } + return parseEdges(n.toString()); +} + +/** `path` as ninja.ts writes it into build.ninja. */ +function ninjaPath(cfg: Config, path: string): string { + return relative(cfg.buildDir, path); +} + +/** The edge producing `/<...output>`. */ +function edgeProducing(cfg: Config, edges: Edge[], ...output: string[]): Edge { + const rel = ninjaPath(cfg, resolve(cfg.codegenDir, ...output)); + const edge = edges.find(e => e.outputs.includes(rel)); + if (edge === undefined) throw new Error(`no edge produces ${rel}`); + return edge; +} + +/** The manifest format: repo-relative, forward slashes, one file per line. */ +function manifestText(cfg: Config, files: string[]): string { + return files.map(f => relative(cfg.cwd, f).replaceAll("\\", "/")).join("\n") + "\n"; +} + +function readManifest(cfg: Config, name: string): string { + return readFileSync(resolve(cfg.codegenDir, name), "utf8"); +} + +/** + * Per step: the manifest its edge has to declare, an output that identifies + * the edge, and the files the manifest has to list. + */ +function steps(cfg: Config): { manifest: string; output: string[]; files: string[] }[] { + const { sources, hostExportsScrape } = fixture(cfg); + return [ + { manifest: "js-sources.txt", output: ["InternalModuleRegistry+enum.h"], files: sources.js }, + { manifest: "bindgen-sources.txt", output: ["GeneratedBindings.cpp"], files: sources.bindgen }, + { manifest: "host-exports-sources.txt", output: ["generated_host_exports.rs"], files: hostExportsScrape }, + { manifest: "bake-sources.txt", output: ["bake.client.js"], files: sources.bakeRuntime }, + { manifest: "bun-error-sources.txt", output: ["bun-error", "index.js"], files: sources.bunError }, + { manifest: "cxx-sources.txt", output: ["cpp.rs"], files: sources.cxx }, + ]; +} + +describe("codegen steps that glob their inputs track the set of inputs", () => { + test("each step's edge has a manifest as an implicit input, listing the files the edge is fed", () => { + using dir = tempDir("build-codegen-source-lists", {}); + const cfg = linuxDebugConfig(String(dir)); + const edges = emit(cfg, fixture(cfg).sources); + + const actual = steps(cfg).map(({ manifest, output }) => { + const edge = edgeProducing(cfg, edges, ...output); + const tracked = new Set([...edge.inputs, ...edge.implicitInputs]); + const content = readManifest(cfg, manifest); + return { + manifest, + declared: edge.implicitInputs.includes(ninjaPath(cfg, resolve(cfg.codegenDir, manifest))), + content, + // The manifest's files are the edge's own inputs, so edits to them + // re-run the step as before; the manifest only adds the set itself. + listedButUntracked: content + .trimEnd() + .split("\n") + .filter(line => !tracked.has(ninjaPath(cfg, resolve(cfg.cwd, line)))), + }; + }); + + expect(actual).toEqual( + steps(cfg).map(({ manifest, files }) => ({ + manifest, + declared: true, + content: manifestText(cfg, files), + listedButUntracked: [], + })), + ); + }); + + test("a manifest is rewritten exactly when its list changes", () => { + using dir = tempDir("build-codegen-source-lists", {}); + const cfg = linuxDebugConfig(String(dir)); + const { sources } = fixture(cfg); + const snapshot = () => + Object.fromEntries( + steps(cfg).map(({ manifest }) => [ + manifest, + { content: readManifest(cfg, manifest), mtimeMs: statSync(resolve(cfg.codegenDir, manifest)).mtimeMs }, + ]), + ); + + emit(cfg, sources); + const before = snapshot(); + + // Reconfigure with nothing changed: no manifest may be touched, or every + // `bun bd` would re-run these steps. + emit(cfg, sources); + expect(snapshot()).toEqual(before); + + // Reconfigure after `rm src/js/internal/b.ts`: the glob no longer returns + // it. The JS manifest is rewritten (its new mtime is what makes ninja + // re-run bundle-modules, which drops the module); nothing else is touched. + const remaining = sources.js.filter(f => !f.endsWith("b.ts")); + expect(remaining).toHaveLength(sources.js.length - 1); + emit(cfg, { ...sources, js: remaining }); + expect(snapshot()).toEqual({ + ...before, + "js-sources.txt": { content: manifestText(cfg, remaining), mtimeMs: expect.any(Number) }, + }); + expect(before["js-sources.txt"]!.content).toBe(manifestText(cfg, sources.js)); + }); +});