Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

skip root scripts if root is filtered out with --filter #16152

Merged
merged 3 commits into from
Jan 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 54 additions & 4 deletions src/install/install.zig
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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

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
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
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
Loading