diff --git a/src/cli/pm_trusted_command.zig b/src/cli/pm_trusted_command.zig index ae9a57a2d106cd..ba8a918523b3e2 100644 --- a/src/cli/pm_trusted_command.zig +++ b/src/cli/pm_trusted_command.zig @@ -344,8 +344,15 @@ pub const TrustCommand = struct { } const output_in_foreground = false; + const optional = false; switch (pm.options.log_level) { - inline else => |log_level| try pm.spawnPackageLifecycleScripts(ctx, info.scripts_list, log_level, output_in_foreground), + inline else => |log_level| try pm.spawnPackageLifecycleScripts( + ctx, + info.scripts_list, + optional, + log_level, + output_in_foreground, + ), } if (pm.options.log_level.showProgress()) { diff --git a/src/install/install.zig b/src/install/install.zig index 9f640681727f3b..e4218f52a872d1 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -12093,6 +12093,7 @@ pub const PackageManager = struct { pending_lifecycle_scripts: std.ArrayListUnmanaged(struct { list: Lockfile.Package.Scripts.List, tree_id: Lockfile.Tree.Id, + optional: bool, }) = .{}, trusted_dependencies_from_update_requests: std.AutoArrayHashMapUnmanaged(TruncatedPackageNameHash, void), @@ -12257,10 +12258,17 @@ pub const PackageManager = struct { const entry = this.pending_lifecycle_scripts.items[i]; const name = entry.list.package_name; const tree_id = entry.tree_id; + const optional = entry.optional; if (this.canRunScripts(tree_id)) { _ = this.pending_lifecycle_scripts.swapRemove(i); const output_in_foreground = false; - this.manager.spawnPackageLifecycleScripts(this.command_ctx, entry.list, log_level, output_in_foreground) catch |err| { + this.manager.spawnPackageLifecycleScripts( + this.command_ctx, + entry.list, + optional, + log_level, + output_in_foreground, + ) catch |err| { if (comptime log_level != .silent) { const fmt = "\nerror: failed to spawn life-cycle scripts for {s}: {s}\n"; const args = .{ name, @errorName(err) }; @@ -12343,8 +12351,9 @@ pub const PackageManager = struct { PackageManager.instance.sleep(); } + const optional = entry.optional; const output_in_foreground = false; - this.manager.spawnPackageLifecycleScripts(this.command_ctx, entry.list, log_level, output_in_foreground) catch |err| { + this.manager.spawnPackageLifecycleScripts(this.command_ctx, entry.list, optional, log_level, output_in_foreground) catch |err| { if (comptime log_level != .silent) { const fmt = "\nerror: failed to spawn life-cycle scripts for {s}: {s}\n"; const args = .{ package_name, @errorName(err) }; @@ -12979,7 +12988,8 @@ pub const PackageManager = struct { this.trees[this.current_tree_id].binaries.add(dependency_id) catch bun.outOfMemory(); } - const name_hash: TruncatedPackageNameHash = @truncate(this.lockfile.buffers.dependencies.items[dependency_id].name_hash); + const dep = this.lockfile.buffers.dependencies.items[dependency_id]; + const name_hash: TruncatedPackageNameHash = @truncate(dep.name_hash); const is_trusted, const is_trusted_through_update_request = brk: { if (this.trusted_dependencies_from_update_requests.contains(name_hash)) break :brk .{ true, true }; if (this.lockfile.hasTrustedDependency(alias)) break :brk .{ true, false }; @@ -12992,6 +13002,7 @@ pub const PackageManager = struct { log_level, destination_dir, package_id, + dep.behavior.optional, resolution, )) { if (is_trusted_through_update_request) { @@ -13115,7 +13126,8 @@ pub const PackageManager = struct { defer if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, destination_dir, !is_pending_package_install, log_level); - const name_hash: TruncatedPackageNameHash = @truncate(this.lockfile.buffers.dependencies.items[dependency_id].name_hash); + const dep = this.lockfile.buffers.dependencies.items[dependency_id]; + const name_hash: TruncatedPackageNameHash = @truncate(dep.name_hash); const is_trusted, const is_trusted_through_update_request, const add_to_lockfile = brk: { // trusted through a --trust dependency. need to enqueue scripts, write to package.json, and add to lockfile if (this.trusted_dependencies_from_update_requests.contains(name_hash)) break :brk .{ true, true, true }; @@ -13133,6 +13145,7 @@ pub const PackageManager = struct { log_level, destination_dir, package_id, + dep.behavior.optional, resolution, )) { if (is_trusted_through_update_request) { @@ -13158,6 +13171,7 @@ pub const PackageManager = struct { comptime log_level: Options.LogLevel, node_modules_folder: std.fs.Dir, package_id: PackageID, + optional: bool, resolution: *const Resolution, ) bool { var scripts: Package.Scripts = this.lockfile.packages.items(.scripts)[package_id]; @@ -13209,6 +13223,7 @@ pub const PackageManager = struct { this.pending_lifecycle_scripts.append(this.manager.allocator, .{ .list = scripts_list.?, .tree_id = this.current_tree_id, + .optional = optional, }) catch bun.outOfMemory(); return true; @@ -14639,8 +14654,9 @@ pub const PackageManager = struct { } // root lifecycle scripts can run now that all dependencies are installed, dependency scripts // have finished, and lockfiles have been saved + const optional = false; const output_in_foreground = true; - try manager.spawnPackageLifecycleScripts(ctx, scripts, log_level, output_in_foreground); + try manager.spawnPackageLifecycleScripts(ctx, scripts, optional, log_level, output_in_foreground); while (manager.pending_lifecycle_script_tasks.load(.monotonic) > 0) { if (PackageManager.verbose_install) { @@ -14824,6 +14840,7 @@ pub const PackageManager = struct { this: *PackageManager, ctx: Command.Context, list: Lockfile.Package.Scripts.List, + optional: bool, comptime log_level: PackageManager.Options.LogLevel, comptime foreground: bool, ) !void { @@ -14873,7 +14890,7 @@ pub const PackageManager = struct { try this_bundler.env.map.put("PATH", original_path); PATH.deinit(); - try LifecycleScriptSubprocess.spawnPackageScripts(this, list, envp, log_level, foreground); + try LifecycleScriptSubprocess.spawnPackageScripts(this, list, envp, optional, log_level, foreground); } }; diff --git a/src/install/lifecycle_script_runner.zig b/src/install/lifecycle_script_runner.zig index f9df8bfd639f28..3900e790e8d35c 100644 --- a/src/install/lifecycle_script_runner.zig +++ b/src/install/lifecycle_script_runner.zig @@ -32,6 +32,7 @@ pub const LifecycleScriptSubprocess = struct { has_incremented_alive_count: bool = false, foreground: bool = false, + optional: bool = false, pub usingnamespace bun.New(@This()); @@ -301,6 +302,11 @@ pub const LifecycleScriptSubprocess = struct { const maybe_duration = if (this.timer) |*t| t.read() else null; if (exit.code > 0) { + if (this.optional) { + _ = this.manager.pending_lifecycle_script_tasks.fetchSub(1, .monotonic); + this.deinitAndDeletePackage(); + return; + } this.printOutput(); Output.prettyErrorln("error: {s} script from \"{s}\" exited with {d}", .{ this.scriptName(), @@ -364,6 +370,12 @@ pub const LifecycleScriptSubprocess = struct { Global.raiseIgnoringPanicHandler(signal); }, .err => |err| { + if (this.optional) { + _ = this.manager.pending_lifecycle_script_tasks.fetchSub(1, .monotonic); + this.deinitAndDeletePackage(); + return; + } + Output.prettyErrorln("error: Failed to run {s} script from \"{s}\" due to\n{}", .{ this.scriptName(), this.package_name, @@ -421,10 +433,28 @@ pub const LifecycleScriptSubprocess = struct { this.destroy(); } + pub fn deinitAndDeletePackage(this: *LifecycleScriptSubprocess) void { + if (this.manager.options.log_level.isVerbose()) { + Output.warn("deleting optional dependency '{s}' due to failed '{s}' script", .{ + this.package_name, + this.scriptName(), + }); + } + try_delete_dir: { + const dirname = std.fs.path.dirname(this.scripts.cwd) orelse break :try_delete_dir; + const basename = std.fs.path.basename(this.scripts.cwd); + const dir = bun.openDirAbsolute(dirname) catch break :try_delete_dir; + dir.deleteTree(basename) catch break :try_delete_dir; + } + + this.deinit(); + } + pub fn spawnPackageScripts( manager: *PackageManager, list: Lockfile.Package.Scripts.List, envp: [:null]?[*:0]u8, + optional: bool, comptime log_level: PackageManager.Options.LogLevel, comptime foreground: bool, ) !void { @@ -434,6 +464,7 @@ pub const LifecycleScriptSubprocess = struct { .scripts = list, .package_name = list.package_name, .foreground = foreground, + .optional = optional, }); if (comptime log_level.isVerbose()) { diff --git a/test/cli/install/registry/bun-install-registry.test.ts b/test/cli/install/registry/bun-install-registry.test.ts index ac772ba7125ec9..42ede1f57b4eae 100644 --- a/test/cli/install/registry/bun-install-registry.test.ts +++ b/test/cli/install/registry/bun-install-registry.test.ts @@ -1673,9 +1673,6 @@ describe("optionalDependencies", () => { expect(err).toMatch(`warn: GET http://localhost:${port}/this-package-does-not-exist-in-the-registry - 404`); }); } -}); - -describe("optionalDependencies", () => { test("should not install optional deps if false in bunfig", async () => { await writeFile( join(packageDir, "bunfig.toml"), @@ -1725,12 +1722,38 @@ describe("optionalDependencies", () => { "", "1 package installed", ]); - expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual([ - "no-deps", - ]); + expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual(["no-deps"]); expect(await exited).toBe(0); assertManifestsPopulated(join(packageDir, ".bun-cache"), registryUrl()); }); + + test("lifecycle scripts failures from transitive dependencies are ignored", async () => { + // Dependency with a transitive optional dependency that fails during its preinstall script. + await write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "foo", + version: "2.2.2", + dependencies: { + "optional-lifecycle-fail": "1.1.1", + }, + trustedDependencies: ["lifecycle-fail"], + }), + ); + + const { err, exited } = await runBunInstall(env, packageDir); + expect(err).not.toContain("error:"); + expect(err).not.toContain("warn:"); + expect(await exited).toBe(0); + assertManifestsPopulated(join(packageDir, ".bun-cache"), registryUrl()); + + expect( + await Promise.all([ + exists(join(packageDir, "node_modules", "optional-lifecycle-fail", "package.json")), + exists(join(packageDir, "node_modules", "lifecycle-fail", "package.json")), + ]), + ).toEqual([true, false]); + }); }); test("tarball override does not crash", async () => { @@ -11847,4 +11870,3 @@ registry = "http://localhost:${port}/" }); } }); - diff --git a/test/cli/install/registry/packages/lifecycle-fail/lifecycle-fail-1.1.1.tgz b/test/cli/install/registry/packages/lifecycle-fail/lifecycle-fail-1.1.1.tgz new file mode 100644 index 00000000000000..2783b586757b60 Binary files /dev/null and b/test/cli/install/registry/packages/lifecycle-fail/lifecycle-fail-1.1.1.tgz differ diff --git a/test/cli/install/registry/packages/lifecycle-fail/package.json b/test/cli/install/registry/packages/lifecycle-fail/package.json new file mode 100644 index 00000000000000..15797c7f682536 --- /dev/null +++ b/test/cli/install/registry/packages/lifecycle-fail/package.json @@ -0,0 +1,45 @@ +{ + "name": "lifecycle-fail", + "versions": { + "1.1.1": { + "name": "lifecycle-fail", + "version": "1.1.1", + "scripts": { + "preinstall": "bun fail.js", + "postinstall": "bun create.js" + }, + "_id": "lifecycle-fail@1.1.1", + "_integrity": "sha512-tqxnPDUZAsW0sy8JUO3LuzuyS7MkvRjGvnnkvbzv1Oo3sRZHMA2yhWDEQ9Bjo0ozxhU+H6vrW2q8EKDAXwnlQw==", + "_nodeVersion": "22.6.0", + "_npmVersion": "10.8.3", + "integrity": "sha512-tqxnPDUZAsW0sy8JUO3LuzuyS7MkvRjGvnnkvbzv1Oo3sRZHMA2yhWDEQ9Bjo0ozxhU+H6vrW2q8EKDAXwnlQw==", + "shasum": "455d2b86cb654b846325e808804573bf56c5f1a4", + "dist": { + "integrity": "sha512-tqxnPDUZAsW0sy8JUO3LuzuyS7MkvRjGvnnkvbzv1Oo3sRZHMA2yhWDEQ9Bjo0ozxhU+H6vrW2q8EKDAXwnlQw==", + "shasum": "455d2b86cb654b846325e808804573bf56c5f1a4", + "tarball": "http://http://localhost:4873/lifecycle-fail/-/lifecycle-fail-1.1.1.tgz" + }, + "contributors": [] + } + }, + "time": { + "modified": "2024-10-24T03:03:12.460Z", + "created": "2024-10-24T03:03:12.460Z", + "1.1.1": "2024-10-24T03:03:12.460Z" + }, + "users": {}, + "dist-tags": { + "latest": "1.1.1" + }, + "_uplinks": {}, + "_distfiles": {}, + "_attachments": { + "lifecycle-fail-1.1.1.tgz": { + "shasum": "455d2b86cb654b846325e808804573bf56c5f1a4", + "version": "1.1.1" + } + }, + "_rev": "", + "_id": "lifecycle-fail", + "readme": "" +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-lifecycle-fail/optional-lifecycle-fail-1.1.1.tgz b/test/cli/install/registry/packages/optional-lifecycle-fail/optional-lifecycle-fail-1.1.1.tgz new file mode 100644 index 00000000000000..c7e44ed80df387 Binary files /dev/null and b/test/cli/install/registry/packages/optional-lifecycle-fail/optional-lifecycle-fail-1.1.1.tgz differ diff --git a/test/cli/install/registry/packages/optional-lifecycle-fail/package.json b/test/cli/install/registry/packages/optional-lifecycle-fail/package.json new file mode 100644 index 00000000000000..9f9dbb25d6f0ec --- /dev/null +++ b/test/cli/install/registry/packages/optional-lifecycle-fail/package.json @@ -0,0 +1,44 @@ +{ + "name": "optional-lifecycle-fail", + "versions": { + "1.1.1": { + "name": "optional-lifecycle-fail", + "version": "1.1.1", + "optionalDependencies": { + "lifecycle-fail": "1.1.1" + }, + "_id": "optional-lifecycle-fail@1.1.1", + "_integrity": "sha512-1nfxe/RpzOaJm7RVSeIux8UMUvrbVDgCCN9lJ1IlnOzd4B6oy9BoKjM4Ij+d9Cmjy+WvhaDKd6AndadHZw5aRw==", + "_nodeVersion": "22.6.0", + "_npmVersion": "10.8.3", + "integrity": "sha512-1nfxe/RpzOaJm7RVSeIux8UMUvrbVDgCCN9lJ1IlnOzd4B6oy9BoKjM4Ij+d9Cmjy+WvhaDKd6AndadHZw5aRw==", + "shasum": "3b030a54938f24912a19b4a865210fcac6172350", + "dist": { + "integrity": "sha512-1nfxe/RpzOaJm7RVSeIux8UMUvrbVDgCCN9lJ1IlnOzd4B6oy9BoKjM4Ij+d9Cmjy+WvhaDKd6AndadHZw5aRw==", + "shasum": "3b030a54938f24912a19b4a865210fcac6172350", + "tarball": "http://http://localhost:4873/optional-lifecycle-fail/-/optional-lifecycle-fail-1.1.1.tgz" + }, + "contributors": [] + } + }, + "time": { + "modified": "2024-10-24T03:05:13.038Z", + "created": "2024-10-24T03:05:13.038Z", + "1.1.1": "2024-10-24T03:05:13.038Z" + }, + "users": {}, + "dist-tags": { + "latest": "1.1.1" + }, + "_uplinks": {}, + "_distfiles": {}, + "_attachments": { + "optional-lifecycle-fail-1.1.1.tgz": { + "shasum": "3b030a54938f24912a19b4a865210fcac6172350", + "version": "1.1.1" + } + }, + "_rev": "", + "_id": "optional-lifecycle-fail", + "readme": "" +} \ No newline at end of file