Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ Five independent jobs over the Rust workspace. They share `.github/actions/rust-

- `clippy` lints the default targets only (lib and bin): test code was never held to the clippy lint set. Its second step, `cargo check --workspace --all-targets`, type-checks `#[cfg(test)]` code, `tests/` and `benches/` so they keep compiling. Nothing else builds them except the crates `rust:miri` runs.
- `clippy`, `miri`, `lolhtml` and `aot-rust-tests` pin `RUSTUP_TOOLCHAIN` at the workflow level (kept in sync with `channel` in `rust-toolchain.toml`) so rustup does not install that file's cross-target list; the action installs the toolchain with `--profile minimal` plus the components the job names (`clippy`, `miri rust-src`, none, none).
- `miri` runs one `cargo miri test -p <crate>` for each crate of `MIRI_CRATES` in `scripts/rust-miri.ts`, several at a time, and prints a crate's output when that crate is done. A crate gets 45 minutes, or the number in `BUN_MIRI_CRATE_LIMIT_MINUTES`, for its wait on cargo's build lock, its build and its tests. Past that the script stops the crate, prints what it wrote, which ends at the test that did not finish, and fails. `bun run rust:miri -p <crate>` has no limit and prints while it runs. The job's `timeout-minutes` covers what that limit does not: the setup and the first build. Miri interprets a test, which takes far longer than `cargo test` does, so a test with a lot of work is given less of it under `cfg!(miri)`, or is skipped there with `#[cfg_attr(miri, ignore)]` when it runs no unsafe code.
- `lolhtml` exists because the vendored lol-html is a fork (oven-sh/lol-html, `bun` branch) whose own test suite is the only thing guarding the fork's invariants. It used to trigger only on `scripts/build/deps/lolhtml.ts`; it now shares the workflow's wider path filter.
- `aot-rust-tests` tests the Rust half of the AoT compiler (`bun build --compile --aot`) without linking Bun. Its ninja targets are `codegen engine-test-programs clone-zstd clone-lolhtml clone-rust-argon2`. `codegen` fetches the prebuilt WebKit tarball (about 700 MB) in every job that builds it, because the table of built-ins is generated from the tarball's `Source/JavaScriptCore/aot/builtins/generated` and the schema's Rust files by the tarball's `aot/ir/generator`. `engine-test-programs` (`jsc` and `testaot`) is the only C++ this job compiles and links: the five sources of Bun's table of built-ins, with the tarball's `lib/libjsc-objects.a`, `lib/libtestaot-objects.a` and libraries. The `testaot` in a tarball's `bin/` has another table and reads nothing that Bun's front end writes. `clone-zstd` is for the build script of `bun_sema_standalone`, which compiles zstd's decoder from `vendor/zstd` into the test binaries; the jobs that only check link nothing and do not need it. The tarball is extracted to a cache directory named after the WebKit version and the build configuration, so `bun scripts/build/aot-paths.ts` prints where `testaot`, `jsc`, the schema and the engine's half of the table are, as `NAME=<path>` lines that the job appends to `$GITHUB_ENV`. `bun test test/internal/aot-schema-generator.test.ts` runs here, with `AOT_SCHEMA` set by `aot-paths.ts`: it skips itself without an engine, as on Buildkite's test machines. The Rust files that the schema gives are written by the `codegen` target, by the tarball's generator from the tarball's schema, so they are always those of the **pinned** WebKit. The schema, its generator and its C++ output belong to WebKit, so a schema change is made with `--webkit=local`; it lands in WebKit first, then in Bun as one change: the new `WEBKIT_VERSION` (`scripts/build/deps/webkit.ts`) and the hand-written Rust that follows it.
- `mordant` runs the [mordant](https://github.com/scarletindustries/mordant) lint pack as an advisory (`continue-on-error`) job. Mordant ships its own runner, the way clippy does. `cargo mordant` is `cargo check` with `mordant-driver` (rustc with the lints linked in) as `RUSTC_WORKSPACE_WRAPPER`, and it builds into `target/mordant/check`. The shared action installs mordant's nightly (`MORDANT_TOOLCHAIN`) with `rustc-dev` and `llvm-tools-preview`, and every cargo call in the job names it (`cargo +"$MORDANT_TOOLCHAIN" …`): `cargo install --locked --git … --rev "$MORDANT_REV"` builds the two binaries with it, and `scripts/rust-mordant.ts` runs `cargo mordant` under it. It is not spelled `RUSTUP_TOOLCHAIN` because `test/internal/source-lints/ci-image-pins.test.ts` holds every `RUSTUP_TOOLCHAIN: nightly-…` in a workflow to the channel in `rust-toolchain.toml`, and this one moves with mordant instead. One cache covers them: `~/.cargo/bin/{cargo-mordant,mordant-driver}`, keyed on the nightly and the rev. The job restores it with `actions/cache/restore` and saves it with `actions/cache/save` right after the build, because the post step of `actions/cache` does not run in a job that failed, and a pull request with a finding over the baseline would then build mordant again on every push. Because that nightly is older than ours, the script passes `-A unknown_lints` through `MORDANT_RUSTFLAGS`. It is skipped on `merge_group`.
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/rust-lints.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ jobs:
miri:
name: cargo miri test
runs-on: ubuntu-latest
# scripts/rust-miri.ts stops a crate after 45 minutes and names the test
# that did not finish. This is for what that limit does not cover.
timeout-minutes: 90
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
Expand Down
86 changes: 81 additions & 5 deletions scripts/rust-miri.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/

import { spawn, spawnSync } from "node:child_process";
import { existsSync } from "node:fs";
import { existsSync, readdirSync, readFileSync } from "node:fs";
import { availableParallelism } from "node:os";
import { resolve } from "node:path";

Expand Down Expand Up @@ -105,25 +105,94 @@ if (extraArgs.length > 0) {
process.exit(run("cargo", ["miri", "test", ...extraArgs], { env }).status ?? 1);
}

// How long one crate gets for its wait on cargo's build lock, its build and
// its tests. Past that the crate is stopped and fails, and its output is
// printed: under Miri libtest runs one test at a time and prints each name
// first, so the output ends at the test that did not finish.
const requestedLimit = process.env.BUN_MIRI_CRATE_LIMIT_MINUTES;
const CRATE_LIMIT_MINUTES = requestedLimit ? Number(requestedLimit) : 45;
if (!(CRATE_LIMIT_MINUTES > 0)) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
console.error(
`\x1b[31m[error]\x1b[0m BUN_MIRI_CRATE_LIMIT_MINUTES is "${requestedLimit}": it takes a number of minutes above 0`,
);
process.exit(1);
}
// A longer delay than this overflows in `setTimeout` and fires at once.
const CRATE_LIMIT_MS = Math.min(CRATE_LIMIT_MINUTES * 60_000, 2 ** 31 - 1);

// Default set: Miri interprets one test at a time per process, so a single
// `cargo miri test -p a -p b ...` is serial end to end. Build everything once,
// then run one `cargo miri test -p <crate>` per crate concurrently.
const allCrates = MIRI_CRATES.flatMap(c => ["-p", c]);
console.log(`\x1b[36m[miri]\x1b[0m cargo miri test --no-run ${allCrates.join(" ")}`);
if (run("cargo", ["miri", "test", "--no-run", ...allCrates], { env }).status !== 0) process.exit(1);

type Result = { crate: string; ok: boolean; seconds: number; output: string };
type Result = { crate: string; ok: boolean; overLimit: boolean; seconds: number; output: string };

// The processes that `pid` started. Linux lists them under each thread of
// `pid`. `pgrep` does it elsewhere, and on a kernel without that list.
function childrenOf(pid: number): number[] {
let list: string;
try {
const threads = readdirSync(`/proc/${pid}/task`);
list = threads.map(thread => readFileSync(`/proc/${pid}/task/${thread}/children`, "utf8")).join(" ");
} catch {
const listed = spawnSync("pgrep", ["-P", `${pid}`], { encoding: "utf8" });
if (listed.error) {
console.error(`\x1b[31m[miri]\x1b[0m \`pgrep\` did not run: the processes that ${pid} started are not stopped`);
}
list = listed.stdout ?? "";
}
return list.split(/\s+/).filter(Boolean).map(Number);
}

// Stops `cargo miri test` and every process under it, Miri among them. A
// process is frozen before its children are listed, so that it starts no more.
function stopTree(pid: number) {
if (process.platform === "win32") {
spawnSync("taskkill", ["/pid", `${pid}`, "/t", "/f"], { stdio: "ignore" });
return;
}
try {
process.kill(pid, "SIGSTOP");
} catch {
return;
}
childrenOf(pid).forEach(stopTree);
try {
process.kill(pid, "SIGKILL");
} catch {
// Already gone.
}
}

function testCrate(crate: string): Promise<Result> {
return new Promise(done => {
const started = Date.now();
const chunks: Buffer[] = [];
const child = spawn("cargo", ["miri", "test", "-p", crate, "--color", "always"], { cwd: repo, env });
let overLimit = false;
let timer = setTimeout(() => {
// Once `cargo` has exited, its process id can be that of another process.
overLimit = child.exitCode === null && child.signalCode === null;
if (overLimit && child.pid !== undefined) stopTree(child.pid);
// `close` comes when every process that holds the pipes is gone. Do not
// wait for one that `stopTree` did not reach.
timer = setTimeout(() => {
child.stdout.destroy();
child.stderr.destroy();
finish(child.exitCode);
}, 5_000);
}, CRATE_LIMIT_MS);
const finish = (code: number | null) => {
clearTimeout(timer);
const ok = code === 0;
const seconds = (Date.now() - started) / 1000;
done({ crate, ok, overLimit: overLimit && !ok, seconds, output: Buffer.concat(chunks).toString() });
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
child.stdout.on("data", chunk => chunks.push(chunk));
child.stderr.on("data", chunk => chunks.push(chunk));
child.on("close", code =>
done({ crate, ok: code === 0, seconds: (Date.now() - started) / 1000, output: Buffer.concat(chunks).toString() }),
);
child.on("close", finish);
});
}

Expand All @@ -144,6 +213,13 @@ async function worker() {
} else {
console.log(`::group::${result.crate} output\n${result.output}\n::endgroup::`);
}
if (result.overLimit) {
console.log(
`\x1b[31m[miri]\x1b[0m ${result.crate} was stopped at the limit of ${CRATE_LIMIT_MINUTES} minutes ` +
"(BUN_MIRI_CRATE_LIMIT_MINUTES). Its output ends at the test that did not finish. Give that test " +
"less to do under `cfg!(miri)`, or skip it there with `#[cfg_attr(miri, ignore)]`.",
);
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/sema/check/regexp_scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1826,7 +1826,9 @@ mod spelling_tests {
names
}

/// Skipped under Miri: the 380,880 comparisons run no unsafe code, and Miri needs hours for them.
#[test]
#[cfg_attr(miri, ignore)]
fn characters_refuse_only_names_that_are_too_far_apart() {
let names = names();
let mut accepted = 0;
Expand Down
Loading