Repository navigation
orderfile: fix inheriting, and generate when there is nothing to inherit - #33345
Merged
Merged
Conversation
Collaborator
|
Updated 10:21 PM PT - Jul 4th, 2026
@autofix-ci[bot], your commit 1ea2834 is building: |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Disabled knowledge base sources:
WalkthroughThis PR refactors CI order-file handling around explicit Buildkite context, new generation and inheritance gates, validated artifact inheritance, bootstrap reporting, and updated tests. ChangesOrder file inheritance and bootstrap rework
Sequence Diagram(s)sequenceDiagram
participant scripts/build.ts
participant scripts/build/ci.ts
participant buildkiteEnv as Buildkite env
participant buildkiteAgent as buildkite-agent
scripts/build.ts->>scripts/build/ci.ts: orderFileContext()
scripts/build.ts->>scripts/build/ci.ts: mustGenerateOrderFile(cfg, ctx, inherited)
scripts/build/ci.ts->>buildkiteEnv: read Buildkite state once
scripts/build/ci.ts->>scripts/build/ci.ts: candidateBuilds(ctx)
scripts/build/ci.ts->>buildkiteAgent: download *.order artifact
buildkiteAgent-->>scripts/build/ci.ts: artifact or failure
scripts/build.ts->>scripts/build/ci.ts: reportOrderFileBootstrap(cfg)
Compact metadata: 4 files changed (+228/-61 lines) across Related issues: None provided. Related PRs: None provided. Suggested labels: ci, build-tooling, tests Suggested reviewers: None provided. Poem 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build/ci.ts`:
- Around line 714-734: The Buildkite metadata lookups in the build-order flow
can still hang indefinitely, blocking symbol-order inheritance. Update the
`newest` fetch path in `scripts/build/ci.ts` and the probe loop that calls
`fetchBuild`/`utils.curl()` to use an abortable timeout, so stalled
`builds/latest` redirects and build JSON requests fail fast. Keep the change
localized around the existing `newest` async block and the `NUMBER_PROBE_BUDGET`
loop, reusing the same request helpers where possible. Ensure timeout failures
are handled the same way as other fetch errors and do not break the existing
fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 20489c74-2bb2-4d1b-be27-1abb8b741ad9
📒 Files selected for processing (4)
scripts/build.tsscripts/build/ci.tsscripts/orderfile/generate.tstest/js/bun/perf/linker-order.test.ts
Comment on lines
+714
to
+734
| const newest = await (async () => { | ||
| try { | ||
| const latest = `${pipeline}/builds/latest?branch=${encodeURIComponent(branch)}&state=passed`; | ||
| const location = (await fetch(latest, { redirect: "manual" })).headers.get("location"); | ||
| return location ? await fetchBuild(`${location}.json`) : undefined; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| })(); | ||
| if (newest?.id) { | ||
| seen.add(newest.id); | ||
| yield { id: newest.id, number: newest.number }; | ||
| } | ||
|
|
||
| let number = Number(process.env.BUILDKITE_BUILD_NUMBER); | ||
| if (!Number.isFinite(number)) return; | ||
|
|
||
| for (let probes = 0; probes < NUMBER_PROBE_BUDGET; probes++) { | ||
| number -= 1; | ||
| if (number < 1) return; | ||
| const body = await fetchBuild(`${pipeline}/builds/${number}.json`); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect utils.curl for a default/enforced timeout.
rg -nP -C4 '\bexport (async )?function curl\b|\bcurl\s*=' scripts/utils.mjsRepository: oven-sh/bun
Length of output: 485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Buildkite metadata probe logic and the curl helper implementation.
sed -n '700,760p' scripts/build/ci.ts
printf '\n---- utils.mjs curl ----\n'
sed -n '826,940p' scripts/utils.mjs
printf '\n---- timeout-related references ----\n'
rg -n "AbortSignal\.timeout|timeout|signal" scripts/build/ci.ts scripts/utils.mjsRepository: oven-sh/bun
Length of output: 7810
Add a timeout to the Buildkite metadata requests in scripts/build/ci.ts
inheritOrderFile already bounds the artifact download, but the builds/latest redirect fetch and the utils.curl() probe loop can still hang indefinitely without an abort/timeout. A stalled Buildkite response would block symbol-order inheritance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build/ci.ts` around lines 714 - 734, The Buildkite metadata lookups
in the build-order flow can still hang indefinitely, blocking symbol-order
inheritance. Update the `newest` fetch path in `scripts/build/ci.ts` and the
probe loop that calls `fetchBuild`/`utils.curl()` to use an abortable timeout,
so stalled `builds/latest` redirects and build JSON requests fail fast. Keep the
change localized around the existing `newest` async block and the
`NUMBER_PROBE_BUDGET` loop, reusing the same request helpers where possible.
Ensure timeout failures are handled the same way as other fetch errors and do
not break the existing fallback behavior.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Inheriting never worked. `previousBuilds()` walked `prev_branch_build` from
Buildkite's public build JSON, and that field no longer exists, so the walk
found nothing and every build linked unordered:
~ symbol order: no previous build on this branch (0s) — linking unordered
The same field is why `utils.getLastSuccessfulBuild()` always returns undefined,
so it cannot be used here either. What does work, unauthenticated, is
`builds/latest?branch=X&state=passed`, which 302s at the build; read the
redirect rather than following it, because it drops the `.json` on the way.
Older builds are then found by probing build numbers, which is only a safety
net: candidates are produced lazily so the normal path is one lookup.
Even fixed, the chain could never start. A build that inherits nothing has no
order file, so it publishes none, so the next build inherits nothing either.
Nothing seeds it short of a release. Now a build that inherits nothing traces
its own binary and relinks, which both bootstraps the chain and heals it after
a gap. That costs a second link, so it announces itself: seeing it once is
expected, seeing it on every build means inheriting is broken again.
Also:
- an empty or truncated artifact no longer counts as inherited (it would
have published nothing, silently breaking the chain for the next build)
- probing starts from this build rather than the newest passed one, so a
build that failed its tests but still published is considered
- cross-compiled targets never trace: they cannot run the binary they linked.
They can still inherit and link ordered.
Jarred-Sumner
force-pushed
the
claude/orderfile-inherit-fix
branch
from
July 5, 2026 05:19
c04749e to
b89d63f
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/build/ci.ts (1)
700-708: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBuildkite metadata lookups still lack a timeout.
fetch(latest, { redirect: "manual" })and theutils.curl()calls in the probe loop have no timeout/AbortSignal. A stalled Buildkite response can hang the "Inherit symbol order file" step (which runs before ninja, on the critical path for every eligible build), for as many asNUMBER_PROBE_BUDGET(200) sequential requests. This was flagged on an earlier revision of this PR and appears unresolved in the current code.🛡️ Proposed fix: bound each metadata request
const fetchBuild = async (target: string): Promise<any | undefined> => { - const response: { error?: unknown; body?: any } = await utils.curl(target, { json: true, cache: true }); + const response: { error?: unknown; body?: any } = await utils.curl(target, { + json: true, + cache: true, + timeout: METADATA_TIMEOUT_MS, + }); return response.error ? undefined : response.body; }; ... const latest = `${pipeline}/builds/latest?branch=${encodeURIComponent(branch)}&state=passed`; - const location = (await fetch(latest, { redirect: "manual" })).headers.get("location"); + const location = ( + await fetch(latest, { redirect: "manual", signal: AbortSignal.timeout(METADATA_TIMEOUT_MS) }) + ).headers.get("location");Also applies to: 719-726
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/build/ci.ts` around lines 700 - 708, Buildkite metadata lookups in the newest-build fetch and the probe loop still have no timeout, so these requests can hang the step indefinitely. Update the logic around the async newest lookup and the probe loop that uses utils.curl() to pass a bounded timeout or AbortSignal for every metadata request, using a shared helper if available so both paths are handled consistently. Keep the change localized to the newest build resolution and the probe request loop so stalled Buildkite responses fail fast instead of blocking the critical path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@scripts/build/ci.ts`:
- Around line 700-708: Buildkite metadata lookups in the newest-build fetch and
the probe loop still have no timeout, so these requests can hang the step
indefinitely. Update the logic around the async newest lookup and the probe loop
that uses utils.curl() to pass a bounded timeout or AbortSignal for every
metadata request, using a shared helper if available so both paths are handled
consistently. Keep the change localized to the newest build resolution and the
probe request loop so stalled Buildkite responses fail fast instead of blocking
the critical path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fa6de538-2a06-48c8-a816-0af2592eaf8a
📒 Files selected for processing (4)
scripts/build.tsscripts/build/ci.tsscripts/orderfile/generate.tstest/js/bun/perf/linker-order.test.ts
Comment on lines
761
to
+789
| rmSync(downloaded, { force: true }); | ||
| // An empty artifact would make us publish nothing, breaking the next build. | ||
| const functions = orderFileFunctionCount(cfg); | ||
| if (functions === 0) { | ||
| console.log(` #${build.number ?? "?"}: ${artifact} is empty — looking further back`); | ||
| continue; | ||
| } | ||
|
|
||
| console.log( | ||
| `+ symbol order: inherited ${artifact}, ${orderFileFunctionCount(cfg)} functions ` + | ||
| `from build #${build.number ?? "?"} in ${since(start)}`, | ||
| `+ symbol order: inherited ${artifact}, ${functions} functions from #${build.number ?? "?"} in ${since(start)}`, | ||
| ); | ||
| return; | ||
| return true; | ||
| } | ||
|
|
||
| console.log( | ||
| `~ symbol order: none of the last ${candidates.length} builds published ${artifact} ` + | ||
| `(${since(start)}) — linking unordered`, | ||
| ); | ||
| const what = | ||
| tried === 0 ? "found no earlier build to inherit from" : `none of the ${tried} builds tried published it`; | ||
| console.log(`~ symbol order: ${what} (${since(start)})`); | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * After pass 1: trace this build's binary and overwrite the order file. The | ||
| * caller must re-run ninja, which relinks and nothing else — `linkDepends()` | ||
| * lists the order file, so it is the only edge whose input changed. | ||
| * Trace the binary from pass 1 and overwrite the order file. The caller re-runs | ||
| * ninja, which relinks and nothing else: `linkDepends()` lists the order file, | ||
| * so it is the only edge whose input changed. | ||
| */ | ||
| export function regenerateOrderFile(cfg: Config): void { | ||
| export function regenerateOrderFile(cfg: Config, ctx: OrderFileContext): void { | ||
| const start = Date.now(); | ||
| const exeName = bunExeName(cfg); // bun-profile, or bun-assertions on an assertions build | ||
| const why = cfg.canary ? "[generate symbol order] in the commit message" : "release build"; | ||
| const why = !cfg.canary |
Contributor
There was a problem hiding this comment.
🟡 Nit: when the loop breaks on the 51st candidate, ++tried becomes 51 before the break, so the fall-through log says "none of the 51 builds tried published it" even though only 50 downloads were attempted. if (tried++ >= PREVIOUS_BUILDS_TO_TRY) break; (or interpolating Math.min(tried, PREVIOUS_BUILDS_TO_TRY)) keeps the count honest. Purely cosmetic — the cap and fallback behavior are correct.
Extended reasoning...
What the bug is
In inheritOrderFile(), the loop that walks candidate builds is capped by:
for await (const build of candidateBuilds(ctx)) {
if (++tried > PREVIOUS_BUILDS_TO_TRY) break;
const result = spawnSync("buildkite-agent", [...]);
...
}Because tried is pre-incremented before the comparison, on the iteration that hits the cap the counter is bumped to 51 and the loop breaks without attempting a download for that candidate. The fall-through message then interpolates ${tried}:
const what = tried === 0 ? "..." : `none of the ${tried} builds tried published it`;so it reports "none of the 51 builds tried published it" when only 50 buildkite-agent artifact download invocations actually ran.
Step-by-step proof
With PREVIOUS_BUILDS_TO_TRY = 50 and candidateBuilds(ctx) yielding ≥ 51 candidates:
tried = 0.- Iteration 1:
++tried→ 1;1 > 50is false; download Fix ?? operator #1 attempted. - …
- Iteration 50:
++tried→ 50;50 > 50is false; download Bun v0.0.44 #50 attempted. - Iteration 51:
++tried→ 51;51 > 50is true; break — no download. - After the loop,
tried === 51; log line reads "none of the 51 builds tried published it".
Only 50 spawnSync calls were made, so the number is off by one.
Reachability
candidateBuilds on main yields the latest passed build plus up to NUMBER_PROBE_BUDGET = 200 branch-matching probes, so it can easily produce > 50 candidates. The path only fires when all 50 attempted downloads fail, which — after this PR's own bootstrap fix (a build that inherits nothing generates and publishes) — should be extremely rare. But it is reachable.
Why nothing prevents it
Nothing else adjusts tried between the break and the log line, and the message uses the raw counter rather than the cap constant. The tried === 0 branch handles only the "no candidates at all" case.
Impact
Purely cosmetic. This is a diagnostic console.log in a CI build script's rare failure path; nothing consumes the number programmatically, the 50-download cap is enforced correctly, and the fallback behavior (return false → generate from scratch) is unchanged. A human reading "51" vs "50" gets the same signal.
Fix
Either post-increment so the break happens before the counter advances past the number of attempts:
if (tried++ >= PREVIOUS_BUILDS_TO_TRY) break;or clamp in the message:
`none of the ${Math.min(tried, PREVIOUS_BUILDS_TO_TRY)} builds tried published it`
Jarred-Sumner
pushed a commit
that referenced
this pull request
Jul 6, 2026
) Follow-up to #33345, fixing the `run()` stdin wiring flagged in [this review](#33345 (comment)). ### The bug `run()` asks for `input` and for stdin to be `"ignore"` at the same time: ```ts input: options.input, stdio: ["ignore", "pipe", "pipe"], ``` Node only delivers `input` when stdin is a pipe. With `"ignore"` it drops it on the floor, no error. Bun's `spawnSync` delivers it either way, which is why `bun run orderfile` works locally and nobody noticed: CI builds with `node --experimental-strip-types scripts/build.ts` (`.buildkite/ci.mjs:595`), so on CI the two interactive workloads are the only ones that are typed anything and they get nothing. Running the real `cli-fixture.js` through the real `run()` options, once under each runtime: ``` ### under NODE (what CI uses): stdin=ignore pipe: {"status":0,"read":0,"ms":50} stdin=ignore tty : {"err":"ETIMEDOUT","ms":10007} stdin=pipe pipe: {"status":0,"read":3,"ms":50} stdin=pipe tty : {"status":0,"read":3,"ms":58} ### under BUN (what a dev uses locally): stdin=ignore pipe: {"status":0,"read":3,"ms":43} stdin=ignore tty : {"status":0,"read":3,"ms":52} ``` So on CI today the pipe workload reads `0` lines, exits `0`, and never drives readline. The tty one is worse: `ptyrun` gets `/dev/null` for stdin, types its `^D` before bun has even started, readline then waits for a line that never arrives, and the workload sits there until `WORKLOAD_TIMEOUT_MS` (120s) kills it. `run()` turns that into a throw, `build.ts` catches it and annotates, and the build ships unordered. Either way the ~2k tty and readline functions those two workloads exist to capture are missing, which is the whole reason they were added. ### The fix Pipe stdin when there is input to deliver. That is what the rest of the repo already does (`scripts/utils.mjs:246`, `scripts/utils.mjs:358`, `scripts/build/fetch-cli.ts:250`); `generate.ts` was the only site that got it wrong, and I grepped the others to be sure. `run()` captured nothing from its closure, so it is hoisted to module scope as `runCommand` and exported. That is what lets the test drive it under node, which is the only runtime where any of this is observable. ### Verification `test/js/bun/perf/linker-order.test.ts` gains a case that spawns `node --experimental-strip-types` on a fixture importing `runCommand`, runs `cli-fixture.js` through it, and checks the fixture was actually typed its input. An in-process test would be useless here: it would run under bun and pass either way. With stdin reverted to `"ignore"`: ``` { "exitCode": 0, - "greeted": true, - "read": "3", + "greeted": false, + "read": "0", } (fail) interactive workload stdin > reaches the workload when the generator runs under node, as CI does ``` and with the fix, `17 pass, 1 skip, 0 fail` for the file. Agents install node 26.3.0 (`scripts/bootstrap.sh`), so the case runs rather than skips.
liooil
pushed a commit
to liooil/poly
that referenced
this pull request
Aug 7, 2026
…358) Follow-up to #33345, fixing the `run()` stdin wiring flagged in [this review](oven-sh/bun#33345 (comment)). ### The bug `run()` asks for `input` and for stdin to be `"ignore"` at the same time: ```ts input: options.input, stdio: ["ignore", "pipe", "pipe"], ``` Node only delivers `input` when stdin is a pipe. With `"ignore"` it drops it on the floor, no error. Bun's `spawnSync` delivers it either way, which is why `bun run orderfile` works locally and nobody noticed: CI builds with `node --experimental-strip-types scripts/build.ts` (`.buildkite/ci.mjs:595`), so on CI the two interactive workloads are the only ones that are typed anything and they get nothing. Running the real `cli-fixture.js` through the real `run()` options, once under each runtime: ``` ### under NODE (what CI uses): stdin=ignore pipe: {"status":0,"read":0,"ms":50} stdin=ignore tty : {"err":"ETIMEDOUT","ms":10007} stdin=pipe pipe: {"status":0,"read":3,"ms":50} stdin=pipe tty : {"status":0,"read":3,"ms":58} ### under BUN (what a dev uses locally): stdin=ignore pipe: {"status":0,"read":3,"ms":43} stdin=ignore tty : {"status":0,"read":3,"ms":52} ``` So on CI today the pipe workload reads `0` lines, exits `0`, and never drives readline. The tty one is worse: `ptyrun` gets `/dev/null` for stdin, types its `^D` before bun has even started, readline then waits for a line that never arrives, and the workload sits there until `WORKLOAD_TIMEOUT_MS` (120s) kills it. `run()` turns that into a throw, `build.ts` catches it and annotates, and the build ships unordered. Either way the ~2k tty and readline functions those two workloads exist to capture are missing, which is the whole reason they were added. ### The fix Pipe stdin when there is input to deliver. That is what the rest of the repo already does (`scripts/utils.mjs:246`, `scripts/utils.mjs:358`, `scripts/build/fetch-cli.ts:250`); `generate.ts` was the only site that got it wrong, and I grepped the others to be sure. `run()` captured nothing from its closure, so it is hoisted to module scope as `runCommand` and exported. That is what lets the test drive it under node, which is the only runtime where any of this is observable. ### Verification `test/js/bun/perf/linker-order.test.ts` gains a case that spawns `node --experimental-strip-types` on a fixture importing `runCommand`, runs `cli-fixture.js` through it, and checks the fixture was actually typed its input. An in-process test would be useless here: it would run under bun and pass either way. With stdin reverted to `"ignore"`: ``` { "exitCode": 0, - "greeted": true, - "read": "3", + "greeted": false, + "read": "0", } (fail) interactive workload stdin > reaches the workload when the generator runs under node, as CI does ``` and with the fix, `17 pass, 1 skip, 0 fail` for the file. Agents install node 26.3.0 (`scripts/bootstrap.sh`), so the case runs rather than skips.
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 16, 2026
) ### Problem - Some builds of main ship unordered binaries. [#113190](https://buildkite.com/bun/bun/builds/113190) and [#112691](https://buildkite.com/bun/bun/builds/112691) linked linux-x64, darwin-aarch64 and both windows targets with an empty order file (`~ symbol order: found no earlier build to inherit from (34s)`). Unordered, `bun hello.js` keeps about 22 MB of text resident, not 7 MB. - The cause is `candidateBuilds()` (`scripts/build/ci.ts:827`). It appends `.json` to the `Location` of Buildkite's `builds/latest?branch=main&state=passed` redirect. Since 2026-08-21 the `Location` repeats the query, so the request is `<build>?branch=main&state=passed.json`. Buildkite answers with HTML, and the lookup returns no build. - That leaves the probe of the 200 build numbers below the current one. PR builds use numbers too, so a quiet night exceeds 200. ### Fix - Put `.json` on the path of the redirect target. - Probe first, and use the newest passed build as the fallback. The nearest file matches the link best. When the newest passed build came first, one stale file served every build until main passed again: [#102154](https://buildkite.com/bun/bun/builds/102154) and [#102263](https://buildkite.com/bun/bun/builds/102263) both resolved 506 of the 1000 hottest names. - Verified: `test/js/bun/perf/linker-order.test.ts` (six new tests, the fallback test fails with the old URL). Run for real from #113190, the walk now yields the newest passed build. ### Background - A symbol order file lists the functions that run at start-up. The linker places them together, so a start touches fewer pages. - Most release lanes cross-compile and cannot trace their own binary. A `trace-order` step traces it on a native host and publishes `bun-<target>.order`. The next build of main inherits that file. - Buildkite has no public list of a branch's builds, so `candidateBuilds()` probes build numbers. <details><summary>Notes</summary> **How often.** I read the `Inherit symbol order file` group of the linux-x64 build step for 35 builds of main from 09-05 to 09-16. 33 inherited a file. #112691 (`none of the 3 builds tried published it (33s)`) and #113190 did not, on any lane. In both, linux-aarch64, which builds natively, paid the second link (`Tracing bun-profile to build a fresh order file (nothing to inherit)`). #112691 is a passed build, so its binaries were uploaded as the canary. A sample of August builds has a third case: [#103671](https://buildkite.com/bun/bun/builds/103671) on 08-22 (`found no earlier build to inherit from (37s)`). **When it broke.** The lookup worked from #33345 (07-05) until 08-21. #69595 inherited from #68645, 950 build numbers back, in 1s. [#102263](https://buildkite.com/bun/bun/builds/102263) (08-21 04:33Z) inherited from #102067 in 0s. [#102418](https://buildkite.com/bun/bun/builds/102418) (08-21 06:54Z) took 33s to inherit from #102263, 155 numbers back. No commit touched `scripts/build/ci.ts` or `scripts/utils.mjs` in that window, so the redirect changed on Buildkite's side. **What it looks like since.** The time to inherit follows the distance to the build inherited from, at about 0.16 s per build number on top of a fixed 6 s. The fixed part is `utils.curl` retrying the JSON parse of the HTML page three times, with sleeps of 2 s and 3 s. | build | inherited from | distance | time | | --- | --- | --- | --- | | #115612 | #115611 | 1 | 6s | | #116354 | #116338 | 16 | 9s | | #114270 | #114150 | 120 | 23s | | #115994 | #115833 | 161 | 31s | | #111080 | #110907 | 173 | 35s | ``` $ curl -sI 'https://buildkite.com/bun/bun/builds/latest?branch=main&state=passed' | grep -i ^location location: https://buildkite.com/bun/bun/builds/116199?branch=main&state=passed $ curl -s -o /dev/null -w '%{content_type}\n' 'https://buildkite.com/bun/bun/builds/116199?branch=main&state=passed.json' text/html; charset=utf-8 $ curl -s -o /dev/null -w '%{content_type}\n' 'https://buildkite.com/bun/bun/builds/116199.json' application/json; charset=utf-8 ``` **Why the probe goes first.** The code on main asks the newest passed build first. With only the URL corrected, every build of main inherits from the newest passed build again, and not from the nearest build. Most builds of main fail a test or get cancelled, so that file is often many commits old. Rust symbol names (v0 mangling) embed a hash per crate. The hash changes when the dependencies of the crate change, or when the toolchain changes, and then the names in an older file no longer resolve. The `+ symbol order: applied` line of the linux-x64 build step shows both orders: - Newest passed first (until 08-21). #100959, #100992, #101024 and #101032 all inherited the file of #100916 and resolved 847, 847, 843 and 843 of 1000. #102154 and #102263 both inherited the file of #102067 and resolved 506 of 1000. - Nearest first (since 08-21, by accident). Of the 33 sampled builds that inherited, 31 resolved 947 or more. #112799 resolved 515 (its parent commit, 8b672e4, removes 21 dependency edges), and the next build, #112830, resolved 990. #116425, the first build after the LLVM 23 and Rust nightly upgrade, resolved 443. The probe costs about 30 s when the window is empty, in a build step that takes many minutes. **The fallback is one candidate.** The `trace-order` step is `soft_fail`, so a passed build can lack the file. In that case nothing is inherited, as today. The cap of 50 candidates now counts inside the probe, so a crowded window cannot keep the newest passed build from being asked. **Releases.** A release build of a cross-compiled lane inherits the same way. The linux-x64 build of v1.4.2 (#110323) inherited from #110317. A release cut more than 200 build numbers after the previous build of main ships unordered on those lanes. **The seam.** `BuildLookups` follows `OrderFileContext` in the same file: the facts come in as an argument so that the decision runs in a test. The default value makes the same two requests as before. `inheritOrderFile()` is the only caller and uses the default. **Related.** #37711 touches the same files for a different problem (a `trace-order` step for linux-aarch64). With it, a lane that inherits nothing links unordered and no longer traces itself, so this fallback matters more. **Not in this PR.** `curl()` in `scripts/utils.mjs` keeps the `error` of a failed attempt after a retry succeeds, so a build whose JSON needed one retry is skipped here. The bug is from 2024 and affects every caller of `curl()` and `curlSafe()`. It gets its own PR. **Self-review.** A review of this description raised three points. One was a wrong claim in my draft, that the lookup never worked: the July and August logs show that it worked until 08-21, and the text above says so. The other two are the single fallback candidate and the cost of the probe. I accept both, for the reasons above. **Test runs.** `bun bd test test/js/bun/perf/linker-order.test.ts`: 32 pass, 4 skip (windows only), 0 fail. I ran the real walk against Buildkite under bun and under Node 26, which is what CI runs the build scripts with. From #116354 it yields #116353, #116352 and #116338 within 3 s, and CI inherited from #116338. From #113190 it finds no build of main in the window and then yields the newest passed build. `bunx tsc --noEmit -p scripts/build/tsconfig.json` reports the same four errors as main, all in `jsonByteClass.ts` and `xmlByteClass.ts`. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/perf/linker-order.test.ts <!-- robobun:evidence:end -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #33302. Inheriting never actually worked, and even fixed it could never have started.
Inheriting never worked
previousBuilds()walkedprev_branch_buildfrom Buildkite's public build JSON. That field no longer exists, so the walk found nothing and every build linked unordered:Zero seconds, because it never made a request. Note this also means
utils.getLastSuccessfulBuild()always returnsundefined— it reads the same field — so it can't be used here either, andskip-buildspresumably hasn't worked for a while.What does work, unauthenticated, is
builds/latest?branch=X&state=passed, which 302s at the build. The redirect drops the.jsonon the way, so we read thelocationheader rather than following it. Older builds are found by probing build numbers, but that's only a safety net for a run of builds that published nothing — candidates are produced lazily, so the normal path is one lookup (measured: 2 requests, 420ms, against the live API).The chain could never start
A build that inherits nothing has no order file, so
packageAndUploadpublishes none, so the next build inherits nothing either. Nothing seeds it short of a release.So a build that inherits nothing now traces its own binary and relinks. That bootstraps the chain and heals it after any gap. It costs a second link, so it announces itself with a warning annotation — seeing it once is expected, seeing it on every build means inheriting is broken again and someone should look. That's the alarm this pipeline was missing the first time.
Also
Tests
test/js/bun/perf/linker-order.test.tsnow covers the decision rules: releases always generate, canaries don't unless asked, PRs never do and never publish, a canary that inherited nothing generates anyway, and cross-compiled targets never generate. I verified each is load-bearing by breaking the rule and watching a test fail.The lookup itself was verified against the live Buildkite API using the exact env of the build that failed (#68425):
latest?branch=main&state=passed→ 302 → #68409 → JSON. That's the call the broken code never made.