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

Glob support for workspace names in bun install #10462

Merged
merged 27 commits into from May 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
979758d
commit the wip stuff
zackradisic Apr 19, 2024
8895f85
make it pass tests
zackradisic Apr 20, 2024
cdc17b5
more tests
zackradisic Apr 22, 2024
be0593b
wrong name
zackradisic Apr 23, 2024
86ee330
Update docs
zackradisic Apr 23, 2024
aeeaaf8
Merge branch 'main' into zack/glob-workspace
zackradisic Apr 23, 2024
79a97c6
Fix windows build
zackradisic Apr 23, 2024
d99954a
fix thing broken by merge
zackradisic Apr 23, 2024
f6e26b5
Resolve some comments
zackradisic Apr 24, 2024
b23a5e6
Merge branch 'zack/glob-workspace' of github.com:oven-sh/bun into zac…
zackradisic Apr 24, 2024
9da8db7
Merge branch 'main' into zack/glob-workspace
zackradisic Apr 24, 2024
14aba25
Remove extra `error:` prefix
zackradisic Apr 24, 2024
17259ff
avoid heap allocation
zackradisic Apr 24, 2024
9ac349c
fix compile errors
zackradisic Apr 24, 2024
52dbbe5
Merge branch 'main' into zack/glob-workspace
zackradisic Apr 24, 2024
5205805
Merge branch 'main' into zack/glob-workspace
Jarred-Sumner Apr 25, 2024
d6ed527
Fix tests
zackradisic Apr 25, 2024
a4c381d
Merge branch 'main' into zack/glob-workspace
zackradisic Apr 25, 2024
6b6d474
Apply formatting changes
zackradisic Apr 25, 2024
0f66860
Merge branch 'main' into zack/glob-workspace
Jarred-Sumner Apr 25, 2024
824e581
Fix migration test
zackradisic Apr 26, 2024
296fd8c
Update `bad-workspace.test.ts` to reflect new change to workspace globs
zackradisic Apr 26, 2024
95da9ae
Fix `cli/run/filter-workspace.test.ts` tests
zackradisic Apr 26, 2024
789fbbb
Merge branch 'main' into zack/glob-workspace
zackradisic Apr 26, 2024
220e2e5
Merge branch 'main' into zack/glob-workspace
Jarred-Sumner Apr 27, 2024
bfe576f
Merge branch 'main' into zack/glob-workspace
zackradisic Apr 29, 2024
00e0e4c
Merge branch 'main' into zack/glob-workspace
zackradisic May 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/install/workspaces.md
Expand Up @@ -38,7 +38,7 @@ In the root `package.json`, the `"workspaces"` key is used to indicate which sub
```

{% callout %}
**Glob support** — Bun supports simple `<directory>/*` globs in `"workspaces"`. Full glob syntax (e.g. `**` and `?`) is not yet supported.
**Glob support** — Bun supports full glob syntax in `"workspaces"` (see [here](/docs/api/glob#supported-glob-patterns) for a comprehensive list of supported syntax), _except_ for exclusions (e.g. `!**/excluded/**`), which are not implemented yet.
{% /callout %}

Each workspace has it's own `package.json` When referencing other packages in the monorepo, use `"workspace:*"` as the version field in your `package.json`.
Expand Down
4 changes: 2 additions & 2 deletions src/bun.js/api/glob.zig
Expand Up @@ -231,11 +231,11 @@ pub const WalkTask = struct {
};

fn globWalkResultToJS(globWalk: *GlobWalker, globalThis: *JSGlobalObject) JSValue {
if (globWalk.matchedPaths.items.len == 0) {
if (globWalk.matchedPaths.keys().len == 0) {
return JSC.JSValue.createEmptyArray(globalThis, 0);
}

return BunString.toJSArray(globalThis, globWalk.matchedPaths.items[0..]);
return BunString.toJSArray(globalThis, globWalk.matchedPaths.keys());
}

/// The reference to the arena is not used after the scope because it is copied
Expand Down
2 changes: 2 additions & 0 deletions src/bun.zig
Expand Up @@ -47,6 +47,8 @@ pub const PackageJSON = @import("./resolver/package_json.zig").PackageJSON;
pub const fmt = @import("./fmt.zig");
pub const allocators = @import("./allocators.zig");

pub const glob = @import("./glob.zig");

pub const shell = struct {
pub usingnamespace @import("./shell/shell.zig");
pub const ShellSubprocess = @import("./shell/subproc.zig").ShellSubprocess;
Expand Down
311 changes: 270 additions & 41 deletions src/glob.zig

Large diffs are not rendered by default.

192 changes: 101 additions & 91 deletions src/install/lockfile.zig
Expand Up @@ -93,6 +93,20 @@ pub const VersionHashMap = std.ArrayHashMapUnmanaged(PackageNameHash, Semver.Ver
const File = bun.sys.File;
const assertNoUninitializedPadding = @import("./padding_checker.zig").assertNoUninitializedPadding;

const IGNORED_PATHS: []const []const u8 = &.{
"node_modules",
".git",
"CMakeFiles",
};
fn ignoredWorkspacePaths(path: []const u8) bool {
inline for (IGNORED_PATHS) |ignored| {
if (bun.strings.eqlComptime(path, ignored)) return true;
}
return false;
}

const GlobWalker = bun.glob.GlobWalker_(ignoredWorkspacePaths, bun.glob.SyscallAccessor, false);

// Serialized data
/// The version of the lockfile format, intended to prevent data corruption for format changes.
format: FormatVersion = FormatVersion.current,
Expand Down Expand Up @@ -3854,6 +3868,27 @@ pub const Package = extern struct {

defer workspace_file.close();

return processWorkspaceNameImpl(
allocator,
workspace_allocator,
workspace_file,
path,
path_to_use,
name_to_copy,
log,
);
}

fn processWorkspaceNameImpl(
allocator: std.mem.Allocator,
workspace_allocator: std.mem.Allocator,
workspace_file: std.fs.File,
path: []const u8,
path_to_use: []const u8,
name_to_copy: *[1024]u8,
log: *logger.Log,
) !WorkspaceEntry {
_ = path_to_use; // autofix
const workspace_bytes = try workspace_file.readToEndAlloc(workspace_allocator, std.math.maxInt(usize));
defer workspace_allocator.free(workspace_bytes);
const workspace_source = logger.Source.initPathString(path, workspace_bytes);
Expand Down Expand Up @@ -3894,14 +3929,15 @@ pub const Package = extern struct {

const orig_msgs_len = log.msgs.items.len;

var asterisked_workspace_paths = std.ArrayList(string).init(allocator);
defer asterisked_workspace_paths.deinit();
var workspace_globs = std.ArrayList(string).init(allocator);
defer workspace_globs.deinit();
const filepath_bufOS = allocator.create(bun.PathBuffer) catch unreachable;
const filepath_buf = std.mem.asBytes(filepath_bufOS);
defer allocator.destroy(filepath_bufOS);

for (arr.slice()) |item| {
defer fallback.fixed_buffer_allocator.reset();
// TODO: when does this get deallocated?
var input_path = item.asString(allocator) orelse {
log.addErrorFmt(source, item.loc, allocator,
\\Workspaces expects an array of strings, like:
Expand All @@ -3912,29 +3948,8 @@ pub const Package = extern struct {
return error.InvalidPackageJSON;
};

if (strings.containsChar(input_path, '*')) {
if (strings.contains(input_path, "**")) {
log.addError(source, item.loc,
\\TODO multi level globs. For now, try something like "packages/*"
) catch {};
continue;
}

const without_trailing_slash = strings.withoutTrailingSlash(input_path);

if (!strings.endsWithComptime(without_trailing_slash, "/*") and !strings.eqlComptime(without_trailing_slash, "*")) {
log.addError(source, item.loc,
\\TODO glob star * in the middle of a path. For now, try something like "packages/*", at the end of the path.
) catch {};
continue;
}

asterisked_workspace_paths.append(without_trailing_slash) catch unreachable;
continue;
} else if (strings.containsAny(input_path, "!{}[]")) {
log.addError(source, item.loc,
\\TODO fancy glob patterns. For now, try something like "packages/*"
) catch {};
if (bun.glob.detectGlobSyntax(input_path)) {
workspace_globs.append(input_path) catch bun.outOfMemory();
continue;
} else if (string_builder == null) {
input_path = Path.joinAbsStringBuf(source.path.name.dir, filepath_buf, &[_]string{input_path}, .auto);
Expand Down Expand Up @@ -4002,89 +4017,84 @@ pub const Package = extern struct {
});
}

if (asterisked_workspace_paths.items.len > 0) {
for (asterisked_workspace_paths.items) |user_path| {
var dir_prefix = if (string_builder) |_|
strings.withoutLeadingSlash(user_path)
else
Path.joinAbsStringBuf(source.path.name.dir, filepath_buf, &[_]string{user_path}, .auto);
if (workspace_globs.items.len > 0) {
var arena = std.heap.ArenaAllocator.init(allocator);
defer arena.deinit();
for (workspace_globs.items) |user_pattern| {
defer _ = arena.reset(.retain_capacity);

dir_prefix = dir_prefix[0 .. strings.indexOfChar(dir_prefix, '*') orelse continue];
if (dir_prefix.len == 0 or
strings.eqlComptime(dir_prefix, ".") or
strings.eqlComptime(dir_prefix, &.{ '.', std.fs.path.sep }))
{
if (comptime Environment.isWindows)
dir_prefix = Fs.FileSystem.instance.top_level_dir
else
dir_prefix = ".";
const glob_pattern = if (user_pattern.len == 0) "package.json" else brk: {
const parts = [_][]const u8{ user_pattern, "package.json" };
break :brk arena.allocator().dupe(u8, bun.path.join(parts, .auto)) catch bun.outOfMemory();
};

var walker: GlobWalker = .{};
var cwd = bun.path.dirname(source.path.textZ(), .auto);
cwd = if (bun.strings.eql(cwd, "")) bun.fs.FileSystem.instance.top_level_dir else cwd;
if ((try walker.initWithCwd(&arena, glob_pattern, cwd, false, false, false, false, true)).asErr()) |e| {
log.addErrorFmt(
source,
loc,
allocator,
"Failed to run workspace pattern <b>{s}<r> due to error <b>{s}<r>",
.{ user_pattern, @tagName(e.getErrno()) },
) catch {};
return error.GlobError;
}
defer walker.deinit(false);

const entries_option = FileSystem.instance.fs.readDirectory(
dir_prefix,
null,
0,
true,
) catch |err| switch (err) {
error.ENOENT => {
log.addWarningFmt(
source,
loc,
allocator,
"workspaces directory prefix not found \"{s}\"",
.{dir_prefix},
) catch {};
continue;
},
error.ENOTDIR => {
log.addWarningFmt(
var iter: GlobWalker.Iterator = .{
.walker = &walker,
};
defer iter.deinit();
if ((try iter.init()).asErr()) |e| {
zackradisic marked this conversation as resolved.
Show resolved Hide resolved
log.addErrorFmt(
source,
loc,
allocator,
"Failed to run workspace pattern <b>{s}<r> due to error <b>{s}<r>",
.{ user_pattern, @tagName(e.getErrno()) },
) catch {};
return error.GlobError;
}

while (switch (try iter.next()) {
.result => |r| r,
.err => |e| {
log.addErrorFmt(
source,
loc,
allocator,
"workspaces directory prefix is not a directory \"{s}\"",
.{dir_prefix},
"Failed to run workspace pattern <b>{s}<r> due to error <b>{s}<r>",
.{ user_pattern, @tagName(e.getErrno()) },
) catch {};
continue;
return error.GlobError;
},
else => continue,
};
if (entries_option.* != .entries) continue;
var entries = entries_option.entries.data.iterator();
const skipped_names = &[_][]const u8{ "node_modules", ".git" };
}) |matched_path| {
const workspace_file = iter.cwd_fd.value.asDir().openFile(matched_path, .{ .mode = .read_only }) catch |err| {
debug("processWorkspaceName({s}) = {} ", .{ glob_pattern, err });
return err;
};
defer workspace_file.close();

while (entries.next()) |entry_iter| {
const name = entry_iter.key_ptr.*;
if (strings.eqlAnyComptime(name, skipped_names))
continue;
var entry: *FileSystem.Entry = entry_iter.value_ptr.*;
if (entry.kind(&Fs.FileSystem.instance.fs, true) != .dir) continue;
const entry_dir: []const u8 = Path.dirname(matched_path, .auto);
const entry_base: []const u8 = Path.basename(matched_path);
debug("matched path: {s}, dirname: {s}\n", .{ matched_path, entry_dir });

var parts = [2]string{ entry.dir, entry.base() };
var parts = [_]string{entry_dir};
const entry_path = Path.joinAbsStringBufZ(
Fs.FileSystem.instance.topLevelDirWithoutTrailingSlash(),
cwd,
filepath_buf,
&parts,
.auto,
);

if (entry.cache.fd == .zero) {
entry.cache.fd = bun.toFD(bun.sys.open(
entry_path,
std.os.O.DIRECTORY | std.os.O.CLOEXEC | std.os.O.NOCTTY | std.os.O.RDONLY,
0,
).unwrap() catch continue);
}

const dir_fd = entry.cache.fd;
assert(dir_fd != bun.invalid_fd); // kind() should've opened
defer fallback.fixed_buffer_allocator.reset();

const workspace_entry = processWorkspaceName(
const workspace_entry = processWorkspaceNameImpl(
allocator,
workspace_allocator,
dir_fd.asDir(),
workspace_file,
"",
filepath_bufOS,
matched_path,
workspace_name_buf,
log,
) catch |err| {
Expand All @@ -4096,7 +4106,7 @@ pub const Package = extern struct {
logger.Loc.Empty,
allocator,
"Missing \"name\" from package.json in {s}" ++ std.fs.path.sep_str ++ "{s}",
.{ entry.dir, entry.base() },
.{ entry_dir, entry_base },
) catch {};
},
else => {
Expand All @@ -4105,7 +4115,7 @@ pub const Package = extern struct {
logger.Loc.Empty,
allocator,
"{s} reading package.json for workspace package \"{s}\" from \"{s}\"",
.{ @errorName(err), entry.dir, entry.base() },
.{ @errorName(err), entry_dir, entry_base },
) catch {};
},
}
Expand Down
2 changes: 1 addition & 1 deletion src/shell/interpreter.zig
Expand Up @@ -2401,7 +2401,7 @@ pub const Interpreter = struct {

var iter = GlobWalker.Iterator{ .walker = this.walker };
defer iter.deinit();
switch (iter.init() catch |e| OOM(e)) {
switch (iter.init() catch bun.outOfMemory()) {
.err => |err| return .{ .err = err },
else => {},
}
Expand Down
6 changes: 5 additions & 1 deletion src/sys.zig
Expand Up @@ -513,7 +513,11 @@ pub fn mkdiratW(dir_fd: bun.FileDescriptor, file_path: []const u16, _: i32) Mayb
pub fn fstatat(fd: bun.FileDescriptor, path: [:0]const u8) Maybe(bun.Stat) {
if (Environment.isWindows) @compileError("TODO");
zackradisic marked this conversation as resolved.
Show resolved Hide resolved
var stat_ = mem.zeroes(bun.Stat);
if (Maybe(bun.Stat).errnoSys(sys.fstatat(fd.int(), path, &stat_, 0), .fstatat)) |err| return err;
if (Maybe(bun.Stat).errnoSys(sys.fstatat(fd.int(), path, &stat_, 0), .fstatat)) |err| {
log("fstatat({}, {s}) = {s}", .{ fd, path, @tagName(err.getErrno()) });
return err;
}
log("fstatat({}, {s}) = 0", .{ fd, path });
return Maybe(bun.Stat){ .result = stat_ };
}

Expand Down
4 changes: 1 addition & 3 deletions test/cli/install/bad-workspace.test.ts
Expand Up @@ -17,7 +17,7 @@ test("bad workspace path", () => {
JSON.stringify(
{
name: "hey",
workspaces: ["i-dont-exist", "**/i-have-a-2-stars-and-i-dont-exist", "*/i-have-a-star-and-i-dont-exist"],
workspaces: ["i-dont-exist"],
},
null,
2,
Expand All @@ -33,8 +33,6 @@ test("bad workspace path", () => {
const text = stderr!.toString();

expect(text).toContain('Workspace not found "i-dont-exist"');
expect(text).toContain("multi level globs");
expect(text).toContain("glob star * in the middle of a path");

expect(exitCode).toBe(1);
});