From ba7debfeb585030e55b5fb1a0465e9355cedd41c Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Mon, 20 Oct 2025 15:21:18 +0000 Subject: [PATCH] Add skipLifecycleScripts to skip unnecessary postinstall scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a new `skipLifecycleScripts` field to package.json that allows skipping lifecycle scripts for packages where they are unnecessary. This has higher precedence than trustedDependencies and scripts in this list are: 1. Never executed (even if in trustedDependencies) 2. Excluded from the "X postinstall scripts blocked" message Key changes: - Added `skip_lifecycle_scripts` field to Lockfile struct - Created `default-skipped-lifecycle-scripts.txt` with esbuild as default - Added parsing for `skipLifecycleScripts` array in package.json - Updated PackageInstaller to check skip list before blocking or enqueueing scripts - Added test verifying skip behavior and precedence over trustedDependencies This prevents nagging users about packages like esbuild whose postinstall scripts are unnecessary for Bun users and just waste CI time. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/install/PackageInstaller.zig | 94 ++++++++++-------- .../default-skipped-lifecycle-scripts.txt | 1 + src/install/lockfile.zig | 75 +++++++++++++++ src/install/lockfile/Package.zig | 30 ++++++ .../bun-install-lifecycle-scripts.test.ts | 96 +++++++++++++++++++ 5 files changed, 254 insertions(+), 42 deletions(-) create mode 100644 src/install/default-skipped-lifecycle-scripts.txt diff --git a/src/install/PackageInstaller.zig b/src/install/PackageInstaller.zig index fa615f787b..9b57728f5e 100644 --- a/src/install/PackageInstaller.zig +++ b/src/install/PackageInstaller.zig @@ -1125,30 +1125,35 @@ pub const PackageInstaller = struct { // these will never be blocked }, else => if (!is_trusted and this.metas[package_id].hasInstallScript()) { - // Check if the package actually has scripts. `hasInstallScript` can be false positive if a package is published with - // an auto binding.gyp rebuild script but binding.gyp is excluded from the published files. - var folder_path: bun.AbsPath(.{ .sep = .auto }) = .from(this.node_modules.path.items); - defer folder_path.deinit(); - folder_path.append(alias.slice(this.lockfile.buffers.string_bytes.items)); + const alias_str = alias.slice(this.lockfile.buffers.string_bytes.items); + const should_skip = this.lockfile.shouldSkipLifecycleScripts(alias_str); - const count = this.getInstalledPackageScriptsCount( - alias.slice(this.lockfile.buffers.string_bytes.items), - package_id, - resolution.tag, - &folder_path, - log_level, - ); - if (count > 0) { - if (log_level.isVerbose()) { - Output.prettyError("Blocked {d} scripts for: {s}@{}\n", .{ - count, - alias.slice(this.lockfile.buffers.string_bytes.items), - resolution.fmt(this.lockfile.buffers.string_bytes.items, .posix), - }); + if (!should_skip) { + // Check if the package actually has scripts. `hasInstallScript` can be false positive if a package is published with + // an auto binding.gyp rebuild script but binding.gyp is excluded from the published files. + var folder_path: bun.AbsPath(.{ .sep = .auto }) = .from(this.node_modules.path.items); + defer folder_path.deinit(); + folder_path.append(alias_str); + + const count = this.getInstalledPackageScriptsCount( + alias_str, + package_id, + resolution.tag, + &folder_path, + log_level, + ); + if (count > 0) { + if (log_level.isVerbose()) { + Output.prettyError("Blocked {d} scripts for: {s}@{}\n", .{ + count, + alias_str, + resolution.fmt(this.lockfile.buffers.string_bytes.items, .posix), + }); + } + const entry = bun.handleOom(this.summary.packages_with_blocked_scripts.getOrPut(this.manager.allocator, truncated_dep_name_hash)); + if (!entry.found_existing) entry.value_ptr.* = 0; + entry.value_ptr.* += count; } - const entry = bun.handleOom(this.summary.packages_with_blocked_scripts.getOrPut(this.manager.allocator, truncated_dep_name_hash)); - if (!entry.found_existing) entry.value_ptr.* = 0; - entry.value_ptr.* += count; } }, } @@ -1268,28 +1273,33 @@ pub const PackageInstaller = struct { }; if (resolution.tag != .root and is_trusted) { - var folder_path: bun.AbsPath(.{ .sep = .auto }) = .from(this.node_modules.path.items); - defer folder_path.deinit(); - folder_path.append(alias.slice(this.lockfile.buffers.string_bytes.items)); + const alias_str = alias.slice(this.lockfile.buffers.string_bytes.items); + const should_skip = this.lockfile.shouldSkipLifecycleScripts(alias_str); - if (this.enqueueLifecycleScripts( - alias.slice(this.lockfile.buffers.string_bytes.items), - log_level, - &folder_path, - package_id, - dep.behavior.optional, - resolution, - )) { - if (is_trusted_through_update_request) { - this.manager.trusted_deps_to_add_to_package_json.append( - this.manager.allocator, - bun.handleOom(this.manager.allocator.dupe(u8, alias.slice(this.lockfile.buffers.string_bytes.items))), - ) catch |err| bun.handleOom(err); - } + if (!should_skip) { + var folder_path: bun.AbsPath(.{ .sep = .auto }) = .from(this.node_modules.path.items); + defer folder_path.deinit(); + folder_path.append(alias_str); - if (add_to_lockfile) { - if (this.lockfile.trusted_dependencies == null) this.lockfile.trusted_dependencies = .{}; - this.lockfile.trusted_dependencies.?.put(this.manager.allocator, truncated_dep_name_hash, {}) catch |err| bun.handleOom(err); + if (this.enqueueLifecycleScripts( + alias_str, + log_level, + &folder_path, + package_id, + dep.behavior.optional, + resolution, + )) { + if (is_trusted_through_update_request) { + this.manager.trusted_deps_to_add_to_package_json.append( + this.manager.allocator, + bun.handleOom(this.manager.allocator.dupe(u8, alias_str)), + ) catch |err| bun.handleOom(err); + } + + if (add_to_lockfile) { + if (this.lockfile.trusted_dependencies == null) this.lockfile.trusted_dependencies = .{}; + this.lockfile.trusted_dependencies.?.put(this.manager.allocator, truncated_dep_name_hash, {}) catch |err| bun.handleOom(err); + } } } } diff --git a/src/install/default-skipped-lifecycle-scripts.txt b/src/install/default-skipped-lifecycle-scripts.txt new file mode 100644 index 0000000000..5ddffd8bb2 --- /dev/null +++ b/src/install/default-skipped-lifecycle-scripts.txt @@ -0,0 +1 @@ +esbuild diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index 3f4a5c7313..40e0033205 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -24,6 +24,8 @@ workspace_versions: VersionHashMap = .{}, /// Optional because `trustedDependencies` in package.json might be an /// empty list or it might not exist trusted_dependencies: ?TrustedDependenciesSet = null, +/// Optional: packages whose lifecycle scripts should be skipped AND excluded from "blocked" count +skip_lifecycle_scripts: ?TrustedDependenciesSet = null, patched_dependencies: PatchedDependenciesMap = .{}, overrides: OverrideMap = .{}, catalogs: CatalogMap = .{}, @@ -2108,6 +2110,79 @@ pub fn hasTrustedDependency(this: *const Lockfile, name: []const u8) bool { return default_trusted_dependencies.has(name); } +const max_default_skipped_lifecycle_scripts = 512; + +pub const default_skipped_lifecycle_scripts_list: []const []const u8 = brk: { + // This file contains a list of packages whose lifecycle scripts should be skipped + // and excluded from the "blocked" count message + const data = @embedFile("./default-skipped-lifecycle-scripts.txt"); + @setEvalBranchQuota(999999); + var buf: [max_default_skipped_lifecycle_scripts][]const u8 = undefined; + var i: usize = 0; + var iter = std.mem.tokenizeAny(u8, data, " \r\n\t"); + while (iter.next()) |package_ptr| { + const package = package_ptr[0..].*; + buf[i] = &package; + i += 1; + } + + const Sorter = struct { + pub fn lessThan(_: void, lhs: []const u8, rhs: []const u8) bool { + return std.mem.order(u8, lhs, rhs) == .lt; + } + }; + + // alphabetical so we don't need to sort later + std.sort.pdq([]const u8, buf[0..i], {}, Sorter.lessThan); + + var names: [i][]const u8 = undefined; + @memcpy(names[0..i], buf[0..i]); + const final = names; + break :brk &final; +}; + +/// The default list of skipped lifecycle scripts is a static hashmap +pub const default_skipped_lifecycle_scripts = brk: { + const StringHashContext = struct { + pub fn hash(_: @This(), s: []const u8) u64 { + @setEvalBranchQuota(999999); + // truncate to u32 because Lockfile uses the same u32 string hash + return @intCast(@as(u32, @truncate(String.Builder.stringHash(s)))); + } + pub fn eql(_: @This(), a: []const u8, b: []const u8) bool { + @setEvalBranchQuota(999999); + return std.mem.eql(u8, a, b); + } + }; + + var map: StaticHashMap([]const u8, void, StringHashContext, max_default_skipped_lifecycle_scripts) = .{}; + + for (default_skipped_lifecycle_scripts_list) |dep| { + if (map.len == max_default_skipped_lifecycle_scripts) { + @compileError("default-skipped-lifecycle-scripts.txt is too large, please increase 'max_default_skipped_lifecycle_scripts' in lockfile.zig"); + } + + const entry = map.getOrPutAssumeCapacity(dep); + if (entry.found_existing) { + @compileError("Duplicate skipped lifecycle script: " ++ dep); + } + } + + const final = map; + break :brk &final; +}; + +pub fn shouldSkipLifecycleScripts(this: *const Lockfile, name: []const u8) bool { + // Check user-specified skipLifecycleScripts first (higher precedence) + if (this.skip_lifecycle_scripts) |skip_scripts| { + const hash = @as(u32, @truncate(String.Builder.stringHash(name))); + if (skip_scripts.contains(hash)) return true; + } + + // Fall back to default skipped list + return default_skipped_lifecycle_scripts.has(name); +} + pub const NameHashMap = std.ArrayHashMapUnmanaged(PackageNameHash, String, ArrayIdentityContext.U64, false); pub const TrustedDependenciesSet = std.ArrayHashMapUnmanaged(TruncatedPackageNameHash, void, ArrayIdentityContext, false); pub const VersionHashMap = std.ArrayHashMapUnmanaged(PackageNameHash, Semver.Version, ArrayIdentityContext.U64, false); diff --git a/src/install/lockfile/Package.zig b/src/install/lockfile/Package.zig index c015adfde6..beae3c0d80 100644 --- a/src/install/lockfile/Package.zig +++ b/src/install/lockfile/Package.zig @@ -1571,6 +1571,36 @@ pub fn Package(comptime SemverIntType: type) type { }, } } + + if (json.asProperty("skipLifecycleScripts")) |q| { + switch (q.expr.data) { + .e_array => |arr| { + if (lockfile.skip_lifecycle_scripts == null) lockfile.skip_lifecycle_scripts = .{}; + try lockfile.skip_lifecycle_scripts.?.ensureUnusedCapacity(allocator, arr.items.len); + for (arr.slice()) |item| { + const name = item.asString(allocator) orelse { + log.addErrorFmt(source, q.loc, allocator, + \\skipLifecycleScripts expects an array of strings, e.g. + \\ "skipLifecycleScripts": [ + \\ "package_name" + \\ ] + , .{}) catch {}; + return error.InvalidPackageJSON; + }; + lockfile.skip_lifecycle_scripts.?.putAssumeCapacity(@as(TruncatedPackageNameHash, @truncate(String.Builder.stringHash(name))), {}); + } + }, + else => { + log.addErrorFmt(source, q.loc, allocator, + \\skipLifecycleScripts expects an array of strings, e.g. + \\ "skipLifecycleScripts": [ + \\ "package_name" + \\ ] + , .{}) catch {}; + return error.InvalidPackageJSON; + }, + } + } } if (comptime features.is_main) { diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 4761fc4a82..70263720ce 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -2921,6 +2921,102 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) { expect(await exited).toBe(0); assertManifestsPopulated(join(packageDir, ".bun-cache"), verdaccio.registryUrl()); }); + + test("skipLifecycleScripts prevents scripts and excludes from blocked count", async () => { + const testEnv = forceWaiterThread ? { ...env, BUN_FEATURE_FLAG_FORCE_WAITER_THREAD: "1" } : env; + await writeFile( + packageJson, + JSON.stringify({ + name: "foo", + version: "1.0.0", + dependencies: { + "all-lifecycle-scripts": "1.0.0", + "uses-what-bin": "1.0.0", + }, + skipLifecycleScripts: ["all-lifecycle-scripts"], + }), + ); + + var { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env: testEnv, + }); + + var err = await stderr.text(); + var out = await stdout.text(); + expect(err).toContain("Saved lockfile"); + expect(err).not.toContain("not found"); + expect(err).not.toContain("error:"); + + // Should only show 1 blocked script (uses-what-bin), not 4 (all-lifecycle-scripts + uses-what-bin) + expect(out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/)).toEqual([ + expect.stringContaining("bun install v1."), + "", + "+ all-lifecycle-scripts@1.0.0", + "+ uses-what-bin@1.0.0", + "", + "2 packages installed", + "", + "Blocked 1 postinstall. Run `bun pm untrusted` for details.", + "", + ]); + expect(await exited).toBe(0); + assertManifestsPopulated(join(packageDir, ".bun-cache"), verdaccio.registryUrl()); + + // Verify scripts were skipped for all-lifecycle-scripts + const depDir = join(packageDir, "node_modules", "all-lifecycle-scripts"); + expect(await exists(join(depDir, "preinstall.txt"))).toBeFalse(); + expect(await exists(join(depDir, "install.txt"))).toBeFalse(); + expect(await exists(join(depDir, "postinstall.txt"))).toBeFalse(); + expect(await exists(join(depDir, "preprepare.txt"))).toBeFalse(); + expect(await exists(join(depDir, "prepare.txt"))).toBeTrue(); // prepare always runs + expect(await exists(join(depDir, "postprepare.txt"))).toBeFalse(); + + // Verify scripts were blocked for uses-what-bin + const whatBinDir = join(packageDir, "node_modules", "uses-what-bin"); + expect(await exists(join(whatBinDir, "what-bin.txt"))).toBeFalse(); + + // Verify skipLifecycleScripts has higher precedence than trustedDependencies + await writeFile( + packageJson, + JSON.stringify({ + name: "foo", + version: "1.0.0", + dependencies: { + "all-lifecycle-scripts": "1.0.0", + }, + trustedDependencies: ["all-lifecycle-scripts"], + skipLifecycleScripts: ["all-lifecycle-scripts"], + }), + ); + + ({ stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env: testEnv, + })); + + err = await stderr.text(); + out = await stdout.text(); + expect(err).not.toContain("error:"); + + // Should not show any blocked scripts message because the only package is in skipLifecycleScripts + const outLines = out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/); + expect(outLines).not.toContain(expect.stringContaining("Blocked")); + expect(await exited).toBe(0); + + // Scripts should still be skipped even though it's in trustedDependencies + expect(await exists(join(depDir, "preinstall.txt"))).toBeFalse(); + expect(await exists(join(depDir, "install.txt"))).toBeFalse(); + expect(await exists(join(depDir, "postinstall.txt"))).toBeFalse(); + }); }); }