Repository navigation
Conversation
Implement support for the `libc` field in package.json, which is used by packages like @rollup/rollup-linux-x64-gnu and @rollup/rollup-linux-x64-musl to specify which libc variant they require. Changes: - Add `libc` field to lockfile `Meta` struct - Implement compile-time libc detection (glibc vs musl on Linux, .all elsewhere) - Update `isDisabled()` to check libc in addition to cpu and os - Add `--libc` CLI flag for manually overriding libc (like `--cpu` and `--os`) - Add libc serialization/deserialization for bun.lock and pnpm lockfiles - Parse `libc` field from package.json The isMatch logic handles backwards compatibility by treating `.none` (unset) the same as `.all`, so packages without a libc field match any system libc. Fixes #26156 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughAdds libc as a platform dimension: CLI flag and Options field, runtime libc detection, libc stored in package metadata and lockfiles, and libc-aware logic throughout dependency filtering, native binlink replacement, lifecycle script gating, and resolution/installation. Tests for CLI parsing and libc filtering were added. (≤50 words) Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)src/**/*.zig📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
🧠 Learnings (12)📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:37:11.466ZApplied to files:
📚 Learning: 2025-10-16T17:32:03.074ZApplied to files:
📚 Learning: 2026-01-15T03:22:50.711ZApplied to files:
📚 Learning: 2025-10-15T22:03:50.832ZApplied to files:
📚 Learning: 2025-10-16T21:24:52.779ZApplied to files:
📚 Learning: 2025-10-24T10:43:09.398ZApplied to files:
📚 Learning: 2025-12-11T02:11:47.024ZApplied to files:
📚 Learning: 2026-01-15T03:22:50.711ZApplied to files:
📚 Learning: 2026-01-05T16:32:07.551ZApplied to files:
📚 Learning: 2025-10-16T02:17:35.237ZApplied to files:
📚 Learning: 2025-10-17T01:21:35.060ZApplied to files:
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting 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
🤖 Fix all issues with AI agents
In `@src/install/npm.zig`:
- Around line 717-735: Libc.current is a compile-time const using
Environment.isMusl, which misdetects the hosting libc for binaries compiled for
a different ABI; change it to a runtime-detected value by replacing the const
with a function (e.g., pub fn current() Libc) that, when Environment.isLinux is
true, performs runtime checks for musl (such as testing for the musl dynamic
loader binary paths like /lib*/ld-musl-*-*.so.1 or using a safe runtime probe)
and returns .musl or .glibc accordingly, otherwise returns .all; keep isMatch
and negatable as-is and ensure callers use Libc.current() (function) instead of
the old const.
In `@test/regression/issue/26156.test.ts`:
- Around line 45-100: The test "should correctly filter glibc vs musl optional
dependencies" only checks exit codes and leaves bun.lockb around, so update the
test to (1) add concrete mock optional deps with libc-specific package names in
the tempDir package.json (e.g. include both "@rollup/rollup-linux-x64-gnu" and
"@rollup/rollup-linux-x64-musl" or equivalent registry-mocked packages) so the
installer must choose between them, (2) after each Bun.spawn install
(proc1/proc2/proc3) assert which package got installed (verify node_modules
contains the expected libc-specific package or check bun.lock/bun.lockb
contents), and (3) ensure you explicitly remove both lockfile variants (bun.lock
and bun.lockb) between runs instead of only bun.lock so prior resolutions don't
leak into subsequent installs.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (25)
src/install/PackageInstaller.zigsrc/install/PackageManager/CommandLineArguments.zigsrc/install/PackageManager/PackageManagerLifecycle.zigsrc/install/PackageManager/PackageManagerOptions.zigsrc/install/isolated_install/Installer.zigsrc/install/lockfile.zigsrc/install/lockfile/Package.zigsrc/install/lockfile/Package/Meta.zigsrc/install/lockfile/Tree.zigsrc/install/lockfile/bun.lock.zigsrc/install/lockfile/lockfile_json_stringify_for_debugging.zigsrc/install/lockfile/printer/tree_printer.zigsrc/install/npm.zigsrc/install/pnpm.zigsrc/install/postinstall_optimizer.zigsrc/resolver/package_json.zigtest/cli/install/bun-install-cpu-os.test.tstest/cli/install/dep-arm64-linux-glibc-3.0.0.tgztest/cli/install/dep-glibc-1.0.0.tgztest/cli/install/dep-glibc-only-1.0.0.tgztest/cli/install/dep-musl-2.0.0.tgztest/cli/install/dep-musl-only-2.0.0.tgztest/cli/install/dep-x64-linux-glibc-1.0.0.tgztest/cli/install/dep-x64-linux-musl-2.0.0.tgztest/regression/issue/26156.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Use the#prefix for private fields in Zig structs, e.g.,struct {#foo: u32 };
Use Decl literals in Zig, e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };
Place@importstatements at the bottom of the file in Zig (auto formatter will handle positioning)
Never use@import()inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Files:
src/install/PackageManager/PackageManagerOptions.zigsrc/install/pnpm.zigsrc/install/PackageManager/PackageManagerLifecycle.zigsrc/install/lockfile/Tree.zigsrc/install/isolated_install/Installer.zigsrc/install/lockfile/lockfile_json_stringify_for_debugging.zigsrc/install/lockfile/bun.lock.zigsrc/install/lockfile/printer/tree_printer.zigsrc/install/npm.zigsrc/install/lockfile.zigsrc/install/lockfile/Package.zigsrc/install/PackageManager/CommandLineArguments.zigsrc/install/lockfile/Package/Meta.zigsrc/install/postinstall_optimizer.zigsrc/install/PackageInstaller.zigsrc/resolver/package_json.zig
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.ts: Use Bun's Jest-compatible test runner with proper test fixtures and imports fromharness
Always useport: 0when binding to ports in tests - never hardcode ports or use custom random port functions
UsenormalizeBunSnapshotto normalize snapshot output in tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output as these will never fail in CI
UsetempDirfromharnessto create temporary directories in tests - do not usetmpdirSyncorfs.mkdtempSync
In spawned process tests, use expect(stdout).toBe(...) BEFORE expect(exitCode).toBe(0) for more useful error messages
Do not usesetTimeoutin tests - await the condition to be met instead, as you are testing the CONDITION not TIME PASSING
Files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
🧠 Learnings (30)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/install/PackageManager/PackageManagerOptions.zigsrc/install/pnpm.zigsrc/install/PackageManager/PackageManagerLifecycle.zigsrc/install/lockfile/Tree.zigsrc/install/isolated_install/Installer.zigsrc/install/lockfile/lockfile_json_stringify_for_debugging.zigsrc/install/lockfile/bun.lock.zigsrc/install/lockfile/printer/tree_printer.zigsrc/install/npm.zigsrc/install/lockfile.zigsrc/install/lockfile/Package.zigsrc/install/PackageManager/CommandLineArguments.zigsrc/install/lockfile/Package/Meta.zigsrc/install/postinstall_optimizer.zigsrc/install/PackageInstaller.zigsrc/resolver/package_json.zig
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/install/isolated_install/Installer.zig
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Applied to files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
📚 Learning: 2026-01-14T21:08:10.438Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/js/node/test/parallel/CLAUDE.md:0-0
Timestamp: 2026-01-14T21:08:10.438Z
Learning: Run Node.js compatibility tests using `bun bd <file-path>` command, not `bun bd test <file-path>`
Applied to files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
📚 Learning: 2026-01-14T21:08:10.438Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/js/node/test/parallel/CLAUDE.md:0-0
Timestamp: 2026-01-14T21:08:10.438Z
Learning: These are Node.js compatibility tests not written by Bun and cannot be modified
Applied to files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
test/cli/install/bun-install-cpu-os.test.tssrc/install/lockfile/bun.lock.zigtest/regression/issue/26156.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
test/cli/install/bun-install-cpu-os.test.tssrc/install/lockfile/bun.lock.zigsrc/resolver/package_json.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Applied to files:
test/cli/install/bun-install-cpu-os.test.tssrc/install/lockfile/bun.lock.zigsrc/install/npm.zigsrc/install/PackageManager/CommandLineArguments.zigsrc/install/postinstall_optimizer.zigsrc/install/PackageInstaller.zig
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.
Applied to files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/cli/install/bun-install-cpu-os.test.tstest/regression/issue/26156.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
src/install/lockfile/bun.lock.zigsrc/install/npm.zigsrc/install/postinstall_optimizer.zigsrc/resolver/package_json.zig
📚 Learning: 2025-11-20T19:51:32.288Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Applied to files:
src/install/lockfile/bun.lock.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/install/lockfile/bun.lock.zig
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Run `bun run zig:check-all` to compile Zig code on all platforms when making platform-specific changes
Applied to files:
src/install/npm.zigsrc/resolver/package_json.zig
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/install/lockfile.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use string literal `require()` statements only; dynamic requires are not permitted
Applied to files:
src/install/postinstall_optimizer.zig
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Applies to test/**/*.test.ts : Use `normalizeBunSnapshot` to normalize snapshot output in tests
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Applies to test/**/*.test.ts : Use Bun's Jest-compatible test runner with proper test fixtures and imports from `harness`
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - a test is invalid if it passes with `USE_SYSTEM_BUN=1`
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced.
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2025-08-30T09:09:18.384Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22231
File: src/bundler/bundle_v2.zig:48-48
Timestamp: 2025-08-30T09:09:18.384Z
Learning: In Zig, when a module exports a top-level struct, import("./Module.zig") directly returns that struct type and can be used as a type alias without needing to access a field within the module. This is a common pattern in the Bun codebase.
Applied to files:
src/resolver/package_json.zig
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/resolver/package_json.zig
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/resolver/package_json.zig
📚 Learning: 2025-10-26T04:50:17.892Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24086
File: src/bun.js/api/server.zig:2534-2535
Timestamp: 2025-10-26T04:50:17.892Z
Learning: In src/bun.js/api/server.zig, u32 is acceptable for route indices and websocket_context_index fields, as the practical limit of 4 billion contexts will never be reached.
Applied to files:
src/resolver/package_json.zig
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Zig code should be organized in `src/*.zig` for core runtime, JavaScript bindings, and package manager
Applied to files:
src/resolver/package_json.zig
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/resolver/package_json.zig
🧬 Code graph analysis (1)
test/regression/issue/26156.test.ts (3)
src/bun.js/bindings/ConcatCStrings.h (1)
it(29-35)test/harness.ts (1)
tempDir(277-284)test/js/node/test_runner/fixtures/02-hooks.js (1)
JSON(7-7)
🔇 Additional comments (36)
src/install/lockfile/printer/tree_printer.zig (1)
139-146: LGTM: libc forwarded into disabled filtering.This keeps the print/disable logic consistent with libc-aware resolution.
src/install/PackageManager/PackageManagerOptions.zig (2)
86-91: LGTM: libc option added with sensible default.
613-617: LGTM: CLI libc is propagated into options.src/install/pnpm.zig (1)
625-627: LGTM: pnpm libc field mapped into package metadata.src/install/lockfile/lockfile_json_stringify_for_debugging.zig (1)
316-326: LGTM: libc included in lockfile debug JSON output.src/install/isolated_install/Installer.zig (2)
930-944: LGTM: libc propagated into lifecycle-script gating.Keeps script skipping consistent with cpu/os filtering.
1323-1332: LGTM: native binlink replacement now libc-aware.Good alignment with the new selection criteria.
test/regression/issue/26156.test.ts (2)
16-42: LGTM: help output includes--libc.Simple and effective flag visibility check.
102-123: LGTM: invalid libc values are rejected.Covers the expected error path cleanly.
src/install/lockfile/Tree.zig (1)
366-377: LGTM: libc included in disablement check and logging.Nice parity with existing cpu/os mismatch reporting.
src/install/lockfile/bun.lock.zig (2)
761-769: LGTM: libc serialized when non-default.Comma handling and Negatable serialization look correct.
1823-1832: LGTM: libc parsed from lockfile metadata.Keeps text lockfile parsing consistent with new field support.
src/install/lockfile.zig (2)
407-416: LGTM: libc added to resolved-dependency disablement logic.Matches the new meta matching behavior.
1026-1035: LGTM: migration path now carries libc metadata.Good parity with os/cpu updates.
src/install/PackageInstaller.zig (3)
292-298: LGTM! Libc propagation to native binlink optimization looks correct.The
target_libcis correctly read frommanager.optionsand passed togetNativeBinlinkReplacementPackageIDalongsidetarget_cpuandtarget_os, following the established pattern for platform-specific package selection.
1167-1177: LGTM! Libc parameter correctly added to lifecycle script filtering.The
shouldIgnoreLifecycleScriptscall now includesthis.manager.options.libc, consistent with the existingcpuandosparameters. This ensures lifecycle scripts are properly skipped for packages that don't match the target libc.
1362-1373: LGTM! Second call site for lifecycle script filtering updated consistently.Both call sites for
shouldIgnoreLifecycleScriptsnow include the libc parameter, ensuring consistent behavior across the trusted dependency path.test/cli/install/bun-install-cpu-os.test.ts (4)
608-666: LGTM! Comprehensive libc filtering test.The test correctly validates:
- glibc filtering installs only glibc-compatible packages
- musl filtering installs only musl-compatible packages
- Proper cleanup between test runs including both
bun.lockbandbun.lock
668-716: LGTM! Combined cpu/os/libc filter test validates the complete matrix.This test ensures that all three platform constraints (cpu: x64, os: linux, libc: glibc) work together correctly, with only the exact matching package being installed.
718-742: LGTM! Error handling test for invalid libc.The test correctly validates that an invalid libc value results in exit code 1 and appropriate error messaging, consistent with the existing invalid CPU/OS tests.
744-783: LGTM! Wildcard libc test ensures backward compatibility.The
--libc "*"wildcard correctly installs all packages regardless of libc constraint, matching the behavior of--cpu "*"and--os "*".src/install/PackageManager/PackageManagerLifecycle.zig (1)
84-89: LGTM! Preinstall state determination now considers libc.The
isDisabledcheck correctly includesmanager.options.libc, ensuring packages incompatible with the target libc are marked as done and not downloaded. This aligns with the existing cpu/os handling pattern.src/install/lockfile/Package.zig (3)
60-62: LGTM! Public API updated to include libc parameter.The
isDisabledmethod signature correctly forwards the libc parameter tothis.meta.isDisabled, maintaining consistency with the cpu and os parameters.
283-286: LGTM! Package metadata correctly populated from package.json.The
libcfield is assigned alongsidearchandos, following the established pattern for platform-specific metadata.
493-496: LGTM! Package metadata correctly populated from npm manifest.The
libcfield is assigned frompackage_version.libcalongsidecpu,os, andintegrity, maintaining consistency with how other platform metadata is handled for npm packages.src/resolver/package_json.zig (3)
65-67: LGTM — libc default aligns with arch/os defaults.
967-979: LGTM — libc parsing mirrors cpu/os negatable handling.
2167-2167: LGTM — Libc import is required for the new field.src/install/lockfile/Package/Meta.zig (3)
10-15: Verify Meta extern layout stays lockfile‑compatible.
Metaisexternand serialized; the newlibcfield must fit into reclaimed padding. Please confirm@sizeOf(Npm.Libc)matches the padding size (u8) or adjust padding/bump the lockfile version to avoid compatibility breaks.🛡️ Optional compile-time guard
pub const Meta = extern struct { // ... libc: Npm.Libc = .all, _padding_libc: u8 = 0, // ... }; + +comptime { + if (`@sizeOf`(Npm.Libc) != `@sizeOf`(u8)) { + `@compileError`("Meta layout assumes 1-byte Npm.Libc; adjust padding or bump lockfile version."); + } +}
35-39: LGTM — libc is now part of the disable predicate.
62-69: LGTM — clone now copies libc.src/install/postinstall_optimizer.zig (2)
48-67: LGTM — binlink replacement selection is now libc‑aware.
101-129: LGTM — lifecycle ignore decision now threads libc.src/install/PackageManager/CommandLineArguments.zig (3)
54-56: LGTM —--libcflag is exposed in shared params.
253-256: LGTM — cli.libc default matches cpu/os pattern.
1022-1044: LGTM —--libcparsing mirrors cpu/os wildcard/negation handling.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| it("should correctly filter glibc vs musl optional dependencies", async () => { | ||
| // This test creates a mock scenario similar to the reported issue | ||
| // where packages like @rollup/rollup-linux-x64-gnu (glibc) and | ||
| // @rollup/rollup-linux-x64-musl (musl) should be filtered based on libc | ||
|
|
||
| using dir = tempDir("issue-26156-filtering", { | ||
| "package.json": JSON.stringify({ | ||
| name: "test-libc-filtering-real", | ||
| version: "1.0.0", | ||
| // In a real scenario, these would be packages with libc field in their package.json | ||
| // The filtering happens based on the registry metadata | ||
| }), | ||
| }); | ||
|
|
||
| // Test that explicit --libc glibc flag is accepted | ||
| await using proc1 = Bun.spawn({ | ||
| cmd: [bunExe(), "install", "--libc", "glibc"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const exitCode1 = await proc1.exited; | ||
| expect(exitCode1).toBe(0); | ||
|
|
||
| // Test that explicit --libc musl flag is accepted | ||
| await rm(join(String(dir), "node_modules"), { recursive: true, force: true }); | ||
| await rm(join(String(dir), "bun.lock"), { force: true }); | ||
|
|
||
| await using proc2 = Bun.spawn({ | ||
| cmd: [bunExe(), "install", "--libc", "musl"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const exitCode2 = await proc2.exited; | ||
| expect(exitCode2).toBe(0); | ||
|
|
||
| // Test that --libc * (wildcard) is accepted | ||
| await rm(join(String(dir), "node_modules"), { recursive: true, force: true }); | ||
| await rm(join(String(dir), "bun.lock"), { force: true }); | ||
|
|
||
| await using proc3 = Bun.spawn({ | ||
| cmd: [bunExe(), "install", "--libc", "*"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const exitCode3 = await proc3.exited; | ||
| expect(exitCode3).toBe(0); | ||
| }); |
There was a problem hiding this comment.
Test doesn’t validate libc filtering and leaves bun.lockb behind.
This block only asserts exit codes, and the optional deps aren’t defined, so it won’t catch a regression in libc-based selection. Additionally, deleting only bun.lock can leave bun.lockb in place, causing later installs to reuse the previous resolution and masking libc changes. Please add concrete libc-constrained packages and assert which one is installed, and clear both lockfile variants between runs.
🔧 Suggested cleanup fix for lockfiles
- await rm(join(String(dir), "bun.lock"), { force: true });
+ await rm(join(String(dir), "bun.lock"), { force: true });
+ await rm(join(String(dir), "bun.lockb"), { force: true });
...
- await rm(join(String(dir), "bun.lock"), { force: true });
+ await rm(join(String(dir), "bun.lock"), { force: true });
+ await rm(join(String(dir), "bun.lockb"), { force: true });🤖 Prompt for AI Agents
In `@test/regression/issue/26156.test.ts` around lines 45 - 100, The test "should
correctly filter glibc vs musl optional dependencies" only checks exit codes and
leaves bun.lockb around, so update the test to (1) add concrete mock optional
deps with libc-specific package names in the tempDir package.json (e.g. include
both "@rollup/rollup-linux-x64-gnu" and "@rollup/rollup-linux-x64-musl" or
equivalent registry-mocked packages) so the installer must choose between them,
(2) after each Bun.spawn install (proc1/proc2/proc3) assert which package got
installed (verify node_modules contains the expected libc-specific package or
check bun.lock/bun.lockb contents), and (3) ensure you explicitly remove both
lockfile variants (bun.lock and bun.lockb) between runs instead of only bun.lock
so prior resolutions don't leak into subsequent installs.
Address review comments on PR #26159: 1. Changed `Libc.current` from compile-time const to runtime function `Libc.current()` that detects musl vs glibc at runtime. This handles cases where a glibc-compiled Bun binary runs on a musl system (e.g., via gcompat on Alpine Linux). 2. Runtime detection checks for musl's dynamic loader paths: - /lib/ld-musl-x86_64.so.1 (x64) - /lib/ld-musl-aarch64.so.1 (arm64) The result is cached after first detection. 3. Updated all usages of `options.libc` to use `options.getLibc()` which resolves the `.none` default to the runtime-detected value. 4. Simplified regression test to focus on CLI interface testing. The detailed libc filtering is already tested in bun-install-cpu-os.test.ts using a mock registry. 5. Added cleanup of both bun.lock and bun.lockb between test runs. Fixes #26156 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/install/npm.zig`:
- Around line 729-774: The global cached_current can race if Libc.current() is
called from multiple threads; replace the plain mutable global with a
thread-safe one-time initialization (e.g., use std.sync.Once or an equivalent
once-init helper) so detectLibcRuntime() is executed exactly once and cached
safely, or make cached_current an atomic and perform an atomic compare-exchange
when setting it; update the use in Libc.current() to call the once-init (or
atomic setter) instead of the current cached_current orelse pattern and ensure
detectLibcRuntime() is the initializer.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
src/install/PackageInstaller.zigsrc/install/PackageManager/CommandLineArguments.zigsrc/install/PackageManager/PackageManagerLifecycle.zigsrc/install/PackageManager/PackageManagerOptions.zigsrc/install/isolated_install/Installer.zigsrc/install/lockfile/Tree.zigsrc/install/lockfile/printer/tree_printer.zigsrc/install/npm.zigtest/regression/issue/26156.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Use the#prefix for private fields in Zig structs, e.g.,struct {#foo: u32 };
Use Decl literals in Zig, e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };
Place@importstatements at the bottom of the file in Zig (auto formatter will handle positioning)
Never use@import()inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Files:
src/install/PackageManager/PackageManagerOptions.zigsrc/install/lockfile/Tree.zigsrc/install/PackageManager/CommandLineArguments.zigsrc/install/lockfile/printer/tree_printer.zigsrc/install/npm.zigsrc/install/isolated_install/Installer.zigsrc/install/PackageManager/PackageManagerLifecycle.zigsrc/install/PackageInstaller.zig
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/regression/issue/26156.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.ts: Use Bun's Jest-compatible test runner with proper test fixtures and imports fromharness
Always useport: 0when binding to ports in tests - never hardcode ports or use custom random port functions
UsenormalizeBunSnapshotto normalize snapshot output in tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output as these will never fail in CI
UsetempDirfromharnessto create temporary directories in tests - do not usetmpdirSyncorfs.mkdtempSync
In spawned process tests, use expect(stdout).toBe(...) BEFORE expect(exitCode).toBe(0) for more useful error messages
Do not usesetTimeoutin tests - await the condition to be met instead, as you are testing the CONDITION not TIME PASSING
Files:
test/regression/issue/26156.test.ts
🧠 Learnings (29)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/install/PackageManager/PackageManagerOptions.zigsrc/install/npm.zig
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/install/PackageManager/PackageManagerOptions.zigsrc/install/lockfile/Tree.zigsrc/install/PackageManager/CommandLineArguments.zigsrc/install/lockfile/printer/tree_printer.zigsrc/install/npm.zigsrc/install/isolated_install/Installer.zigsrc/install/PackageManager/PackageManagerLifecycle.zigsrc/install/PackageInstaller.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Applied to files:
src/install/PackageManager/CommandLineArguments.zigsrc/install/npm.zigsrc/install/PackageInstaller.zig
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
src/install/npm.zig
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Zig code should be organized in `src/*.zig` for core runtime, JavaScript bindings, and package manager
Applied to files:
src/install/npm.zig
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Run `bun run zig:check-all` to compile Zig code on all platforms when making platform-specific changes
Applied to files:
src/install/npm.zig
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/install/npm.zig
📚 Learning: 2025-10-16T21:24:52.779Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/crash_handler.zig:1415-1423
Timestamp: 2025-10-16T21:24:52.779Z
Learning: When a boolean EnvVar in src/envvars.zig is defined with a default value (e.g., `.default = false`), the `get()` method returns `bool` instead of `?bool`. This means you cannot distinguish between "environment variable not set" and "environment variable explicitly set to the default value". For opt-out scenarios where detection of explicit setting is needed (like `BUN_ENABLE_CRASH_REPORTING` on platforms where crash reporting defaults to enabled), either: (1) don't provide a default value so `get()` returns `?bool`, or (2) use the returned boolean directly instead of only checking if it's true.
Applied to files:
src/install/npm.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/install/npm.zig
📚 Learning: 2025-12-11T02:11:47.024Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25462
File: src/ast/visitExpr.zig:1644-1695
Timestamp: 2025-12-11T02:11:47.024Z
Learning: In Bun's bundler feature flag implementation (src/ast/visitExpr.zig), the validation for feature() flag names intentionally only rejects UTF-16 strings (checking `is_utf16`) while allowing UTF-8 strings, even though the error message says "must be an ASCII string". This is the intended behavior and should not be changed to enforce strict ASCII validation.
Applied to files:
src/install/npm.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/install/npm.zig
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/install/npm.zig
📚 Learning: 2025-10-17T01:21:35.060Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/bindings/BunIDLConvert.h:79-95
Timestamp: 2025-10-17T01:21:35.060Z
Learning: In JavaScriptCore (used by Bun), the `toBoolean()` method implements ECMAScript's ToBoolean abstract operation, which is side-effect-free and cannot throw exceptions. It does not execute arbitrary JavaScript or invoke user-defined methods like Symbol.toPrimitive. Exception handling is not needed after toBoolean() calls.
Applied to files:
src/install/npm.zig
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-14T21:08:10.438Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/js/node/test/parallel/CLAUDE.md:0-0
Timestamp: 2026-01-14T21:08:10.438Z
Learning: Run Node.js compatibility tests using `bun bd <file-path>` command, not `bun bd test <file-path>`
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Applies to test/**/*.test.ts : Use `normalizeBunSnapshot` to normalize snapshot output in tests
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Applies to test/**/*.test.ts : Use Bun's Jest-compatible test runner with proper test fixtures and imports from `harness`
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2025-11-08T04:06:33.198Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-14T21:08:10.438Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/js/node/test/parallel/CLAUDE.md:0-0
Timestamp: 2026-01-14T21:08:10.438Z
Learning: These are Node.js compatibility tests not written by Bun and cannot be modified
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2026-01-15T03:22:50.711Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T03:22:50.711Z
Learning: Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - a test is invalid if it passes with `USE_SYSTEM_BUN=1`
Applied to files:
test/regression/issue/26156.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/regression/issue/26156.test.ts
🧬 Code graph analysis (1)
test/regression/issue/26156.test.ts (2)
test/js/node/module/module-resolve-filename-paths.test.js (1)
dirPath(42-42)test/harness.ts (1)
tempDir(277-284)
🔇 Additional comments (19)
src/install/PackageInstaller.zig (3)
290-299: LGTM — libc-aware native binlink selection is wired correctly.Passing the resolved libc into the replacement lookup matches the new platform filtering expectations.
1167-1179: LGTM — lifecycle script gating now includes libc.This keeps lifecycle decisions consistent with libc-specific optional dependency filtering.
1362-1374: LGTM — libc propagation for trusted dependency scripts.Good to see libc included in the optimizer’s ignore logic here as well.
src/install/npm.zig (2)
717-723: LGTM — libc matching semantics align with backward compatibility.Treating
.none/.allas compatible and handling target.allmatches expected optional-dependency behavior.
784-789: LGTM — JS binding now uses runtime-detected libc.Switching to
current()keeps the JS helper consistent with the new runtime detection.test/regression/issue/26156.test.ts (4)
23-27: LGTM — lockfile cleanup is thorough.Removing both
bun.lockandbun.lockbavoids stale resolutions between runs.
29-52: LGTM — CLI help coverage for--libc.This ensures the flag is surfaced in user-facing help output.
54-103: LGTM — valid libc value parsing is covered.The three accepted values are exercised with clean state between installs.
105-126: LGTM — invalid libc value handling is asserted.Good coverage of exit code and error message content.
src/install/PackageManager/PackageManagerOptions.zig (3)
86-92: LGTM — libc option added with clear default.The default
.noneplus inline comment is clear and consistent with other platform overrides.
129-132: LGTM — helper resolves effective libc cleanly.
getLibc()makes downstream call sites simpler and avoids duplicate logic.
618-623: LGTM — CLI libc resolution wired correctly.Resolving
.nonetoNpm.Libc.current()here keeps options consistent for the rest of the install flow.src/install/isolated_install/Installer.zig (1)
1322-1332: Libc-aware native binlink replacement looks correct.Plumbing
target_libcinto replacement selection keeps native binlink choices aligned with libc filtering.src/install/PackageManager/PackageManagerLifecycle.zig (1)
84-88: Libc included in preinstall disable checks is correct.This keeps preinstall state in sync with libc-aware dependency filtering.
src/install/PackageManager/CommandLineArguments.zig (3)
54-56:--libcflag addition is consistent with cpu/os overrides.Help text is clear and matches the existing override pattern.
253-256: CLI args now store libc override for downstream options.This cleanly mirrors
cpu/osdefaults and resolution flow.
1022-1044: Libc parsing mirrors cpu/os negatable handling.Wildcard support and validation behavior look consistent.
src/install/lockfile/printer/tree_printer.zig (1)
139-146: Libc-aware disabled check keeps output consistent.This aligns the tree printer with libc filtering decisions elsewhere.
src/install/lockfile/Tree.zig (1)
366-378: Libc-aware filtering and mismatch logging look correct.Using
target_libchere keeps filter decisions and verbose diagnostics aligned.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Address thread safety review comment: The cached_current variable could race if Libc.current() is called from multiple threads. Changed to use std.once for thread-safe one-time initialization: - cached_current_once ensures detectLibcRuntimeOnce() runs exactly once - cached_current stores the result and is written during init Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Addressed the thread safety review comment in commit a9ad1d5:
This follows the same pattern used elsewhere in the codebase (e.g., |
|
Closing this PR because it has been inactive for more than 90 days. |
Summary
libcfield in package.json for filtering optional dependencies--libcCLI flag for manually overriding the target libc type.allon other platformsTest plan
bun bd test test/cli/install/bun-install-cpu-os.test.ts- all 17 tests pass (including 4 new libc tests)bun bd test test/regression/issue/26156.test.ts- all 3 tests pass--libc glibc,--libc musl, and--libc *flags work correctlyDetails
This PR adds libc filtering support to the package manager, allowing packages like
@rollup/rollup-linux-x64-gnu(glibc) and@rollup/rollup-linux-x64-musl(musl) to be correctly filtered based on the target system's libc type.Key changes:
npm.zig: Updated
Libc.currentto detect glibc vs musl at compile time on Linux. On non-Linux platforms (macOS, Windows), returns.allsince libc is not relevant.Meta.zig: Added
libcfield to the package metadata struct, replacing the padding byte with the actual field. UpdatedisDisabled()to check libc in addition to cpu and os.isMatch() fix: The
isMatchfunction now treats.none(unset) the same as.all, ensuring backwards compatibility with existing lockfiles where the padding bytes would be 0.CLI flag: Added
--libcflag to override the target libc, similar to existing--cpuand--osflags.Lockfile support: Added libc serialization/deserialization for both bun.lock and pnpm lockfile formats.
Fixes #26156
🤖 Generated with Claude Code