From c24fa9759abcf0fa443bb3e4537bf2c8caba0c9f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:35:26 +0000 Subject: [PATCH 1/3] install: stop hoisting a dependency cycle forever The tree builder nests a dependency inside its dependent whenever the nearest node_modules holding that name has a different version. Around a cycle through two versions of the same packages (x@1 -> y@1 -> x@2 -> y@2 -> x@1 ...) every copy conflicts with the copy one level up, so copies were nested without end and `bun install` never returned while its memory grew. A package bundling a dependency that depends back on it looped as well: the bundled subtree's walk stops at the bundler's own node_modules without seeing the bundler one level up, so the bundler was copied into itself and the copy bundled the dependency again. process_subtree now leaves a copy of a package that is nested below another copy of the same package without a node_modules of its own (the point at which npm links back to the copy above), which bounds every path by the number of packages, and hoist_dependency resolves a dependency onto the package whose node_modules it would otherwise be placed in. The bun.lock loader bound a package's dependencies from each of its rows in turn, so the unexpanded copy, always the deepest row, rebound them to the versions it sits under and a clean checkout installed a smaller tree while --frozen-lockfile passed. Rows below another row of the same package are skipped now; the copy the tree was built from binds the package. --- src/install/lockfile/Tree.rs | 35 ++++ src/install/lockfile/bun.lock.rs | 25 +++ test/cli/install/hoist.test.ts | 190 +++++++++++++++++- .../packages/create-hoist-cycle-packages.ts | 129 ++++++++++++ .../hoist-bundled-cycle-host-1.0.0.tgz | Bin 0 -> 265 bytes .../hoist-bundled-cycle-host/package.json | 25 +++ .../hoist-bundled-cycle-plugin-1.0.0.tgz | Bin 0 -> 208 bytes .../hoist-bundled-cycle-plugin/package.json | 22 ++ .../hoist-cycle-x/hoist-cycle-x-1.0.0.tgz | Bin 0 -> 188 bytes .../hoist-cycle-x/hoist-cycle-x-2.0.0.tgz | Bin 0 -> 188 bytes .../packages/hoist-cycle-x/package.json | 35 ++++ .../hoist-cycle-y/hoist-cycle-y-1.0.0.tgz | Bin 0 -> 192 bytes .../hoist-cycle-y/hoist-cycle-y-2.0.0.tgz | Bin 0 -> 192 bytes .../packages/hoist-cycle-y/package.json | 35 ++++ .../hoist-optional-peer-cycle-entry-1.0.0.tgz | Bin 0 -> 205 bytes .../package.json | 22 ++ .../hoist-optional-peer-cycle-x-1.0.0.tgz | Bin 0 -> 209 bytes .../hoist-optional-peer-cycle-x-2.0.0.tgz | Bin 0 -> 209 bytes .../hoist-optional-peer-cycle-x/package.json | 37 ++++ .../hoist-optional-peer-cycle-y-1.0.0.tgz | Bin 0 -> 209 bytes .../hoist-optional-peer-cycle-y-2.0.0.tgz | Bin 0 -> 232 bytes .../hoist-optional-peer-cycle-y/package.json | 40 ++++ .../hoist-optional-peer-cycle-z-1.0.0.tgz | Bin 0 -> 205 bytes .../hoist-optional-peer-cycle-z-2.0.0.tgz | Bin 0 -> 173 bytes .../hoist-optional-peer-cycle-z/package.json | 32 +++ 25 files changed, 625 insertions(+), 2 deletions(-) create mode 100644 test/cli/install/registry/packages/create-hoist-cycle-packages.ts create mode 100644 test/cli/install/registry/packages/hoist-bundled-cycle-host/hoist-bundled-cycle-host-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-bundled-cycle-host/package.json create mode 100644 test/cli/install/registry/packages/hoist-bundled-cycle-plugin/hoist-bundled-cycle-plugin-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-bundled-cycle-plugin/package.json create mode 100644 test/cli/install/registry/packages/hoist-cycle-x/hoist-cycle-x-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-cycle-x/hoist-cycle-x-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-cycle-x/package.json create mode 100644 test/cli/install/registry/packages/hoist-cycle-y/hoist-cycle-y-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-cycle-y/hoist-cycle-y-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-cycle-y/package.json create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-entry-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-entry/package.json create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-x-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-x-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-x/package.json create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-y-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-y-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-y/package.json create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-z/hoist-optional-peer-cycle-z-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-z/hoist-optional-peer-cycle-z-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/hoist-optional-peer-cycle-z/package.json diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index 66d3f9597a28..7d946e95776e 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -649,6 +649,29 @@ impl Tree { return Ok(()); } + // A copy of a package nested somewhere below another copy of itself only exists + // because a different version of its name shadows the copy above; its + // dependencies were all placed when that copy was processed. Laying them out again + // here would nest whichever of them the levels in between shadow, and in a cycle + // through two versions of the same names (a@1 -> b@1 -> a@2 -> b@2 -> a@1 ...) + // that shadows the next one in turn, so the copies never end. This is the point at + // which npm links back to the copy above; the copy stays as is here, and its + // dependencies resolve to whatever is on the path. bun.lock skips these copies when + // it binds dependencies from the tree, so this check and that one have to agree. + { + let trees = builder.list.items_tree(); + let mut ancestor_id = self.id; + while ancestor_id != INVALID_ID { + let ancestor = &trees[ancestor_id as usize]; + if (ancestor.dependency_id as usize) < builder.resolutions.len() + && builder.resolutions[ancestor.dependency_id as usize] == parent_pkg_id + { + return Ok(()); + } + ancestor_id = ancestor.parent; + } + } + builder.list.append(BuilderEntry { tree: Tree { parent: self.id, @@ -1106,6 +1129,18 @@ impl Tree { } } + // The package this node_modules belongs to is itself resolvable from everything + // inside it. The walk stops at a bundled root before reaching the folder that holds + // it, so a bundled dependency depending back on the package bundling it would copy + // that package into its own node_modules, and processing the copy bundles the + // dependency again, without end. + if (this.dependency_id as usize) < deps.len() + && builder.resolutions[this.dependency_id as usize] == package_id + && deps[this.dependency_id as usize].name_hash == target_name_hash + { + return Ok(HoistDependencyResult::Hoisted); // 1 + } + // place the dependency in the current tree Ok(HoistDependencyResult::Placement(Placement { id: this.id, diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index 2efb8de91097..ae51929573aa 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -1817,6 +1817,23 @@ impl PkgMap { } } +impl PkgMap { + /// Whether one of the folders `pkg_path` is nested in holds `pkg_id` as well. + /// Every folder on the way down to a package is a key of its own; a prefix cut + /// inside a scoped name is not a key and does not match anything. + fn is_below_copy_of(&self, pkg_path: &[u8], pkg_id: PackageID) -> bool { + let mut end: usize = 0; + while let Some(i) = strings::index_of_char_usize(&pkg_path[end..], b'/') { + end += i; + if self.get(&pkg_path[..end]) == Some(&pkg_id) { + return true; + } + end += 1; + } + false + } +} + // const PkgMap = struct {}; fn object_rows(expr: &Expr) -> &[JSON::E::PropertyJSON] { @@ -3230,6 +3247,14 @@ pub(crate) fn parse_into_binary_lockfile( continue; } + // `Tree::process_subtree` gives a copy nested below another copy of the same + // package no node_modules of its own, so from this path its dependencies walk + // to whatever versions the copy happens to sit under. The copy above, which the + // tree was built from, binds them; this row is only a folder. + if pkg_map.is_below_copy_of(pkg_path, pkg_id) { + continue; + } + // find resolutions. iterate up to root through the pkg path. let deps = pkg_deps[pkg_id as usize]; 'deps: for _dep_id in deps.begin()..deps.end() { diff --git a/test/cli/install/hoist.test.ts b/test/cli/install/hoist.test.ts index 12919c36b139..18ab3776405a 100644 --- a/test/cli/install/hoist.test.ts +++ b/test/cli/install/hoist.test.ts @@ -1,5 +1,6 @@ -import { afterAll, beforeAll, test } from "bun:test"; -import { VerdaccioRegistry, bunEnv, runBunInstall } from "harness"; +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { VerdaccioRegistry, bunEnv, bunExe, runBunInstall } from "harness"; +import { join } from "node:path"; const registry = new VerdaccioRegistry(); @@ -28,3 +29,188 @@ test("should handle resolving optional peer from multiple instances of same pack // this shouldn't hit an assertion await runBunInstall(bunEnv, packageDir); }); + +// The hoist-*-cycle-* fixtures are described in registry/packages/create-hoist-cycle-packages.ts. +// +// These graphs can only be laid out by nesting: every package in the cycle conflicts with the +// version of its name one level up, so each copy used to get another copy nested below it and +// `bun install` never finished. The tree now ends at the first copy of a package that is nested +// below another copy of itself; that copy gets no node_modules of its own, so its dependency in +// the cycle resolves to the conflicting version next to it (no finite layout of these graphs avoids +// that). The trees below are what that produces, keyed the way bun.lock keys `packages`. +// +// bun.lock has to load back into the same tree. That copy's row resolves its dependencies to the +// wrong versions by path, so the loader has to bind the package from the copy above it instead; the +// reload half of each test fails if it does not, without --frozen-lockfile noticing. + +type Linker = "hoisted" | "isolated"; + +async function install(cwd: string, ...args: string[]) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install", ...args], + cwd, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ args, stdout, stderr, exitCode }).toMatchObject({ + stderr: expect.not.stringContaining("error:"), + exitCode: 0, + }); +} + +async function lockfileTree(dir: string) { + const { packages } = Bun.JSONC.parse(await Bun.file(join(dir, "bun.lock")).text()) as { + packages: Record; + }; + return Object.fromEntries(Object.entries(packages).map(([path, [resolution]]) => [path, resolution])); +} + +async function installedPackageJsons(dir: string) { + const nodeModules = join(dir, "node_modules"); + return (await Array.fromAsync(new Bun.Glob("**/package.json").scan({ cwd: nodeModules, dot: true }))).sort(); +} + +// Installs `files` from scratch, then again in a new directory from the bun.lock that produced, +// and returns the first install's directory. +async function installFreshAndFromLockfile( + files: Record, + linker: Linker, + expectedTree: Record, +) { + const { packageDir } = await registry.createTestDir({ bunfigOpts: { linker }, files }); + await install(packageDir); + const lockfile = await Bun.file(join(packageDir, "bun.lock")).text(); + expect(await lockfileTree(packageDir)).toEqual(expectedTree); + + const { packageDir: reloadDir } = await registry.createTestDir({ + bunfigOpts: { linker }, + files: { ...files, "bun.lock": lockfile }, + }); + await install(reloadDir, "--frozen-lockfile"); + expect(await installedPackageJsons(reloadDir)).toEqual(await installedPackageJsons(packageDir)); + + // --lockfile-only always writes, so this is the tree a reload builds, printed back. + await install(reloadDir, "--lockfile-only"); + expect(await Bun.file(join(reloadDir, "bun.lock")).text()).toBe(lockfile); + + return packageDir; +} + +test.each(["hoisted", "isolated"] as const)( + "a dependency cycle through two versions of the same packages is installed (%s linker)", + async linker => { + const packageDir = await installFreshAndFromLockfile( + { "package.json": JSON.stringify({ name: "pkg", dependencies: { "hoist-cycle-x": "1.0.0" } }) }, + linker, + { + "hoist-cycle-x": "hoist-cycle-x@1.0.0", + "hoist-cycle-y": "hoist-cycle-y@1.0.0", + "hoist-cycle-y/hoist-cycle-x": "hoist-cycle-x@2.0.0", + "hoist-cycle-y/hoist-cycle-x/hoist-cycle-y": "hoist-cycle-y@2.0.0", + "hoist-cycle-y/hoist-cycle-x/hoist-cycle-y/hoist-cycle-x": "hoist-cycle-x@1.0.0", + // This y@1.0.0 is below the y@1.0.0 at the root, so it gets no node_modules of its own. + "hoist-cycle-y/hoist-cycle-x/hoist-cycle-y/hoist-cycle-x/hoist-cycle-y": "hoist-cycle-y@1.0.0", + }, + ); + + if (linker === "hoisted") { + expect(await installedPackageJsons(packageDir)).toEqual([ + "hoist-cycle-x/package.json", + "hoist-cycle-y/node_modules/hoist-cycle-x/node_modules/hoist-cycle-y/node_modules/hoist-cycle-x/node_modules/hoist-cycle-y/package.json", + "hoist-cycle-y/node_modules/hoist-cycle-x/node_modules/hoist-cycle-y/node_modules/hoist-cycle-x/package.json", + "hoist-cycle-y/node_modules/hoist-cycle-x/node_modules/hoist-cycle-y/package.json", + "hoist-cycle-y/node_modules/hoist-cycle-x/package.json", + "hoist-cycle-y/package.json", + ]); + } + }, +); + +test("a cycle entered at both of its versions, from the root and from a workspace, is installed", async () => { + await installFreshAndFromLockfile( + { + "package.json": JSON.stringify({ + name: "pkg", + workspaces: ["packages/*"], + dependencies: { "hoist-cycle-x": "1.0.0" }, + }), + "packages/app/package.json": JSON.stringify({ + name: "app", + version: "1.0.0", + dependencies: { "hoist-cycle-x": "2.0.0" }, + }), + }, + "hoisted", + { + "app": "app@workspace:packages/app", + "hoist-cycle-x": "hoist-cycle-x@1.0.0", + "hoist-cycle-y": "hoist-cycle-y@1.0.0", + "hoist-cycle-y/hoist-cycle-x": "hoist-cycle-x@2.0.0", + "hoist-cycle-y/hoist-cycle-x/hoist-cycle-y": "hoist-cycle-y@2.0.0", + "hoist-cycle-y/hoist-cycle-x/hoist-cycle-y/hoist-cycle-x": "hoist-cycle-x@1.0.0", + "hoist-cycle-y/hoist-cycle-x/hoist-cycle-y/hoist-cycle-x/hoist-cycle-y": "hoist-cycle-y@1.0.0", + // The workspace's branch enters the cycle at x@2.0.0, so it is x@2.0.0's copy that ends it. + "app/hoist-cycle-x": "hoist-cycle-x@2.0.0", + "app/hoist-cycle-x/hoist-cycle-y": "hoist-cycle-y@2.0.0", + "app/hoist-cycle-x/hoist-cycle-y/hoist-cycle-x": "hoist-cycle-x@1.0.0", + "app/hoist-cycle-x/hoist-cycle-y/hoist-cycle-x/hoist-cycle-y": "hoist-cycle-y@1.0.0", + "app/hoist-cycle-x/hoist-cycle-y/hoist-cycle-x/hoist-cycle-y/hoist-cycle-x": "hoist-cycle-x@2.0.0", + }, + ); +}); + +test.each(["hoisted", "isolated"] as const)( + "a bundled dependency with a peer on the package bundling it is installed (%s linker)", + async linker => { + // The plugin's peer is satisfied by the host it is bundled in. The host used to be copied into + // its own node_modules instead, and the copy bundled the plugin again below it, and so on. + await installFreshAndFromLockfile( + { "package.json": JSON.stringify({ name: "pkg", dependencies: { "hoist-bundled-cycle-host": "1.0.0" } }) }, + linker, + { + "hoist-bundled-cycle-host": "hoist-bundled-cycle-host@1.0.0", + "hoist-bundled-cycle-host/hoist-bundled-cycle-plugin": "hoist-bundled-cycle-plugin@1.0.0", + }, + ); + }, +); + +test("a cycle closed through an optional peer is installed", async () => { + await installFreshAndFromLockfile( + { + "package.json": JSON.stringify({ + name: "pkg", + dependencies: { "hoist-optional-peer-cycle-x": "2.0.0" }, + devDependencies: { "hoist-optional-peer-cycle-entry": "1.0.0" }, + }), + }, + "hoisted", + { + "hoist-optional-peer-cycle-entry": "hoist-optional-peer-cycle-entry@1.0.0", + "hoist-optional-peer-cycle-x": "hoist-optional-peer-cycle-x@2.0.0", + "hoist-optional-peer-cycle-y": "hoist-optional-peer-cycle-y@2.0.0", + "hoist-optional-peer-cycle-z": "hoist-optional-peer-cycle-z@2.0.0", + "hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-x": "hoist-optional-peer-cycle-x@1.0.0", + "hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-y": + "hoist-optional-peer-cycle-y@1.0.0", + "hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-z": + "hoist-optional-peer-cycle-z@1.0.0", + // y@1.0.0's peer on x@2.0.0, nested because x@1.0.0 is above it. + "hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-x": + "hoist-optional-peer-cycle-x@2.0.0", + // z@1.0.0's y@2.0.0, whose optional peer binds to the x@1.0.0 above it here. + "hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-z/hoist-optional-peer-cycle-y": + "hoist-optional-peer-cycle-y@2.0.0", + "hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-y": + "hoist-optional-peer-cycle-y@2.0.0", + "hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-z": + "hoist-optional-peer-cycle-z@2.0.0", + // That y@2.0.0 sits below x@2.0.0, so the bound x@1.0.0 is nested under it; this x@1.0.0 is + // below the x@1.0.0 under `entry` and is where the tree ends. + "hoist-optional-peer-cycle-entry/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-x": + "hoist-optional-peer-cycle-x@1.0.0", + }, + ); +}); diff --git a/test/cli/install/registry/packages/create-hoist-cycle-packages.ts b/test/cli/install/registry/packages/create-hoist-cycle-packages.ts new file mode 100644 index 000000000000..2ab5f295b71c --- /dev/null +++ b/test/cli/install/registry/packages/create-hoist-cycle-packages.ts @@ -0,0 +1,129 @@ +#!/usr/bin/env bun +/** + * Generates the `hoist-*-cycle-*` fixtures used by hoist.test.ts. + * + * Each shape is a dependency cycle the hoister can only lay out by nesting: + * every package in the cycle conflicts with the version of its name one level + * up, so without a cutoff the tree builder nests copies forever. + * + * hoist-cycle (plain dependencies, two versions of each name): + * + * - hoist-cycle-x@1.0.0 -> hoist-cycle-y@1.0.0 + * - hoist-cycle-y@1.0.0 -> hoist-cycle-x@2.0.0 + * - hoist-cycle-x@2.0.0 -> hoist-cycle-y@2.0.0 + * - hoist-cycle-y@2.0.0 -> hoist-cycle-x@1.0.0 + * + * hoist-bundled-cycle: a package bundling a plugin that has a peer dependency + * back on the package bundling it. + * + * - hoist-bundled-cycle-host@1.0.0 -> plugin@1.0.0 (bundled) + * - hoist-bundled-cycle-plugin@1.0.0 peer on host@1.0.0 + * + * hoist-optional-peer-cycle: the cycle is closed through an optional peer. + * An optional peer is bound while the tree is built, to whichever version of + * its name is closest to the copy being processed, and that one binding is + * shared by every copy of the package. With x@2.0.0 at the root and x@1.0.0 + * nested under `entry`, y@2.0.0 is reached both next to x@1.0.0 (through z, + * which binds the peer to it) and below a nested x@2.0.0, where that binding + * conflicts and x@1.0.0 is nested again, which starts the cycle over. + * + * - hoist-optional-peer-cycle-entry@1.0.0 -> x@1.0.0 + * - hoist-optional-peer-cycle-x@1.0.0 -> y@1.0.0, z@1.0.0 + * - hoist-optional-peer-cycle-x@2.0.0 -> y@2.0.0, z@2.0.0 + * - hoist-optional-peer-cycle-y@1.0.0 peer on x@2.0.0 + * - hoist-optional-peer-cycle-y@2.0.0 optional peer on x@1.0.0 + * - hoist-optional-peer-cycle-z@1.0.0 -> y@2.0.0 + * - hoist-optional-peer-cycle-z@2.0.0 no dependencies + */ + +import { mkdir, writeFile } from "fs/promises"; +import { join } from "path"; + +const packagesDir = import.meta.dir; + +type Manifest = { + version: string; + dependencies?: Record; + peerDependencies?: Record; + peerDependenciesMeta?: Record; + bundleDependencies?: string[]; +}; + +const cycle = "hoist-cycle-"; +const bundled = "hoist-bundled-cycle-"; +const optional = "hoist-optional-peer-cycle-"; + +const packages: Record = { + [`${cycle}x`]: [ + { version: "1.0.0", dependencies: { [`${cycle}y`]: "1.0.0" } }, + { version: "2.0.0", dependencies: { [`${cycle}y`]: "2.0.0" } }, + ], + [`${cycle}y`]: [ + { version: "1.0.0", dependencies: { [`${cycle}x`]: "2.0.0" } }, + { version: "2.0.0", dependencies: { [`${cycle}x`]: "1.0.0" } }, + ], + + [`${bundled}host`]: [ + { + version: "1.0.0", + dependencies: { [`${bundled}plugin`]: "1.0.0" }, + bundleDependencies: [`${bundled}plugin`], + }, + ], + [`${bundled}plugin`]: [{ version: "1.0.0", peerDependencies: { [`${bundled}host`]: "1.0.0" } }], + + [`${optional}entry`]: [{ version: "1.0.0", dependencies: { [`${optional}x`]: "1.0.0" } }], + [`${optional}x`]: [ + { version: "1.0.0", dependencies: { [`${optional}y`]: "1.0.0", [`${optional}z`]: "1.0.0" } }, + { version: "2.0.0", dependencies: { [`${optional}y`]: "2.0.0", [`${optional}z`]: "2.0.0" } }, + ], + [`${optional}y`]: [ + { version: "1.0.0", peerDependencies: { [`${optional}x`]: "2.0.0" } }, + { + version: "2.0.0", + peerDependencies: { [`${optional}x`]: "1.0.0" }, + peerDependenciesMeta: { [`${optional}x`]: { optional: true } }, + }, + ], + [`${optional}z`]: [{ version: "1.0.0", dependencies: { [`${optional}y`]: "2.0.0" } }, { version: "2.0.0" }], +}; + +for (const [name, manifests] of Object.entries(packages)) { + const dir = join(packagesDir, name); + await mkdir(dir, { recursive: true }); + + const versions: Record = {}; + let latest = ""; + for (const manifest of manifests) { + const pkgJson = { name, ...manifest }; + const files: Record = { "package/package.json": JSON.stringify(pkgJson, null, 2) }; + for (const dep of manifest.bundleDependencies ?? []) { + files[`package/node_modules/${dep}/package.json`] = JSON.stringify( + { name: dep, version: manifest.dependencies![dep] }, + null, + 2, + ); + } + const tarball = join(dir, `${name}-${manifest.version}.tgz`); + await Bun.Archive.write(tarball, files, { compress: "gzip" }); + + const bytes = await Bun.file(tarball).bytes(); + versions[manifest.version] = { + ...pkgJson, + _id: `${name}@${manifest.version}`, + dist: { + integrity: `sha512-${Buffer.from(new Bun.CryptoHasher("sha512").update(bytes).digest()).toString("base64")}`, + shasum: new Bun.CryptoHasher("sha1").update(bytes).digest("hex"), + tarball: `http://localhost:4873/${name}/-/${name}-${manifest.version}.tgz`, + }, + }; + latest = manifest.version; + } + + await writeFile( + join(dir, "package.json"), + JSON.stringify({ _id: name, name, "dist-tags": { latest }, versions }, null, 2), + ); +} + +console.log("Created hoist cycle test packages"); diff --git a/test/cli/install/registry/packages/hoist-bundled-cycle-host/hoist-bundled-cycle-host-1.0.0.tgz b/test/cli/install/registry/packages/hoist-bundled-cycle-host/hoist-bundled-cycle-host-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..37507eb9689eda5df9cf21eada4c17429988e27e GIT binary patch literal 265 zcmb2|=3oGW|8KAOW*s&VU`<$C+U5BAmfZg>Z0oDLgSxVLJEPr=CdO(gmg^shG*LWy zp{3=(Z!=~mDaC`c_vP8WZdLgk^0@!O++7A~by{3Odx~Rb7W-+|p5b~n=kp#FzhXUZ z*EQ=Bc%1i!)qIOR-oIN|+BYR;uTb#B$X{kW7k|IVQ8&xlH`{N~QvT-33og&RuPyv~ zE+pmBjO2M?Yri~Nvt!xR1g%>cN>+DQzCY&r`}GdBTkoQLZqMVF-h0OElFRz4)emHT z-%&poT^U_zT_!YXzy06%;v~KQnx7N@ZC;b&_HTWn+5g`g=FVg2kcW5`P99kMm!V66 K(}_WYfdK$Oqj+2Z literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/hoist-bundled-cycle-host/package.json b/test/cli/install/registry/packages/hoist-bundled-cycle-host/package.json new file mode 100644 index 000000000000..3ab9a86e8308 --- /dev/null +++ b/test/cli/install/registry/packages/hoist-bundled-cycle-host/package.json @@ -0,0 +1,25 @@ +{ + "_id": "hoist-bundled-cycle-host", + "name": "hoist-bundled-cycle-host", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "hoist-bundled-cycle-host", + "version": "1.0.0", + "dependencies": { + "hoist-bundled-cycle-plugin": "1.0.0" + }, + "bundleDependencies": [ + "hoist-bundled-cycle-plugin" + ], + "_id": "hoist-bundled-cycle-host@1.0.0", + "dist": { + "integrity": "sha512-SFFfaFA5g9jnrCbqHjFhQ2jfd+GFqWQWrwJw3RboD8z/Pn091rfOCiVfyoYwCoByZuCUyCiHojzoXIXYFlSYXA==", + "shasum": "51b0aeed7e396f12316b46e763fa6cb1972ecb63", + "tarball": "http://localhost:4873/hoist-bundled-cycle-host/-/hoist-bundled-cycle-host-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/hoist-bundled-cycle-plugin/hoist-bundled-cycle-plugin-1.0.0.tgz b/test/cli/install/registry/packages/hoist-bundled-cycle-plugin/hoist-bundled-cycle-plugin-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..722a07a00cc6ccaa5ec3359a4fa569f5b2c5881e GIT binary patch literal 208 zcmb2|=3oGW|8LJ9^g8Sy()uv+xu)J`vyaO>c~jnYA9-@g?QzNpm8L2Cb|;8C6)JZH z{MdiCaks^E;obk1THmPMlk#!Svj_iXtg^eZ)#7i-+*Q_hy?5Tfx@u}zsB%=;Efc>h zf17pJ-hK4t`u=st?{);MJ#zk@wNJEu&hahJ{G%n*q(A-JJ71^&a+yT){?jghX6t6o zY0f`7y;ZDgr`zVI7Z;Lm;UEtY>>tsrcfp3V+&G?e_YlK-Y`QU-$t G0|Nji{$=?9 literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/hoist-bundled-cycle-plugin/package.json b/test/cli/install/registry/packages/hoist-bundled-cycle-plugin/package.json new file mode 100644 index 000000000000..d7ee81ad39b1 --- /dev/null +++ b/test/cli/install/registry/packages/hoist-bundled-cycle-plugin/package.json @@ -0,0 +1,22 @@ +{ + "_id": "hoist-bundled-cycle-plugin", + "name": "hoist-bundled-cycle-plugin", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "hoist-bundled-cycle-plugin", + "version": "1.0.0", + "peerDependencies": { + "hoist-bundled-cycle-host": "1.0.0" + }, + "_id": "hoist-bundled-cycle-plugin@1.0.0", + "dist": { + "integrity": "sha512-Ln5B/R9XfmUnYRRzx4BXwV8ncQ+xWCWkga1eiuX+HwgYsiCYZlsZfFmz0i+R86eYv/oKuQo5Cy8qxGr1vqp8BQ==", + "shasum": "5de566ac4690ed53cf7d3e5a6590c5bef47c6ef0", + "tarball": "http://localhost:4873/hoist-bundled-cycle-plugin/-/hoist-bundled-cycle-plugin-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/hoist-cycle-x/hoist-cycle-x-1.0.0.tgz b/test/cli/install/registry/packages/hoist-cycle-x/hoist-cycle-x-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..4fbb67e76b27d285dbe56eb4c64cad25ac73496d GIT binary patch literal 188 zcmb2|=3oGW|8LKq<~n2`(E4!AXA$RXQR>^GUfiA{wI%LSQi#aMyR5MtdAdy>>Sr@E zw?F(+x_RaF#7p+Z(aZH_HLiTMN8`wg!|AJj%@ntvEw*asvr5fZ#k$_gS#=L)@O{1a z;`)BO<9|C>ixzIY#~R)lU7BmvGuwMJ@8iwVk%Hx1zps>C41H9yY^L4%?hEZZI5uWF k7fp>>+p*`m{`xl2oi}GQLL3Msd0z50Yu3JD&|qKy09wvfZU6uP literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/hoist-cycle-x/hoist-cycle-x-2.0.0.tgz b/test/cli/install/registry/packages/hoist-cycle-x/hoist-cycle-x-2.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..72032921d3a88b0df3037bc89f2d84a944f543f2 GIT binary patch literal 188 zcmb2|=3oGW|8LKqIpeEpehT@*Nyf*#)o*BZ{dm{3u4FaGq5rvj zA~IdZ+wQLV+rR&@ZFj2Nq-ta1->bC>%RbLZOB7Q%vutW)W@==2>!nw>I(PElcpkgE z^xLQP&kN%py>T_!I(tR$i=-Ey^DfQZRXS(Rm*d?p6!VJhRdQXN6I<`D|7>y0q<`h> nP&t=hS-oF|Q8Vn2o(lS{{ literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/hoist-cycle-y/hoist-cycle-y-2.0.0.tgz b/test/cli/install/registry/packages/hoist-cycle-y/hoist-cycle-y-2.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..752a17fcfc9e65b60ee01df0bb01abede34b5aaa GIT binary patch literal 192 zcmb2|=3oGW|8LKq?mFZk(E2d$d$7n>IpeEpehT@*Nyf*#)o*BZ{dm{3u4FaGq5rvj zA~IdZ+wQLV+rR&@ZFj2Nq-ta1->bC>%RbLZOB7Q%vutW)W@==2>!nw>I(PElcpkgE z^xLQP&kN%py>T_!I(tR$i=-Ey^DfQZRXS(Rm*d?p6!VJhRdQXHw~ChSKlfPTvdyw} n{gKj^)q<64v|qOB`(Xt?``Bf#)L1ynXFG1azVCilTio_$;nM#P zwyrBI{PjA1`qOnsHVS^!+&4r1sQt5@>vHv8&k4S>{b)|+^21xUU%RwR=8v!Pqx(54 zi~Z_E-~;7vnDe`Tnr`u9AVdOUvI#m!N33j DiS1y@ literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/hoist-optional-peer-cycle-entry/package.json b/test/cli/install/registry/packages/hoist-optional-peer-cycle-entry/package.json new file mode 100644 index 000000000000..604e9a659ecc --- /dev/null +++ b/test/cli/install/registry/packages/hoist-optional-peer-cycle-entry/package.json @@ -0,0 +1,22 @@ +{ + "_id": "hoist-optional-peer-cycle-entry", + "name": "hoist-optional-peer-cycle-entry", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "hoist-optional-peer-cycle-entry", + "version": "1.0.0", + "dependencies": { + "hoist-optional-peer-cycle-x": "1.0.0" + }, + "_id": "hoist-optional-peer-cycle-entry@1.0.0", + "dist": { + "integrity": "sha512-FDfiSd8eCpyJ3n324Hk8KdEoNLkB/go8tQvT9txL1qt/ULRhuzYfCjT0StP5a3pYykhvjQFYA17VzF1ZMPsEqw==", + "shasum": "be52741a45e564b667febacfdcfcd2433ba474ef", + "tarball": "http://localhost:4873/hoist-optional-peer-cycle-entry/-/hoist-optional-peer-cycle-entry-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-x-1.0.0.tgz b/test/cli/install/registry/packages/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-x-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..9c948777879fc7d44a810128b56b461184144b4b GIT binary patch literal 209 zcmb2|=3oGW|8LJ9^kOy?IQDVQXA$Rf@2p;~6aAKZ$m~Uy@1Zo&?3cT_zbSmX(7iO^ z$A3NM!{)|5oR70By1qY+xIKN=Rqw3 z>HDshn=kg~&$TO8{uXv*=?Ag93hylaOEZfim(4k}P1Eq)*&Wi`H~uUs4?9wOWT&6y zPRBX2Z?t`j>`ckln4zBzt>q;}B;c4mm9p`^jF8TrkIZXX#m G7#IMwI%MGh literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-x-2.0.0.tgz b/test/cli/install/registry/packages/hoist-optional-peer-cycle-x/hoist-optional-peer-cycle-x-2.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..e194805325bc7779e59fd2d9c3b7a2f109ddb6c9 GIT binary patch literal 209 zcmb2|=3oGW|8LJ9^g8SyaO|V)`(U1GvqG~@(NAXHyE3-C)4Qp0?NYh;8^<>b#a%W2 z)K71GI9qaA%S%g}Be#oIy$wEl@{eKYU7lu%zeRDOyL-j!uk)_j`|a=E74P>}DQEpV zm>qjK>C5%)=jQJgV%L5YRMD9y@_okhU1mGi6#FG!pCU0m?Uw)U8+&$3Yd^a8DCqf| zI~wb|RTWFaQ6q)b5?z8(Rg?81t@r%w*7DU;qHm Con-w0 literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-y-2.0.0.tgz b/test/cli/install/registry/packages/hoist-optional-peer-cycle-y/hoist-optional-peer-cycle-y-2.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..d489f89c7b6e7fae80dffd13c65b9fd8927fe452 GIT binary patch literal 232 zcmb2|=3oGW|8LJbdLMQWV0~cwetpH6n@2n+Ydv{$Res|U$Hk?aRiZae`BScV&|6SY zWZ~1=>5UIRws|hB=Ik$Y&&^DZ~? XyxfexZ#po5-U)W2SqhyD8Vn2o<}Y#b literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/hoist-optional-peer-cycle-y/package.json b/test/cli/install/registry/packages/hoist-optional-peer-cycle-y/package.json new file mode 100644 index 000000000000..86abf8665e9c --- /dev/null +++ b/test/cli/install/registry/packages/hoist-optional-peer-cycle-y/package.json @@ -0,0 +1,40 @@ +{ + "_id": "hoist-optional-peer-cycle-y", + "name": "hoist-optional-peer-cycle-y", + "dist-tags": { + "latest": "2.0.0" + }, + "versions": { + "1.0.0": { + "name": "hoist-optional-peer-cycle-y", + "version": "1.0.0", + "peerDependencies": { + "hoist-optional-peer-cycle-x": "2.0.0" + }, + "_id": "hoist-optional-peer-cycle-y@1.0.0", + "dist": { + "integrity": "sha512-K6r6RYiWvhsjN0UMpVoBXWRvE7tUUXGIiS1ZxMf+HW4dsgMNXB+hfWMoQ8DlwNHZuM1kTdSIkc5u4rDIrf1x0A==", + "shasum": "c450c4030181e5c778268eab31bffa98d595cfbf", + "tarball": "http://localhost:4873/hoist-optional-peer-cycle-y/-/hoist-optional-peer-cycle-y-1.0.0.tgz" + } + }, + "2.0.0": { + "name": "hoist-optional-peer-cycle-y", + "version": "2.0.0", + "peerDependencies": { + "hoist-optional-peer-cycle-x": "1.0.0" + }, + "peerDependenciesMeta": { + "hoist-optional-peer-cycle-x": { + "optional": true + } + }, + "_id": "hoist-optional-peer-cycle-y@2.0.0", + "dist": { + "integrity": "sha512-ycp0N508WdyGz/ECodwmYUWevdQrSeCSKvjjUFL+oEkJyz0l8lKn7cHcD7Png+oQ5heJbJUkje91bIZKhyeLaA==", + "shasum": "d2befacfc7eb260db2369e59d2d3ce0755af5d23", + "tarball": "http://localhost:4873/hoist-optional-peer-cycle-y/-/hoist-optional-peer-cycle-y-2.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/hoist-optional-peer-cycle-z/hoist-optional-peer-cycle-z-1.0.0.tgz b/test/cli/install/registry/packages/hoist-optional-peer-cycle-z/hoist-optional-peer-cycle-z-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..292f583dca19939707963464904e487e2e1e3c83 GIT binary patch literal 205 zcmb2|=3oGW|8LLV^*Zbz(Du;wTqsX<&Z8&G*^11)tunT})4Qp0Ai#QJb zo>OCBU|eH)M*rc(cb`pXKcBSdcj~HN20V|xJTeQlzU#UBHQUv=-*qCn)w1sDul^l~ zzIo`?m*e~MPX85N?seq&hpalG|5K)Ix~iG)-Ssdaee>3N{htrM;;#?y{81i#deWl# zN4EExRn2{;8+|8wj<|N+q+nI0cvGhaTp@A!>MNN~wxaW#*qRUzFIgg&WEzZwB zd;V`{v{F)a`Il{CySa1cTr%* Date: Sat, 15 Aug 2026 09:56:44 +0000 Subject: [PATCH 2/3] test: compare installed paths with forward slashes on Windows --- test/cli/install/hoist.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/cli/install/hoist.test.ts b/test/cli/install/hoist.test.ts index 18ab3776405a..b5b480b60ab7 100644 --- a/test/cli/install/hoist.test.ts +++ b/test/cli/install/hoist.test.ts @@ -69,7 +69,8 @@ async function lockfileTree(dir: string) { async function installedPackageJsons(dir: string) { const nodeModules = join(dir, "node_modules"); - return (await Array.fromAsync(new Bun.Glob("**/package.json").scan({ cwd: nodeModules, dot: true }))).sort(); + const paths = await Array.fromAsync(new Bun.Glob("**/package.json").scan({ cwd: nodeModules, dot: true })); + return paths.map(path => path.replaceAll("\\", "/")).sort(); } // Installs `files` from scratch, then again in a new directory from the bun.lock that produced, From 86a1bc6c90ad7da766f4dd15c42862ca527cbef9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:02:28 +0000 Subject: [PATCH 3/3] install: shorten the comments on the cycle checks --- src/install/lockfile/Tree.rs | 21 +++++++-------------- src/install/lockfile/bun.lock.rs | 12 +++++------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index 7d946e95776e..45696237fb26 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -649,15 +649,10 @@ impl Tree { return Ok(()); } - // A copy of a package nested somewhere below another copy of itself only exists - // because a different version of its name shadows the copy above; its - // dependencies were all placed when that copy was processed. Laying them out again - // here would nest whichever of them the levels in between shadow, and in a cycle - // through two versions of the same names (a@1 -> b@1 -> a@2 -> b@2 -> a@1 ...) - // that shadows the next one in turn, so the copies never end. This is the point at - // which npm links back to the copy above; the copy stays as is here, and its - // dependencies resolve to whatever is on the path. bun.lock skips these copies when - // it binds dependencies from the tree, so this check and that one have to agree. + // Below another copy of itself, a package's dependencies are already placed; laying + // them out again only re-nests what the levels in between shadow, which for a cycle + // through two versions (a@1 -> b@1 -> a@2 -> b@2 -> a@1 ...) never ends. npm stops + // here as well (linking to the copy above); bun.lock.rs skips these copies to match. { let trees = builder.list.items_tree(); let mut ancestor_id = self.id; @@ -1129,11 +1124,9 @@ impl Tree { } } - // The package this node_modules belongs to is itself resolvable from everything - // inside it. The walk stops at a bundled root before reaching the folder that holds - // it, so a bundled dependency depending back on the package bundling it would copy - // that package into its own node_modules, and processing the copy bundles the - // dependency again, without end. + // A bundled root's walk ends below the folder holding the bundling package, which is + // resolvable from inside it; a bundled dependency depending back on it would otherwise + // copy it into itself, and the copy's bundled dependencies bring it back, forever. if (this.dependency_id as usize) < deps.len() && builder.resolutions[this.dependency_id as usize] == package_id && deps[this.dependency_id as usize].name_hash == target_name_hash diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index ae51929573aa..ff2ffc856d7b 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -1818,9 +1818,8 @@ impl PkgMap { } impl PkgMap { - /// Whether one of the folders `pkg_path` is nested in holds `pkg_id` as well. - /// Every folder on the way down to a package is a key of its own; a prefix cut - /// inside a scoped name is not a key and does not match anything. + /// Whether a folder on the way down to `pkg_path` holds `pkg_id` too. Each of those + /// folders is a key of its own; a prefix ending inside a scoped name matches nothing. fn is_below_copy_of(&self, pkg_path: &[u8], pkg_id: PackageID) -> bool { let mut end: usize = 0; while let Some(i) = strings::index_of_char_usize(&pkg_path[end..], b'/') { @@ -3247,10 +3246,9 @@ pub(crate) fn parse_into_binary_lockfile( continue; } - // `Tree::process_subtree` gives a copy nested below another copy of the same - // package no node_modules of its own, so from this path its dependencies walk - // to whatever versions the copy happens to sit under. The copy above, which the - // tree was built from, binds them; this row is only a folder. + // A copy below another copy of its package has no node_modules of its own + // (`Tree::process_subtree`), so walking up from it finds the versions it sits + // under rather than the package's resolutions; the copy above binds those. if pkg_map.is_below_copy_of(pkg_path, pkg_id) { continue; }