Skip to content

Commit

Permalink
skip root scripts if root is filtered out with --filter (#16152)
Browse files Browse the repository at this point in the history 
Co-authored-by: Jarred Sumner <[email protected]>
  • Loading branch information
@dylan-conway @Jarred-Sumner
dylan-conway and Jarred-Sumner authored Jan 4, 2025
1 parent 4454ebb commit 79aa5d1
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 38 deletions.
58 changes: 54 additions & 4 deletions src/install/install.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 strings.withoutTrailingSlash(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 "!<name>"
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,
);
}
Expand Down Expand Up @@ -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);
Expand Down
56 changes: 24 additions & 32 deletions src/install/lockfile.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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`
Expand All @@ -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();
Expand All @@ -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,
Expand Down
56 changes: 54 additions & 2 deletions test/cli/install/bun-workspaces.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 79aa5d1

Please sign in to comment.