Repository navigation
Conversation
|
Warning Review limit reached
Next review available in: 11 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 ignored due to path filters (1)
📒 Files selected for processing (2)
Comment |
|
Status: reproduced with the fixture in the PR description (released 1.4.0 and a main build): a workspace listing a sibling in two dependency groups makes every |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Review follow-ups:
|
There was a problem hiding this comment.
The linking-off arm now goes through catalogs.resolve_range(...).tag == Npm, so catalog: → workspace:* and dist-tags fall through untagged with linkWorkspacePackages off — that closes the case I raised earlier, and the added test rows cover both shapes. I didn't find further issues; a human look is still worthwhile since this rewrites the workspace-link rule in the bun.lock loader (mirroring Package::parse_dependency) and changes six standing snapshots in dependency on workspace without version in package.json.
Checked: the linking-on Npm arm matches the parser's link_workspace_packages && satisfies gate at src/install/lockfile/Package.rs:1886; resolve_range returns the dep's own version for non-catalog tags so DistTag/Workspace/Catalog-of-workspace correctly fall to false; link_dep_to_workspace preserves the whole-struct-assign so DependencyVersion::Drop still frees the prior npm chain; seen_deps removal has no other reader; the snapshot deltas keep package_id at the workspace and only change the version tag.
Extended reasoning...
Overview
The PR changes how parse_into_binary_lockfile (src/install/lockfile/bun.lock.rs) retags root and workspace dependencies that bun.lock bound to a workspace package. Previously it retagged every such entry to Workspace unconditionally (via map_dep_to_pkg), except that a seen_deps set skipped the retag for the second and later entries of a name — so a workspace listed in two dependency groups loaded with one entry retagged and one not, permanently diffing against the fresh parse. The fix replaces seen_deps and the unconditional retag with map_manifest_dep_to_pkg + WorkspaceLinks::links(), which retags only when Package::parse_dependency would: workspace: always; Npm ranges only when the recorded workspace version satisfies them and linkWorkspacePackages is on; with linking off, only non-peer entries whose (catalog-resolved) tag is Npm keep loading as linked so the diff re-resolves them once against the registry. Installed-package deps still go through the old map_dep_to_pkg. Six snapshots in dependency on workspace without version in package.json change (wildcard/dist-tag on a versionless workspace now keep their parsed tag; package_id is unchanged). Twenty new tests are added.
Prior review
My earlier run flagged that the linking-off fallback arm was a _ catch-all that also matched Catalog and DistTag, so a catalog: entry holding workspace:* would still permanently diff with linking off. c5a8052 replaced that with !is_peer() && catalogs.resolve_range(...).tag == Npm; I traced resolve_range (CatalogMap.rs:152) — for non-Catalog deps it returns &dep.version verbatim, so DistTag → DistTag → false, and for a catalog entry it returns the looked-up entry's version, so workspace:* → Workspace → false and ^1.0.0 → Npm → true. The new test table has both catalog shapes with linking on and off, plus a re-resolution test for a catalog range. The comment-cop feedback was addressed by trimming to a single doc comment on links().
Security risks
None. This is in-memory tag classification during lockfile load; resolutions (package_id) are unchanged, the saved bun.lock is byte-identical, and no untrusted input handling changes.
Level of scrutiny
High. This is the bun install lockfile loader — every workspace install goes through it, and the correctness argument depends on WorkspaceLinks::links staying in lockstep with Package::parse_dependency's linking rule in a separate file. I verified the linking-on Npm arm against Package.rs:1880-1904 and it matches (link_workspace_packages && satisfies), and the linking-off arm's convergence argument holds for the shapes covered. But this is exactly the kind of two-place-mirror that a maintainer who owns Package::parse_dependency should confirm, and the six snapshot changes deserve a human eye to confirm no downstream consumer of the loaded lockfile relies on those entries being tagged Workspace.
Other factors
Extensive test coverage (16-row table across group combinations, satisfied/unsatisfied ranges, versionless, catalog, dist-tag, ×linking on/off; plus root variant, bump, and linking-off re-resolution for plain and catalog ranges); the description states 16/20 fail against main's loader. link_dep_to_workspace is a straight extraction of the previous inline body, keeping the whole-struct assign for Drop. The description notes overlap with open #37249 (versionless-wildcard shape) which will need a rebase either way.
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
32-bit hash collisions can happen here so we should compare nmaes after hash passes
|
Done in 4eb1d80: For reference, the keys involved here are the 64-bit |
There was a problem hiding this comment.
I re-reviewed after c5a8052/4eb1d80 and didn't find any bugs — the linking-off arm now resolves through catalogs.resolve_range and only retags when the underlying tag is Npm, so the catalog-holding-workspace:* and dist-tag cases I flagged earlier fall through to false (both now covered in the test table). Given this rewrites how root/workspace dependencies are retagged on load — a predicate that has to exactly mirror Package::parse_dependency across tag × behavior × linkWorkspacePackages — a human look from someone who owns src/install/lockfile would still be worthwhile.
What was reviewed:
WorkspaceLinks::linksagainst the parser's rule:workspace:always; Npm ranges satisfied by the recorded workspace version; catalog/dist-tag never retagged with linking on; with linking off, only Npm ranges (direct or behind a catalog) load as linked so they diff once and re-resolve.seen_depsremoval: every entry is now classified independently, andresolutions[dep_id]is still written for every entry viamap_manifest_dep_to_pkg.- The 6 snapshot changes: only the in-memory tag flips (npm/dist_tag instead of workspace);
package_idunchanged. - The one open comment-cop lint on line 3504 is on a two-line comment explaining the name-hash-vs-name check — looks like a false positive.
Extended reasoning...
Overview
The PR changes how bun.lock's loader (parse_into_binary_lockfile in src/install/lockfile/bun.lock.rs) rewrites root and workspace dependency entries that resolved to a workspace package. Previously it unconditionally retagged the first entry per name to Workspace and skipped later ones via a seen_deps set; now every entry goes through a new map_manifest_dep_to_pkg that consults WorkspaceLinks::links to retag only when Package::parse_dependency would produce a Workspace tag for the same input. This makes the loaded dependency list byte-equal to a fresh parse, so Diff::generate stops reporting phantom updates on every install. 20 new tests in bun-workspaces.test.ts cover the shape matrix; 6 existing snapshots change (in-memory tag only, package_id unchanged).
Security risks
None identified. This is in-memory lockfile state reconstruction; no network, filesystem, or auth surface changes. The saved bun.lock bytes are unchanged.
Level of scrutiny
High. bun install lockfile loading runs on every install in every workspace monorepo, and the correctness of the no-op fast path depends on links() exactly mirroring the parser's linking rule across every combination of dependency tag, behavior flags, and the linkWorkspacePackages setting. A mismatch in either direction produces either the original bug (permanent phantom updates) or a stale resolution that never re-resolves. The predicate reads correctly to me and the test table is thorough, but the interaction with catalogs.resolve_range, peer-binding-by-name-hash (the reason for the explicit name check at line 3505), and the linking-off fallback are subtle enough that a maintainer who knows Package::parse_dependency should confirm the rule is complete.
Other factors
- My earlier inline finding (the
_ if !self.link_workspace_packagesarm catching Catalog/DistTag) was addressed in c5a8052 by resolving throughcatalogs.resolve_rangeand gating ontag == Npm; the two shapes it would have broken are now in the test table with linking on and off, plus a catalog-range re-resolution test. link_workspace_packagesdefaults totruewhenmanagerisNone(is_none_or), matching the option's default — checked that the debug-dump path (install_test_helpers.parseLockfile, no manager) sees linking-on behavior, which is what the tests assert.- The removed
seen_depsallocation and?onget_or_putare gone;map_manifest_dep_to_pkgstill writesresolutions[dep_id]unconditionally, so no entry loses its binding. - The PR notes overlap with #37249 (versionless-wildcard shape) — one will need a rebase; not a blocker for this review.
- One unresolved comment-cop lint remains on the two-line name-hash comment at line 3504; the author already defended the (longer) doc comment on
links()and trimmed the rest in 7f0a983.
…ackage.json parses them The bun.lock loader marked every root or workspace dependency that resolved to a workspace package as a workspace dependency, except the second and later entries of a name listed in more than one dependency group, which it left as parsed. Package.parse marks an entry as a workspace dependency exactly when its specifier links the workspace (workspace:, or a range the workspace's version satisfies while linkWorkspacePackages is on), and marks every such entry. Wherever the two disagreed, Diff.generate counted an update on every install of an unchanged repo and the install re-resolved the workspace each time: a workspace listed in two dependency groups, an unsatisfied peer range on a workspace, a range on a versionless workspace, a catalog entry pointing at a workspace, a dist-tag the registry does not have, an override sending a range to a workspace. Root and workspace entries now follow the parser's rule, one entry at a time. A range links when it names the bound workspace package (compared by name, not only by the hash the version map and peer bindings are keyed by) and the version recorded in the lockfile satisfies it, so a workspace that moved out of range still diffs. With linkWorkspacePackages off the parser links nothing, so ranges (plain or behind a catalog entry) that the lockfile still binds to a workspace keep loading as linked, as before, and the diff re-resolves them; peers, overridden entries, catalog entries holding workspace: and dist-tags are bound to the workspace under either setting and load as parsed. Entries of installed packages are not diffed against anything and keep the old behavior.
4eb1d80 to
0097912
Compare
| /// Whether `Package::parse_dependency` links `dep` to the workspace `bun.lock` | ||
| /// bound it to, so that `Diff::generate` sees the loaded and the parsed | ||
| /// dependency as equal. With `linkWorkspacePackages` off it links no range, and | ||
| /// a range still bound to a workspace is loaded as linked on purpose: the diff | ||
| /// re-resolves it. Peers, overridden entries and dist-tags bind to a workspace | ||
| /// either way. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| /// Same exemption as `enqueue_dependency_with_main_and_success_fn`: an `npm:` | ||
| /// alias is never overridden. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
|
Squashed and rebased onto current main (0097912; the only conflict was the new
23 tests now; 19 fail against main's |
| fn overridden(&self, dep: &Dependency) -> bool { | ||
| let is_alias = dep.version.tag == DependencyVersionTag::Npm && dep.version.npm().is_alias; | ||
| !is_alias && self.overrides.has_rule_for_name(dep.name_hash) | ||
| } |
There was a problem hiding this comment.
🟡 overridden() uses has_rule_for_name(dep.name_hash), which also matches scoped/range-gated overrides (scoped_names) whose parent scope does not apply to this edge. In the !link_workspace_packages branch that makes links() return false for a plain range that was previously linked, so it loads as Npm, matches the parser, never diffs, and stays workspace-bound instead of re-resolving against the registry — a regression from pre-PR behavior for the (obscure) combination of a scoped override on a workspace-named package plus toggling linkWorkspacePackages off. enqueue_dependency_with_main_and_success_fn uses the precise overrides.get(&lockfile, id, name_hash) for the same distinction.
Extended reasoning...
What the bug is
WorkspaceLinks::overridden() (src/install/lockfile/bun.lock.rs:3539-3542) exists to tell the !link_workspace_packages branch of links() whether an npm range that bun.lock bound to a workspace got there because an override sent it there (in which case re-resolving would just bind the workspace again, so it should load as parsed and not diff) or because linking was previously on (in which case it should keep loading as linked, diff once, and re-resolve against the registry). It answers that question with self.overrides.has_rule_for_name(dep.name_hash).
OverrideMap::has_rule_for_name (src/install/lockfile/OverrideMap.rs:316-318) is self.map.contains(&name_hash) || self.scoped_names.contains(&name_hash). scoped_names is populated by push_scoped (line 511) with the target name of every scoped and range-gated rule, regardless of its parent scope — so a rule like "some-transitive-pkg > no-deps": "2.0.0" or "no-deps@^2": "..." puts hash("no-deps") into scoped_names even though overrides.get() returns None for a direct pkg1 → no-deps edge that does not match the scope.
The code path that triggers it
In links() with link_workspace_packages == false, after the Workspace-tag early return:
if dep.behavior.is_peer() || self.overridden(dep) {
return false;
}When overridden() over-broadly returns true, links() returns false, map_manifest_dep_to_pkg does not call link_dep_to_workspace, and the dependency stays tagged Npm. The parser (linking off) also produces Npm, so Diff::generate sees no change and the resolution recorded in bun.lock — the workspace package — is kept.
Why existing code does not prevent it
The precise check for "does an override apply to this edge" is overrides.get(&lockfile, dependency_id, name_hash), which walks the parent chain and matches scoped rules only when the scope actually applies (OverrideMap.rs:152-157). That is what enqueue_dependency_with_main_and_success_fn uses for the same distinction. has_rule_for_name was borrowed from resolve_peer_dep_version_based, where a false positive falls through to the tree-path lookup and is harmless; here there is no fallback — a false positive silently keeps the wrong resolution.
WorkspaceLinks does not currently have access to the dependency_id or the lockfile parent chain, so wiring the precise check in is not a one-liner, but the coarse check is still an incompleteness in the "load as parsed" rule the PR establishes.
Impact
A regression from pre-PR behavior in a narrow case. Concretely:
- Root has
overrides: { "other-pkg > no-deps": "2.0.0" }(or"no-deps@^2": "..."). - Workspace
no-deps@1.0.0atpackages/pkg2;pkg1.dependencies: { "no-deps": "^1.0.0" }. The scoped override does not apply topkg1 → no-deps. - Install with
linkWorkspacePackageson: parser links^1.0.0→ resolver binds the workspace →bun.lockrecords it workspace-bound. - User turns
linkWorkspacePackagesoff and installs again. - Loader:
dep.version.tag == Npm,res.tag == Workspace, linking off.overridden(dep)→has_rule_for_name(hash("no-deps"))→true(viascoped_names) →links()returnsfalse→ dep staysNpm. - Parser (linking off) also produces
Npm. No diff.pkg1 → no-depsstays bound to the workspace and is never re-resolved against the registry.
Before this PR, map_dep_to_pkg unconditionally retagged the entry to Workspace when res.tag == Workspace, so with linking off the diff fired and re-resolved to the registry on the next install. The PR's own a linked range (^1.0.0) is re-resolved after linkWorkspacePackages is turned off test would fail if a scoped override on no-deps (under an unrelated parent) were added to the fixture.
The trigger is a triple conjunction — a scoped/range-gated override whose target name matches a workspace, a workspace declaring a satisfied npm range on that name, and the user toggling linkWorkspacePackages off — and the workaround (delete bun.lock) is easy, so this is not worth blocking merge over. But it is a real gap in the rule and worth noting.
How to fix
Give WorkspaceLinks::overridden() (or links()) enough context to call the precise overrides.get(lockfile, dep_id, name_hash) instead of has_rule_for_name — e.g. pass the dependency_id down from map_manifest_dep_to_pkg and hold a reference to lockfile (or the parent chain). Then overridden() returns true only when an override actually applies to this edge, matching the exemption enqueue_dependency_with_main_and_success_fn already uses.
Step-by-step proof
- Root
package.json:{ "workspaces": ["packages/*"], "overrides": { "unrelated > no-deps": "2.0.0" } }. packages/pkg2/package.json:{ "name": "no-deps", "version": "1.0.0" }.packages/pkg1/package.json:{ "name": "pkg1", "dependencies": { "no-deps": "^1.0.0" } }.bun install(linking on).Package::parse_dependencysees^1.0.0,link_workspace_packages && satisfies→ links.bun.lockrecordspkg1 > no-deps→ workspace.- Loading
bun.lock:parse_overrides→put_lockfile_rule→push_scopedinsertshash("no-deps")intolockfile.overrides.scoped_names. - Set
install.linkWorkspacePackages = falsein bunfig.toml;bun install. map_manifest_dep_to_pkgon pkg1'sno-deps:res.tag == Workspace, callsworkspace_links.links(dep, pkg_id).links():dep.version.tag == Npm(notWorkspace),!self.link_workspace_packages→ checksdep.behavior.is_peer()(false)|| self.overridden(dep).overridden(dep):is_alias= false,self.overrides.has_rule_for_name(hash("no-deps"))→scoped_names.contains(...)= true → returnstrue.links()returnsfalse.link_dep_to_workspaceis skipped. Loaded dep staystag = Npm.Diff::generate: parsed dep (linking off, override does not apply here peroverrides.get()returningNone) is alsotag = Npm,Dependency::eqlpasses.summary.update= 0.- Resolution stays workspace-bound. Pre-PR, step 8 unconditionally retagged to
Workspace, step 11 sawWorkspacevsNpm,summary.update= 1, and re-resolution went to the registry.
|
Issue #40366 is the same root cause surfaced through bun prune and bun dedupe: the skipped rewrite on a duplicate name makes Diff::generate report the lockfile as stale, and both commands exit 1 with "bun.lock does not match package.json" on a lockfile bun install just wrote. I reproduced it and fixed it independently on branch farm/a9e82d7a/prune-dedupe-workspace-peer-dup (https://github.com/oven-sh/bun/tree/farm/a9e82d7a/prune-dedupe-workspace-peer-dup) with a narrower change: keep the seen_deps shortcut but apply the workspace rewrite when the duplicate itself resolves to the workspace. This PR covers that fix and more, so I am not opening a second PR. The branch also carries a prune/dedupe regression test in test/cli/install/bun-workspaces.test.ts ("workspace dependency duplicated as a peer dependency stays in sync") that this PR could adopt to close #40366. |
|
Status after #41302 (merged as f942cb0): the bug in this PR is fixed on main. Three of the new cases still fail on main, so this PR stays open for now. I checked with a debug build of main (4661e49):
These cases fail on main:
For cases 2 and 3, main keeps the workspace link. The install with The #41302 description states this behavior: "With
|
|
This is Claude (an AI assistant) writing on Jarred's behalf. Obsolete: fixed on main by #41302. |
Problem
packages/core/sharedhas"@activepieces/core-piece-types": "workspace:*"under bothdependenciesanddevDependencies), everybun installreports the workspace as changed, with nothing changed on disk:bun install --verboseprintsWorkspace package "packages/core/shared" has added 0 dependencies, removed 0 dependencies, and updated 1 dependencieson every run,summary.updateis non-zero, and the install takes the re-resolve path instead of the no-op path. Same when the root lists a workspace in two groups. Reproduces on 1.3.14, 1.4.0 and main.bun.lockstores a workspace's dependencies as the literal specifiers, and the loader rewrites each entry that resolved to a workspace package into the formPackage::parse_dependencyproduces (map_dep_to_pkg, src/install/lockfile/bun.lock.rs), but aseen_depscheck skipped the rewrite for the second and later entries of a name (root loop and workspace loop, formerly around lines 3102 and 3187). The parser rewrites every entry, so the twoDependencylists never compare equal inDiff::generate_inner(src/install/lockfile/Package.rs, theDependency::eqlcheck) andsummary.updateis bumped once per extra entry, forever.workspace:specifier or a range the workspace's version satisfies; the loader rewrote anything bound to a workspace. Dropping the check alone makes the existing testmatching workspace devDependency and npm peerDependency(aworkspace:*devDependency plus an unsatisfied2.0.0peer on the same workspace) report an update on every install instead, because the peer entry then gets rewritten too. The same over-broad rewrite already makes these single-entry shapes report an update on every install on main: an unsatisfied peer range on a workspace (which also re-resolves the peer against the registry on every install), a range on a versionless workspace (install: stop re-saving an unchanged lockfile for wildcard deps on versionless workspace members #37249's shape), acatalog:entry pointing at a workspace, and a dist-tag the registry does not have (which the resolver answers with the workspace).Fix
map_manifest_dep_to_pkg, which binds the entry and rewrites it only whenWorkspaceLinks::linkssays the parser would:workspace:specifiers always; npm ranges when they name the bound workspace package (the name inside the range is compared against the package's name, not only the hashes the version map and peer bindings are keyed by) and the versionbun.lockrecorded for it satisfies them; nothing else. Every entry is classified on its own, so a name listed in several groups gets each entry rewritten or left alone exactly as the parser does.seen_depsis gone.Diff::generatecompares the loaded list against a fresh parse of the same package.json, so the loader has to reproduce the parser's output, and the parser's rule is the one inPackage::parse_dependency(src/install/lockfile/Package.rs, theTag::Npmarm linking a satisfied range and theTag::Workspacearm). Resolutions are untouched: every entry still binds to the same package as before, only the in-memory version differs, and that in-memory state is now the one a fresh install has too (the fresh resolver binds unsatisfied peers and versionless wildcards to the workspace without retagging the dependency). The savedbun.lockis byte-identical before and after, so existing lockfiles load as they are.bun.lock, not the one on disk, so a workspace bumped out of an entry's range still loads as linked, diffs against the parse, and gets re-resolved (tested).linkWorkspacePackages = false: the parser then links no range, so a rangebun.lockstill binds to a workspace, written out or behind acatalog:entry, was linked while the setting was on. Those keep loading as linked, which is the previous behavior and is what makes them diff once and re-resolve against the registry (tested for both forms, including that the next install is quiet again). Everything a fresh install binds to the workspace under either setting loads as parsed: peers (bound to the workspace of their name by the hoister), entries an override redirects (annpm:alias is never overridden, the same exemption the resolver applies),catalog:entries holdingworkspace:, and dist-tags the registry does not have; marking any of those would be the same permanent update, and was, on main (tested for each). This is the rule install: fail --frozen-lockfile on manifest drift and fix the spurious lockfile re-saves behind it #33632's loader commit arrives at as well, so that PR can rebase its loader half onto this.map_dep_to_pkg; nothing diffs them, and this change does not touch their behavior.dependencies on a workspace load from bun.lock the way package.json parses them: a table of 19 shapes (the two-group shapes from the report in every group combination, satisfied ranges plain and as annpm:alias, unsatisfied ranges, versionless wildcard, catalog entry, missing dist-tag, an override sending a range to the workspace, and six of those with linking off) asserting the loaded entries via the lockfile dump and that a second--verboseinstall does not report the workspace (the dump loads with the test process's default settings, so for the linking-off rows the--verboseinstall in the fixture is the assertion that exercises that setting); a root variant; the bump test; and the linking-off re-resolution test for a plain and for a catalog range, each also checking the third install is quiet. Against main's bun.lock.rs 19 of the 23 fail (the dump showsworkspace *for the second entry, orworkspace packages/pkg2for the peer/wildcard/catalog/dist-tag/override entries); the 4 that pass on main pin behavior that must not change (the dev+unsatisfied-peer shape with linking on and off, the bump, the plain-range re-resolution after turning linking off). With the override exemption removed, the linking-off override row fails on the--verboseline, with the re-resolve binding the workspace again. All 23 pass with the fix.dependency on workspace without version in package.jsonshow the only in-memory change for that test: wildcard and dist-tag entries on a versionless workspace keep their parsed tag,package_idunchanged. (The"tag": "no-deps"value in the new dist-tag snapshot is a pre-existing bug in the debug dump, already certified by three other snapshots in that file; install: fail --frozen-lockfile on manifest drift and fix the spurious lockfile re-saves behind it #33632's last commit fixes it.)cargo clippy -p bun_installclean. migration/complex-workspace needs gitlab/bitbucket access and fails here with or without the change.seen_depsremoval, amap_manifest_dep_to_pkgwith the same rule, the same 6 snapshot hunks) plus unrelated pieces; this PR is that loader slice on its own, with the name check and the catalog handling added in review, and install: fail --frozen-lockfile on manifest drift and fix the spurious lockfile re-saves behind it #33632 would rebase down to its remaining pieces. install: stop re-saving an unchanged lockfile for wildcard deps on versionless workspace members #37249 gates only npm entries and keepsseen_deps, so it fixes the versionless-wildcard shape but not the two-group ones; this supersedes it (noted there). install: link wildcard deps to prerelease workspace members in any position #37264 is changing the parser's range rule (a wildcard accepting a prerelease member) through a shared predicate;WorkspaceLinks::linksmirrors the rule as it is on main today, and whichever of the two lands second points the loader at that predicate (noted there). The merged install: collapse a workspace's same-name dependency slots into one entry so --frozen-lockfile is stable #36303 is about hoisting two entries with different versions and does not interact with this.Earlier revision
The first push exempted only peers from the linking-off fallback, so with linking off a
catalog:entry holdingworkspace:or a missing dist-tag was still marked and diffed forever (as on main). Review caught it; the fallback now applies to ranges only, with the catalog and dist-tag shapes in the table and a catalog-range variant of the re-resolution test. A second round asked for the workspace's name to be compared rather than trusting the hash-keyed lookups alone; that is the name check inlinksand the aliased-range row. A third pass compared the rule with #33632's and took its one extra exemption (overridden entries with linking off), with the two override rows; the branch was also squashed and rebased onto current main at that point.Background
Dependencyinlockfile.buffers.dependencies(name, behavior = which group, parsed version); a package owns a contiguous range of them, andbuffers.resolutionsmaps each one to a package id. The same name in two groups is two entries with different behaviors.Dependencywhose version tag isworkspaceand whose value is the workspace's path. The parser produces one for aworkspace:specifier, and, whilelinkWorkspacePackagesis on, for an npm range satisfied by the workspace's version (the literal is kept either way, which is whatbun.lockprints).Dependency::eqlcompares tag and value, so aworkspace:*entry left with the value*is not equal to one carrying the path.bun.lockloading (parse_into_binary_lockfile): parses every literal back into aDependency, resolves the root's and each workspace's entries by name against the packages map, then the installed packages' entries by tree path. The rewrite discussed here happens at that binding step.Diff::generate(Package.rs): on every install, compares the loaded root with a fresh parse of package.json and recurses into each workspace; any entry that is noteqlcounts as an update, which prints the verbose line above, makeshad_any_diffstrue and re-resolves the workspace.bun install --frozen-lockfiledoes not consult these counts, so this shape never failed CI; it only did the extra work and (for unsatisfied peers) the extra registry round trip on every install.install_test_helpers.parseLockfile) loadsbun.lockthrough this same loader and prints each entry keyed by its version tag, which is how the tests observe the loaded form directly.Repro (released bun)
mkdir -p repro/packages/{a,b} && cd repro echo '{"name":"root","private":true,"workspaces":["packages/*"]}' > package.json echo '{"name":"a","version":"1.0.0"}' > packages/a/package.json echo '{"name":"b","version":"1.0.0","dependencies":{"a":"workspace:*"},"devDependencies":{"a":"workspace:*"}}' > packages/b/package.json bun install bun install --verbose 2>&1 | grep "Workspace package" # Workspace package "packages/b" has added 0 dependencies, removed 0 dependencies, and updated 1 dependenciesListing
ain three groups printsupdated 2. The same line appears fordependencies+peerDependencies,dependencies+optionalDependencies,^1.0.0in two groups, a lonepeerDependencies: {"a": "2.0.0"},dependencies: {"a": "*"}on a versionlessa, anddependencies: {"a": "catalog:"}withcatalog: {"a": "workspace:*"}; a singleworkspace:*,^1.0.0orworkspace:*+2.0.0peer entry is quiet. On 1.3.x/1.4.0 the second install also printsSaved lockfile; main since #38333 skips the write when the bytes are unchanged, so there the symptom is the verbose line and the re-resolve.