From 11a2f43b43ad7484fdc94354450ec9ae3790ae1e Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Fri, 3 Jan 2025 20:44:34 -0800 Subject: [PATCH 1/3] skip root scripts if not installed --- src/install/install.zig | 58 +++++++++++++++++++++++-- src/install/lockfile.zig | 56 ++++++++++-------------- test/cli/install/bun-workspaces.test.ts | 56 +++++++++++++++++++++++- 3 files changed, 132 insertions(+), 38 deletions(-) diff --git a/src/install/install.zig b/src/install/install.zig index 3e1f3e89d91fab..6faafc600b8777 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -13903,13 +13903,14 @@ pub const PackageManager = struct { pub fn installPackages( this: *PackageManager, ctx: Command.Context, - original_cwd: string, + workspace_filters: []const WorkspaceFilter, + install_root_dependencies: bool, comptime log_level: PackageManager.Options.LogLevel, ) !PackageInstall.Summary { const original_trees = this.lockfile.buffers.trees; const original_tree_dep_ids = this.lockfile.buffers.hoisted_dependencies; - try this.lockfile.filter(this.log, this, original_cwd); + try this.lockfile.filter(this.log, this, install_root_dependencies, workspace_filters); defer { this.lockfile.buffers.trees = original_trees; @@ -15027,11 +15028,60 @@ pub const PackageManager = struct { return; } + var path_buf: bun.PathBuffer = undefined; + var workspace_filters: std.ArrayListUnmanaged(WorkspaceFilter) = .{}; + // only populated when subcommand is `.install` + if (manager.subcommand == .install and manager.options.filter_patterns.len > 0) { + try workspace_filters.ensureUnusedCapacity(manager.allocator, manager.options.filter_patterns.len); + for (manager.options.filter_patterns) |pattern| { + try workspace_filters.append(manager.allocator, try WorkspaceFilter.init(manager.allocator, pattern, original_cwd, &path_buf)); + } + } + defer workspace_filters.deinit(manager.allocator); + + var install_root_dependencies = workspace_filters.items.len == 0; + if (!install_root_dependencies) { + const pkg_names = manager.lockfile.packages.items(.name); + + const abs_root_path = abs_root_path: { + if (comptime !Environment.isWindows) { + break :abs_root_path strings.withoutTrailingSlash(FileSystem.instance.top_level_dir); + } + + var abs_path = Path.pathToPosixBuf(u8, FileSystem.instance.top_level_dir, &path_buf); + break :abs_root_path abs_path[Path.windowsVolumeNameLen(abs_path)[0]..]; + }; + + for (workspace_filters.items) |filter| { + const pattern, const path_or_name = switch (filter) { + .name => |pattern| .{ pattern, pkg_names[0].slice(manager.lockfile.buffers.string_bytes.items) }, + .path => |pattern| .{ pattern, abs_root_path }, + .all => { + install_root_dependencies = true; + continue; + }, + }; + + switch (bun.glob.walk.matchImpl(pattern, path_or_name)) { + .match, .negate_match => install_root_dependencies = true, + + .negate_no_match => { + // always skip if a pattern specifically says "!" + install_root_dependencies = false; + break; + }, + + .no_match => {}, + } + } + } + var install_summary = PackageInstall.Summary{}; if (manager.options.do.install_packages) { install_summary = try manager.installPackages( ctx, - original_cwd, + workspace_filters.items, + install_root_dependencies, log_level, ); } @@ -15091,7 +15141,7 @@ pub const PackageManager = struct { } } - if (manager.options.do.run_scripts) { + if (manager.options.do.run_scripts and install_root_dependencies) { if (manager.root_lifecycle_scripts) |scripts| { if (comptime Environment.allow_assert) { bun.assert(scripts.total > 0); diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index d57a66ae93f0c1..1d7c6692403cc1 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -690,6 +690,7 @@ pub const Tree = struct { manager: if (method == .filter) *const PackageManager else void, sort_buf: std.ArrayListUnmanaged(DependencyID) = .{}, workspace_filters: if (method == .filter) []const WorkspaceFilter else void = if (method == .filter) &.{} else {}, + install_root_dependencies: if (method == .filter) bool else void, path_buf: []u8, pub fn maybeReportError(this: *@This(), comptime fmt: string, args: anytype) void { @@ -747,13 +748,6 @@ pub const Tree = struct { this.queue.deinit(); this.sort_buf.deinit(this.allocator); - if (comptime method == .filter) { - for (this.workspace_filters) |workspace_filter| { - workspace_filter.deinit(this.lockfile.allocator); - } - this.lockfile.allocator.free(this.workspace_filters); - } - // take over the `builder.list` pointer for only trees if (@intFromPtr(trees.ptr) != @intFromPtr(list_ptr)) { var new: [*]Tree = @ptrCast(list_ptr); @@ -870,27 +864,32 @@ pub const Tree = struct { continue; } - if (builder.manager.subcommand == .install) { + if (builder.manager.subcommand == .install) dont_skip: { // only do this when parent is root. workspaces are always dependencies of the root // package, and the root package is always called with `processSubtree` if (parent_pkg_id == 0 and builder.workspace_filters.len > 0) { + if (!builder.dependencies[dep_id].behavior.isWorkspaceOnly()) { + if (builder.install_root_dependencies) { + break :dont_skip; + } + + continue; + } + var match = false; for (builder.workspace_filters) |workspace_filter| { const res_id = builder.resolutions[dep_id]; const pattern, const path_or_name = switch (workspace_filter) { - .name => |pattern| .{ pattern, if (builder.dependencies[dep_id].behavior.isWorkspaceOnly()) - pkg_names[res_id].slice(builder.buf()) - else - pkg_names[0].slice(builder.buf()) }, + .name => |pattern| .{ pattern, pkg_names[res_id].slice(builder.buf()) }, .path => |pattern| path: { - const res_path = if (builder.dependencies[dep_id].behavior.isWorkspaceOnly() and pkg_resolutions[res_id].tag == .workspace) - pkg_resolutions[res_id].value.workspace.slice(builder.buf()) - else - // dependnecy of the root package.json. use top level dir - FileSystem.instance.top_level_dir; + const res = pkg_resolutions[res_id]; + if (res.tag != .workspace) { + break :dont_skip; + } + const res_path = res.value.workspace.slice(builder.buf()); // occupy `builder.path_buf` var abs_res_path = strings.withoutTrailingSlash(bun.path.joinAbsStringBuf( @@ -1570,16 +1569,17 @@ pub fn resolve( lockfile: *Lockfile, log: *logger.Log, ) Tree.SubtreeError!void { - return lockfile.hoist(log, .resolvable, {}, {}); + return lockfile.hoist(log, .resolvable, {}, {}, {}); } pub fn filter( lockfile: *Lockfile, log: *logger.Log, manager: *PackageManager, - cwd: string, + install_root_dependencies: bool, + workspace_filters: []const WorkspaceFilter, ) Tree.SubtreeError!void { - return lockfile.hoist(log, .filter, manager, cwd); + return lockfile.hoist(log, .filter, manager, install_root_dependencies, workspace_filters); } /// Sets `buffers.trees` and `buffers.hoisted_dependencies` @@ -1588,7 +1588,8 @@ pub fn hoist( log: *logger.Log, comptime method: Tree.BuilderMethod, manager: if (method == .filter) *PackageManager else void, - cwd: if (method == .filter) string else void, + install_root_dependencies: if (method == .filter) bool else void, + workspace_filters: if (method == .filter) []const WorkspaceFilter else void, ) Tree.SubtreeError!void { const allocator = lockfile.allocator; var slice = lockfile.packages.slice(); @@ -1606,19 +1607,10 @@ pub fn hoist( .lockfile = lockfile, .manager = manager, .path_buf = &path_buf, + .install_root_dependencies = install_root_dependencies, + .workspace_filters = workspace_filters, }; - if (comptime method == .filter) { - if (manager.options.filter_patterns.len > 0) { - var filters = try std.ArrayListUnmanaged(WorkspaceFilter).initCapacity(allocator, manager.options.filter_patterns.len); - for (manager.options.filter_patterns) |pattern| { - try filters.append(allocator, try WorkspaceFilter.init(allocator, pattern, cwd, &path_buf)); - } - - builder.workspace_filters = filters.items; - } - } - try (Tree{}).processSubtree( Tree.root_dep_id, Tree.invalid_id, diff --git a/test/cli/install/bun-workspaces.test.ts b/test/cli/install/bun-workspaces.test.ts index b282e6c1be2775..a72ece22806f4b 100644 --- a/test/cli/install/bun-workspaces.test.ts +++ b/test/cli/install/bun-workspaces.test.ts @@ -381,8 +381,6 @@ describe("workspace aliases", async () => { ), ]); - console.log({ packageDir }); - await runBunInstall(env, packageDir); const files = await Promise.all( ["a0", "a1", "a2", "a3", "a4", "a5"].map(name => @@ -1284,6 +1282,60 @@ for (const version of versions) { } describe("install --filter", () => { + test("does not run root scripts if root is filtered out", async () => { + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root", + workspaces: ["packages/*"], + scripts: { + postinstall: `${bunExe()} root.js`, + }, + }), + ), + write(join(packageDir, "root.js"), `require("fs").writeFileSync("root.txt", "")`), + write( + join(packageDir, "packages", "pkg1", "package.json"), + JSON.stringify({ + name: "pkg1", + scripts: { + postinstall: `${bunExe()} pkg1.js`, + }, + }), + ), + write(join(packageDir, "packages", "pkg1", "pkg1.js"), `require("fs").writeFileSync("pkg1.txt", "")`), + ]); + + var { exited } = spawn({ + cmd: [bunExe(), "install", "--filter", "pkg1"], + cwd: packageDir, + stdout: "ignore", + stderr: "ignore", + env, + }); + + expect(await exited).toBe(0); + + expect(await exists(join(packageDir, "root.txt"))).toBeFalse(); + expect(await exists(join(packageDir, "packages", "pkg1", "pkg1.txt"))).toBeTrue(); + + await rm(join(packageDir, "packages", "pkg1", "pkg1.txt")); + + ({ exited } = spawn({ + cmd: [bunExe(), "install", "--filter", "root"], + cwd: packageDir, + stdout: "ignore", + stderr: "ignore", + env, + })); + + expect(await exited).toBe(0); + + expect(await exists(join(packageDir, "root.txt"))).toBeTrue(); + expect(await exists(join(packageDir, "packages", "pkg1.txt"))).toBeFalse(); + }); + test("basic", async () => { await Promise.all([ write( From e3ba940db80404d7e712342f4757a11f1333e6f3 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Fri, 3 Jan 2025 20:59:57 -0800 Subject: [PATCH 2/3] without trailing slash --- src/install/install.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/install/install.zig b/src/install/install.zig index 6faafc600b8777..d16c20ce83c175 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -15049,7 +15049,7 @@ pub const PackageManager = struct { } var abs_path = Path.pathToPosixBuf(u8, FileSystem.instance.top_level_dir, &path_buf); - break :abs_root_path abs_path[Path.windowsVolumeNameLen(abs_path)[0]..]; + break :abs_root_path strings.withoutTrailingSlash(abs_path[Path.windowsVolumeNameLen(abs_path)[0]..]); }; for (workspace_filters.items) |filter| { From 917e9822941925838c0c8d1cb1bc24bf206dc0bc Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 4 Jan 2025 00:33:56 -0800 Subject: [PATCH 3/3] Update src/install/lockfile.zig --- src/install/lockfile.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index 1d7c6692403cc1..47f4c6d8ac1c81 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -885,7 +885,7 @@ pub const Tree = struct { .name => |pattern| .{ pattern, pkg_names[res_id].slice(builder.buf()) }, .path => |pattern| path: { - const res = pkg_resolutions[res_id]; + const res = &pkg_resolutions[res_id]; if (res.tag != .workspace) { break :dont_skip; }