build: generate bun_core::build_options from Config#30749
Conversation
|
Updated 1:14 AM PT - May 15th, 2026
❌ @dylan-conway, your commit ca6b0cb has 1 failures in
🧪 To try this PR locally: bunx bun-pr 30749That installs a local version of the PR into your bun-30749 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
WalkthroughThis PR migrates ChangesBuild options codegen migration
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
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/buildOptionsRs.ts`:
- Around line 31-33: rbstr currently emits Rust byte-string literals using
JSON.stringify which leaves non-ASCII chars intact and produces invalid b"..."
syntax; change rbstr to encode the input string as UTF-8 bytes (e.g.,
Buffer.from(s, "utf8") or similar) and emit a Rust byte-array literal like
&[<comma-separated 0xNN values>] (or &[<decimal bytes>]) instead of b"...",
ensuring the length is preserved if you need a fixed-size array; update the
rbstr function to build that comma-separated byte list from the UTF-8 bytes
rather than using JSON.stringify(s).
🪄 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: 49aaa0d9-c587-4959-8ab1-279982995a08
📒 Files selected for processing (6)
scripts/build/CLAUDE.mdscripts/build/buildOptionsRs.tsscripts/build/codegen.tsscripts/build/rust.tssrc/bun_core/build.rssrc/bun_core/lib.rs
|
@robobun can you address the reviews |
…aned const_parse_u32
There was a problem hiding this comment.
LGTM — both prior review points addressed (.as_bytes() for non-ASCII paths, const_parse_u32 removed).
Extended reasoning...
Overview
This PR migrates bun_core::build_options from a dozen option_env!("BUN_*") reads (with names duplicated between rust.ts and lib.rs) to a single generated build_options.rs written at configure time and include!()'d. Touches 7 files: a new TS generator (buildOptionsRs.ts), wiring in codegen.ts, env-var removal from rust.ts, a new src/bun_core/build.rs (mirrors src/{jsc,runtime}/build.rs), the include! swap in lib.rs, dead-code removal in util.rs, and a doc table row in CLAUDE.md.
Prior feedback addressed
My previous review flagged two issues, both now fixed in commit ca6b0cb:
rbstr()now emits"...".as_bytes()instead ofb"...", so non-ASCII repo paths (e.g.C:\Users\Müller\) compile — restoring parity with the oldoption_env! → v.as_bytes()path.- The orphaned
const_parse_u32helper has been deleted fromutil.rs.
Security risks
None. This is build-system plumbing that changes how compile-time constants reach bun_core (file vs env vars), not what they are. No user input, no network, no auth, no runtime code paths affected. The generated values (sha, version, paths) are identical to what the env-var handshake produced.
Level of scrutiny
Low-to-moderate. The pattern is not novel — it exactly mirrors the existing generated_classes.rs / cpp.rs / generated_host_exports.rs codegen-then-include! flow, and the new build.rs is a near-copy of src/jsc/build.rs / src/runtime/build.rs. If anything here were wrong, it would manifest as an immediate, loud build failure (CI already validates cargo build end-to-end), not a subtle runtime bug. The one CI failure (node-http-backpressure.test.ts SIGKILL on aarch64) is unrelated to build infrastructure.
Other factors
writeIfChangedpreserves mtime so unchanged reconfigures don't cascade-recompilebun_core— author verified this explicitly.- Target-derived constants (
ENABLE_TINYCC/ENABLE_ASAN/ENABLE_LOGS) correctly stay ascfg!()so cross-targetcargo checkstill works. rustInputspush +rerun-if-changedgive correct incremental-rebuild semantics on both the ninja and cargo sides.- Net deletion of duplicated state (env-var name list maintained in two places → one generator).
bun_core::build_options(the Rust analogue of Zig's@import("build_options")— version, sha, baseline, canary, paths) was populated via a dozenBUN_*env vars thatscripts/build/rust.tsexported andbun_core/lib.rsread back withoption_env!. The names were maintained in two places, and a barecargo check/ rust-analyzer saw placeholder defaults ("0000…"sha, fake version, mixed-separator WindowsBASE_PATH).Now
scripts/build/buildOptionsRs.tswrites${codegenDir}/build_options.rsat configure time with literalpub consts straight from the resolvedConfig, andbun_coreinclude!()s it (same pattern asgenerated_classes.rs/cpp.rs).writeIfChangedkeeps the mtime stable so an unchanged sha doesn't recompilebun_coreand its dependents.Target/profile-derived constants (
ENABLE_TINYCC,ENABLE_ASAN,ENABLE_LOGS) stay ascfg!()inside the generated file so cross-targetcargo checkstill evaluates them per-triple.Also:
src/bun_core/build.rsto exportBUN_CODEGEN_DIR+rerun-if-changed(mirrorssrc/{jsc,runtime}/build.rs)cfg.fuzzilli→ENABLE_FUZZILLI(was hardcodedfalse)BASE_PATHfallback hackVerified:
bun bd --revisionreports the configured sha; empty-commit + rebuild reports the new sha; reconfigure with unchangedConfigpreservesbuild_options.rsmtime;cargo check -p bun_corepasses.