diff --git a/scripts/build/bun.ts b/scripts/build/bun.ts index cd66d264a06e..be8aa5a23074 100644 --- a/scripts/build/bun.ts +++ b/scripts/build/bun.ts @@ -509,40 +509,8 @@ export function emitBun(n: Ninja, cfg: Config, sources: Sources): BunOutput { linkerMapOutput: cfg.linux && cfg.release && !cfg.asan && !cfg.valgrind ? linkerMapPath(cfg) : undefined, }); - // ─── Step 8: post-link (strip + dsymutil) ─── - // Plain release only: produce stripped `bun` alongside `bun-profile`. - // Debug/asan/etc. keep symbols (you want them for debugging). - let strippedExe: string | undefined; - let dsym: string | undefined; - if (shouldStrip(cfg)) { - strippedExe = emitStrip(n, cfg, exe, flags.stripflags); - // darwin: extract debug symbols from the UNSTRIPPED exe into a .dSYM - // bundle. dsymutil reads DWARF from bun-profile, writes bun-profile.dSYM. - // Must run BEFORE stripping could discard sections it needs (we don't - // strip bun-profile itself, only copy → bun, so this is safe). - if (cfg.darwin) { - dsym = emitDsymutil(n, cfg, exe, exeName); - } - } - - // Phony `bun` target for convenience — only when strip DIDN'T produce a - // literal file named `bun` (which would collide with the phony). When - // strip runs, `ninja bun` builds the actual stripped file; no phony needed. - if (strippedExe === undefined) { - n.phony("bun", [exe]); - } - - // ─── Step 9: smoke test ─── - // Run ` --revision`. If it exits non-zero or crashes, something - // broke at load time (missing symbol, static initializer blowup, ABI - // mismatch). Catching this HERE is much better than "CI passes, user - // runs bun, it segfaults". - // - // Linux+ASAN quirk: some systems need ASLR disabled (`setarch -R`) for - // ASAN binaries to run from subprocesses (shadow memory layout conflict - // with ELF_ET_DYN_BASE, see sanitizers/856). We try with setarch first, - // fall back to direct invocation. - emitSmokeTest(n, cfg, exe, exeName); + // ─── Step 8: post-link (strip, dsymutil, smoke test) ─── + const { strippedExe, dsym } = emitPostLink(n, cfg, exe, exeName, flags.stripflags); return { exe, strippedExe, dsym, deps, codegen, rustObjects, objects: allObjects }; } @@ -652,14 +620,7 @@ function emitLinkOnly(n: Ninja, cfg: Config): BunOutput { }); // Strip + smoke test — same as full mode. - let strippedExe: string | undefined; - let dsym: string | undefined; - if (shouldStrip(cfg)) { - strippedExe = emitStrip(n, cfg, exe, flags.stripflags); - if (cfg.darwin) dsym = emitDsymutil(n, cfg, exe, exeName); - } - if (strippedExe === undefined) n.phony("bun", [exe]); - emitSmokeTest(n, cfg, exe, exeName); + const { strippedExe, dsym } = emitPostLink(n, cfg, exe, exeName, flags.stripflags); return { exe, @@ -733,14 +694,7 @@ function emitRustAndLink(n: Ninja, cfg: Config, sources: Sources): BunOutput { linkerMapOutput: cfg.linux && cfg.release && !cfg.asan && !cfg.valgrind ? linkerMapPath(cfg) : undefined, }); - let strippedExe: string | undefined; - let dsym: string | undefined; - if (shouldStrip(cfg)) { - strippedExe = emitStrip(n, cfg, exe, flags.stripflags); - if (cfg.darwin) dsym = emitDsymutil(n, cfg, exe, exeName); - } - if (strippedExe === undefined) n.phony("bun", [exe]); - emitSmokeTest(n, cfg, exe, exeName); + const { strippedExe, dsym } = emitPostLink(n, cfg, exe, exeName, flags.stripflags); return { exe, @@ -753,13 +707,71 @@ function emitRustAndLink(n: Ninja, cfg: Config, sources: Sources): BunOutput { }; } +/** + * Post-link steps shared by every linking mode (full, link-only, + * rust-and-link): strip, dsymutil, the `bun` phony, and the `--revision` + * smoke test. + * + * Centralized because the smoke_test and dsymutil edges must be ordered + * after strip — their rule commands wrap through `cfg.jsRuntime` + * (process.execPath), which can BE the strip output when `bun` on PATH + * resolves into the build directory (build/release/bun). Without the + * ordering, ninja runs strip and the wrapper exec concurrently (both + * depend only on `exe`) and the wrapper fails with "Permission denied" on + * the half-written file. Open-coding this in each mode already caused one + * call site to be missed (#30539), so the invariant lives here. + */ +export function emitPostLink( + n: Ninja, + cfg: Config, + exe: string, + exeName: string, + stripflags: string[], +): { strippedExe: string | undefined; dsym: string | undefined } { + // Plain release only: produce stripped `bun` alongside `bun-profile`. + // Debug/asan/valgrind/assertions keep symbols (you want them for + // debugging). + let strippedExe: string | undefined; + let dsym: string | undefined; + if (shouldStrip(cfg)) { + strippedExe = emitStrip(n, cfg, exe, stripflags); + // darwin: extract debug symbols from the UNSTRIPPED exe into a .dSYM + // bundle. dsymutil reads DWARF from bun-profile, writes + // bun-profile.dSYM. The input exe is never stripped in-place (strip + // writes a new file via -o), so the read is safe. + if (cfg.darwin) dsym = emitDsymutil(n, cfg, exe, exeName, strippedExe); + } + + // `bun` phony — only when strip didn't produce a literal file named + // `bun` (which would collide with the phony). When strip runs, `ninja + // bun` builds the stripped file; no phony needed. + if (strippedExe === undefined) n.phony("bun", [exe]); + + // Run ` --revision`. If it exits non-zero or crashes, something + // broke at load time (missing symbol, static initializer blowup, ABI + // mismatch). Catching this HERE is much better than "CI passes, user + // runs bun, it segfaults". + // + // Linux+ASAN quirk: some systems need ASLR disabled (`setarch -R`) for + // ASAN binaries to run from subprocesses (shadow memory layout conflict + // with ELF_ET_DYN_BASE, see sanitizers/856). We try with setarch first, + // fall back to direct invocation. + emitSmokeTest(n, cfg, exe, exeName, strippedExe); + + return { strippedExe, dsym }; +} + /** * Smoke test: run the built executable with --revision. If it crashes or * errors, the build failed — typically means a link-time issue that the * linker didn't catch (missing symbol only referenced at init, ICU ABI * mismatch, etc.). + * + * `strippedExe` is the strip output (release builds only), added as an + * order-only input so this rule never runs while strip is mid-write; see + * emitPostLink for why. */ -function emitSmokeTest(n: Ninja, cfg: Config, exe: string, exeName: string): void { +function emitSmokeTest(n: Ninja, cfg: Config, exe: string, exeName: string, strippedExe: string | undefined): void { // Skip when the binary can't run on this host (different os/arch/abi) — // `ninja check` becomes a no-op alias for the exe. if (!cfg.canRunOnHost) { @@ -804,6 +816,7 @@ function emitSmokeTest(n: Ninja, cfg: Config, exe: string, exeName: string): voi outputs: [stamp], rule: "smoke_test", inputs: [exe], + ...(strippedExe !== undefined ? { orderOnlyInputs: [strippedExe] } : {}), }); // Phony target — `ninja check` runs the smoke test. @@ -860,8 +873,11 @@ function emitStrip(n: Ninja, cfg: Config, inputExe: string, stripflags: string[] * Runs dsymutil on bun-profile (which has full DWARF). The .dSYM lets you * symbolicate crash logs from the stripped `bun` — lldb/Instruments find * it automatically by UUID. + * + * `strippedExe` is order-only for the same reason as emitSmokeTest: the + * `cfg.jsRuntime` wrapper may be the strip output itself. */ -function emitDsymutil(n: Ninja, cfg: Config, inputExe: string, exeName: string): string { +function emitDsymutil(n: Ninja, cfg: Config, inputExe: string, exeName: string, strippedExe: string): string { assert(cfg.darwin, "dsymutil is darwin-only"); assert(cfg.dsymutil !== undefined, "dsymutil not found in toolchain"); @@ -892,6 +908,7 @@ function emitDsymutil(n: Ninja, cfg: Config, inputExe: string, exeName: string): outputs: [out], rule: "dsymutil", inputs: [inputExe], + orderOnlyInputs: [strippedExe], }); return out; diff --git a/test/internal/build-post-link-ordering.test.ts b/test/internal/build-post-link-ordering.test.ts new file mode 100644 index 000000000000..0a82b72bf6cb --- /dev/null +++ b/test/internal/build-post-link-ordering.test.ts @@ -0,0 +1,138 @@ +/** + * Regression tests for ninja ordering of post-link steps (strip, smoke test, + * dsymutil) in scripts/build/bun.ts. + * + * The smoke_test and dsymutil rule commands are wrapped through + * `cfg.jsRuntime` (= process.execPath). When `bun` on PATH resolves inside the + * build directory, that path is the strip output itself (build/release/bun), + * and without an ordering edge ninja will run strip and the wrapper exec + * concurrently, failing with "Permission denied" on the half-written file. + * + * These exercise the ninja-emission logic only (no compiler or ninja needed), + * so they run on every host. + */ +import { describe, expect, test } from "bun:test"; +import { isMacOS, tempDir } from "harness"; +import { join, resolve } from "node:path"; + +import { emitPostLink } from "../../scripts/build/bun.ts"; +import { resolveConfig, type Config, type PartialConfig, type Toolchain } from "../../scripts/build/config.ts"; +import { Ninja } from "../../scripts/build/ninja.ts"; + +/** A fully-populated fake toolchain; resolveConfig never spawns any of these. */ +function mockToolchain(overrides: Partial = {}): 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, + ...overrides, + }; +} + +/** + * Resolve a host-targeted config: no os/arch override, so `canRunOnHost` is + * true and the smoke_test rule emits the real edge (not the phony short-circuit). + */ +function hostConfig(partial: PartialConfig, buildDir: string): Config { + return resolveConfig( + { buildDir, ...partial }, + // jsRuntime = the strip output: what resolveToolchain() produces when + // `bun` on PATH resolves into build/release/. + mockToolchain({ jsRuntime: join(buildDir, "bun") }), + ); +} + +/** Find one build-edge line in the generated ninja text (continuations unwrapped). */ +function buildEdge(ninja: string, rule: string): string { + const flat = ninja.replace(/ \$\n +/g, " "); + const line = flat.split("\n").find(l => l.startsWith("build ") && l.includes(`: ${rule} `)); + if (line === undefined) throw new Error(`no '${rule}' edge in ninja output:\n${ninja}`); + return line; +} + +describe("emitPostLink ninja ordering", () => { + test("release smoke_test is ordered after strip", () => { + using dir = tempDir("build-post-link", {}); + const buildDir = String(dir); + const cfg = hostConfig({ buildType: "Release" }, buildDir); + expect(cfg.canRunOnHost).toBe(true); + + const n = new Ninja({ buildDir }); + const exe = resolve(buildDir, `bun-profile${cfg.exeSuffix}`); + const { strippedExe } = emitPostLink(n, cfg, exe, "bun-profile", []); + const out = n.toString(); + + expect(strippedExe).toBe(resolve(buildDir, `bun${cfg.exeSuffix}`)); + // strip writes `bun`; the smoke_test wrapper execs cfg.jsRuntime + // (= `bun` here). Without `|| bun` ninja schedules them concurrently + // and the wrapper sees a half-written file. + expect(buildEdge(out, "smoke_test")).toBe( + `build bun-profile.smoke-test-passed: smoke_test bun-profile${cfg.exeSuffix} || bun${cfg.exeSuffix}`, + ); + expect(buildEdge(out, "strip")).toBe(`build bun${cfg.exeSuffix}: strip bun-profile${cfg.exeSuffix}`); + }); + + test("debug smoke_test has no strip dep (nothing to order against)", () => { + using dir = tempDir("build-post-link", {}); + const buildDir = String(dir); + const cfg = hostConfig({ buildType: "Debug", assertions: true }, buildDir); + + const n = new Ninja({ buildDir }); + const exe = resolve(buildDir, `bun-debug${cfg.exeSuffix}`); + const { strippedExe, dsym } = emitPostLink(n, cfg, exe, "bun-debug", []); + const out = n.toString(); + + expect({ strippedExe, dsym }).toEqual({ strippedExe: undefined, dsym: undefined }); + expect(buildEdge(out, "smoke_test")).toBe( + `build bun-debug.smoke-test-passed: smoke_test bun-debug${cfg.exeSuffix}`, + ); + expect(buildEdge(out, "phony")).toBe(`build bun: phony bun-debug${cfg.exeSuffix}`); + }); + + // Cross-config path only: on macOS, resolveConfig({ os: "darwin" }) probes + // xcode-select for the real SDK, which belongs to the native test above. + // The ordering logic is identical to the smoke_test case. + test.skipIf(isMacOS)("darwin release dsymutil is ordered after strip", () => { + using dir = tempDir("build-post-link", {}); + const buildDir = String(dir); + const cfg = resolveConfig({ os: "darwin", arch: "aarch64", buildType: "Release", buildDir }, mockToolchain()); + expect(cfg.canRunOnHost).toBe(false); + + const n = new Ninja({ buildDir }); + const exe = resolve(buildDir, "bun-profile"); + const { dsym } = emitPostLink(n, cfg, exe, "bun-profile", []); + const out = n.toString(); + + expect(dsym).toBe(resolve(buildDir, "bun-profile.dSYM")); + expect(buildEdge(out, "dsymutil")).toBe("build bun-profile.dSYM: dsymutil bun-profile || bun"); + // Cross-compile: smoke_test short-circuits to a `check` phony (the + // binary can't run on this host), so the strip race can't happen there. + expect(buildEdge(out, "phony")).toBe("build check: phony bun-profile"); + }); +});