bunfig: look up XDG-conventional global config paths - #30853
Conversation
|
Updated 10:05 PM PT - May 20th, 2026
❌ @autofix-ci[bot], your commit fbc960a has 1 failures in
🧪 To try this PR locally: bunx bun-pr 30853That installs a local version of the PR into your bun-30853 --bun |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR implements XDG Base Directory specification compliance for Bun's global ChangesXDG-compliant bunfig.toml path resolution
🚥 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: 2
🤖 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 `@test/config/bunfig/global-config-xdg.test.ts`:
- Around line 16-40: The helper runPmCache currently returns stderr but the
tests calling it don't assert or surface stderr on failure; update each of the
six test cases that call runPmCache so they destructure stderr from the returned
object and, immediately before the existing expect(exitCode).toBe(0) assertion,
add a conditional that if (exitCode !== 0) { expect(stderr).toBe(""); } to
ensure failing child-process output is shown in test failures; locate the calls
to runPmCache in the test file (the six cases around the existing
expect(exitCode).toBe(0) checks) and apply this pattern using the exitCode and
stderr variables returned by runPmCache.
- Around line 42-45: The helper writeBunfigCacheDir is using
path.substring(...lastIndexOf("/")) which breaks on Windows; replace that manual
dirname logic with Node's path.dirname (i.e., call path.dirname(path) when
creating directories) and ensure the test file imports/references the Node
'path' module (or a variable named path) so mkdirSync receives a
platform-correct directory; keep the writeFileSync call and contents unchanged
(still using writeFileSync(path, `[install.cache]\ndir =
${JSON.stringify(cacheDir)}\n`)).
🪄 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: 68a13902-f5e8-4f25-929b-a6131911ba87
📒 Files selected for processing (3)
docs/runtime/bunfig.mdxsrc/bunfig/arguments.rstest/config/bunfig/global-config-xdg.test.ts
962d4df to
a10706f
Compare
There was a problem hiding this comment.
All earlier review feedback has been addressed and I found no remaining issues, but since this changes global bunfig.toml resolution precedence for every user (and CI is currently red on the build-rust shards), it's worth a maintainer sign-off before merge.
Extended reasoning...
Overview
The substantive change is a ~45-line rewrite of get_home_config_path() in src/bunfig/arguments.rs to probe XDG-conventional locations ($XDG_CONFIG_HOME/bun/bunfig.toml, with the spec default $HOME/.config applied when unset/empty) before falling back to the legacy $HOME/.bunfig.toml. It ships with a 7-case test suite and updated docs. The other 10 touched files (crash_handler, errno, tracy, run_command, etc.) are pure autofix.ci #[cfg]/#[cfg_attr] reformatting with no semantic change — the author tried to revert them in 37bb3ef and autofix re-applied them in 01e2ab1.
Prior feedback
I left six inline findings on earlier revisions (Windows dirname/USERPROFILE, XDG empty-string handling, BUN_INSTALL_CACHE_DIR masking the test signal, describe.concurrent, docs framing, and the xdg_scratch buffer size). All six are marked resolved and I verified each fix is present in the current diff. The bug-hunting pass on this revision found nothing.
Security risks
None identified. The new code reads two well-known env vars and stats a small fixed set of paths under them; there's no shell-out, no untrusted-input parsing, and the existing load_bunfig(auto_loaded=true) path already swallows "file not found". The unsafe { ZStr::from_raw } is locally justified (re-borrowing a buffer that join_abs_string_buf_z just NUL-terminated).
Level of scrutiny
Medium. The logic itself is simple and well-tested, but it changes which config file wins at startup for every install-related command. A user with a stale ~/.config/bun/bunfig.toml that was previously ignored will now have it take precedence over ~/.bunfig.toml — that's the intended fix for #30842, but it's a product/behavior decision a maintainer should ack rather than something to auto-approve.
Other factors
- Buildkite is currently failing
build-rust/build-cppon several shards for 37bb3ef (visible in the robobun comment); the latest autofix commit 01e2ab1 may or may not have cleared that — worth confirming green before merge. - The 10-file autofix reformatting noise is out of scope for this PR but appears to be unavoidable given the autofix.ci re-application.
|
CI summary for maintainers (latest — Buildkite #56531 on fbc960a):
|
Closes #30842. Global `bunfig.toml` lookup previously checked only `$XDG_CONFIG_HOME/.bunfig.toml` (a hidden file inside a directory that XDG designates precisely to avoid dotfiles) and `$HOME/.bunfig.toml`, and did not apply the XDG spec default of `$HOME/.config` when `$XDG_CONFIG_HOME` was unset. Other XDG-compliant tools use an app subdir, which Bun did not support. `get_home_config_path` now probes candidates in order, first existing file wins: 1. $XDG_CONFIG_HOME/bun/bunfig.toml (XDG-conventional) 2. $XDG_CONFIG_HOME/.bunfig.toml (legacy, retained) 3. $HOME/.config/bun/bunfig.toml (spec default when XDG_CONFIG_HOME unset) 4. $HOME/.config/.bunfig.toml (legacy under spec default) 5. $HOME/.bunfig.toml (original home dotfile) Candidates under the XDG base are existence-checked so we fall through; the home dotfile is returned unconditionally so `load_bunfig`'s auto-load branch can swallow a missing file uniformly (prior behaviour). Docs updated to advertise the XDG-conventional path and the spec default.
…ing in tests Addresses two review findings: 1. XDG spec says `XDG_CONFIG_HOME` unset *or empty* should fall back to `$HOME/.config`. `env_var::XDG_CONFIG_HOME.get()` returns `Some(b"")" for a bare `XDG_CONFIG_HOME=""`, making the first match arm take an empty base. Switch to `get_not_empty()` (codebase already provides it and uses it for `TMPDIR`) so empty falls through to the `$HOME/.config` arm. Applied to `HOME` in the tail branch too for consistency. 2. Windows: the test helper deleted `USERPROFILE` but only set `HOME`. `env_var::HOME` reads `USERPROFILE` on Windows, so `HOME`-dependent tests wouldn't see the intended home dir. Mirror `HOME` → `USERPROFILE` in `runPmCache` so the suite is portable (the feature itself already works on Windows — `XDG_CONFIG_HOME` is honoured there too). New test case: `XDG_CONFIG_HOME=""` falls back to `$HOME/.config`.
CI runner (scripts/runner.node.mjs:1223) injects BUN_INSTALL_CACHE_DIR into every test process. fetch_cache_directory_path() checks that env var before the bunfig [install.cache].dir option (PackageManagerDirectories.rs:434), so on Buildkite the CI tmpdir masked our sentinel and every expect(stdout).toBe(cacheDir) failed. Strip BUN_INSTALL_CACHE_DIR (plus BUN_INSTALL and XDG_CACHE_HOME for hygiene) in runPmCache so the per-test sentinel wins. Switch to describe.concurrent now that the environment is fully contained per-test (own tempDir, own env map, no shared mutation).
…k chain get_home_config_path() resolves *one* XDG base ($XDG_CONFIG_HOME when set/non-empty, else $HOME/.config) then probes exactly 2 candidates under it — $HOME/.config is not a per-candidate fallback. The previous 5-item list read as "check all five in order", which would mislead a reader with $XDG_CONFIG_HOME set to a non-default location and a config under ~/.config/.
MAX_PATH_BYTES is 98302 on Windows (32767 * 3 + 1), so [0u8; MAX_PATH_BYTES / 2] was a 48 KB zero-initialized stack array on every call — contradicting the 'small stack array' comment and close to the 64 KB Windows stack-alloc ceiling documented in src/CLAUDE.md. $HOME is well under a few hundred bytes on every supported platform; 512 is ample, and longer homes still fall through to $HOME/.bunfig.toml via the existing bounds check.
…s 2); diff is cross-platform
e61179a to
ab76d0e
Compare
There was a problem hiding this comment.
All prior review feedback is addressed and I found no new issues, but this changes global config-discovery precedence (a user-visible design decision) and adds an unsafe reborrow in a startup hot path, so it's worth a maintainer sign-off — also to confirm the remaining Buildkite failures are indeed infra-only.
Extended reasoning...
Overview
This PR rewrites get_home_config_path() in src/bunfig/arguments.rs (~60 net lines) to probe XDG-conventional global bunfig locations ($XDG_CONFIG_HOME/bun/bunfig.toml, with the spec default $HOME/.config applied when unset/empty) ahead of the legacy hidden-dotfile paths, updates docs/runtime/bunfig.mdx to document the new lookup order, and adds a 7-case integration test (test/config/bunfig/global-config-xdg.test.ts) that exercises each candidate and precedence rule via bun pm cache.
Security risks
None identified. The new code only reads two well-known environment variables and stats files under the user's own home/XDG directory; it introduces no new untrusted input, network access, or privilege boundaries. The 512-byte scratch buffer is bounds-checked (if total <= xdg_scratch.len()) before copy_from_slice, and the unsafe { ZStr::from_raw(...) } reborrows a NUL-terminated path that join_abs_string_buf_z just wrote into the caller-owned PathBuffer.
Level of scrutiny
Moderate-to-high. This runs on the CLI startup path for every command that reads global config, and it changes user-visible behaviour: a previously-ignored ~/.config/bun/bunfig.toml will now take effect, and the relative precedence between XDG and $HOME/.bunfig.toml shifts. That's the right outcome per the linked issue, but the precedence ordering and back-compat strategy are design choices a maintainer should ratify rather than a bot. The unsafe block looks correct but is another reason for a human eye.
Other factors
I left seven inline comments on earlier revisions (Windows dirname/USERPROFILE, get_not_empty() for empty XDG_CONFIG_HOME, stripping BUN_INSTALL_CACHE_DIR in tests, describe.concurrent, docs framing, and the scratch-buffer size) — all are addressed and resolved in the current diff, and the bug-hunting pass found nothing new. The remaining Buildkite reds (musl/freebsd build-rust, darwin build-cpp, darwin-aarch64 runner expiry) appear infra-related and unrelated to the three files touched here, but I'd rather a maintainer confirm that before merge.
Closes #30842.
Repro
$XDG_CONFIG_HOME/bun/bunfig.toml(the XDG-conventional path used by every other XDG-compliant tool) was silently ignored.$XDG_CONFIG_HOME/.bunfig.tomlonly worked when$XDG_CONFIG_HOMEwas explicitly set — the spec default of$HOME/.configwas not applied when the variable was unset.Cause
get_home_config_path(src/bunfig/arguments.rs) only knew two paths — both hidden dotfiles. It checked$XDG_CONFIG_HOME/.bunfig.tomlif the env var was set, else$HOME/.bunfig.toml. No app subdir, no spec default whenXDG_CONFIG_HOMEwas unset.Fix
Probe candidates in this order and return the first existing file:
$XDG_CONFIG_HOME/bun/bunfig.toml— XDG-conventional (app subdir)$XDG_CONFIG_HOME/.bunfig.toml— legacy, retained for back-compat$HOME/.config/bun/bunfig.toml— applies the XDG spec default whenXDG_CONFIG_HOMEis unset$HOME/.config/.bunfig.toml— legacy under spec default$HOME/.bunfig.toml— original home dotfileCandidates under the XDG base are
exists_z-checked before probing the next. The home dotfile is returned unconditionally (even if missing) to preserve the prior uniform handling —load_bunfigswallows "file not found" whenauto_loaded=true.Docs (
docs/runtime/bunfig.mdx) now advertise the XDG-conventional path as the recommended option and reference the XDG Base Directory Specification.Verification
test/config/bunfig/global-config-xdg.test.ts— 6 cases covering each candidate, priority between XDG app subdir vs legacy hidden-file, and that an explicit$XDG_CONFIG_HOMEstill beats the spec default.Rebase note (2026-05-15): rebased onto current main and dropped three unrelated commits that were dangling from earlier iterations — two
[autofix.ci] apply automated fixescommits (rustfmt multi-line#[cfg(...)]reformatting ofcrash_handler/errno/tracy/cli/spawn/...) and mychore: revert autofix-ci reformattingthat offset them. Main has since landed #31116 (clippy: 45 deny lints + fix 2735 violations across workspace) which already covers that formatting. Final diff is just the three intended files:src/bunfig/arguments.rs,docs/runtime/bunfig.mdx,test/config/bunfig/global-config-xdg.test.ts.