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
6 changes: 4 additions & 2 deletions .github/workflows/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,18 @@ export LLVM_VERSION_MAJOR=19

## rust-lints.yml Workflow

Four independent jobs that each run one cargo command over the Rust workspace. They share `.github/actions/rust-lint-setup`, a composite action that installs LLVM from apt.llvm.org (configure resolves a clang even though nothing here compiles C++), Bun, optionally a pinned Rust toolchain plus components, runs `bun install`, then `bun scripts/build.ts --configure-only` and the ninja targets a job asks for: `clone-lolhtml` (cargo cannot resolve the workspace until the vendored `lol_html` path dependency exists) and, for jobs that check `bun_runtime`/`bun_jsc`/`bun_core`, `codegen` (their `include!()`d sources under `build/debug/codegen`).
Five independent jobs that each run one cargo command over the Rust workspace. They share `.github/actions/rust-lint-setup`, a composite action that installs LLVM from apt.llvm.org (configure resolves a clang even though nothing here compiles C++), Bun, optionally a pinned Rust toolchain plus components, runs `bun install`, then `bun scripts/build.ts --configure-only` and the ninja targets a job asks for: `clone-lolhtml` (cargo cannot resolve the workspace until the vendored `lol_html` path dependency exists) and, for jobs that check `bun_runtime`/`bun_jsc`/`bun_core`, `codegen` (their `include!()`d sources under `build/debug/codegen`).

| Job | Check name | Runs | Blocking |
| --------- | --------------------- | -------------------------------------------- | ------------------------------ |
| `clippy` | `cargo clippy` | `bun run rust:clippy` | yes |
| `miri` | `cargo miri test` | `bun run rust:miri` (`scripts/rust-miri.ts`) | yes |
| `test` | `cargo test` | `bun run rust:test` (`scripts/rust-test.ts`) | yes |
| `lolhtml` | `lol-html cargo test` | `cargo test` in `vendor/lolhtml` | yes |
| `mordant` | `mordant` | `cargo dylint --all --workspace` | advisory (`continue-on-error`) |

- `clippy`, `miri` and `lolhtml` 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).
- `clippy`, `miri`, `test` and `lolhtml` 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).
- `test` runs a native `cargo test --locked -p <crate>` for each crate listed in `scripts/rust-test.ts`. A test binary links only the crate's Rust dependencies, so this is what catches a test reaching one of the externs that exist only in the full bun link (Miri never links). `scripts/rust-workspace.ts` holds the configure prerequisite check `rust-miri.ts` and `rust-test.ts` share; `test/internal/rust-native-cargo-test.test.ts` runs the same script under `bun bd test`.
- `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.
- `mordant` runs the [mordant](https://github.com/scarletindustries/mordant) dylint pack. It sets `RUSTUP_TOOLCHAIN: stable` instead: mordant is built with, and lints us using, the nightly named in its own rust-toolchain file, which dylint fetches on demand, so the outer cargo only needs to exist. Because that nightly is older than ours, the job passes `-A unknown_lints` through `DYLINT_RUSTFLAGS`. Two caches cover the slow parts: `~/.cargo/bin/{cargo-dylint,dylint-link}` keyed on `DYLINT_VERSION`, and `~/.dylint_drivers` + `target/dylint/libraries` keyed on `DYLINT_VERSION` plus the pinned mordant rev read out of `Cargo.toml`. It is skipped on `merge_group`.

Expand Down
27 changes: 27 additions & 0 deletions .github/workflows/rust-lints.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ on:
- "scripts/build/**"
- "scripts/build.ts"
- "scripts/rust-miri.ts"
- "scripts/rust-test.ts"
- "scripts/rust-workspace.ts"
- "package.json"
- "Cargo.toml"
- "Cargo.lock"
Expand Down Expand Up @@ -85,6 +87,31 @@ jobs:
BUN_CODEGEN_DIR: ${{ github.workspace }}/build/debug/codegen
run: bun run rust:miri

test:
# The thing Miri cannot check: a crate's test binary links only its Rust
# dependencies, so a test that reaches one of the externs only the full bun
# link provides fails here (scripts/rust-test.ts).
name: cargo test
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false

- name: Setup
uses: ./.github/actions/rust-lint-setup
with:
bun-version: ${{ env.BUN_VERSION }}
llvm-version: ${{ env.LLVM_VERSION_MAJOR }}
toolchain: ${{ env.RUSTUP_TOOLCHAIN }}

- name: cargo test
env:
BUN_CODEGEN_DIR: ${{ github.workspace }}/build/debug/codegen
run: bun run rust:test

lolhtml:
# The vendored lol-html is a fork (oven-sh/lol-html, `bun` branch) carrying
# content-handler suspension, and its own test suite is the only thing that
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"rust:mordant": "DYLINT_RUSTFLAGS='-A unknown_lints' cargo dylint --all --workspace -- --keep-going",
"rust:mordant:baseline": "DYLINT_RUSTFLAGS='-A unknown_lints' MORDANT_BASELINE_WRITE=1 cargo dylint --all --workspace",
"rust:miri": "bun scripts/rust-miri.ts",
"rust:test": "bun scripts/rust-test.ts",
"rust:timings": "bun scripts/rust-timings.ts",
"codegen:string-maps": "for f in src/**/*.string-map.ts; do bun src/codegen/generate-string-map.ts \"$f\" \"${f%.string-map.ts}.generated.rs\"; done",
"codegen:verify": "bun run codegen:string-maps && git diff --exit-code 'src/**/*.generated.rs' || (echo '\\n*.generated.rs is stale — run `bun run codegen:string-maps` and commit the result.' >&2; exit 1)",
Expand Down
39 changes: 6 additions & 33 deletions scripts/rust-miri.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@
* is the candidate replacement spec, allows that pattern, and still catches
* the bugs we care about.
*
* Miri never links, so it says nothing about whether these crates' test
* binaries link natively; `rust-test.ts` covers that.
*
* Usage:
* bun run rust:miri # default crate set, crates run concurrently
* bun run rust:miri -p bun_foo # extra args go straight to one `cargo miri test`
*/

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

const repo = resolve(import.meta.dirname, "..");
import { ensureCargoWorkspace, repo, run } from "./rust-workspace.ts";

// Crates that pass `cargo miri test` under Tree Borrows. To add one it must
// (a) have at least one `#[test]`, (b) compile under `--cfg test`, (c) at test
Expand Down Expand Up @@ -54,34 +54,7 @@ const MIRI_CRATES = [
"bun_wyhash",
];

function run(cmd: string, args: string[], opts: Parameters<typeof spawnSync>[2] = {}) {
return spawnSync(cmd, args, { stdio: "inherit", cwd: repo, ...opts });
}

// `bun_core/build.rs` needs `build_options.rs`; cargo can't resolve the
// workspace until `vendor/lolhtml/` (a path dep) exists. Both come from the
// configure step, which is a no-op when already done.
const buildOptionsRs = resolve(repo, "build/debug/codegen/build_options.rs");
const lolhtmlCargo = resolve(repo, "vendor/lolhtml/Cargo.toml");
if (!existsSync(buildOptionsRs) || !existsSync(lolhtmlCargo)) {
console.log("\x1b[36m[setup]\x1b[0m bun run build --configure-only");
if (run("bun", ["run", "build", "--configure-only"]).status !== 0) process.exit(1);
if (!existsSync(lolhtmlCargo) && run("ninja", ["-C", "build/debug", "clone-lolhtml"]).status !== 0) {
process.exit(1);
}
// Re-check: configure can succeed without producing these (e.g. partial
// checkout, ninja target rename) — fail fast instead of letting cargo
// produce a confusing workspace-resolution error.
for (const [path, hint] of [
[buildOptionsRs, "bun run build --configure-only"],
[lolhtmlCargo, "ninja -C build/debug clone-lolhtml"],
] as const) {
if (!existsSync(path)) {
console.error(`\x1b[31m[error]\x1b[0m ${path} still missing after setup — try: ${hint}`);
process.exit(1);
}
}
}
ensureCargoWorkspace();

const env = {
...process.env,
Expand Down
78 changes: 78 additions & 0 deletions scripts/rust-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
#!/usr/bin/env bun
/**
* Native `cargo test` for the crates whose unit tests are meant to run on
* their own, outside the bun build.
*
* A crate's test binary links its Rust dependencies and nothing else; the C/C++
* behind the workspace's externs (highway kernels, simdutf, mimalloc, the
* OutputSink interface implemented in bun_sys, ...) exists only in the full bun
* link. A test that reaches one of them fails to link with an `undefined
* symbol`, and nothing else notices: the Miri lane (`rust-miri.ts`) never links
* and runs bun_highway's scalar paths, and `cargo check --tests` stops before
* the linker. A crate whose tests need the byte-search kernels enables
* bun_highway's `scalar` feature from its `[dev-dependencies]` (see
* src/clap/Cargo.toml); anything else has to be stubbed in the crate, or stays
* a link error naming the symbol.
*
* One cargo invocation per crate on purpose: cargo unifies features within an
* invocation, so `cargo test -p a -p b` would build bun_highway with `scalar`
* for both as soon as either asks for it, and a crate missing its own
* `[dev-dependencies]` entry would pass here yet fail for whoever runs
* `cargo test -p <crate>` alone. Shared dependencies are still built once per
* feature set, not once per crate.
*
* Usage:
* bun run rust:test # every crate below
* bun run rust:test -p bun_foo # extra args go straight to cargo test
*/

import { ensureCargoWorkspace, run } from "./rust-workspace.ts";

// Crates whose `cargo test --locked -p <crate>` links and passes on a
// configured checkout. Of the Miri set, bun_ast (its tests reach mimalloc) and
// bun_ptr (highway kernels and the OutputSink interface) do not link yet, and
// bun_collections' pool tests count drops that its thread-local pools perform
// at thread exit, so they race once libtest actually runs them in parallel.
const NATIVE_TEST_CRATES = [
"bun_base64",
"bun_clap",
"bun_dispatch",
"bun_errno",
"bun_hash",
"bun_http_types",
"bun_md",
"bun_paths",
"bun_resolve_builtins",
"bun_shell_parser",
"bun_threading",
"bun_wyhash",
];

ensureCargoWorkspace();

// `scalar` makes every byte search scalar, so it must only ever reach test
// binaries. Cargo does not activate [dev-dependencies] when building bun_bin;
// a `[dependencies]` entry asking for the feature would, silently.
const treeArgs = ["tree", "--locked", "-p", "bun_bin", "-i", "bun_highway", "-e", "features"];
console.log(`\x1b[36m[test]\x1b[0m cargo ${treeArgs.join(" ")}`);
const tree = run("cargo", treeArgs, { stdio: ["ignore", "pipe", "inherit"] });
if (tree.status !== 0) process.exit(tree.status ?? 1);
if (tree.stdout.toString().includes('feature "scalar"')) {
console.error(tree.stdout.toString());
console.error("\x1b[31m[error]\x1b[0m bun_highway's `scalar` feature is enabled in bun_bin's dependency graph");
process.exit(1);
}

const extraArgs = process.argv.slice(2);
const invocations = extraArgs.length > 0 ? [extraArgs] : NATIVE_TEST_CRATES.map(crate => ["-p", crate]);

const failed: string[] = [];
for (const args of invocations) {
console.log(`\x1b[36m[test]\x1b[0m cargo test --locked ${args.join(" ")}`);
if (run("cargo", ["test", "--locked", ...args]).status !== 0) failed.push(args.join(" "));
}

if (failed.length > 0) {
console.error(`\x1b[31m[error]\x1b[0m cargo test failed for: ${failed.join(", ")}`);
process.exit(1);
}
48 changes: 48 additions & 0 deletions scripts/rust-workspace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Shared setup for the scripts that run cargo on individual crates
* (`rust-miri.ts`, `rust-test.ts`).
*
* `bun_core/build.rs` needs `build_options.rs`, and cargo can't resolve the
* workspace at all until `vendor/lolhtml/` (a path dep) exists. Both come from
* the configure step, which is a no-op when already done.
*/

import { spawnSync } from "node:child_process";
import { existsSync } from "node:fs";
import { resolve } from "node:path";

export const repo = resolve(import.meta.dirname, "..");

const buildOptionsRs = resolve(repo, "build/debug/codegen/build_options.rs");
const lolhtmlCargo = resolve(repo, "vendor/lolhtml/Cargo.toml");

export function cargoWorkspaceConfigured(): boolean {
return existsSync(buildOptionsRs) && existsSync(lolhtmlCargo);
}

export function run(cmd: string, args: string[], opts: Parameters<typeof spawnSync>[2] = {}) {
return spawnSync(cmd, args, { stdio: "inherit", cwd: repo, ...opts });
}

/** Configures the tree if needed; exits the process when that does not produce what cargo needs. */
export function ensureCargoWorkspace(): void {
if (cargoWorkspaceConfigured()) return;

console.log("\x1b[36m[setup]\x1b[0m bun run build --configure-only");
if (run("bun", ["run", "build", "--configure-only"]).status !== 0) process.exit(1);
if (!existsSync(lolhtmlCargo) && run("ninja", ["-C", "build/debug", "clone-lolhtml"]).status !== 0) {
process.exit(1);
}
// Re-check: configure can succeed without producing these (e.g. partial
// checkout, ninja target rename). Fail fast instead of letting cargo
// produce a confusing workspace-resolution error.
for (const [path, hint] of [
[buildOptionsRs, "bun run build --configure-only"],
[lolhtmlCargo, "ninja -C build/debug clone-lolhtml"],
] as const) {
if (!existsSync(path)) {
console.error(`\x1b[31m[error]\x1b[0m ${path} still missing after setup; try: ${hint}`);
process.exit(1);
}
}
}
6 changes: 6 additions & 0 deletions src/clap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,9 @@ bitflags.workspace = true
thiserror.workspace = true
bun_clap_macros.workspace = true
bun_core.workspace = true

[dev-dependencies]
# The streaming tests reach `bun_core::strings` (`StreamingClap::normal`,
# `test_err`); without this the test binary fails to link against the highway
# kernels (`bun run rust:test`).
bun_highway = { workspace = true, features = ["scalar"] }
4 changes: 1 addition & 3 deletions src/clap/streaming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,10 +432,8 @@ mod tests {
&diag.arg
};
let quoted = [b"'".as_slice(), captured, b"'"].concat();
// Naive search: `cargo test -p bun_clap` does not link the
// highway kernels behind `bun_core::strings::contains`.
assert!(
(0..expected.len()).any(|i| expected[i..].starts_with(&quoted)),
strings::contains(expected, &quoted),
"expected message {:?} does not name captured arg {:?}",
bstr::BStr::new(expected),
bstr::BStr::new(captured),
Expand Down
8 changes: 8 additions & 0 deletions src/highway/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ edition.workspace = true
[lib]
path = "lib.rs"

[features]
# Take the scalar paths the search wrappers use under Miri at every length, so
# nothing references the highway_* C++ kernels, which only exist in the full bun
# link. For a crate whose standalone `cargo test` binary reaches a search
# wrapper: enable this from that crate's [dev-dependencies]. Never enabled in
# the bun build itself.
scalar = []

[lints]
workspace = true

Expand Down
Loading
Loading