install: only re-resolve rows a package still owns when overrides or catalogs change

src/install/PackageManager/install_with_manager.rs

@@ -1,6 +1,7 @@
 use core::sync::atomic::Ordering;
 
 use bun_collections::DynamicBitSet;
+use bun_collections::bit_set::Range;
 use bun_core::UnwrapOrOom as _;
 use bun_core::time::nano_timestamp;
 use bun_core::{Global, Output};
@@ -523,33 +524,10 @@ pub fn install_with_manager(
                         pinned_rows = enqueue_transitive(manager, &transitive, invalidates_rows)?;
                     }
 
-                    // `enqueueDependencyWithMain` can reach `Lockfile.Package.fromNPM`,
-                    // which grows `buffers.dependencies` and may reallocate it.
-                    // Iterate by index against a snapshot of the original length and
-                    // copy each entry to the stack so neither the loop nor the callee
-                    // ever reads through a pointer into the old backing storage.
                     if manager.summary.overrides_changed && !all_name_hashes.is_empty() {
-                        let dependencies_len = manager.lockfile.buffers.dependencies.len();
-                        for dependency_i in 0..dependencies_len {
-                            if pinned_rows.is_set_allow_out_of_bound(dependency_i, false) {
-                                continue;
-                            }
-                            let dependency =
-                                manager.lockfile.buffers.dependencies[dependency_i].clone();
-                            if all_name_hashes.binary_search(&dependency.name_hash).is_ok() {
-                                manager.lockfile.buffers.resolutions[dependency_i] =
-                                    invalid_package_id;
-                                if let Err(err) = enqueue_dependency_with_main(
-                                    manager,
-                                    dependency_i as u32,
-                                    &dependency,
-                                    invalid_package_id,
-                                    false,
-                                ) {
-                                    add_dependency_error(manager, &dependency, err);
-                                }
-                            }
-                        }
+                        reresolve_owned_rows(manager, &pinned_rows, |dependency| {
+                            all_name_hashes.binary_search(&dependency.name_hash).is_ok()
+                        })?;
                     }
 
                     if manager.summary.catalogs_changed {
@@ -560,33 +538,12 @@ pub fn install_with_manager(
                             .append_catalog_valued_name_hashes(&mut catalog_overridden);
                         catalog_overridden.sort_unstable();
                         catalog_overridden.dedup();
-                        let dependencies_len = manager.lockfile.buffers.dependencies.len();
-                        for _dep_id in 0..dependencies_len {
-                            let dep_id: DependencyID = u32::try_from(_dep_id).expect("int cast");
-                            if pinned_rows.is_set_allow_out_of_bound(_dep_id, false) {
-                                continue;
-                            }
-                            let dep =
-                                manager.lockfile.buffers.dependencies[dep_id as usize].clone();
-                            if dep.version.tag != DependencyVersionTag::Catalog
-                                && (catalog_overridden.is_empty()
-                                    || catalog_overridden.binary_search(&dep.name_hash).is_err())
-                            {
-                                continue;
-                            }
-
-                            manager.lockfile.buffers.resolutions[dep_id as usize] =
-                                invalid_package_id;
-                            if let Err(err) = enqueue_dependency_with_main(
-                                manager,
-                                dep_id,
-                                &dep,
-                                invalid_package_id,
-                                false,
-                            ) {
-                                add_dependency_error(manager, &dep, err);
-                            }
-                        }
+                        reresolve_owned_rows(manager, &pinned_rows, |dependency| {
+                            dependency.version.tag == DependencyVersionTag::Catalog
+                                || catalog_overridden
+                                    .binary_search(&dependency.name_hash)
+                                    .is_ok()
+                        })?;
                     }
 
                     // Split this into two passes because the below may allocate memory or invalidate pointers
@@ -1590,6 +1547,57 @@ fn enqueue_transitive(
     transitive.enqueue_tracked(manager)
 }
 
+/// Invalidates and re-resolves every row `selects`, except `pinned_rows` and rows no package owns any more:
+/// the differ just gave the root a fresh dependency list, so the rows it was loaded with are dead. The resolver
+/// decides how far to trust a row by its owner (`Lockfile::is_workspace_dependency`), so re-resolving a dead
+/// root row is wasted work at best and, for a `file:` path outside the project that nothing overrides any
+/// more, a resolution error.
+fn reresolve_owned_rows(
+    manager: &mut PackageManager,
+    pinned_rows: &DynamicBitSet,
+    selects: impl Fn(&Dependency) -> bool,
+) -> crate::Result<()> {
+    let owned_rows = {
+        let lockfile = &*manager.lockfile;
+        let mut owned = DynamicBitSet::init_empty(lockfile.buffers.dependencies.len())?;
+        for slice in lockfile.packages.items_dependencies() {
+            if slice.len == 0 {
+                continue;
+            }
+            owned.set_range_value(
+                Range {
+                    start: slice.begin() as usize,
+                    end: slice.end() as usize,
+                },
+                true,
+            );
+        }
+        owned
+    };
+    // Resolving may append packages and grow `buffers.dependencies`, so only the rows present when this pass
+    // started are visited, each copied out before the resolver runs.
+    for dep_id in 0..owned_rows.bit_length() {
+        if !owned_rows.is_set(dep_id) || pinned_rows.is_set_allow_out_of_bound(dep_id, false) {
+            continue;
+        }
+        let dependency = manager.lockfile.buffers.dependencies[dep_id].clone();
+        if !selects(&dependency) {
+            continue;
+        }
+        manager.lockfile.buffers.resolutions[dep_id] = invalid_package_id;
+        if let Err(err) = enqueue_dependency_with_main(
+            manager,
+            dep_id as DependencyID,
+            &dependency,
+            invalid_package_id,
+            false,
+        ) {
+            add_dependency_error(manager, &dependency, err);
+        }
+    }
+    Ok(())
+}
+
 #[derive(Default)]
 struct NamedUpdates {
     /// Invalidated rows paired with the package they resolved to, for redirect_moved_edges.

test/cli/install/catalogs.test.ts

@@ -259,6 +259,54 @@ describe("basic", () => {
     await runBunInstall(bunEnv, packageDir, { savesLockfile: false });
   });
 
+  // A file: path outside the project is only accepted on a row the root (or a workspace) owns. A catalog change
+  // re-resolves every catalog: row; the rows the root was loaded with have been replaced by then and belong to
+  // nobody, so re-resolving them fails the way an escaping transitive file: dependency does.
+  test.concurrent("changing the entry of a catalog: dependency pointing outside the project", async () => {
+    const packageJson = (xPath: string) =>
+      JSON.stringify({
+        name: "catalog-file-dep",
+        workspaces: { packages: [], catalog: { x: xPath } },
+        dependencies: { x: "catalog:" },
+      });
+    using dir = tempDir("catalog-file-dep", {
+      "x/package.json": JSON.stringify({ name: "x", version: "1.0.0" }),
+      "x2/package.json": JSON.stringify({ name: "x", version: "2.0.0" }),
+      "project/package.json": packageJson("file:../x"),
+    });
+    const packageDir = join(String(dir), "project");
+    const installedVersion = async () =>
+      (await file(join(packageDir, "node_modules", "x", "package.json")).json()).version;
+
+    await runBunInstall(bunEnv, packageDir);
+    expect(await installedVersion()).toBe("1.0.0");
+
+    await write(join(packageDir, "package.json"), packageJson("file:../x2"));
+    await runBunInstall(bunEnv, packageDir);
+    expect(await installedVersion()).toBe("2.0.0");
+    expect(normalizeBunSnapshot(await file(join(packageDir, "bun.lock")).text(), packageDir)).toMatchInlineSnapshot(`
+      "{
+        "lockfileVersion": 2,
+        "configVersion": 1,
+        "workspaces": {
+          "": {
+            "name": "catalog-file-dep",
+            "dependencies": {
+              "x": "catalog:",
+            },
+          },
+        },
+        "catalog": {
+          "x": "file:../x2",
+        },
+        "packages": {
+          "x": ["x@file:../x2", {}],
+        }
+      }"
+    `);
+    await runBunInstall(bunEnv, packageDir, { frozenLockfile: true });
+  });
+
   test.concurrent("catalog and catalogs.default may split different packages between them", async () => {
     const { packageDir } = await registry.createTestDir({
       files: {

test/cli/install/nested-overrides.test.ts

@@ -2,7 +2,7 @@ import { file, write } from "bun";
 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
 import { existsSync, realpathSync } from "fs";
 import { rm } from "fs/promises";
-import { VerdaccioRegistry, bunEnv, bunExe } from "harness";
+import { VerdaccioRegistry, bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
 import { join } from "path";
 
 const registry = new VerdaccioRegistry();
@@ -1290,6 +1290,105 @@ describe.concurrent("lockfile", () => {
     await installOk(dir, "--frozen-lockfile");
   });
 
+  // An overrides change re-resolves every row naming a previously or newly overridden package. By then the root's
+  // rows from bun.lock have been replaced by freshly parsed ones and belong to no package; they must not be
+  // re-resolved as well: a range that is no longer in package.json would add a package the current row then dedupes
+  // onto, and a file: path outside the project is only accepted on a row the root (or a workspace) owns.
+  describe.concurrent("removing a flat rule re-resolves only the root's current rows", () => {
+    test("a range changed in the same edit resolves on its own", async () => {
+      const dir = await project({ dependencies: { "no-deps": "~1.0.0" }, overrides: { "no-deps": "2.0.0" } });
+      await installOk(dir);
+      expect(await versionSeenBy(dir, undefined, "no-deps")).toBe("2.0.0");
+
+      await write(
+        join(dir, "package.json"),
+        JSON.stringify({ name: "nested-overrides", dependencies: { "no-deps": "^1.0.0" } }),
+      );
+      const { err } = await installOk(dir);
+      expect(err).toContain("Saved lockfile");
+      // 1.0.1 is what the dropped ~1.0.0 range would pick.
+      expect(await versionSeenBy(dir, undefined, "no-deps")).toBe("1.1.0");
+      const after = await lock(dir);
+      expect(after).not.toContain('"overrides"');
+      expect(after).not.toContain("no-deps@1.0.1");
+      expect(after).not.toContain("no-deps@2.0.0");
+      await installOk(dir, "--frozen-lockfile");
+    });
+
+    const outside = {
+      "x/package.json": JSON.stringify({ name: "x", version: "1.0.0" }),
+      "x2/package.json": JSON.stringify({ name: "x", version: "2.0.0" }),
+      "project/y/package.json": JSON.stringify({ name: "y", version: "1.0.0" }),
+    };
+    const rootPackageJson = (pkg: Record<string, unknown>) => JSON.stringify({ name: "nested-overrides", ...pkg });
+    const before = rootPackageJson({
+      dependencies: { x: "file:../x", y: "file:./y" },
+      overrides: { x: "file:../x2" },
+    });
+
+    test("a file: dependency outside the project re-resolves to its own path", async () => {
+      using root = tempDir("override-removed-file-dep", { ...outside, "project/package.json": before });
+      const dir = join(String(root), "project");
+      await installOk(dir);
+      expect(await versionSeenBy(dir, undefined, "x")).toBe("2.0.0");
+
+      await write(join(dir, "package.json"), rootPackageJson({ dependencies: { x: "file:../x", y: "file:./y" } }));
+      const { err } = await installOk(dir);
+      expect(err).toContain("Saved lockfile");
+      expect(await versionSeenBy(dir, undefined, "x")).toBe("1.0.0");
+      expect(normalizeBunSnapshot(await lock(dir), dir)).toMatchInlineSnapshot(`
+        "{
+          "lockfileVersion": 2,
+          "configVersion": 1,
+          "workspaces": {
+            "": {
+              "name": "nested-overrides",
+              "dependencies": {
+                "x": "file:../x",
+                "y": "file:./y",
+              },
+            },
+          },
+          "packages": {
+            "x": ["x@file:../x", {}],
+
+            "y": ["y@file:y", {}],
+          }
+        }"
+      `);
+      await installOk(dir, "--frozen-lockfile");
+    });
+
+    test("a file: dependency outside the project removed together with its rule", async () => {
+      using root = tempDir("override-and-file-dep-removed", { ...outside, "project/package.json": before });
+      const dir = join(String(root), "project");
+      await installOk(dir);
+      expect(await versionSeenBy(dir, undefined, "x")).toBe("2.0.0");
+
+      await write(join(dir, "package.json"), rootPackageJson({ dependencies: { y: "file:./y" } }));
+      const { err } = await installOk(dir);
+      expect(err).toContain("Saved lockfile");
+      expect(normalizeBunSnapshot(await lock(dir), dir)).toMatchInlineSnapshot(`
+        "{
+          "lockfileVersion": 2,
+          "configVersion": 1,
+          "workspaces": {
+            "": {
+              "name": "nested-overrides",
+              "dependencies": {
+                "y": "file:./y",
+              },
+            },
+          },
+          "packages": {
+            "y": ["y@file:y", {}],
+          }
+        }"
+      `);
+      await installOk(dir, "--frozen-lockfile");
+    });
+  });
+
   test("changing only the parent's range text is a frozen-lockfile change", async () => {
     const dir = await project({ dependencies: twoParents, overrides: { "one-fixed-dep@1": { "no-deps": "1.1.0" } } });
     await installOk(dir);