Skip to content

fix life-cycle script execution #1799

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.

Sign up for GitHub

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

Merged
Jarred-Sumner merged 1 commit into from
Jan 15, 2023
Merged

fix life-cycle script execution #1799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
100 changes: 43 additions & 57 deletions src/cli/run_command.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,82 +533,68 @@ pub const RunCommand = struct {
var needs_to_force_bun = force_using_bun or !found_node;
var optional_bun_self_path: string = "";

if (bin_dirs.len > 0 or package_json_dir.len > 0) {
var new_path_len: usize = PATH.len + 2;
for (bin_dirs) |bin| {
new_path_len += bin.len + 1;
}
var new_path_len: usize = PATH.len + 2;
for (bin_dirs) |bin| {
new_path_len += bin.len + 1;
}

if (package_json_dir.len > 0) {
new_path_len += package_json_dir.len + 1;
}
if (package_json_dir.len > 0) {
new_path_len += package_json_dir.len + 1;
}

new_path_len += if (needs_to_force_bun) bun_node_dir.len + 1 else 0;
new_path_len += root_dir_info.abs_path.len + "node_modules/.bin".len + 1;

var new_path = try std.ArrayList(u8).initCapacity(ctx.allocator, new_path_len);
if (needs_to_force_bun) {
new_path_len += bun_node_dir.len + 1;
}

if (needs_to_force_bun) {
createFakeTemporaryNodeExecutable(&new_path, &optional_bun_self_path) catch unreachable;
if (!force_using_bun) {
this_bundler.env.map.put("NODE", bun_node_dir ++ "/node") catch unreachable;
this_bundler.env.map.put("npm_node_execpath", bun_node_dir ++ "/node") catch unreachable;
this_bundler.env.map.put("npm_execpath", optional_bun_self_path) catch unreachable;
}
var new_path = try std.ArrayList(u8).initCapacity(ctx.allocator, new_path_len);

needs_to_force_bun = false;
if (needs_to_force_bun) {
createFakeTemporaryNodeExecutable(&new_path, &optional_bun_self_path) catch unreachable;
if (!force_using_bun) {
this_bundler.env.map.put("NODE", bun_node_dir ++ "/node") catch unreachable;
this_bundler.env.map.put("npm_node_execpath", bun_node_dir ++ "/node") catch unreachable;
this_bundler.env.map.put("npm_execpath", optional_bun_self_path) catch unreachable;
}

{
var needs_colon = false;
if (package_json_dir.len > 0) {
defer needs_colon = true;
if (needs_colon) {
try new_path.append(':');
}
try new_path.appendSlice(package_json_dir);
}
needs_to_force_bun = false;
}

var bin_dir_i: i32 = @intCast(i32, bin_dirs.len) - 1;
// Iterate in reverse order
// Directories are added to bin_dirs in top-down order
// That means the parent-most node_modules/.bin will be first
while (bin_dir_i >= 0) : (bin_dir_i -= 1) {
defer needs_colon = true;
if (needs_colon) {
try new_path.append(':');
}
try new_path.appendSlice(bin_dirs[@intCast(usize, bin_dir_i)]);
{
var needs_colon = false;
if (package_json_dir.len > 0) {
defer needs_colon = true;
if (needs_colon) {
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Jan 15, 2023

Choose a reason for hiding this comment

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

This branch will always be true

Copy link
Contributor Author

@alexlamsl alexlamsl Jan 15, 2023

Choose a reason for hiding this comment

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

would this defer be executed at the end of if (package_json_dir.len > 0) {...}? 🤔

Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Jan 16, 2023

Choose a reason for hiding this comment

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

yeah

try new_path.append(':');
}
try new_path.appendSlice(package_json_dir);
}

var bin_dir_i: i32 = @intCast(i32, bin_dirs.len) - 1;
// Iterate in reverse order
// Directories are added to bin_dirs in top-down order
// That means the parent-most node_modules/.bin will be first
while (bin_dir_i >= 0) : (bin_dir_i -= 1) {
defer needs_colon = true;
if (needs_colon) {
try new_path.append(':');
}
try new_path.appendSlice(PATH);
try new_path.appendSlice(bin_dirs[@intCast(usize, bin_dir_i)]);
}

this_bundler.env.map.put("PATH", new_path.items) catch unreachable;
PATH = new_path.items;
}

if (needs_to_force_bun) {
needs_to_force_bun = false;

var new_path = try std.ArrayList(u8).initCapacity(ctx.allocator, PATH.len);
createFakeTemporaryNodeExecutable(&new_path, &optional_bun_self_path) catch unreachable;
if (new_path.items.len > 0)
if (needs_colon) {
try new_path.append(':');
try new_path.appendSlice(PATH);

this_bundler.env.map.put("PATH", new_path.items) catch unreachable;

if (!force_using_bun) {
this_bundler.env.map.put("NODE", bun_node_dir ++ "/node") catch unreachable;
this_bundler.env.map.put("npm_node_execpath", bun_node_dir ++ "/node") catch unreachable;
this_bundler.env.map.put("npm_execpath", optional_bun_self_path) catch unreachable;
}
PATH = new_path.items;
try new_path.appendSlice(root_dir_info.abs_path);
try new_path.appendSlice("node_modules/.bin");
try new_path.append(':');
try new_path.appendSlice(PATH);
}

this_bundler.env.map.put("PATH", new_path.items) catch unreachable;
PATH = new_path.items;

this_bundler.env.map.putDefault("npm_config_local_prefix", this_bundler.fs.top_level_dir) catch unreachable;

// we have no way of knowing what version they're expecting without running the node executable
Expand Down
48 changes: 33 additions & 15 deletions src/install/lockfile.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,11 @@ const Stream = std.io.FixedBufferStream([]u8);
pub const default_filename = "bun.lockb";

pub const Scripts = struct {
const StringArrayList = std.ArrayListUnmanaged(string);
const Entry = struct {
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner Jan 15, 2023

Choose a reason for hiding this comment

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

we almost could add a --filter with this! Just missing another map to say "which package.json name does it belong to"

cwd: string,
script: string,
};
const StringArrayList = std.ArrayListUnmanaged(Entry);
const RunCommand = @import("../cli/run_command.zig").RunCommand;

preinstall: StringArrayList = .{},
Expand All @@ -122,7 +126,7 @@ pub const Scripts = struct {
prepare: StringArrayList = .{},
postprepare: StringArrayList = .{},

pub fn hasAny(this: Scripts) bool {
pub fn hasAny(this: *Scripts) bool {
return (this.preinstall.items.len +
this.install.items.len +
this.postinstall.items.len +
Expand All @@ -131,13 +135,28 @@ pub const Scripts = struct {
this.postprepare.items.len) > 0;
}

pub fn run(this: Scripts, allocator: std.mem.Allocator, env: *DotEnv.Loader, silent: bool, comptime hook: []const u8) !void {
for (@field(this, hook).items) |script| {
pub fn run(this: *Scripts, allocator: std.mem.Allocator, env: *DotEnv.Loader, silent: bool, comptime hook: []const u8) !void {
for (@field(this, hook).items) |entry| {
std.debug.assert(Fs.FileSystem.instance_loaded);
const cwd = Fs.FileSystem.instance.top_level_dir;
_ = try RunCommand.runPackageScript(allocator, script, hook, cwd, env, &.{}, silent);
const cwd = Path.joinAbsString(
FileSystem.instance.top_level_dir,
&[_]string{
entry.cwd,
},
.posix,
);
_ = try RunCommand.runPackageScript(allocator, entry.script, hook, cwd, env, &.{}, silent);
}
}

pub fn deinit(this: *Scripts, allocator: std.mem.Allocator) void {
this.preinstall.deinit(allocator);
this.install.deinit(allocator);
this.postinstall.deinit(allocator);
this.preprepare.deinit(allocator);
this.prepare.deinit(allocator);
this.postprepare.deinit(allocator);
}
};

pub fn isEmpty(this: *const Lockfile) bool {
Expand Down Expand Up @@ -2593,20 +2612,18 @@ pub const Package = extern struct {
"prepare",
"preprepare",
};
var cwd: string = "";

inline for (scripts) |script_name| {
if (scripts_prop.expr.get(script_name)) |script| {
if (script.asString(allocator)) |input| {
var list = @field(lockfile.scripts, script_name);
if (list.capacity == 0) {
list.capacity = 1;
list.items = try allocator.alloc(string, 1);
list.items[0] = input;
} else {
try list.append(allocator, input);
if (cwd.len == 0 and source.path.name.dir.len > 0) {
cwd = try allocator.dupe(u8, source.path.name.dir);
}

@field(lockfile.scripts, script_name) = list;
try @field(lockfile.scripts, script_name).append(allocator, .{
.cwd = cwd,
.script = input,
});
}
}
}
Expand Down Expand Up @@ -3100,6 +3117,7 @@ pub fn deinit(this: *Lockfile) void {
this.packages.deinit(this.allocator);
this.unique_packages.deinit(this.allocator);
this.string_pool.deinit();
this.scripts.deinit(this.allocator);
this.workspace_paths.deinit(this.allocator);
}

Expand Down
23 changes: 7 additions & 16 deletions src/js_ast.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2403,10 +2403,7 @@ pub const Expr = struct {

pub inline fn asString(expr: *const Expr, allocator: std.mem.Allocator) ?string {
if (std.meta.activeTag(expr.data) != .e_string) return null;

const key_str = expr.data.e_string;

return if (key_str.isUTF8()) key_str.data else key_str.string(allocator) catch null;
return expr.data.e_string.string(allocator) catch null;
}

pub fn asBool(
Expand Down Expand Up @@ -6841,11 +6838,9 @@ pub const Macro = struct {
var p = self.p;

const node_type: JSNode.Tag = JSNode.Tag.names.get(str.data) orelse {
if (!str.isUTF8()) {
self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{strings.toUTF8Alloc(self.p.allocator, str.slice16()) catch unreachable}) catch unreachable;
} else {
self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{str.data}) catch unreachable;
}
self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{
str.string(self.p.allocator) catch unreachable,
}) catch unreachable;
return false;
};

Expand All @@ -6863,13 +6858,9 @@ pub const Macro = struct {
var p = self.p;

const node_type: JSNode.Tag = JSNode.Tag.names.get(str.data) orelse {
if (!str.isUTF8()) {
self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{
strings.toUTF8Alloc(self.p.allocator, str.slice16()) catch unreachable,
}) catch unreachable;
} else {
self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{str.data}) catch unreachable;
}
self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{
str.string(self.p.allocator) catch unreachable,
}) catch unreachable;
return false;
};

Expand Down
Loading