fix(install): continue install if optional postinstall fails (#14783)

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()) {

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 = "\n<r><red>error:<r> failed to spawn life-cycle scripts for <b>{s}<r>: {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 = "\n<r><red>error:<r> failed to spawn life-cycle scripts for <b>{s}<r>: {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);
     }
 };
 

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("<r><red>error<r><d>:<r> <b>{s}<r> script from \"<b>{s}<r>\" exited with {d}<r>", .{
                         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("<r><red>error<r>: Failed to run <b>{s}<r> script from \"<b>{s}<r>\" 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()) {

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}/"
     });
   }
 });
-

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": "[email protected]",
+			"_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": ""
+}

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": "[email protected]",
+			"_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": ""
+}