install: keep a bundle's dependency out of a slot the bundling package resolves through

src/install/lockfile/Tree.rs

@@ -1107,12 +1107,110 @@ impl Tree {
             }
         }
 
+        // a bundle's dependency stops here, but must not shadow what resolves through here
+        if !AS_DEFINED
+            && this.id == hoist_root_id
+            && Tree::hoist_root_resolves_elsewhere(
+                this.id,
+                package_id,
+                dependency.name_hash,
+                builder,
+            )
+        {
+            return Ok(HoistDependencyResult::DependencyLoop); // 3
+        }
+
         // place the dependency in the current tree
         Ok(HoistDependencyResult::Placement(Placement {
             id: this.id,
             bundled: false,
         })) // 2
     }
+
+    /// Whether a `name_hash` edge that a loaded `bun.lock` would rebind to `hoist_root` resolves elsewhere.
+    fn hoist_root_resolves_elsewhere<const METHOD: BuilderMethod>(
+        hoist_root: Id,
+        package_id: PackageID,
+        name_hash: PackageNameHash,
+        builder: &Builder<'_, METHOD>,
+    ) -> bool {
+        // an unbound optional peer saves no entry; once bound, `Lockfile::resolve` hoists again
+        if package_id == invalid_package_id {
+            return false;
+        }
+
+        let trees = builder.list.items_tree();
+        if trees[hoist_root as usize].parent == INVALID_ID {
+            return false;
+        }
+
+        let entry_lists = builder.list.items_dependencies();
+        let deps: &[Dependency] = builder.dependencies;
+        let resolutions: &[PackageID] = &*builder.resolutions;
+        let resolution_lists = builder.resolution_lists;
+
+        let resolves_elsewhere = |pkg_id: PackageID| -> bool {
+            let pkg_deps = resolution_lists[pkg_id as usize];
+            (pkg_deps.begin()..pkg_deps.end()).any(|dep_id| {
+                let dep = &deps[dep_id as usize];
+                dep.name_hash == name_hash
+                    && !dep.behavior.is_bundled()
+                    && resolutions[dep_id as usize] != package_id
+            })
+        };
+
+        // `tree_owner`'s regular dependencies placed in its own node_modules, `tree_id`
+        let nested_in = move |tree_id: Id, tree_owner: PackageID| {
+            let own_deps = resolution_lists[tree_owner as usize];
+            trees[tree_id as usize]
+                .dependencies
+                .get(entry_lists[tree_id as usize].as_slice())
+                .iter()
+                .filter(move |&&dep_id| {
+                    own_deps.contains(dep_id) && !deps[dep_id as usize].behavior.is_bundled()
+                })
+                .map(move |&dep_id| resolutions[dep_id as usize])
+                .filter(|&pkg_id| pkg_id != invalid_package_id)
+        };
+
+        let owner = resolutions[trees[hoist_root as usize].dependency_id as usize];
+        if resolves_elsewhere(owner) {
+            return true;
+        }
+
+        let mut any_nested = false;
+        for pkg_id in nested_in(hoist_root, owner) {
+            if resolves_elsewhere(pkg_id) {
+                return true;
+            }
+            any_nested = true;
+        }
+        if !any_nested {
+            return false;
+        }
+
+        // The nested packages' own trees hold more of them. A tree comes after its parent.
+        let mut scope: Vec<(Id, PackageID)> = vec![(hoist_root, owner)];
+        for (id, tree) in trees.iter().enumerate().skip(hoist_root as usize + 1) {
+            let Some(&(_, parent_owner)) =
+                scope.iter().find(|(scope_id, _)| *scope_id == tree.parent)
+            else {
+                continue;
+            };
+            if !resolution_lists[parent_owner as usize].contains(tree.dependency_id)
+                || deps[tree.dependency_id as usize].behavior.is_bundled()
+            {
+                continue;
+            }
+            let tree_owner = resolutions[tree.dependency_id as usize];
+            if nested_in(id as Id, tree_owner).any(&resolves_elsewhere) {
+                return true;
+            }
+            scope.push((id as Id, tree_owner));
+        }
+
+        false
+    }
 }
 
 // ──────────────────────────────────────────────────────────────────────────

test/cli/install/bun-lock.test.ts

@@ -1376,3 +1376,175 @@ it("an optional peer is rebound when another version of its package takes the sl
   await run(["install", "--lockfile-only"]);
   expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile);
 });
+
+// The bundled-shadow-* fixtures are described in
+// registry/packages/create-bundled-shadow-packages.ts. In short: host depends
+// on shared@1.0.0, which is hoisted to the root, and bundles inner, which
+// depends on shared@2.0.0. A bundle's dependencies hoist no further than the
+// bundling package's node_modules, but `host/shared` in bun.lock is also what
+// host's own `shared` edge resolves to when the lockfile is loaded again, and
+// likewise for anything nested under host that resolves shared from the root.
+// https://github.com/oven-sh/bun/issues/29263
+it("a bundled dependency's dependency does not take a slot the bundling package resolves through", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({
+    bunfigOpts: { saveTextLockfile: true, linker: "hoisted" },
+  });
+  const run = makeInstallRunner(packageDir);
+  const hostNodeModules = join(packageDir, "node_modules", "bundled-shadow-host", "node_modules");
+
+  await write(
+    packageJson,
+    JSON.stringify({
+      name: "foo",
+      dependencies: { "bundled-shadow-host": "1.0.0", "bundled-shadow-shared": "1.0.0" },
+    }),
+  );
+  await run(["install"]);
+  const fresh = await file(join(packageDir, "bun.lock")).text();
+  expect(fresh).toContain('"bundled-shadow-shared": ["bundled-shadow-shared@1.0.0"');
+  expect(fresh).toContain(
+    '"bundled-shadow-host/bundled-shadow-inner/bundled-shadow-shared": ["bundled-shadow-shared@2.0.0"',
+  );
+  expect(fresh).not.toContain('"bundled-shadow-host/bundled-shadow-shared"');
+
+  // inner's shared@2.0.0 ships inside host's tarball; host itself uses the root copy.
+  const layout = async () => ({
+    root: (await file(join(packageDir, "node_modules", "bundled-shadow-shared", "package.json")).json()).version,
+    host: await readdirSorted(hostNodeModules),
+    inner: (
+      await file(
+        join(hostNodeModules, "bundled-shadow-inner", "node_modules", "bundled-shadow-shared", "package.json"),
+      ).json()
+    ).version,
+  });
+  const expectedLayout = { root: "1.0.0", host: ["bundled-shadow-inner"], inner: "2.0.0" };
+  expect(await layout()).toEqual(expectedLayout);
+
+  // Every install that starts from the lockfile has to give host the same
+  // shared as the install that wrote it.
+  await run(["install"]);
+  expect(await layout()).toEqual(expectedLayout);
+
+  await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
+  await run(["install", "--frozen-lockfile"]);
+  expect(await layout()).toEqual(expectedLayout);
+
+  await run(["install", "--lockfile-only"]);
+  expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh);
+});
+
+it("the isolated linker links the bundling package against the same dependency on a reinstall", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({
+    bunfigOpts: { saveTextLockfile: true, linker: "isolated" },
+  });
+  const run = makeInstallRunner(packageDir);
+  const hostStoreEntry = join(packageDir, "node_modules", ".bun", "bundled-shadow-host@1.0.0", "node_modules");
+  const sharedVersion = async (...dir: string[]) =>
+    (await file(join(...dir, "bundled-shadow-shared", "package.json")).json()).version;
+  // `host` is what gets linked next to host in its store entry; `inner` is the
+  // copy that came out of host's tarball.
+  const layout = async () => ({
+    host: await sharedVersion(hostStoreEntry),
+    inner: await sharedVersion(
+      hostStoreEntry,
+      "bundled-shadow-host",
+      "node_modules",
+      "bundled-shadow-inner",
+      "node_modules",
+    ),
+  });
+
+  await write(
+    packageJson,
+    JSON.stringify({
+      name: "foo",
+      dependencies: { "bundled-shadow-host": "1.0.0", "bundled-shadow-shared": "1.0.0" },
+    }),
+  );
+  await run(["install"]);
+  expect(await layout()).toEqual({ host: "1.0.0", inner: "2.0.0" });
+
+  // The store is rebuilt from the resolutions bun.lock loads, which come from
+  // the saved paths.
+  await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
+  await run(["install", "--frozen-lockfile"]);
+  expect(await layout()).toEqual({ host: "1.0.0", inner: "2.0.0" });
+});
+
+it("a bundled dependency's dependency does not take a slot a dependency nested under the bundling package resolves through", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({
+    bunfigOpts: { saveTextLockfile: true, linker: "hoisted" },
+  });
+  const run = makeInstallRunner(packageDir);
+
+  // deep-host itself does not depend on shared. consumer@1.0.0 nests under it
+  // (consumer@2.0.0 holds the root), mid@1.0.0 nests under consumer (mid@2.0.0
+  // holds the root), and mid's shared@1.0.0 is hoisted to the root, up through
+  // deep-host's node_modules. deep-host bundles wrapper, whose inner depends on
+  // shared@2.0.0, and that is hoisted after mid's shared is already at the root.
+  await write(
+    packageJson,
+    JSON.stringify({
+      name: "foo",
+      dependencies: {
+        "bundled-shadow-deep-host": "1.0.0",
+        "bundled-shadow-consumer": "2.0.0",
+        "bundled-shadow-mid": "2.0.0",
+      },
+    }),
+  );
+  await run(["install"]);
+  const fresh = await file(join(packageDir, "bun.lock")).text();
+  expect(fresh).toContain('"bundled-shadow-shared": ["bundled-shadow-shared@1.0.0"');
+  expect(fresh).toContain(
+    '"bundled-shadow-deep-host/bundled-shadow-consumer/bundled-shadow-mid": ["bundled-shadow-mid@1.0.0"',
+  );
+  expect(fresh).toContain(
+    '"bundled-shadow-deep-host/bundled-shadow-inner/bundled-shadow-shared": ["bundled-shadow-shared@2.0.0"',
+  );
+  expect(fresh).not.toContain('"bundled-shadow-deep-host/bundled-shadow-shared"');
+
+  // mid@1.0.0 has to keep finding shared@1.0.0 at the root on a reinstall.
+  await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
+  await run(["install", "--frozen-lockfile"]);
+  expect({
+    root: (await file(join(packageDir, "node_modules", "bundled-shadow-shared", "package.json")).json()).version,
+    deepHost: await readdirSorted(join(packageDir, "node_modules", "bundled-shadow-deep-host", "node_modules")),
+  }).toEqual({ root: "1.0.0", deepHost: ["bundled-shadow-consumer", "bundled-shadow-wrapper"] });
+
+  await run(["install", "--lockfile-only"]);
+  expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh);
+});
+
+it("a bundled dependency's optional peer bound late does not take a slot the bundling package resolves through", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({
+    bunfigOpts: { saveTextLockfile: true, linker: "hoisted" },
+  });
+  const run = makeInstallRunner(packageDir);
+
+  // peer-inner (bundled) has an optional peer on shared and depends on
+  // peer-leaf, which depends on shared@2.0.0. The peer is still unbound when
+  // peer-inner is hoisted and gets bound to peer-leaf's shared@2.0.0 inside the
+  // bundle; peer-host's own shared@1.0.0 at the root must not be shadowed by
+  // either of them.
+  await write(packageJson, JSON.stringify({ name: "foo", dependencies: { "bundled-shadow-peer-host": "1.0.0" } }));
+  await run(["install"]);
+  const fresh = await file(join(packageDir, "bun.lock")).text();
+  expect(fresh).toContain('"bundled-shadow-shared": ["bundled-shadow-shared@1.0.0"');
+  expect(fresh).toContain(
+    '"bundled-shadow-peer-host/bundled-shadow-peer-inner/bundled-shadow-shared": ["bundled-shadow-shared@2.0.0"',
+  );
+  expect(fresh).toContain(
+    '"bundled-shadow-peer-host/bundled-shadow-peer-leaf/bundled-shadow-shared": ["bundled-shadow-shared@2.0.0"',
+  );
+  expect(fresh).not.toContain('"bundled-shadow-peer-host/bundled-shadow-shared"');
+
+  await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
+  await run(["install", "--frozen-lockfile"]);
+  expect(await readdirSorted(join(packageDir, "node_modules", "bundled-shadow-peer-host", "node_modules"))).toEqual([
+    "bundled-shadow-peer-inner",
+  ]);
+
+  await run(["install", "--lockfile-only"]);
+  expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh);
+});

test/cli/install/registry/packages/bundled-shadow-consumer/package.json

@@ -0,0 +1,32 @@
+{
+  "_id": "bundled-shadow-consumer",
+  "name": "bundled-shadow-consumer",
+  "dist-tags": {
+    "latest": "2.0.0"
+  },
+  "versions": {
+    "1.0.0": {
+      "name": "bundled-shadow-consumer",
+      "version": "1.0.0",
+      "dependencies": {
+        "bundled-shadow-mid": "1.0.0"
+      },
+      "_id": "bundled-shadow-consumer@1.0.0",
+      "dist": {
+        "integrity": "sha512-bYPAnSxRZS5jNQuvfR+PV/RC9MhWSCW0C+aB2GZi0xEQ9jmhUO+HAgK18dvtgF7IaXTqXM3F2ToI1eH5nsVLTA==",
+        "shasum": "825e428cd7ddc7af4be726cc1728ab0c735385da",
+        "tarball": "http://localhost:4873/bundled-shadow-consumer/-/bundled-shadow-consumer-1.0.0.tgz"
+      }
+    },
+    "2.0.0": {
+      "name": "bundled-shadow-consumer",
+      "version": "2.0.0",
+      "_id": "bundled-shadow-consumer@2.0.0",
+      "dist": {
+        "integrity": "sha512-5rOVsz3v9j2P1FaFAxsgnp50QmpGwjlPs3KO29gvUoO3RWlmYS5WQDJStSGMO8lzyWW+UCoR08IW3/WodfMjSg==",
+        "shasum": "79c93410f8ab5a6135323994a946978174e46dca",
+        "tarball": "http://localhost:4873/bundled-shadow-consumer/-/bundled-shadow-consumer-2.0.0.tgz"
+      }
+    }
+  }
+}

test/cli/install/registry/packages/bundled-shadow-deep-host/package.json

@@ -0,0 +1,26 @@
+{
+  "_id": "bundled-shadow-deep-host",
+  "name": "bundled-shadow-deep-host",
+  "dist-tags": {
+    "latest": "1.0.0"
+  },
+  "versions": {
+    "1.0.0": {
+      "name": "bundled-shadow-deep-host",
+      "version": "1.0.0",
+      "dependencies": {
+        "bundled-shadow-consumer": "1.0.0",
+        "bundled-shadow-wrapper": "1.0.0"
+      },
+      "bundleDependencies": [
+        "bundled-shadow-wrapper"
+      ],
+      "_id": "bundled-shadow-deep-host@1.0.0",
+      "dist": {
+        "integrity": "sha512-0AapvZZN+czyjr1l05qbGQaBHKNiLw+2BFxouwkst7BvNfJUv09ZgHtRL1tb7QGHhm1VwwR/Vg70Uz+IOzmeXA==",
+        "shasum": "9e14944bae0586dc76c6ec9f0cd68af710da5a94",
+        "tarball": "http://localhost:4873/bundled-shadow-deep-host/-/bundled-shadow-deep-host-1.0.0.tgz"
+      }
+    }
+  }
+}

test/cli/install/registry/packages/bundled-shadow-host/package.json

@@ -0,0 +1,26 @@
+{
+  "_id": "bundled-shadow-host",
+  "name": "bundled-shadow-host",
+  "dist-tags": {
+    "latest": "1.0.0"
+  },
+  "versions": {
+    "1.0.0": {
+      "name": "bundled-shadow-host",
+      "version": "1.0.0",
+      "dependencies": {
+        "bundled-shadow-inner": "1.0.0",
+        "bundled-shadow-shared": "1.0.0"
+      },
+      "bundleDependencies": [
+        "bundled-shadow-inner"
+      ],
+      "_id": "bundled-shadow-host@1.0.0",
+      "dist": {
+        "integrity": "sha512-8OfobtACp9H+WHwCFhoRxW43zR9DIrtdPucaIwom3zp5jNJYQ1d6yu7y4x2FG8bixlhxR+iQ8E3/p0P+c4Ir1A==",
+        "shasum": "44e59de3f83ac5536a0391ec996b6a7cda06d203",
+        "tarball": "http://localhost:4873/bundled-shadow-host/-/bundled-shadow-host-1.0.0.tgz"
+      }
+    }
+  }
+}