diff --git a/src/cli/package_manager_command.zig b/src/cli/package_manager_command.zig index e50dcbfa33..ba91e85f22 100644 --- a/src/cli/package_manager_command.zig +++ b/src/cli/package_manager_command.zig @@ -347,6 +347,10 @@ pub const PackageManagerCommand = struct { }); } + if (directories.items.len == 0) { + return; + } + const first_directory = directories.orderedRemove(0); var more_packages = try ctx.allocator.alloc(bool, max_depth); @@ -532,6 +536,12 @@ fn printNodeModulesFolderStructure( } const package_id = lockfile.buffers.resolutions.items[dependency_id]; + + if (package_id >= lockfile.packages.len) { + // in case we are loading from a binary lockfile with invalid package ids + continue; + } + var dir_index: usize = 0; var found_node_modules = false; while (dir_index < directories.items.len) : (dir_index += 1) { @@ -589,12 +599,6 @@ const PmPkgCommand = @import("./pm_pkg_command.zig").PmPkgCommand; const PmVersionCommand = @import("./pm_version_command.zig").PmVersionCommand; const PmWhyCommand = @import("./pm_why_command.zig").PmWhyCommand; -const Install = @import("../install/install.zig"); -const DependencyID = Install.DependencyID; -const Npm = Install.Npm; -const PackageID = Install.PackageID; -const PackageManager = Install.PackageManager; - const DefaultTrustedCommand = @import("./pm_trusted_command.zig").DefaultTrustedCommand; const TrustCommand = @import("./pm_trusted_command.zig").TrustCommand; const UntrustedCommand = @import("./pm_trusted_command.zig").UntrustedCommand; @@ -606,3 +610,9 @@ const Output = bun.Output; const log = bun.log; const strings = bun.strings; const File = bun.sys.File; + +const install = bun.install; +const DependencyID = install.DependencyID; +const Npm = install.Npm; +const PackageID = install.PackageID; +const PackageManager = install.PackageManager; diff --git a/src/cli/pm_trusted_command.zig b/src/cli/pm_trusted_command.zig index 90aee43433..817f724516 100644 --- a/src/cli/pm_trusted_command.zig +++ b/src/cli/pm_trusted_command.zig @@ -67,6 +67,11 @@ pub const UntrustedCommand = struct { const dep = pm.lockfile.buffers.dependencies.items[dep_id]; const alias = dep.name.slice(buf); const package_id = pm.lockfile.buffers.resolutions.items[dep_id]; + + if (package_id >= packages.len) { + continue; + } + const resolution = &resolutions[package_id]; var package_scripts = scripts[package_id]; @@ -230,9 +235,11 @@ pub const TrustCommand = struct { const dep = pm.lockfile.buffers.dependencies.items[dep_id]; const alias = dep.name.slice(buf); const package_id = pm.lockfile.buffers.resolutions.items[dep_id]; - if (comptime Environment.allow_assert) { - bun.assertWithLocation(package_id != Install.invalid_package_id, @src()); + + if (package_id >= packages.len) { + continue; } + const resolution = &resolutions[package_id]; var package_scripts = scripts[package_id]; diff --git a/src/install/PackageInstaller.zig b/src/install/PackageInstaller.zig index eb2fa02a63..91e2112022 100644 --- a/src/install/PackageInstaller.zig +++ b/src/install/PackageInstaller.zig @@ -22,7 +22,6 @@ pub const PackageInstaller = struct { destination_dir_subpath_buf: bun.PathBuffer = undefined, folder_path_buf: bun.PathBuffer = undefined, successfully_installed: Bitset, - tree_iterator: *Lockfile.Tree.Iterator(.node_modules), command_ctx: Command.Context, current_tree_id: Lockfile.Tree.Id = Lockfile.Tree.invalid_id, diff --git a/src/install/hoisted_install.zig b/src/install/hoisted_install.zig index 456921494c..3efddb055a 100644 --- a/src/install/hoisted_install.zig +++ b/src/install/hoisted_install.zig @@ -172,7 +172,6 @@ pub fn installHoistedPackages( this.allocator, this.lockfile.packages.len, ), - .tree_iterator = &iterator, .command_ctx = ctx, .tree_ids_to_trees_the_id_depends_on = tree_ids_to_trees_the_id_depends_on, .completed_trees = completed_trees, @@ -204,10 +203,6 @@ pub fn installHoistedPackages( var remaining = node_modules.dependencies; installer.current_tree_id = node_modules.tree_id; - if (comptime Environment.allow_assert) { - bun.assert(node_modules.dependencies.len == this.lockfile.buffers.trees.items[installer.current_tree_id].dependencies.len); - } - // cache line is 64 bytes on ARM64 and x64 // PackageIDs are 4 bytes // Hence, we can fit up to 64 / 4 = 16 package IDs in a cache line diff --git a/src/install/lockfile/Tree.zig b/src/install/lockfile/Tree.zig index 471876ae39..a49751adfe 100644 --- a/src/install/lockfile/Tree.zig +++ b/src/install/lockfile/Tree.zig @@ -295,19 +295,27 @@ pub fn Builder(comptime method: BuilderMethod) type { total += tree.dependencies.len; } - var dependency_ids = try DependencyIDList.initCapacity(z_allocator, total); - var next = PackageIDSlice{}; + var dep_ids = try DependencyIDList.initCapacity(this.allocator, total); for (trees, dependencies) |*tree, *child| { - if (tree.dependencies.len > 0) { - const len = @as(PackageID, @truncate(child.items.len)); - next.off += next.len; - next.len = len; - tree.dependencies = next; - dependency_ids.appendSliceAssumeCapacity(child.items); - child.deinit(this.allocator); + defer child.deinit(this.allocator); + + const off: u32 = @intCast(dep_ids.items.len); + for (child.items) |dep_id| { + const pkg_id = this.lockfile.buffers.resolutions.items[dep_id]; + if (pkg_id == invalid_package_id) { + // optional peers that never resolved + continue; + } + + dep_ids.appendAssumeCapacity(dep_id); } + const len: u32 = @intCast(dep_ids.items.len - off); + + tree.dependencies.off = off; + tree.dependencies.len = len; } + this.queue.deinit(); this.sort_buf.deinit(this.allocator); this.pending_optional_peers.deinit(); @@ -321,7 +329,7 @@ pub fn Builder(comptime method: BuilderMethod) type { return .{ .trees = std.ArrayListUnmanaged(Tree).fromOwnedSlice(trees), - .dep_ids = dependency_ids, + .dep_ids = dep_ids, }; } }; @@ -779,7 +787,6 @@ const Output = bun.Output; const Path = bun.path; const assert = bun.assert; const logger = bun.logger; -const z_allocator = bun.z_allocator; const Bitset = bun.bit_set.DynamicBitSetUnmanaged; const String = bun.Semver.String; @@ -795,7 +802,6 @@ const invalid_package_id = install.invalid_package_id; const Lockfile = install.Lockfile; const DependencyIDList = Lockfile.DependencyIDList; const ExternalSlice = Lockfile.ExternalSlice; -const PackageIDSlice = Lockfile.PackageIDSlice; const PackageManager = bun.install.PackageManager; const WorkspaceFilter = install.PackageManager.WorkspaceFilter; diff --git a/test/cli/install/__snapshots__/migrate-bun-lockb-v2.test.ts.snap b/test/cli/install/__snapshots__/migrate-bun-lockb-v2.test.ts.snap index 27d3a4899d..e0c6629381 100644 --- a/test/cli/install/__snapshots__/migrate-bun-lockb-v2.test.ts.snap +++ b/test/cli/install/__snapshots__/migrate-bun-lockb-v2.test.ts.snap @@ -2007,10 +2007,6 @@ exports[`migrate migrate-bun-lockb-v2-most-features 1`] = ` "id": 44, "package_id": 43, }, - "lodash": { - "id": 10, - "package_id": 4294967295, - }, "loose-envify": { "id": 15, "package_id": 8, diff --git a/test/integration/next-pages/test/__snapshots__/dev-server-ssr-100.test.ts.snap b/test/integration/next-pages/test/__snapshots__/dev-server-ssr-100.test.ts.snap index 8b3ef2323b..89959331b6 100644 --- a/test/integration/next-pages/test/__snapshots__/dev-server-ssr-100.test.ts.snap +++ b/test/integration/next-pages/test/__snapshots__/dev-server-ssr-100.test.ts.snap @@ -23958,18 +23958,10 @@ exports[`ssr works for 100-ish requests 1`] = ` "id": 407, "package_id": 61, }, - "@opentelemetry/api": { - "id": 633, - "package_id": 4294967295, - }, "@pkgjs/parseargs": { "id": 599, "package_id": 62, }, - "@playwright/test": { - "id": 634, - "package_id": 4294967295, - }, "@puppeteer/browsers": { "id": 719, "package_id": 63, @@ -24246,18 +24238,10 @@ exports[`ssr works for 100-ish requests 1`] = ` "id": 905, "package_id": 131, }, - "babel-plugin-react-compiler": { - "id": 635, - "package_id": 4294967295, - }, "balanced-match": { "id": 202, "package_id": 132, }, - "bare-buffer": { - "id": 197, - "package_id": 4294967295, - }, "bare-events": { "id": 194, "package_id": 133, @@ -24302,10 +24286,6 @@ exports[`ssr works for 100-ish requests 1`] = ` "id": 1014, "package_id": 143, }, - "bufferutil": { - "id": 1005, - "package_id": 4294967295, - }, "bun-types": { "id": 4, "package_id": 144, @@ -24550,10 +24530,6 @@ exports[`ssr works for 100-ish requests 1`] = ` "id": 398, "package_id": 204, }, - "eslint-plugin-import-x": { - "id": 416, - "package_id": 4294967295, - }, "eslint-plugin-jsx-a11y": { "id": 399, "package_id": 205, @@ -25318,10 +25294,6 @@ exports[`ssr works for 100-ish requests 1`] = ` "id": 451, "package_id": 395, }, - "sass": { - "id": 638, - "package_id": 4294967295, - }, "scheduler": { "id": 731, "package_id": 396, @@ -25518,10 +25490,6 @@ exports[`ssr works for 100-ish requests 1`] = ` "id": 877, "package_id": 442, }, - "ts-node": { - "id": 701, - "package_id": 4294967295, - }, "tsconfig-paths": { "id": 436, "package_id": 443, @@ -25578,10 +25546,6 @@ exports[`ssr works for 100-ish requests 1`] = ` "id": 138, "package_id": 456, }, - "utf-8-validate": { - "id": 1006, - "package_id": 4294967295, - }, "util-deprecate": { "id": 705, "package_id": 457, diff --git a/test/integration/next-pages/test/__snapshots__/dev-server.test.ts.snap b/test/integration/next-pages/test/__snapshots__/dev-server.test.ts.snap index be4a0b6e7d..321cb36341 100644 --- a/test/integration/next-pages/test/__snapshots__/dev-server.test.ts.snap +++ b/test/integration/next-pages/test/__snapshots__/dev-server.test.ts.snap @@ -23958,18 +23958,10 @@ exports[`hot reloading works on the client (+ tailwind hmr) 1`] = ` "id": 407, "package_id": 61, }, - "@opentelemetry/api": { - "id": 633, - "package_id": 4294967295, - }, "@pkgjs/parseargs": { "id": 599, "package_id": 62, }, - "@playwright/test": { - "id": 634, - "package_id": 4294967295, - }, "@puppeteer/browsers": { "id": 719, "package_id": 63, @@ -24246,18 +24238,10 @@ exports[`hot reloading works on the client (+ tailwind hmr) 1`] = ` "id": 905, "package_id": 131, }, - "babel-plugin-react-compiler": { - "id": 635, - "package_id": 4294967295, - }, "balanced-match": { "id": 202, "package_id": 132, }, - "bare-buffer": { - "id": 197, - "package_id": 4294967295, - }, "bare-events": { "id": 194, "package_id": 133, @@ -24302,10 +24286,6 @@ exports[`hot reloading works on the client (+ tailwind hmr) 1`] = ` "id": 1014, "package_id": 143, }, - "bufferutil": { - "id": 1005, - "package_id": 4294967295, - }, "bun-types": { "id": 4, "package_id": 144, @@ -24550,10 +24530,6 @@ exports[`hot reloading works on the client (+ tailwind hmr) 1`] = ` "id": 398, "package_id": 204, }, - "eslint-plugin-import-x": { - "id": 416, - "package_id": 4294967295, - }, "eslint-plugin-jsx-a11y": { "id": 399, "package_id": 205, @@ -25318,10 +25294,6 @@ exports[`hot reloading works on the client (+ tailwind hmr) 1`] = ` "id": 451, "package_id": 395, }, - "sass": { - "id": 638, - "package_id": 4294967295, - }, "scheduler": { "id": 731, "package_id": 396, @@ -25518,10 +25490,6 @@ exports[`hot reloading works on the client (+ tailwind hmr) 1`] = ` "id": 877, "package_id": 442, }, - "ts-node": { - "id": 701, - "package_id": 4294967295, - }, "tsconfig-paths": { "id": 436, "package_id": 443, @@ -25578,10 +25546,6 @@ exports[`hot reloading works on the client (+ tailwind hmr) 1`] = ` "id": 138, "package_id": 456, }, - "utf-8-validate": { - "id": 1006, - "package_id": 4294967295, - }, "util-deprecate": { "id": 705, "package_id": 457, diff --git a/test/integration/next-pages/test/__snapshots__/next-build.test.ts.snap b/test/integration/next-pages/test/__snapshots__/next-build.test.ts.snap index b484a46fa6..05004d7e99 100644 --- a/test/integration/next-pages/test/__snapshots__/next-build.test.ts.snap +++ b/test/integration/next-pages/test/__snapshots__/next-build.test.ts.snap @@ -23958,18 +23958,10 @@ exports[`next build works: bun 1`] = ` "id": 407, "package_id": 61, }, - "@opentelemetry/api": { - "id": 633, - "package_id": 4294967295, - }, "@pkgjs/parseargs": { "id": 599, "package_id": 62, }, - "@playwright/test": { - "id": 634, - "package_id": 4294967295, - }, "@puppeteer/browsers": { "id": 719, "package_id": 63, @@ -24246,18 +24238,10 @@ exports[`next build works: bun 1`] = ` "id": 905, "package_id": 131, }, - "babel-plugin-react-compiler": { - "id": 635, - "package_id": 4294967295, - }, "balanced-match": { "id": 202, "package_id": 132, }, - "bare-buffer": { - "id": 197, - "package_id": 4294967295, - }, "bare-events": { "id": 194, "package_id": 133, @@ -24302,10 +24286,6 @@ exports[`next build works: bun 1`] = ` "id": 1014, "package_id": 143, }, - "bufferutil": { - "id": 1005, - "package_id": 4294967295, - }, "bun-types": { "id": 4, "package_id": 144, @@ -24550,10 +24530,6 @@ exports[`next build works: bun 1`] = ` "id": 398, "package_id": 204, }, - "eslint-plugin-import-x": { - "id": 416, - "package_id": 4294967295, - }, "eslint-plugin-jsx-a11y": { "id": 399, "package_id": 205, @@ -25318,10 +25294,6 @@ exports[`next build works: bun 1`] = ` "id": 451, "package_id": 395, }, - "sass": { - "id": 638, - "package_id": 4294967295, - }, "scheduler": { "id": 731, "package_id": 396, @@ -25518,10 +25490,6 @@ exports[`next build works: bun 1`] = ` "id": 877, "package_id": 442, }, - "ts-node": { - "id": 701, - "package_id": 4294967295, - }, "tsconfig-paths": { "id": 436, "package_id": 443, @@ -25578,10 +25546,6 @@ exports[`next build works: bun 1`] = ` "id": 138, "package_id": 456, }, - "utf-8-validate": { - "id": 1006, - "package_id": 4294967295, - }, "util-deprecate": { "id": 705, "package_id": 457, @@ -49932,18 +49896,10 @@ exports[`next build works: node 1`] = ` "id": 407, "package_id": 61, }, - "@opentelemetry/api": { - "id": 633, - "package_id": 4294967295, - }, "@pkgjs/parseargs": { "id": 599, "package_id": 62, }, - "@playwright/test": { - "id": 634, - "package_id": 4294967295, - }, "@puppeteer/browsers": { "id": 719, "package_id": 63, @@ -50220,18 +50176,10 @@ exports[`next build works: node 1`] = ` "id": 905, "package_id": 131, }, - "babel-plugin-react-compiler": { - "id": 635, - "package_id": 4294967295, - }, "balanced-match": { "id": 202, "package_id": 132, }, - "bare-buffer": { - "id": 197, - "package_id": 4294967295, - }, "bare-events": { "id": 194, "package_id": 133, @@ -50276,10 +50224,6 @@ exports[`next build works: node 1`] = ` "id": 1014, "package_id": 143, }, - "bufferutil": { - "id": 1005, - "package_id": 4294967295, - }, "bun-types": { "id": 4, "package_id": 144, @@ -50524,10 +50468,6 @@ exports[`next build works: node 1`] = ` "id": 398, "package_id": 204, }, - "eslint-plugin-import-x": { - "id": 416, - "package_id": 4294967295, - }, "eslint-plugin-jsx-a11y": { "id": 399, "package_id": 205, @@ -51292,10 +51232,6 @@ exports[`next build works: node 1`] = ` "id": 451, "package_id": 395, }, - "sass": { - "id": 638, - "package_id": 4294967295, - }, "scheduler": { "id": 731, "package_id": 396, @@ -51492,10 +51428,6 @@ exports[`next build works: node 1`] = ` "id": 877, "package_id": 442, }, - "ts-node": { - "id": 701, - "package_id": 4294967295, - }, "tsconfig-paths": { "id": 436, "package_id": 443, @@ -51552,10 +51484,6 @@ exports[`next build works: node 1`] = ` "id": 138, "package_id": 456, }, - "utf-8-validate": { - "id": 1006, - "package_id": 4294967295, - }, "util-deprecate": { "id": 705, "package_id": 457, diff --git a/test/regression/issue/24502/bun-pm-ls-all-invalid-package-id.test.ts b/test/regression/issue/24502/bun-pm-ls-all-invalid-package-id.test.ts new file mode 100644 index 0000000000..b46225a17f --- /dev/null +++ b/test/regression/issue/24502/bun-pm-ls-all-invalid-package-id.test.ts @@ -0,0 +1,31 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, runBunInstall, tempDirWithFiles } from "harness"; + +test("unresolved optional peers don't crash", async () => { + const testDir = tempDirWithFiles("unresolved-optional-peer", { + "package.json": JSON.stringify({ + name: "pkg", + peerDependencies: { + jquery: "3.7.1", + }, + peerDependenciesMeta: { + jquery: { + optional: true, + }, + }, + }), + }); + + await runBunInstall(bunEnv, testDir); + + const { stdout, stderr, exited } = Bun.spawn({ + cmd: [bunExe(), "pm", "ls", "--all"], + cwd: testDir, + stdout: "pipe", + stderr: "pipe", + }); + + expect(await exited).toBe(0); + expect(await stdout.text()).toBe(""); + expect(await stderr.text()).toBe(""); +});