Skip to content

Commit

Permalink
fix(cli): Making sure the bunfig is read before checking workspace
Browse files Browse the repository at this point in the history
  • Loading branch information
@JulesGuesnon
JulesGuesnon committed May 12, 2024
1 parent cf84c90 commit e2e49e0
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 8 deletions.
25 changes: 23 additions & 2 deletions src/cli.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ pub const Arguments = struct {
};

defer config_file.close();

const contents = config_file.readToEndAlloc(allocator, std.math.maxInt(usize)) catch |err| {
if (auto_loaded) return;
Output.prettyErrorln("<r><red>error<r>: {s} reading config \"{s}\"", .{
Expand All @@ -284,6 +285,7 @@ pub const Arguments = struct {
ctx.log.level = original_level;
}
ctx.log.level = logger.Log.Level.warn;

try Bunfig.parse(allocator, logger.Source.initPathString(bun.asByteSlice(config_path), contents), ctx, cmd);
}

Expand All @@ -295,7 +297,18 @@ pub const Arguments = struct {

return null;
}

pub fn loadConfig(allocator: std.mem.Allocator, user_config_path_: ?string, ctx: Command.Context, comptime cmd: Command.Tag) !void {
return loadConfig_(allocator, user_config_path_, ctx, cmd, false);
}

pub fn loadConfig_(
allocator: std.mem.Allocator,
user_config_path_: ?string,
ctx: Command.Context,
comptime cmd: Command.Tag,
override_absolute_working_dir: ?bool,
) !void {
var config_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
if (comptime cmd.readGlobalConfig()) {
if (!ctx.has_loaded_global_config) {
Expand All @@ -308,7 +321,6 @@ pub const Arguments = struct {
}

var config_path_: []const u8 = user_config_path_ orelse "";

var auto_loaded: bool = false;
if (config_path_.len == 0 and (user_config_path_ != null or
Command.Tag.always_loads_config.get(cmd) or
Expand All @@ -332,7 +344,7 @@ pub const Arguments = struct {
config_buf[config_path_.len] = 0;
config_path = config_buf[0..config_path_.len :0];
} else {
if (ctx.args.absolute_working_dir == null) {
if ((override_absolute_working_dir orelse false) or ctx.args.absolute_working_dir == null) {
var secondbuf: [bun.MAX_PATH_BYTES]u8 = undefined;
const cwd = bun.getcwd(&secondbuf) catch return;

Expand All @@ -346,6 +358,7 @@ pub const Arguments = struct {
&parts,
.auto,
);

config_buf[config_path_.len] = 0;
config_path = config_buf[0..config_path_.len :0];
}
Expand Down Expand Up @@ -1435,6 +1448,7 @@ pub const Command = struct {
},
.InstallCommand => {
if (comptime bun.fast_debug_build_mode and bun.fast_debug_build_cmd != .InstallCommand) unreachable;

const ctx = try Command.init(allocator, log, .InstallCommand);

try InstallCommand.exec(ctx);
Expand Down Expand Up @@ -2318,6 +2332,13 @@ pub const Command = struct {
pub const uses_global_options: std.EnumArray(Tag, bool) = std.EnumArray(Tag, bool).initDefault(true, .{
.CreateCommand = false,
.BunxCommand = false,
.InstallCommand = false,
.AddCommand = false,
.RemoveCommand = false,
.UpdateCommand = false,
.PackageManagerCommand = false,
.LinkCommand = false,
.UnlinkCommand = false,
});
};
};
Expand Down
24 changes: 21 additions & 3 deletions src/install/install.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6943,6 +6943,7 @@ pub const PackageManager = struct {

pub fn init(ctx: Command.Context, comptime subcommand: Subcommand) !*PackageManager {
const cli = try CommandLineArguments.parse(ctx.allocator, subcommand);

return initWithCLI(ctx, cli, subcommand);
}

Expand Down Expand Up @@ -6975,6 +6976,8 @@ pub const PackageManager = struct {

var workspace_names = Package.WorkspaceMap.init(ctx.allocator);

const before_top_level_dir = try ctx.allocator.dupe(u8, fs.top_level_dir);

// Step 1. Find the nearest package.json directory
//
// We will walk up from the cwd, trying to find the nearest package.json file.
Expand Down Expand Up @@ -7048,6 +7051,9 @@ pub const PackageManager = struct {

const child_cwd = this_cwd;

// Reading a first time the config to know if we need to ignore the workspace or not
try BunArguments.loadConfig(ctx.allocator, cli.config, ctx, .InstallCommand);

var ignore_workspace = false;

if (cli.ignore_workspace) {
Expand All @@ -7060,6 +7066,7 @@ pub const PackageManager = struct {

// Check if this is a workspace; if so, use root package
var found = false;

if ((comptime subcommand != .link) and !ignore_workspace) {
if (!created_package_json) {
while (std.fs.path.dirname(this_cwd)) |parent| : (this_cwd = parent) {
Expand Down Expand Up @@ -7128,8 +7135,15 @@ pub const PackageManager = struct {
break :brk child_json;
};

try bun.sys.chdir(fs.top_level_dir).unwrap();
try BunArguments.loadConfig(ctx.allocator, cli.config, ctx, .InstallCommand);
// If the top level dir is different than before, then we don't ignore the workspace and we found one,
// so we need to read again the config and override the working dir
if (!strings.eql(before_top_level_dir, fs.top_level_dir)) {
try bun.sys.chdir(fs.top_level_dir).unwrap();
ctx.args.absolute_working_dir = try ctx.allocator.dupe(u8, fs.top_level_dir);

try BunArguments.loadConfig_(ctx.allocator, cli.config, ctx, .InstallCommand, true);
}

bun.copy(u8, &cwd_buf, fs.top_level_dir);
cwd_buf[fs.top_level_dir.len] = '/';
cwd_buf[fs.top_level_dir.len + 1] = 0;
Expand Down Expand Up @@ -7253,6 +7267,7 @@ pub const PackageManager = struct {

break :brk @as(u32, @truncate(@as(u64, @intCast(@max(std.time.timestamp(), 0)))));
};

return manager;
}

Expand Down Expand Up @@ -8102,6 +8117,7 @@ pub const PackageManager = struct {
buf[cwd_.len] = 0;
final_path = buf[0..cwd_.len :0];
}

try bun.sys.chdir(final_path).unwrap();
}

Expand Down Expand Up @@ -8966,7 +8982,7 @@ pub const PackageManager = struct {

// pub fn printTreeDeps(this: *PackageInstaller) void {
// for (this.tree_ids_to_trees_the_id_depends_on, 0..) |deps, j| {
// std.debug.print("tree #{d:3}: ", .{j});
// std.debug.print("tree #{d:3}: ", .{j});
// for (0..this.lockfile.buffers.trees.items.len) |tree_id| {
// std.debug.print("{d} ", .{@intFromBool(deps.isSet(tree_id))});
// }
Expand Down Expand Up @@ -10617,9 +10633,11 @@ pub const PackageManager = struct {
_ = manager.getCacheDirectory();
_ = manager.getTemporaryDirectory();
}

manager.enqueueDependencyList(root.dependencies);
} else {
// Anything that needs to be downloaded from an update needs to be scheduled here

manager.drainDependencyList();
}

Expand Down
78 changes: 75 additions & 3 deletions test/cli/install/bun-install.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5933,6 +5933,7 @@ it("should install dependencies in root package of workspace", async () => {
workspaces: ["moo"],
}),
);

await mkdir(join(package_dir, "moo"));
const moo_package = JSON.stringify({
name: "moo",
Expand All @@ -5955,6 +5956,7 @@ it("should install dependencies in root package of workspace", async () => {
expect(err).toContain("Saved lockfile");
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();

expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
"",
" + moo@workspace:moo",
Expand All @@ -5975,6 +5977,79 @@ it("should install dependencies in root package of workspace", async () => {
await access(join(package_dir, "bun.lockb"));
});

it("should ignore workspaces when installing a package inside a workspace with bunfig", async () => {
await writeFile(
join(package_dir, "package.json"),
JSON.stringify({
name: "Foo",
version: "0.0.1",
workspaces: ["bar", "packages/*"],
}),
);

const barPath = join(package_dir, "bar");
await mkdir(barPath);
await writeFile(
join(package_dir, "bar", "package.json"),
JSON.stringify({
name: "Bar",
version: "0.0.2",
}),
);

await mkdir(join(package_dir, "packages", "nominally-scoped"), { recursive: true });
await writeFile(
join(package_dir, "packages", "nominally-scoped", "package.json"),
JSON.stringify({
name: "@org/nominally-scoped",
version: "0.1.4",
}),
);

await mkdir(join(package_dir, "packages", "second-asterisk"), { recursive: true });
await writeFile(
join(package_dir, "packages", "second-asterisk", "package.json"),
JSON.stringify({
name: "AsteriskTheSecond",
version: "0.1.4",
}),
);

await mkdir(join(package_dir, "packages", "asterisk"), { recursive: true });
await writeFile(
join(package_dir, "packages", "asterisk", "package.json"),
JSON.stringify({
name: "Asterisk",
version: "0.0.4",
}),
);

await writeFile(
join(package_dir, "bar", "bunfig.toml"),
`${await file(join(package_dir, "bunfig.toml")).text()}
ignoreWorkspace=true
`,
);

const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: barPath,
stdout: "pipe",
stdin: "pipe",
stderr: "pipe",
env,
});
expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
expect(err).not.toContain("Saved lockfile");
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*/, "").split(/\r?\n/)).toEqual(["done", ""]);
expect(await exited).toBe(0);
expect(requested).toBe(0);
expect(async () => await readdirSorted(join(package_dir, "node_modules"))).toThrow("No such file or directory");
});

it("should install dependencies in root package of workspace (*)", async () => {
const urls: string[] = [];
setHandler(dummyRegistry(urls));
Expand Down Expand Up @@ -6989,8 +7064,6 @@ it("should handle installing workspaces with multiple glob patterns", async () =
},
});

console.log("TEMPDIR", package_dir);

const { stdout, stderr } = await Bun.$`${bunExe()} install`.env(env).cwd(package_dir).throws(true);
const err1 = stderr.toString();
expect(err1).toContain("Saved lockfile");
Expand Down Expand Up @@ -7059,7 +7132,6 @@ it.todo("should handle installing workspaces with absolute glob patterns", async
},
},
});
console.log("TEMP DIR", package_dir);

const { stdout, stderr } = await Bun.$`${bunExe()} install`.env(env).cwd(package_dir).throws(true);
const err1 = stderr.toString();
Expand Down

0 comments on commit e2e49e0

Please sign in to comment.