bunfig: honor smol and install.prefer for bun file.js / bun test, and let the flags override them for bun run - #38611
Conversation
… let the flags win over the file Arguments::parse wrote ctx.runtime_options.smol and ctx.debug.offline_mode_setting unconditionally, after bunfig.toml had already been applied for `bun file.js`, `bun -e` and `bun test`, so `smol = true`, `[test] smol = true` and `install.prefer` in bunfig.toml had no effect there. For `bun run file.js` bunfig.toml is loaded later and overrode --smol / --prefer-* instead. Only write the two settings when the flag is present, and record flags given on the command line in ctx.cli_overrides so the bunfig parser leaves those keys alone whichever of the two is applied first.
|
Warning Review limit reached
Next review available in: 32 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
|
Status: reproduced on the release build of main with the Linux memfd probe used in CI: 177 of 179 jobs passed with no test failures; the build is marked failed only because the two |
There was a problem hiding this comment.
LGTM — focused fix for CLI-flag vs bunfig.toml precedence on smol and install.prefer.
What was reviewed:
- Verified all three readers of
offline_mode_setting(run_command.rs,repl_command.rs,bake/production.rs) already.unwrap_or(Online), so leaving itNonewhen no flag is passed preserves the default. - Confirmed the
CliOverridesgate inbunfig.rsstill type-checks the key even when the flag wins, and matches the existingpath_ignore_patterns_from_clipattern. - Checked the memfd probe against
LinuxMemFdAllocator::should_use(1 MiB vs 8 MiB threshold) and thememfd-num-label — a 2 MiB Blob distinguishes the two modes;can_use_memfd()failing would produce a false fail, not a false pass. - Traced both load orders (bunfig-before-flags for
bun file.js/-e/testatArguments.rs:921, bunfig-after-flags forbun run) — the flag wins in each.
Extended reasoning...
Overview
Four files: src/runtime/cli/Arguments.rs stops unconditionally overwriting ctx.runtime_options.smol and ctx.debug.offline_mode_setting when the corresponding flags are absent, and records presence in a new ctx.cli_overrides. src/options_types/context.rs adds the two-field CliOverrides struct to ContextData. src/bunfig/bunfig.rs skips assigning smol / [test].smol / install.prefer when the corresponding CLI override bit is set (still runs the type check). test/config/bunfig/smol.test.ts covers 13 combinations across bun file.js, bun -e, bun run, and bun test, with the flag before and after the subcommand.
Security risks
None. This is CLI/bunfig option precedence for --smol (heap sizing) and --prefer-offline/--prefer-latest (auto-install cache preference). No untrusted input parsing, no auth/crypto/permissions.
Level of scrutiny
Low-to-moderate. The change is small (~30 net lines of Rust), mechanical, and follows the exact pattern already used for test.pathIgnorePatterns (path_ignore_patterns_from_cli). The only behavioral risk was that leaving offline_mode_setting as None (rather than Some(Online)) might change a reader's behavior; I grepped all three readers and each does .unwrap_or(OfflineMode::Online), so the default is preserved.
Other factors
- The test is Linux-only by necessity (the memfd threshold is the only externally observable smol-mode signal without adding test-only hooks), but the code under test is platform-agnostic. The probe is anchored to real constants in
LinuxMemFdAllocator::should_use(1 MiB vs 8 MiB) and thememfd-num-label, and asserts the exact blob size so a probe failure surfaces rather than passing vacuously. install.prefergets the same code fix without a test; the description explains this is because--prefer-offlineitself is currently unobservable on main (pending #36776), which is a reasonable justification.- The
CliOverridesstruct is deliberately named to merge cleanly with sibling PR #38599. - No CODEOWNERS coverage on the touched files. No prior reviewer comments to address.
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Not duplicates: none of #38599, #33198, #36669 or #28547 touch |
Problem
smol = truein bunfig.toml is ignored bybun file.jsandbun -e, and[test] smol = trueis ignored bybun test. Onlybun run file.jshonors it.bun run --smol file.jswithsmol = falsein bunfig.toml runs without smol mode: the file overrides the flag, although the bunfig docs say CLI flags override bunfig.toml.install.prefer(--prefer-offline/--prefer-latest) has the same two problems.Arguments::parse(src/runtime/cli/Arguments.rs) assignsctx.runtime_options.smol = args.flag("--smol")andctx.debug.offline_mode_setting = Some(...)unconditionally. Forbun file.js,bun -eandbun testit has already applied bunfig.toml at that point (load_config_with_cmd_args), so with no flag on the command line both assignments reset what the file set (src/bunfig/bunfig.rs: top-levelsmol,[test] smol,install.prefer).bun run <target>loads bunfig.toml later, fromRunCommand, and the bunfig parser assigned the keys unconditionally, so there the file overwrote a flag that was given.Fix
Arguments::parseonly writes the two settings when the flag is present. Without a flagoffline_mode_settingstaysNone, which all three readers (RunCommand,bun repl,bun build --app) already map to online, so the default is unchanged.ctx.cli_overrides(newCliOverridesinsrc/options_types/context.rs), and the bunfig parser skips the assignment for a recorded key (the key is still type-checked). This is the mechanismtest.pathIgnorePatternsalready uses (path_ignore_patterns_from_cli), and it gives the same result in both load orders without touching the late-load sites.cli_overridesstruct for the keys that were only broken in thebun runorder; it leavessmolandinstall.preferto this PR. The struct and field are named the same here so whichever lands second only has to merge the field lists.test/config/bunfig/smol.test.ts. Smol mode has no direct JS reflection; on Linux it lowers the size at which a Blob is backed by a memfd from 8 MiB to 1 MiB (LinuxMemFdAllocator::should_use), so a 2 MiB Blob plus a look at/proc/self/fdtells the two modes apart with an unmodified binary. The cases coverbun file.js,bun -e,bun run file.js,bun test, and--smolbeatingsmol = false/[test] smol = falsewith the flag before and after the subcommand. Linux only because of the probe; the code under test is not platform specific.smol = truewithbun index.jsand withbun -e,[test] smol = truewithbun test,smol = falsewithbun run --smoland withbun --smol run); with this change all 13 pass.install.prefergets the same code change but no behavioural test: on main,--prefer-offlineitself has no observable effect on auto-install (the disk cache lookup it enables never matches; install: make --prefer-offline resolve auto-installs from the disk cache #36776 fixes that, and I confirmed the flag changes nothing on main with that PR's scenario), so neither the flag nor the key can be distinguished from the default yet. Once install: make --prefer-offline resolve auto-installs from the disk cache #36776 lands, its--prefer-offlinetest withprefer = "offline"in bunfig.toml covers this path.test/config/bunfig/,test/cli/run/run-autoinstall.test.ts,test/cli/run/autoinstall-cached-manifest.test.ts,test/regression/issue/24387.test.ts, theexecArgvtests intest/js/node/worker_threads/worker_threads.test.tsandtest/bundler/compile-argv.test.ts(standalone executables parse theirexecArgvthrough the same function): all pass.cargo fmt --checkis clean.Background
ContextData(ctx,src/options_types/context.rs) is the process-wide record of parsed CLI state. BothArguments::parseand the bunfig parser write into it, and the commands read from it afterwards (RunCommand::boot,bun test, the REPL, the parallel test runner); whichever writer runs second owns a field.bun test,bun -e, andbun file.jswith a known extension load it insideArguments::parse, before the runtime flags are applied.bun run <target>,bun <script>, thenodeshim andbun replskip that and load it once argv parsing is done (theloaded_bunfigchecks inrun_command.rs/repl_command.rs). A setting that both argv and the file can set has to come out the same either way.smol(--smol) makesRunCommand/bun test/ the REPL create the JSC VM with a small heap and makes a few native caches release memory eagerly; the memfd threshold the test observes is one of those knobs.install.prefer/--prefer-offline/--prefer-latestsetctx.debug.offline_mode_setting, which becomes the runtime auto-installer'sinstall_preference.