install: fix the package id boundary in Lockfile::eql

Merged
RO
3mo ago
+58-6

Problem

  • bun install --frozen-lockfile fails with error: lockfile had changes, but lockfile is frozen on a bun.lock whose tree has not changed, as soon as the file contains an entry that nothing depends on and that entry is not the last one in the file. Field case: activepieces/activepieces at HEAD, whose committed (1.3-written) lockfile 1.3.14 accepts; with #38853 applied, main still rejects it.
  • Lockfile::eql (src/install/lockfile.rs) compares the cleaned lockfile's tree with the tree the loaded lockfile was hoisted into at load time. It took a cut_off_pkg_id and left out every placement bound to a package id at or past it, on both sides. The callers pass the cleaned lockfile's package count. On the cleaned side that is a no-op. On the loaded side it does two things: it catches edges that resolving rebound to packages appended after load (those ids are past the loaded count; the loaded tree still places them, so leaving them out makes the counts differ; bun update --frozen-lockfile moving a transitive dependency relies on this, bun-update-transitive.test.ts), and, as soon as the clean dropped an entry, it also leaves out the loaded side's own last packages, whose ids are past the cleaned count but below the loaded one. That second effect is the bug: a real placement disappears from the comparison and the counts differ.
  • In activepieces the dropped entry is react-dom@18.3.1: the file nests it under react-json-view for a peer range (^15 || ^16 || ^17) nothing in the file satisfies, and since #32182 the loader binds that edge by version, falling back to the first candidate (react-dom@19.2.5 at the root), so the nested copy is referenced by nothing and the clean drops it (4069 -> 4068 packages). The file's last entry, mdast-util-find-and-replace@1.1.1 (id 4068), is placed twice; both placements were left out on the loaded side. The two hoists otherwise agree placement for placement (trace in the details below).

Fix

  • The boundary on the loaded side is now the loaded lockfile's own package count at load time, which mark_loaded_packages already records for the resolver (loaded_package_count); the two callers in install_with_manager.rs pass lockfile_before_clean.loaded_package_count. A placement bound past it was rebound to a package appended by this install, so eql now returns false on it directly instead of leaving it out and relying on the counts. The cleaned side is compared whole. The debug-build determinism check in save_lockfile compares a lockfile with itself and keeps passing its own count.
  • Correct because the two things the old boundary mixed up are now separate: a rebound edge is a changed resolution and fails, and a package the clean dropped only matters if it was placed in the loaded tree, in which case its placements are now counted on the loaded side and missing on the cleaned side, which still fails. An entry that was in no tree no longer affects the result.
  • Visible consequence outside the frozen check: the same comparison decides whether a non-frozen install re-saves, so a no-op bun install on a lockfile whose only difference is an entry outside the tree no longer rewrites the file just to drop it; it is dropped the next time anything else causes a save. Installs with a package.json diff still save regardless (had_any_diffs).
  • Test: test/cli/install/bun-lock.test.ts, "--frozen-lockfile accepts a bun.lock with an entry nothing depends on, wherever it is listed". A hand-written lockfile with no-deps (depended on) and a-dep (not depended on) in both orders; on main the order with a-dep first fails with the error above, with this change both orders install no-deps only and leave the file untouched. The rebound case stays covered by the existing bun update --frozen-lockfile test in bun-update-transitive.test.ts, which failed on the first version of this PR (it dropped the loaded-side check entirely) and passes now.
  • With this change plus #38853, the committed lockfiles of activepieces, opencode, eliza and supermemory pass --frozen-lockfile with the debug build (activepieces still logs 4069 -> 4068 and passes); hono and remotion still pass, and a further 72 small repos with a committed bun.lock behave identically on 1.3.14, main and the two changes combined (65 pass everywhere, 7 are already out of date everywhere). Suites run with the debug build: bun-lock, bun-update-transitive, bun-update, bun-add, bun-remove, migration/migrate, frozen-lockfile-pruned, frozen-lockfile-missing-workspace, lockfile-only, bun-lockb, bun-workspaces, isolated-install, bun-dedupe, bun-prune, hoist, catalogs, overrides, nested-overrides, lockfile-version-2, bun-install-registry.
  • Not changed here: the loader binding an out-of-range peer to a different version than the entry the file nests next to the dependent (the producer of the unreferenced entry above). That rewrites such 1.3 lockfiles on the next non-frozen install and changes which copy the dependent gets; it is the same family as #38767 / #38768 / #38837 and is tracked separately.

Background

  • bun.lock stores the hoisted tree as keys: a/b means b is installed in a's node_modules. Loading the file creates one package per distinct entry (ids in file order), binds every dependency edge, and hoists the result into an in-memory tree. mark_loaded_packages then records the package count; anything resolving appends afterwards (a newer version bun update picked, a package added to package.json) gets a higher id, and edges may be rebound to those packages in place, while the tree built at load time is not rebuilt.
  • clean_with_logger builds a fresh lockfile by walking from the root (ids in walk order), so packages no edge reaches are dropped, and hoists it again. --frozen-lockfile compares that tree with the load-time tree using Lockfile::eql: placements are listed as (path, package), sorted, and compared pairwise on name, resolution, bins and scripts. The same comparison decides after a normal install whether the lockfile is saved again. bun.lockb uses a hash over the package list instead (packages_len_before_install still feeds that).
Hoist trace on activepieces (temporary instrumentation, not part of the change)

Every decision the hoister made for mdast-util-find-and-replace, first while loading the file (ids 2628/4068), then during the clean (ids 1344/2474). The two runs are identical; only the comparison differed.

hoist dep_id=10546 pkg_id=2628 parent_pkg_id=2640 tree_node=858  -> Placement(ancestor)
hoist dep_id=10499 pkg_id=2628 parent_pkg_id=2631 tree_node=1120 -> Hoisted(dedupe)
hoist dep_id=16465 pkg_id=4068 parent_pkg_id=4043 tree_node=1222 -> Placement(own node)
hoist dep_id=16465 pkg_id=4068 parent_pkg_id=4043 tree_node=1227 -> Placement(own node)
hoist dep_id=3661  pkg_id=1344 parent_pkg_id=1347 tree_node=858  -> Placement(ancestor)
hoist dep_id=3651  pkg_id=1344 parent_pkg_id=1343 tree_node=1120 -> Hoisted(dedupe)
hoist dep_id=7043  pkg_id=2474 parent_pkg_id=2473 tree_node=1222 -> Placement(own node)
hoist dep_id=7043  pkg_id=2474 parent_pkg_id=2473 tree_node=1227 -> Placement(own node)

Placement diff reported by an instrumented eql (raw hoisted_dependencies lengths were equal; only the filtered lists differed, by exactly the two placements of loaded id 4068):

only in cleaned: @tryfabric/martian/remark-gfm/mdast-util-gfm/mdast-util-gfm-autolink-literal :: mdast-util-find-and-replace@1.1.1
only in cleaned: slackify-markdown/remark-gfm/mdast-util-gfm/mdast-util-gfm-autolink-literal  :: mdast-util-find-and-replace@1.1.1
First version of this PR

The first push removed the boundary altogether, on the reasoning that nothing appends packages during the install step any more. That is true of the step after the clean, but resolving before the clean still appends and rebinds, and the loaded tree is not rebuilt, so bun update --frozen-lockfile on a stale lockfile was accepted (bun-update-transitive.test.ts caught it in CI). The current version keeps that check and only moves the boundary to the loaded side's own count.


no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-lock.test.ts