install: fail --frozen-lockfile on manifest drift and fix the spurious lockfile re-saves behind it

Closed
RO
2mo ago
+1694-454

Fixes #22689
Fixes #24223
Fixes #13823

Problem

  • bun install --frozen-lockfile (and bun ci, --production) exits 0 on a stale lockfile whenever the manifest change resolves to packages the lockfile already has: a dependency added to one workspace that another workspace (or a transitive) already pulls in, a removed direct dependency that stays in the tree transitively, a range change the locked version still satisfies. npm/pnpm/yarn all refuse these.
  • Cause: the frozen check in install_with_manager.rs compares the resolved package set (Lockfile::eql for bun.lock, the meta hash for bun.lockb) and, since #38333, the overrides and catalog flags of the differ; the differ's dependency comparison itself (Diff::generate, root and workspace members) is not consulted.
  • Consulting it is only possible once it is quiet on stable repos. On main it reports a diff on every install for a number of shapes (since #38333 the byte-identical re-save is suppressed, so this is invisible, but every one of them would become a permanent frozen failure): a workspace member with a lifecycle script, trustedDependencies declared in a member, a sibling workspace listed in two dependency sections, a catalog: entry or an override pointing at a workspace, * on a versionless workspace, a root entry for a member that the parser does not link ({a: "^9.0.0"} plus an override), every bun.lockb monorepo whose members depend on each other, stale trustedDependencies / patchedDependencies entries, a trustedDependencies entry for a package installed under an npm: alias (which bun.lock also never records, so installing from the lockfile relied on that perpetual diff to trust it), and monorepos migrated from pnpm-lock.yaml whose members depend on each other by version range.

Fix

Five commits:

  1. install: stop reporting manifest diffs for repos whose lockfile is already up to date (Package.rs, lockfile.rs, bun.lock.rs, bun.lockb.rs, a hook in install_with_manager.rs)
    • lifecycle scripts are only compared when the loaded package recorded them (scripts.filled, i.e. bun.lockb); bun.lock stores none and reads them from package.json at install time.
    • overrides, catalogs, trustedDependencies and patchedDependencies are compared once, at the root, and the last two only after the members have been parsed (the lockfile holds the union over all workspaces; before, a list declared in a member was reported removed on every install). The caller already applies changes to these sections across every package's edges.
    • workspace members: Diff::generate used to compare a member only by recursing through the root's workspace-behavior edge onto it. diff_workspace_members now compares every workspace package in the lockfile with its package.json regardless of how the root reaches it (its workspaces entry, an override on its own entry, only through a sibling), records the changed ones in changed_workspaces (which changes_resolutions / changes_dependencies / has_diffs include, so the existing consumers keep their meaning), and install_with_manager re-resolves one dependency onto each changed member (Lockfile::dependency_to_reresolve_workspace, preferring a workspace: edge declared by the root or a member), which re-reads it into the same package id through the folder resolver, exactly what re-resolving the synthetic edge used to do. Root edges onto members always stay mapped. The pass also feeds #38333's survivors / removed_names bookkeeping, so pruned checkouts behave as before.
    • the bun.lock loader tagged every root/workspace edge that resolved to a workspace package as a workspace dependency, except the second entry of a duplicated name, and always synthesized the root's edge to each member. The parser tags only workspace: specifiers and npm ranges satisfied by the member's version, tags every entry, and replaces the synthetic edge with the root's own entry when that entry is an npm range it does not link; map_manifest_dep_to_pkg and the synthesis loop now follow those rules (using the versions recorded in the lockfile, so a member that moved out of range still produces a diff). This is the bun-workspaces.test.ts.snap change: loaded */dist-tag edges onto a versionless member keep their own tag.
    • bun.lockb stores an edge as tag plus literal, so a member's workspace:* edge reloaded holding * while a fresh parse holds the member's path, which is what Version::eql compares. restore_workspace_dependency_paths points the root's and members' workspace edges back at the path of the package they resolved to after loading, as the bun.lock loader already did (the migrate-bun-lockb-v2 snapshot shows the path now; the bytes written are unchanged).
    • bun.lock only writes the trustedDependencies names and patchedDependencies keys that match something in the tree, while the differ compared the full package.json sets. Entries that are only in package.json now count as additions when the lockfile has something for them to apply to (stores_trusted_dependency, stores_any_patched_dependency); what the loaded lockfile holds is compared as before. has_trusted_dependency checks an npm package under the package's own name, so {"x": "npm:real@1"} is trusted by listing real; the writer only looked at edge names and never stored such an entry, and stores_trusted_dependency has to agree with has_trusted_dependency, otherwise the entry is filtered out of the diff and an install from the lockfile stops trusting it. Both now also match the name of the npm package an edge resolved to (one extra insert in the writer's collection loop; for a non-aliased edge it re-inserts the key just inserted, so #38736's byte order is unaffected). And since the differ now leaves some package.json entries out on purpose, install_with_manager copies trustedDependencies and patchedDependencies from the fresh parse whether or not it reported a diff, as it already did for the root scripts (the patched block is the else branch's existing code moved into copy_patched_dependencies, called from both branches). Otherwise a run without a manifest diff kept the loaded subsets, which shows on the one such run that still changes resolutions: bun audit fix moving a package to the version a patch is written for installed it unpatched. Declaring trustedDependencies at all is what turns the default trusted list off, so the writer now emits "trustedDependencies": [] when the field is declared but none of its names are in the tree (bun.lockb already has a tag for this), and a package.json that declares the field while the lockfile does not record it is a diff (trusted_dependencies_declared). On main a bun.lock written for "trustedDependencies": [] drops the field and the default list is back in effect from the second install on, bun ci included (verified with the electron fixture); such lockfiles now get re-saved once.
  2. install: record workspace versions when migrating pnpm-lock.yaml (pnpm.rs): the migration read the version off the pnpm importer entry, which never has one, so migrated lockfiles listed members without versions and the loader rule above could not tell a linked range apart; it now reads the member's package.json, like the package-lock.json migration. Two migration snapshots gain the "version" lines bun's own writer produces.
  3. install: fail --frozen-lockfile when a manifest diverges from bun.lock, and say which one (install_with_manager.rs): the check becomes summary.changes_dependencies() || <eql / meta hash>. changes_dependencies is #38333's notion of "the dependency declarations changed" (root and members, overrides, catalog; not lifecycle scripts), so trustedDependencies and patchedDependencies stay out of the comparison, as the pruned-checkout tests require (turbo prune strips them; they are applied from package.json either way). The notes reuse #38333's wording: 
    error: lockfile had changes, but lockfile is frozen
note: dependencies in packages/pkg2/package.json changed since bun.lock was saved (1 added, 0 removed, 0 updated)
note: try re-running without --frozen-lockfile and commit the updated lockfile
    (dependencies in package.json ... for the root; overrides / resolutions / the catalog in package.json changed since ... exactly as before, so #38333's assertions still hold; the packages resolved from the manifests differ from bun.lock when only the resolution comparison fires.) frozen_changed_section is folded into this.
  4. tests.
  5. install: debug lockfile JSON wrote the package name as a dist-tag dependency's tag: the bun:internal-for-testing dump wrote info.name for a dist-tag's tag; it surfaced because a loaded dist-tag edge now stays a dist-tag.

Dropped while rebasing onto current main: the bun update lockfile-specifier commit and the $ref override storage fix, both of which #38333 implemented independently (package_json_write_back::sync_lockfile, bun-update-lockfile-sync.test.ts), together with their tests.

Verification

test/cli/install/bun-install-registry.test.ts (the file fails on main, passes here):

  • failing direction: direct dep added that is already transitive (asserts the note and that bun.lock is left untouched), direct dep removed but still transitive, range literal changed (#24223), dep added to a second workspace (#22689, asserts the member note and no root note), a bun.lockb variant (#13823); and --frozen-lockfile fails when <member> gains a dependency: a member kept only by an override on the root's entry (bun.lock) and a member only a sibling depends on while the root depends on its published version (bun.lockb; bun.lock cannot load that shape today, #37248). The latter two add a dependency the root already has, so only the member comparison can notice; they check no diff on the second install, the frozen failure naming the member, the member actually being re-read by a plain install (lockfile dump), and frozen passing afterwards. All fail on main.
  • passing direction, --frozen-lockfile passes and nothing is re-saved (<format>): <shape>, 16 shapes under both formats (member depending on a sibling, root depending on members, override-kept unlinked entry, lifecycle script, member trustedDependencies, sibling in dev+peer, sibling linked in two groups, * on a versionless sibling, npm: alias of a sibling, catalog: -> workspace, override -> workspace, $ref override, stale / all-stale / empty trustedDependencies, stale patchedDependencies key). With the gate in place these rows are what proves the differ is quiet; on main's byte-compare save they pass trivially, so they act as guards there.
  • trustedDependencies [] / ["not-a-dependency"] still disables the default trusted list when installing from bun.lock (electron stays blocked on the first install, a plain install from the lockfile and a frozen one; bun.lock contains the empty field), and <change> passes --frozen-lockfile and is stored by the next install for an installed name, [], an all-stale list and a patch (stores ["no-deps"], [], [], the patch entry; frozen passes before and after). The electron tests and the []-storing rows fail on main.
  • trustedDependencies naming the package behind an npm: alias is stored and stays trusted when installing from bun.lock: lifecycle-postinstall installed as postinstall-alias and trusted under its real name; the postinstall runs on the first install, bun.lock records the name, and it runs again on a plain and on a frozen install from the lockfile. Fails on main on the bun.lock content (the name is not stored); failed on the previous revision of this PR at the second install (entry filtered out, bun.lock held [], script blocked).
  • the existing production = true test's inline snapshot gains the new note.

bun-audit.test.ts, applies the patchedDependencies entry written for the version it installs: no-deps@1.0.0 locked as a transitive, a patchedDependencies entry for 1.0.1 (so bun.lock stores nothing for it), and bun audit fix moving it to 1.0.1 has to install it patched and store the entry; frozen passes afterwards. Passes on main, fails on this branch with the unconditional copy removed (checked by building without it), passes here.

migration/migrate.test.ts: the arborist fixture edit-package-json--changed (package.json says abbrev: ^1.1.0, its lockfile was written from ^1.1.1) now records a frozen exit code of 1; #38853 had just recorded 0 for it and deferred this to this PR. It is the range-literal shape from #24223.

bun-install.test.ts: two existing bun.lockb workspace tests now assert the second install neither re-saves nor creates a .cache entry (they re-resolve on every install on main). pnpm-lock-v9.test.ts "link: version with a semver specifier" is the test that exposed commit 2.

Also run locally with the debug build: all of test/cli/install/ (the only failures are the tests that need network access or git clones), which includes #38333's frozen-lockfile-pruned, frozen-lockfile-missing-workspace, nested-overrides, bun-update-lockfile-sync, bun-lock (incl. #38736's byte-order test), bun-workspaces, the migration suites, isolated-install; source lints; cargo clippy -p bun_install. Plus registry-free ad-hoc shapes under both formats: a member reached through a catalog entry and then edited, two members edited at once (one removing a dependency) producing one note each and no root note, and linkWorkspacePackages flipped either way after the lockfile exists.

CI on this head (13f2595): every GitHub check passes (clippy, mordant, miri, format, source lints) and Buildkite build 97980 has all 177 Linux and Windows jobs green, with the only test failures being retried flakes in files this PR does not touch; its two darwin jobs expired because no darwin agent has been connected today, which affects every build, so the build shows as failed until the fleet is back.

Background

  • Diff::generate (Package.rs) compares the root package as loaded from the lockfile against a fresh parse of package.json; its DiffSummary drives the rebuild of the root's edges, should_save_lockfile, and (since #38333) changes_resolutions / changes_dependencies consumers such as dedupe, prune and the transitive update planner. had_any_diffs true on an unchanged repo means a full re-resolve every install; before #38333 it also meant a visible re-save, which is how these shapes were found.
  • bun.lock stores each workspace's dependency sections as literal specifiers plus a resolved packages map; the loader re-parses the literals, synthesizes the root's edge to each member, and fixes up edges that resolved to workspace packages. bun.lockb stores the parsed edges as tag plus literal. Both loaders have to land on exactly what the parser produces for the differ to stay quiet.
  • When the install re-resolves a workspace: dependency declared by the root or a member, the folder resolver re-parses that member's package.json and replaces its package in place (same id), so every other edge onto it sees the new dependency list; the re-read hook relies on that.
  • bun.lock writes trustedDependencies / patchedDependencies filtered to what is in the tree, bun.lockb writes the full sets; a stable repo therefore reloads a subset of what package.json declares under bun.lock, which is why the differ asks "would a save store this" for new entries instead of comparing the sets.
Earlier versions
  • v1 only gated on had_any_diffs; review listed the stable shapes for which it is spuriously true.
  • v2 fixed the bun.lock loader and the argument-less bun update flows; review found bun.lockb monorepos, the synthetic root edge, and bun update <name>.
  • v3 compared members only through root edges and the trusted/patched sets in full; review found members reached via an override going unnoticed and stale trusted/patched entries failing forever, then the all-stale trustedDependencies case re-enabling the default list.
  • v4 was the rebase onto main after #38333 / #38736: the update-specifier and $ref work is dropped as already landed, the member pass is rebuilt on the new generate_inner, the gate uses changes_dependencies so trusted/patched changes no longer fail frozen, the notes use #38333's wording, and the pnpm migration gap it exposed is fixed. Review of v4 found that its stores_trusted_dependency only looked at edge names, so a trusted name for an npm:-aliased package was filtered out and, with the loaded list kept, no longer trusted from the second install on.
  • v5 (this one): the writer and stores_trusted_dependency also match the resolved npm package's name, both sections are copied from package.json regardless of the diff result, the loader's root-only flag became a small struct, and a second rebase picks up #38870 (the frozen comparison passes the loaded package count) and #38853 (the arborist snapshot above).

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