install: read file: tarballs relative to the file: folder package that declares them - #39017
install: read file: tarballs relative to the file: folder package that declares them#39017robobun wants to merge 5 commits into
Conversation
…t declares them A local tarball dependency declared in the package.json of a file: folder dependency was read relative to the project directory instead of the folder, so the install failed with ENOENT, or installed a same-named tarball from the project directory when one existed. Only workspace declarers had their directory joined in. enqueue_local_tarball now asks local_tarball_base_dir for the base: the directory of the declaring workspace or file: folder package when that package's own specifier is the tarball path, and the top-level dir otherwise. The second case covers paths substituted from the root package.json (overrides, resolutions, catalogs), which were previously read relative to the workspace when applied to a workspace dependency. Both the resolve pass and the install from bun.lock go through the same function, so they read the same file.
|
Warning Review limit reached
Next review available in: 10 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 (5)
Comment |
|
Status: fix pushed, waiting on CI and review. Reproduced on the released build and on a debug build of main with a project depending on |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Checked the two issues suggested above: #25835 (root The two flagged doc comments are gone: the condition they explained is now the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes user-visible path-resolution semantics in bun install (including an acknowledged break for folder packages that worked around the old behavior with project-relative paths), a human look would still be worthwhile.
What was reviewed:
- Traced both call sites of
enqueue_local_tarball(resolve pass andenqueue_tarball_for_reading) — thedeclared_by_parentguard correctly falls through to the top-level dir for overrides/catalogs on both paths, since the stored edge keeps the original specifier. - Checked root-declared tarballs still take the
normalize=truepath (ResolutionTag::Rootis not matched, same as before). get_parent_pkg_of_dependencyis the same O(packages) shape as theget_workspace_pkg_if_workspace_depit generalizes; no new perf concern.- Tests plant decoy tarballs at the wrong candidate path and cover both linkers plus
--frozen-lockfileinto an empty cache.
Extended reasoning...
Overview
The PR reworks how enqueue_local_tarball picks the base directory for a file:-tarball path. The old code only recognized workspace declarers and always joined onto the workspace directory; everything else (including file: folder packages) used the project root. The new local_tarball_base_dir helper (a) extends the declarer lookup to ResolutionTag::Folder, and (b) adds a declared_by_parent guard so that when the stored edge's specifier is not the tarball path being read (i.e. an override/resolution/catalog supplied it), the top-level dir is used instead. A new Lockfile::get_parent_pkg_of_dependency generalizes the existing per-package dependency-range scan. Five tests are added across bun-install.test.ts and bun-workspaces.test.ts; the LocalTarballRequest.tarball_path doc comment is shortened.
Security risks
None identified. The change only affects which on-disk file inside the user's own project tree is read for a locally-declared tarball; no new untrusted-input parsing, no network, no privilege boundary.
Level of scrutiny
Medium-high. This is core bun install path resolution and the PR itself calls out an intentional behavior change: projects that worked around the old bug by writing project-relative paths inside a folder dependency's package.json will now need folder-relative paths. That is the correct npm-compatible behavior, but it's a user-visible break that a maintainer should sign off on. The interaction with #38986 (which is separately changing what happens for registry/git/tarball declarers and adds the same get_parent_pkg_of_dependency helper) also deserves a human eye for coordination.
Other factors
- The comment-cop bot fired several times on earlier revisions; the follow-up commits (ef99947, 223e0a4, 5ce5696) shortened the flagged comments and the current diff looks clean on that front.
- Test coverage is strong: each new test plants a different tarball at the wrong candidate path so the failure mode is a wrong install rather than an ENOENT, and each runs both a fresh install and
--frozen-lockfileinto an emptied cache so both code paths intoenqueue_local_tarballare exercised. - I verified the
declared_by_parentguard works on theenqueue_tarball_for_readingpath too: the stored edge for an overridden/catalog dep keeps its original tag (Npm/Catalog), so the tag check short-circuits before the string compare.
|
Updated 11:12 AM PT - Aug 15th, 2026
❌ @robobun, your commit 5ce5696 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 39017That installs a local version of the PR into your bun-39017 --bun |
|
Folded into #38867. |
2 participants
Problem
file:folder dependency whose own package.json declares a local tarball fails to install (released 1.4.0 canary and main, both linkers): project package.json{"dependencies":{"lib":"file:./vendor/lib"}},vendor/lib/package.jsondeclaring"tool": "file:./tool.tgz",vendor/lib/tool.tgzpresent:<project>/tool.tgz. If that file happens to exist it is installed astoolinstead of the folder's copy, with no error. The directory form of the same declaration ("tool": "file:./tool") is already resolved relative tovendor/lib.enqueue_local_tarball(src/install/PackageManager/PackageManagerEnqueue.rs) picks the directory a local tarball path is relative to, and the only declarer it looked at was a workspace (get_workspace_pkg_if_workspace_dep). Every other declarer, including afile:folder package, fell through to the top-level dir.overrides/resolutionsentry or catalog entry pointing a dependency atfile:./x.tgzwas read relative to the workspace when the dependency it applied to was declared by a workspace member, sooverrides: { bar: "file:./bar.tgz" }withbar.tgzin the project root failed with the same ENOENT as soon as a workspace depended onbar. This is Feature: Workspaceoverrideswithfile:paths should resolve relative to workspace root's package.json #25835 (overrides) and Catalog entries withfile:relative paths fail when referenced from workspace packages #25752 (catalogs); both reproduce as reported on the released build and install with this change. The directory form of an override is already resolved relative to the project (Folderarm ofget_or_put_resolved_package).Fixes #25835
Fixes #25752
Fix
enqueue_local_tarballnow takes the base directory fromlocal_tarball_base_dir: the directory of the declaring package when it is a workspace or afile:folder package and that package's own specifier is the tarball path being read; the top-level dir in every other case.file:and what bun already does for workspace declarers and for the directory form. Afile:folder package is read from the project like a workspace is, and itsResolution::Folderpayload is its directory relative to the top-level dir (folder_resolver.rs,NewResolver { folder_path: rel }), the same shape as a workspace'sResolution::Workspacepayload, so both are joined the same way. The only otherResolution::Folderpackages are the stubs created forfile:directories declared by something other than the root or a workspace (Folderarm ofget_or_put_resolved_package); those carry no dependency list, so they are never the declarer of an edge.overrides,resolutionsand catalogs are only parsed from the root package.json (Package.rs,FEATURES.is_main), and applying one leaves the declaring package's stored edge untouched (the replacement is local toenqueue_dependency_with_main_and_success_fn). So when the stored edge's specifier is not the path being read, the root wrote the path and the top-level dir is the only directory it can mean. Without this condition, root overrides applied to a folder-declared dependency, which work today only because of the bug, would start being read from the folder.version_was_replaced:enqueue_local_tarballis also reached fromenqueue_tarball_for_readingwhen a project with abun.lockis installed into an empty cache. The lockfile row keeps the path as declared ("tool": ["bar@./tool.tgz", ...]under"lib": [..., { "dependencies": { "tool": "file:./tool.tgz" } }]), and the edge's declarer and specifier are available there too, so both passes compute the same directory. Lockfile format is unchanged and existing lockfiles keep working; the path in the row is joined onto a different directory only for declarers that previously failed or installed the wrong file.Lockfile::get_parent_pkg_of_dependencyis the same helper that PR adds.bar@./tool.tgz), so two project packages declaring the same relative path to two different files still share one row and one read, the limitation workspaces already have. A package.json inside a folder dependency that worked around this bug by writing a project-relative path (file:./vendor/lib/tool.tgz) will now need the path relative to itself, which is what npm requires for it as well.test/cli/install/bun-install.test.ts,describe("file: tarball declared by a file: folder dependency"): the folder's tarball is installed with the hoisted and with the isolated linker, and a root override supplying the path is read from the project. Each test plants a different tarball at the other candidate path and runs a fresh install followed by--frozen-lockfileinto an emptied cache, so both the resolve pass and the install frombun.lockhave to read the right file.test/cli/install/bun-workspaces.test.ts,relative tarballs > from a root override / catalog entry applied to a workspace dependency, covers Feature: Workspaceoverrideswithfile:paths should resolve relative to workspace root's package.json #25835 and Catalog entries withfile:relative paths fail when referenced from workspace packages #25752 the same way. The four folder/workspace tests install the wrong tarball without thesrc/change (checked against a debug build of main and against the released build); the override-on-folder test passes before and after and pins that case.bun-workspaces.test.ts(74 pass),overrides.test.ts+nested-overrides.test.ts(151 pass),bun-lock.test.ts(40 pass),bun-install.test.ts -t "tarball|tgz|file:|folder|override|resolutions"(51 pass;should treat non-GitHub http(s) URLs as tarballsfails identically on the released build, it needs network),isolated-install.test.ts,bun-add.test.tsandbun-install-registry.test.tsfiltered to tarball/file tests (all pass).cargo clippy -p bun_installis clean.Background
file:dependency on a directory is aTag::Folderdependency; one on a.tgzis aTag::Tarballdependency with a local URI. The latter resolves toResolution::LocalTarball(<path as written>); the path string is both the task id for reading it and the package's identity inbun.lock.file:folder dependencies from disk (Package::parse), and stores for each of the latter two aResolution::Workspace/Resolution::Folderwhose payload is the package directory relative to the top-level dir. Packages from the registry, git or a tarball get their dependency lists from the manifest or the extracted archive instead and have no directory in the project while they resolve.enqueue_local_tarballis called from two places: theTarballarm of the resolve pass (no lockfile row yet, the tarball is read to learn the package's name and dependencies) andenqueue_tarball_for_readingduring install (a row exists inbun.lock, but the extracted package is missing from the cache). It computes the on-disk path on the main thread and hands it to a thread pool task, which only reads the file.