Skip to content

install: resolve overrides that point at a tarball with a mismatched internal name#33194

Open
robobun wants to merge 5 commits into
mainfrom
farm/a96bfdcb/override-tarball-name-mismatch
Open

install: resolve overrides that point at a tarball with a mismatched internal name#33194
robobun wants to merge 5 commits into
mainfrom
farm/a96bfdcb/override-tarball-name-mismatch

Conversation

@robobun

@robobun robobun commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

What

Fixes #33192. A dependency or overrides/resolutions entry that points a name at an HTTP(S) or local tarball is now installed under the dependency key even when the tarball's own package.json name field differs, matching npm and pnpm.

Repro

// package.json
{
  "name": "app",
  "devDependencies": { "vite": "^5.0.0" },
  "overrides": { "vite": "https://example.com/vite-core.tgz" } // tarball's internal name is "@scope/vite-core"
}
$ bun install
error: vite@^5.0.0 failed to resolve
node_modules/vite: NOT INSTALLED

In graphs where the only vite edge is a transitive peer (vitest -> @vitest/mocker), bun install instead exits 0 and silently drops vite rather than erroring.

Cause

When a tarball is extracted, the resulting package is indexed in the lockfile's package index under the tarball's own package.json name (@scope/vite-core). Resolution then looks the package up by name hash.

  • The direct-dependency path works because after extraction it back-propagates the extracted name onto the dependency's stored tarball version, so the re-enqueue computes hash("@scope/vite-core") and finds the package.
  • The override path skipped that step: the dependency's stored version keeps its original tag (e.g. an npm range), so the back-propagation branch fell through (_ => {}), and update_name_and_name_hash_from_version_replacement returned the original name hash (hash("vite")). get_package_id(hash("vite"), ...) could not find the package indexed under hash("@scope/vite-core"), so the edge never resolved.

Fix

Two changes, both in the hoisted installer's tarball-extract path:

  1. runTasks.rs: when the completed tarball's waiting dependency edge keeps its original (non-tarball) tag, it was redirected by overrides/resolutions, so record the extracted name on the override entry (keyed by the original name hash) instead of the stored version.
  2. PackageManagerEnqueue.rs: once a git/github/tarball replacement's package name is known, derive the replacement's name hash from that name so resolution finds the package under the tarball's own name. While the name is still unknown (pre-extract), the original name hash is kept so the override stays keyed by its original name.

The directory name stays the overridden key (node_modules/vite), since assign_resolution only overwrites a dependency's name when it is empty or equals the version literal.

Catalog replacements are unchanged: their package name is never back-propagated, so the new helper returns the original hash (its existing behavior).

Scope

overrides that resolve to a git URL (git+..., cloned rather than tarball-extracted) are intentionally left out of this PR. They take a different completion path (Task::Tag::GitCheckout) with a pre-existing wrong-union-arm write for override edges, and git clones cannot be reproduced hermetically in the test harness, so that sibling is a follow-up. GitHub URLs are covered here because they download as tarballs through the same extract path.

Tests

test/cli/install/bun-install.test.ts, serving in-process tarballs over HTTP and from a local path:

  • direct dependency resolved through a remote-tarball override with a mismatched name
  • transitive peer satisfied by a remote-tarball override with a mismatched name (the issue's real-world shape)
  • direct dependency resolved through a local-tarball override with a mismatched name
  • override tarball whose internal name matches the key still installs (guard)

The three mismatched-name tests fail on the current binary (two with vite@^5.0.0 failed to resolve, the transitive-peer one by silently not installing vite) and pass with the fix; the matching-name guard passes both ways.

…internal name

When an overrides/resolutions entry points a name at an HTTP(S) or local
tarball, the extracted package is indexed under the tarball's own
package.json name. The override edge kept the original name hash when
re-enqueuing, so get_package_id could not find the extracted package and
the dependency (or any transitive peer relying on it) failed to resolve:

    error: vite@^5.0.0 failed to resolve

The direct-dependency path already back-propagates the extracted name
onto the stored tarball version; the override path skipped it because the
stored version keeps its original tag. Record the extracted name on the
override entry in that case, and derive the replacement name hash from the
tarball's own name once it is known, so resolution finds the package under
the key the user overrode (matching npm and pnpm).
@robobun

robobun commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 12:13 PM PT - Jul 1st, 2026

@robobun, your commit 26d042b has 3 failures in Build #67611 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 33194

That installs a local version of the PR into your bun-33194 executable, so you can run:

bun-33194 --bun

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 94a1cc8c-e88c-4d0c-af50-32852d18f6ab

📥 Commits

Reviewing files that changed from the base of the PR and between 8627736 and 26d042b.

📒 Files selected for processing (2)
  • src/install/PackageManager/PackageManagerEnqueue.rs
  • test/cli/install/bun-install.test.ts

Walkthrough

This PR updates tarball override name-hash handling, writes extracted package names back to the correct lockfile location for overrides, and adds install tests for mismatched tarball names across remote, local, peer, and regression cases.

Changes

Tarball Override Name-Hash Fix

Layer / File(s) Summary
Conditional name/hash recomputation for override replacements
src/install/PackageManager/PackageManagerEnqueue.rs
Git/Github/Tarball replacement handling now uses a helper that preserves the original hash for empty extracted names and recomputes it from the extracted package name when present.
Lockfile package_name backfill for overrides
src/install/PackageManager/runTasks.rs
Extract/resolve handling now writes package names to either dependency versions or the corresponding override entry based on the stored version tag and original name hash.
Tarball override install cases
test/cli/install/bun-install.test.ts
Adds helpers to build and serve in-memory tarballs, then adds install coverage for remote overrides, resolutions, peer overrides, local tarball overrides, and a matching-name regression case.

Possibly related issues

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: tarball overrides with mismatched internal names.
Description check ✅ Passed It covers the change and verification, though it uses custom headings instead of the exact template.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/install/PackageManager/runTasks.rs`:
- Around line 1222-1235: The fallback arm in the `target`/`version.tag` match
inside `runTasks` is silently swallowing a state that should not happen. Update
the `if let Some(version) = target` handling for
`bun_install::DependencyVersionTag::Git`, `Github`, and `Tarball` to add a
`debug_assert!` or `scoped_log!` in the `_` branch so mismatched or stale
override entries are surfaced instead of ignored.

In `@test/cli/install/bun-install.test.ts`:
- Around line 9862-10035: These install cases are independent and should run
concurrently instead of serially. Update the surrounding `describe` block in
`bun-install.test.ts` to use `describe.concurrent` (or mark each `test` as
`test.concurrent`) so the four install scenarios can execute in parallel; keep
the existing `tempDir`, `serveTarballs`, `Bun.spawn`, and `npmTarball` setup
unchanged since each test already isolates its own state.
- Around line 9871-9891: The local npmTarball helper in bun-install.test.ts
duplicates tarball construction already used by the install tarball tests. Reuse
or extract the shared tarball-building logic from the existing install suite
helpers, and update this test to call that shared helper instead of maintaining
its own header assembly and gzip packaging.
🪄 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: 843c2e27-d4a6-40d0-bd88-67bb793f9d2e

📥 Commits

Reviewing files that changed from the base of the PR and between d816daf and a5e2672.

📒 Files selected for processing (3)
  • src/install/PackageManager/PackageManagerEnqueue.rs
  • src/install/PackageManager/runTasks.rs
  • test/cli/install/bun-install.test.ts

Comment thread src/install/PackageManager/runTasks.rs
Comment thread test/cli/install/bun-install.test.ts Outdated
Comment thread test/cli/install/bun-install.test.ts
Comment thread src/install/PackageManager/PackageManagerEnqueue.rs
Comment thread src/install/PackageManager/runTasks.rs
@robobun

robobun commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

The diff is green; CI is red only on unrelated flaky tests. This change is install-only (src/install/PackageManager/{PackageManagerEnqueue,runTasks}.rs + the install test), the binary-size check shows every target built fine, and the 5 new bun-install.test.ts cases are not in any failure list.

The telling part is that the two runs failed on different tests, which is the signature of pre-existing flakes rather than a deterministic break:

  • Build 67603: test/bake/dev/production.test.ts (Bake SSR, 90s timeout), test/js/bun/util/v8-heap-snapshot.test.ts (SIGKILL), plus flaky test/cli/hot/hot.test.ts and test/cli/install/migration/complex-workspace.test.ts (the latter uses no overrides/tarballs, so it can't reach this change).
  • Build 67611: test/js/node/test/parallel/test-net-connect-memleak.js (asserts GC collected a socket, timing-dependent), test/js/bun/http/serve-body-leak.test.ts (timed out), test/bake/dev/production.test.ts, plus a flaky test/bake/dev-and-prod.test.ts and an earlier buildkite-agent artifact download timed out on darwin aarch64.

All of these are GC/leak/timeout/SSR/infra flakes in subsystems an install-resolution change cannot touch. Ready for a maintainer to merge or re-run CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bun install discards an HTTP(S) tarball dependency/override when the tarball's internal name differs from the dependency key (npm & pnpm honor it)

1 participant