compile: accept "glibc" as an explicit libc token in --target - #37385
compile: accept "glibc" as an explicit libc token in --target#37385robobun wants to merge 6 commits into
Conversation
Bun.Build.Libc has advertised "glibc" | "musl" since compile targets were added to Bun.build, but CompileTarget::try_from only knew the "musl" and "android" tokens, so "bun-linux-x64-glibc" failed in both the CLI (Unsupported target "glibc") and Bun.build (Unknown compile target). "glibc" now parses to Libc::Default, so it names the same build as the plain Linux targets and never downloads anything on a glibc host. Like "musl" it is only valid together with linux; the OS check keys off the explicit token instead of the libc value so "bun-windows-x64-glibc" is still rejected, with its own message on the CLI.
|
Status: fix pushed (latest fda5879, docs only on top of ccdfd62), waiting on CI. Reproduced with bun 1.4.0: Review: unknown-segment coverage added (4e95696); e1bfee3 makes |
|
Warning Review limit reached
Next review available in: 14 minutes 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 (1)
WalkthroughThe compile target parser now supports explicit Linux ChangesExplicit glibc target support
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/options_types/compile_target.rs`:
- Around line 390-392: Update the target validation logic around the glibc
diagnostic to use exact parsed target tokens or the existing ParseError
validation reason instead of strings::contains substring checks on input.
Preserve the invalid-target diagnostic for glibc targets that lack linux while
avoiding false matches from unrelated token text.
In `@test/bundler/bun-build-compile.test.ts`:
- Around line 127-216: Extend the “compile target -glibc token” suite with CLI
and Bun.build cases using the malformed target bun-linux-${arch}-glibc-invalid.
Assert both paths reject the target, preserving the existing invalid-target
error behavior and ensuring the CLI exits unsuccessfully without building.
🪄 Autofix
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: 5cefe8f6-0021-4748-b000-b414cc0285e5
📒 Files selected for processing (3)
docs/bundler/executables.mdxsrc/options_types/compile_target.rstest/bundler/bun-build-compile.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small, well-scoped parser fix that closes the type/runtime gap for Bun.Build.Libc = "glibc".
What was reviewed:
try_from: the newglibcarm maps toLibc::Default, sonpm_name(), download URL, cache key anddefine_valuesare unchanged;is_default()still holds on a glibc host (no download).- The linux-only check switching from
libc != Defaulttofound_libc— necessary now thatglibcparses toDefault; verified the earlier!found_libchost-libc reset block is unaffected. from():glibcin the known-token filter so…-glibc-invalidstill namesinvalid; the substring diagnostic branch mirrors the existingmusl/androidsiblings.- Tests: both entry points, accept (glibc-host-only, no network) and reject (all hosts), pipes drained concurrently, combined-object assertions;
isGlibcexists in harness.
Extended reasoning...
Overview
Three files: src/options_types/compile_target.rs gains a "glibc" token in CompileTarget::try_from (mapping to Libc::Default), a corresponding entry in the from() known-token filter, a diagnostic branch, and the linux-only check now keys on found_libc instead of libc != Default. docs/bundler/executables.mdx gets one sentence. test/bundler/bun-build-compile.test.ts gets a new describe block covering accept/reject on both CLI and Bun.build.
The motivating gap is real: Bun.Build.Libc is typed "glibc" | "musl" and the types fixture asserts bun-linux-x64-modern-glibc is assignable, yet the runtime parser rejected glibc as an unsupported token.
Security risks
None. This is pure string-token matching over a --target value; no allocation sizing, no filesystem/network at parse time, no user data flowing to a syscall. The only downstream effect (which build to download) is unchanged because Libc::Default.npm_name() is "".
Level of scrutiny
Low-to-medium. The parser is user-facing but the change is a four-line arm mirroring the existing musl arm plus a condition rewrite whose necessity is explained in a one-line comment. I traced the semantic knock-ons: on a glibc host bun-linux-<host arch>-glibc still satisfies is_default() (uses the running binary, no download); on a musl/android host it correctly selects the glibc build; bun-windows-x64-glibc is rejected because found_libc && os != Linux; the earlier !found_libc && libc != Default && os != Linux host-reset block is orthogonal and untouched.
Other factors
Both CodeRabbit threads are resolved — the malformed-suffix coverage was added in 4e95696, and the substring-diagnostic concern was withdrawn as the branch only selects an error message consistent with its musl/android/wasm siblings. Tests follow harness conventions (tempDir, bunEnv, concurrent pipe drain, combined {stdout, stderr, exitCode} assertion, test.concurrent, skipIf(!isGlibc) for the accept path so no network is touched). isGlibc already exists in test/harness.ts. The PR description is unusually thorough and matches the code exactly.
…g it try_from knew which segment it rejected and why, but returned unit variants, so from() re-tokenized the input against a second copy of the token list to name the bad segment and picked the InvalidTarget message by substring-matching the input. Adding the glibc token meant teaching all three places, and the substring chain misattributed inputs such as bun-x64-glibc-v1.2 (an incomplete version) to the libc message. ParseError now carries the segment or the reason and Display renders the same messages; from() only prints it. The libc tokens move into a single LIBC_NAMES map like the arch and OS tables. Tests pin every message, add the musl/glibc selection check that is only observable from a bun built against the other libc (the fetch goes to a local 404 server, so the build fails naming the bun it resolved to), and the docs sentence now describes that a Linux target without a libc segment follows the running bun's libc.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Updated 2:16 AM PT - Aug 11th, 2026
✅ @robobun, your commit fda5879f359e92218bee661631d45280264397b9 passed in 🧪 To try this PR locally: bunx bun-pr 37385That installs a local version of the PR into your bun-37385 --bun |
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 `@docs/bundler/executables.mdx`:
- Line 231: Update the Linux target documentation near the libc segment
description to include -android as a supported segment, while preserving the
existing -glibc and -musl examples and behavior explanation.
In `@test/bundler/bun-build-compile.test.ts`:
- Around line 171-183: Update the compile build subprocess assertions around
Bun.spawn so captured stdout is retained and asserted alongside stderr and
exitCode. Include stdout in the Promise.all result object and expected
successful-build result, preserving concurrent draining of stdout, stderr, and
process exit; apply the same change to the additional build path.
🪄 Autofix
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: 28c4c6b2-0007-40c0-b38d-87782829b9ae
📒 Files selected for processing (3)
docs/bundler/executables.mdxsrc/options_types/compile_target.rstest/bundler/bun-build-compile.test.ts
…bc segment list is not exhaustive
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 `@docs/bundler/executables.mdx`:
- Line 231: Add the parser-supported explicit glibc target literals to the
Bun.Build.CompileTarget union, alongside the existing Linux target variants.
Ensure every documented -glibc architecture target, including
bun-linux-x64-glibc, is represented so the type matches accepted build targets.
🪄 Autofix
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: 73400d0b-d205-4bb1-9ffb-211efdd61688
📒 Files selected for processing (2)
docs/bundler/executables.mdxtest/bundler/bun-build-compile.test.ts
There was a problem hiding this comment.
LGTM — closes the type/runtime gap for bun-linux-*-glibc targets and cleans up the diagnostic path.
What was reviewed:
try_fromsegment loop:glibc → Libc::Defaultkeepsis_default(), download URL, anddefine_valuesunchanged on glibc hosts; on musl hosts it correctly selects the glibc build.found_libc && os != Linuxgate replaceslibc != Default— verifiedbun-windows-x64-glibcstill rejects on all host libcs, and the!found_libcimplicit-reset branch above it is unaffected.ParseError<'a>lifetime: only two callers (options_jsc.rsdiscards the error,Arguments.rsgoes throughfrom()), both compatible;strum::IntoStaticStrremoval has no other consumers.- The open CodeRabbit note about
CompileTargetliterals refers to the docs code block, notbun.d.ts— the actual type already covers-glibcvia${Libc}.
Extended reasoning...
Overview
Three files: src/options_types/compile_target.rs (accept glibc as a libc segment; restructure ParseError to carry the failure reason instead of unit variants that from() re-derived by substring-matching the input), test/bundler/bun-build-compile.test.ts (+~200 lines: accept tests on glibc hosts, exact-stderr rejection tests on all hosts, libc-selection tests behind a local 404 server on Linux), and one docs paragraph in executables.mdx.
Security risks
None. Pure string parsing of a CLI/API option; no allocation sizing, no filesystem/network effects at parse time. The libc-selection test is hermetic (local Bun.serve({port:0}), empty BUN_INSTALL_CACHE_DIR, proxy env vars cleared).
Level of scrutiny
Moderate. The core fix (glibc → Libc::Default in a comptime string map) is a one-liner. The ParseError restructuring is larger but strictly simplifying: it deletes ~50 lines of re-tokenization in from() that kept a second copy of the segment list and picked messages by substring, and moves the exact same message text into a Display impl keyed on the variant the parser actually returned. Every message is pinned by a test.concurrent.each row, including the previously-misattributed bun-x64-glibc-v1.2 case. I traced both callers of try_from/from — the added lifetime parameter is transparent to them, and the dropped IntoStaticStr derive has no consumers.
Other factors
All prior review threads are resolved (comment-cop trim, unknown-segment coverage, stdout stage assertions, non-exhaustive docs wording). The remaining unresolved CodeRabbit note asks to add -glibc literals to the CompileTarget union — but the actual type in packages/bun-types/bun.d.ts already includes them via \bun-linux-${Architecture}-${Libc}`whereLibc = "glibc" | "musl"; the referenced lines are the illustrative docs code block, which this PR does not touch. Tests follow harness conventions (tempDir+using, bunEnv` spread, concurrent pipe drain, combined-object assertions, per-test timeouts only on the compile paths).
There was a problem hiding this comment.
LGTM — focused fix making the runtime honor the "glibc" value that Bun.Build.Libc already declares.
What was reviewed:
try_fromsegment loop:LIBC_NAMESmap replaces the two hardcoded branches,glibc→Libc::Defaultleavesnpm_name/define_values/Displayunchanged;found_libcgate correctly rejectsbun-windows-x64-glibcwhile the earlier!found_libcreset still clears the host libc for non-Linux targets.ParseError<'a>restructuring: both callers checked —options_jsc.rsdiscards the error vialet Ok else,Arguments.rsgoes throughfrom()which just printsDisplay; the lifetime and dropped#[error]attrs (manualDisplaysupplied) are fine for both.- Tests: hermetic (local 404 server, empty cache dir, proxy vars unset), concurrent pipe drains, combined-object assertions,
isGlibcexists in harness; rejection matrix pins the pre-existing musl/android/wasm messages verbatim through the refactor.
Extended reasoning...
Overview
Three files: src/options_types/compile_target.rs teaches CompileTarget::try_from to accept a glibc segment (mapping to Libc::Default) via a new LIBC_NAMES comptime string map alongside the existing arch/OS tables, and restructures ParseError from unit variants into structured variants (UnsupportedSegment{segment, input}, IncompleteVersion, LibcRequiresLinux(Libc), Wasm) whose Display renders the same CLI messages the old from() re-derivation produced. from() shrinks from ~55 lines of re-tokenization + substring heuristics to a two-line print-and-exit. test/bundler/bun-build-compile.test.ts gains a ~200-line suite covering accept (glibc host, both entry points, executable runs), reject (six target strings with exact stderr on the CLI, two on the JS API), and libc selection (local 404 server observing whether the other-libc build is fetched). docs/bundler/executables.mdx gets a paragraph describing the host-following default and adds the two -glibc rows to the illustrative CompileTarget union.
Security risks
None. The change is to a --target string tokenizer that maps segments to enum variants; no filesystem paths, no shell, no network input. The one network-adjacent path (to_npm_registry_url) is untouched — Libc::Default.npm_name() was already "".
Level of scrutiny
Medium. The parser change itself is small (one map + one gate change), but the found_libc vs libc != Default swap is semantically load-bearing and I traced it through the four host×input combinations: glibc host + -glibc → is_default() true (running binary); musl host + -glibc → libc=Default ≠ host default Musl, downloads glibc build; musl host + bun-windows-x64 → !found_libc reset still fires, libc=Default; any host + bun-windows-x64-glibc → found_libc && os≠Linux rejects. The ParseError<'a> lifetime addition required checking both callers (src/bundler_jsc/options_jsc.rs:86 discards it, src/runtime/cli/Arguments.rs:2157 uses from()), neither stores the error. #[derive(thiserror::Error)] without #[error] attrs plus a manual Display impl is valid thiserror usage; Debug was added to Libc because ParseError derives Debug.
Other factors
All five CodeRabbit threads are resolved (two addressed with commits, three withdrawn). The comment-cop flag was addressed in b0ae4fd. The rejection-matrix tests pin every pre-existing message byte-for-byte, so the Display refactor is proven behavior-preserving for musl/android/wasm/incomplete-version, and the bun-x64-glibc-v1.2 row confirms the old substring-misattribution (would have blamed glibc) is gone. Test hygiene follows repo conventions: tempDir with using, Promise.all drains, port: 0, test.concurrent.each, per-test timeouts only on the compile-copying paths.
1 participant
Repro
Bun.Build.Libcis declared as"glibc" | "musl"andBun.Build.CompileTargetincludes`bun-linux-${Architecture}-${Libc}`(packages/bun-types/bun.d.ts, namespaceBuild;test/integration/bun-types/fixture/build.tseven assertsbun-linux-x64-modern-glibcis assignable), so this type-checks and then fails at runtime on both entry points:Reproduced with bun 1.4.0 (and 1.3.14).
Cause
Both the CLI (
src/runtime/cli/Arguments.rs,--targetwith--compile) andBun.build(compile_target_from_sliceinsrc/bundler_jsc/options_jsc.rs, reached fromcompile: "...",compile: { target }andtarget: "bun-...") parse the string withCompileTarget::try_frominsrc/options_types/compile_target.rs. Its segment loop knew arch and OS names,modern/baseline,-vX.Y.Z,muslandandroid; anything else, includingglibc, was rejected.Fix
glibcis now a libc segment that parses toLibc::Default, i.e. exactly the build the plain Linux targets name, so the download URL, cache name anddefine_valuesare unchanged and on a glibc hostbun-linux-x64-glibcis stillis_default()(uses the running binary, downloads nothing). The three libc segments live in oneLIBC_NAMESmap, like the arch and OS tables. The linux-only check keys off "a libc segment was given" rather thanlibc != Default, sinceglibcnow maps toDefault; this is what keepsbun-windows-x64-glibcrejected.The error reporting was restructured at the same time, because the first version of this change had to teach
glibcto three places:try_fromreturned unitParseErrorvariants, sofrom()re-tokenized the input against a second copy of the segment list to name the bad segment, and chose the "invalid target" message by substring-matching the input. That chain also misattributes errors (bun-x64-glibc-v1.2, an incomplete version on a Linux target, would have printed the libc message).ParseErrornow carries the offending segment or the reason (UnsupportedSegment,IncompleteVersion,LibcRequiresLinux(libc),Wasm), itsDisplayrenders the existing messages verbatim plus the new glibc one (invalid target, glibc only exists on linux), andfrom()just prints it.Bun.buildstill throwsUnknown compile target: ...for every rejection, as before.Why accept the segment rather than delete it from the types: the types and the types fixture have advertised it since
Bun.build({ compile })landed, and it is the only way to ask for the glibc build explicitly.CompileTarget::default()carries the host libc andtry_fromonly resets it for non-Linux targets, so on a musl (or Android) build of bun,bun-linux-x64means the host's libc;-glibcis the counterpart of-muslfor that case. That host-following default is unchanged; the docs sentence under "Supported targets" now describes it, since the table there otherwise reads as if plain Linux targets were always glibc.Types are untouched (
Libcalready matches the runtime). The type still does not model-androidor the-vX.Y.Zsuffix; that is a separate, pre-existing gap left for its own change.Verification
New tests in
test/bundler/bun-build-compile.test.ts("compile target libc segments"):Bun.buildandbun build --compileacceptbun-linux-<host arch>-glibcand the produced executable runs (glibc Linux hosts, where the target is the running binary; no network).bun-linux-x64-glibc-invalid(namesinvalid, notglibc),bun-x64-glibc-v1.2(names the version, not glibc), andbun-wasm; plusBun.buildrejections for two of those. The musl, android and wasm rows pass before and after and pin the messages through the restructuring.BUN_COMPILE_TARGET_TARBALL_URL) and an emptyBUN_INSTALL_CACHE_DIR, a target without a libc segment builds without any fetch, while a segment naming the other libc fails withTarget platform 'bun-linux-<arch>[-musl]-vX.Y.Z' is not available for downloadafter exactly one fetch. On the musl CI lanes the "other" libc is glibc, which is the only place theglibc => Libc::Defaultmapping is observable (on glibc hosts it is a no-op); on glibc lanes the same test exercises-musl.Without the
src/change, both accept tests and the glibc,glibc-invalidandglibc-v1.2rejection rows fail; with it the whole file passes underbun bd test.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bun-build-compile.test.ts