From f34a04d80a60927beb602270a0c774808630bec6 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 18 Jun 2025 14:48:20 -0700 Subject: [PATCH] here --- src/bun.js/api/bun/spawn.zig | 24 ++++-- src/env_loader.zig | 10 +-- src/install/GitRunner.zig | 150 +++++++++++++++-------------------- src/install/install.zig | 4 +- src/install/repository.zig | 116 +++++++++++++++++---------- src/string/StringBuilder.zig | 31 ++++++++ 6 files changed, 192 insertions(+), 143 deletions(-) diff --git a/src/bun.js/api/bun/spawn.zig b/src/bun.js/api/bun/spawn.zig index ed56eb5e68..de6e6ab4b2 100644 --- a/src/bun.js/api/bun/spawn.zig +++ b/src/bun.js/api/bun/spawn.zig @@ -14,6 +14,12 @@ const errno = std.posix.errno; const mode_t = std.posix.mode_t; const unexpectedErrno = std.posix.unexpectedErrno; +const NullTerminatedCStringSlice = [*:null]?[*:0]const u8; + +fn allocateSpawnArguments(allocator: std.mem.Allocator, argv: anytype) !struct { NullTerminatedCStringSlice, []u8 } { + return try bun.StringBuilder.createNullDelimited(allocator, argv); +} + pub const BunSpawn = struct { pub const Action = extern struct { pub const FileActionType = enum(u8) { @@ -133,6 +139,9 @@ pub const BunSpawn = struct { _ = this; } }; + + pub const allocateArguments = allocateSpawnArguments; + pub const Argv = NullTerminatedCStringSlice; }; // mostly taken from zig's posix_spawn.zig @@ -292,15 +301,15 @@ pub const PosixSpawn = struct { pid: *c_int, path: [*:0]const u8, request: *const BunSpawnRequest, - argv: [*:null]?[*:0]const u8, - envp: [*:null]?[*:0]const u8, + argv: Argv, + envp: Argv, ) isize; pub fn spawn( path: [*:0]const u8, req_: BunSpawnRequest, - argv: [*:null]?[*:0]const u8, - envp: [*:null]?[*:0]const u8, + argv: Argv, + envp: Argv, ) Maybe(pid_t) { var req = req_; var pid: c_int = 0; @@ -331,8 +340,8 @@ pub const PosixSpawn = struct { path: [*:0]const u8, actions: ?Actions, attr: ?Attr, - argv: [*:null]?[*:0]const u8, - envp: [*:null]?[*:0]const u8, + argv: Argv, + envp: Argv, ) Maybe(pid_t) { if (comptime Environment.isLinux) { return BunSpawnRequest.spawn( @@ -440,4 +449,7 @@ pub const PosixSpawn = struct { pub const Rusage = process.Rusage; pub const Stdio = @import("spawn/stdio.zig").Stdio; + + pub const allocateArguments = allocateSpawnArguments; + pub const Argv = NullTerminatedCStringSlice; }; diff --git a/src/env_loader.zig b/src/env_loader.zig index dd6cfbd78a..766426201b 100644 --- a/src/env_loader.zig +++ b/src/env_loader.zig @@ -13,6 +13,8 @@ const Api = @import("./api/schema.zig").Api; const which = @import("./which.zig").which; const s3 = bun.S3; +pub const NullDelimitedEnvMap = [:null]?[*:0]const u8; + const DotEnvFileSuffix = enum { development, production, @@ -1146,13 +1148,11 @@ pub const Map = struct { map: HashTable, - pub fn createNullDelimitedEnvMap(this: *Map, arena: std.mem.Allocator) ![:null]?[*:0]const u8 { - var env_map = &this.map; - - const envp_count = env_map.count(); + pub fn createNullDelimitedEnvMap(this: *const Map, arena: std.mem.Allocator) !NullDelimitedEnvMap { + const envp_count = this.map.count(); const envp_buf = try arena.allocSentinel(?[*:0]const u8, envp_count, null); { - var it = env_map.iterator(); + var it = this.map.iterator(); var i: usize = 0; while (it.next()) |pair| : (i += 1) { const env_buf = try arena.allocSentinel(u8, pair.key_ptr.len + pair.value_ptr.value.len + 1, 0); diff --git a/src/install/GitRunner.zig b/src/install/GitRunner.zig index f999eaecf3..bbaedf9902 100644 --- a/src/install/GitRunner.zig +++ b/src/install/GitRunner.zig @@ -21,10 +21,12 @@ pub const GitRunner = struct { remaining_fds: i8 = 0, has_called_process_exit: bool = false, completion_context: CompletionContext, - envp: [:null]?[*:0]const u8, + envp: DotEnv.NullDelimitedEnvMap, /// The git command arguments (owned by this runner) - argv: [][]const u8, + argv: bun.spawn.Argv, + argv_bytes: []u8, + arena: bun.ArenaAllocator, /// Allocator for this runner allocator: std.mem.Allocator, @@ -56,33 +58,27 @@ pub const GitRunner = struct { }, }; - pub fn new( + pub fn init( allocator: std.mem.Allocator, manager: *PackageManager, context: CompletionContext, argv: []const []const u8, - env_map: DotEnv.Map, + env_map: *const DotEnv.Map, ) !*GitRunner { + const argv_, const argv_bytes = bun.StringBuilder.createNullDelimited(allocator, argv) catch bun.outOfMemory(); const runner = try allocator.create(GitRunner); - - // Convert env map to envp - need a mutable copy - var env = env_map; - const envp = try env.createNullDelimitedEnvMap(allocator); - - // Duplicate argv strings so we own them - const owned_argv = try allocator.alloc([]const u8, argv.len); - for (argv, 0..) |arg, i| { - owned_argv[i] = try allocator.dupe(u8, arg); - } - runner.* = .{ .manager = manager, .completion_context = context, - .argv = owned_argv, - .envp = envp, + .argv = argv_, + .argv_bytes = argv_bytes, + .arena = bun.ArenaAllocator.init(allocator), + .envp = undefined, .allocator = allocator, }; + runner.envp = try env_map.createNullDelimitedEnvMap(runner.arena.allocator()); + runner.stdout.setParent(runner); runner.stderr.setParent(runner); @@ -92,33 +88,36 @@ pub const GitRunner = struct { pub fn spawn(this: *GitRunner) !void { const spawn_options = bun.spawn.SpawnOptions{ .stdin = .ignore, - .stdout = .buffer, - .stderr = .buffer, - .cwd = bun.fs.FileSystem.instance.top_level_dir, + + .stdout = if (this.manager.options.log_level == .silent) + .ignore + else if (this.manager.options.log_level.isVerbose()) + .inherit + else if (Environment.isPosix) + .buffer + else + .{ + .buffer = this.stdout.source.?.pipe, + }, + .stderr = if (this.manager.options.log_level == .silent) + .ignore + else if (this.manager.options.log_level.isVerbose()) + .inherit + else if (Environment.isPosix) + .buffer + else + .{ + .buffer = this.stderr.source.?.pipe, + }, + .windows = if (Environment.isWindows) .{ .loop = JSC.EventLoopHandle.init(&this.manager.event_loop), - } else {}, + }, + .stream = false, }; - // Convert []const []const u8 to argv format - var argv_buf = try this.allocator.alloc(?[*:0]const u8, this.argv.len + 1); - defer { - // Free the duped null-terminated strings - for (argv_buf[0..this.argv.len]) |arg_ptr| { - if (arg_ptr) |ptr| { - this.allocator.free(std.mem.span(ptr)); - } - } - this.allocator.free(argv_buf); - } - - for (this.argv, 0..) |arg, i| { - argv_buf[i] = try this.allocator.dupeZ(u8, arg); - } - argv_buf[this.argv.len] = null; - - var spawned = try (try bun.spawn.spawnProcess(&spawn_options, @ptrCast(argv_buf.ptr), this.envp)).unwrap(); + var spawned = try (try bun.spawn.spawnProcess(&spawn_options, this.argv, this.envp)).unwrap(); this.remaining_fds = 0; @@ -201,7 +200,8 @@ pub const GitRunner = struct { } pub fn handleExit(this: *GitRunner, status: bun.spawn.Status) void { - defer this.deinit(); + var must_deinit = true; + defer if (must_deinit) this.deinit(); switch (status) { .exited => |exit| { @@ -210,10 +210,12 @@ pub const GitRunner = struct { const stdout_data = this.stdout.finalBuffer(); switch (this.completion_context) { - .download => |ctx| { + .download => |*ctx| { // For download, open the created repo directory - var local_folder_name_buf = std.mem.zeroes([bun.MAX_PATH_BYTES]u8); - const folder_name = std.fmt.bufPrintZ(&local_folder_name_buf, "{any}.git", .{ + const local_folder_name_buf = bun.PathBufferPool.get(); + defer bun.PathBufferPool.put(local_folder_name_buf); + + const folder_name = std.fmt.bufPrintZ(local_folder_name_buf, "{any}.git", .{ bun.fmt.hexIntLower(ctx.task_id), }) catch bun.outOfMemory(); @@ -223,7 +225,7 @@ pub const GitRunner = struct { this.manager.onGitDownloadComplete(ctx.task_id, err) catch {}; } }, - .find_commit => |ctx| { + .find_commit => |*ctx| { // For find_commit, we need to parse the commit hash from stdout const commit_hash = std.mem.trim(u8, stdout_data.items, " \t\r\n"); if (commit_hash.len > 0) { @@ -233,7 +235,7 @@ pub const GitRunner = struct { this.manager.onGitFindCommitComplete(ctx.task_id, error.InstallFailed) catch {}; } }, - .checkout => |ctx| { + .checkout => |*ctx| { if (!this.is_second_step) { const buf = bun.PathBufferPool.get(); defer bun.PathBufferPool.put(buf); @@ -242,39 +244,25 @@ pub const GitRunner = struct { const folder = Path.joinAbsStringBuf(PackageManager.get().cache_directory_path, buf, &.{folder_name}, .auto); const checkout_argv = [_][]const u8{ "git", "-C", folder, "checkout", "--quiet", ctx.resolved }; + this.allocator.free(this.argv_bytes); + const argv, const argv_bytes = bun.spawn.allocateArguments(this.allocator, checkout_argv) catch bun.outOfMemory(); + this.argv = argv; + this.argv_bytes = argv_bytes; + this.is_second_step = true; + this.stdout.deinit(); + this.stderr.deinit(); + this.remaining_fds = 0; - // Create a new completion context for the checkout step - var checkout_context = this.completion_context; - switch (checkout_context) { - .checkout => |*checkout_ctx| { - checkout_ctx.* = checkout_ctx.*; // Copy the context - }, - else => unreachable, - } + this.stdout = bun.io.BufferedReader.init(@This()); + this.stderr = bun.io.BufferedReader.init(@This()); + this.stdout.setParent(this); + this.stderr.setParent(this); - // Create environment map from this runner's envp - const checkout_env = DotEnv.Map.init(this.allocator); - // Note: Creating a new env map from scratch since envp is complex to duplicate - // This should be fine since Repository.shared_env.get() creates the environment anyway - - // Create a new runner for the checkout step using GitRunner.new() - const checkout_runner = GitRunner.new( - this.allocator, - this.manager, - checkout_context, - &checkout_argv, - checkout_env, - ) catch |err| { + this.spawn() catch |err| { this.manager.onGitCheckoutComplete(ctx.task_id, err) catch {}; return; }; - checkout_runner.is_second_step = true; - - checkout_runner.spawn() catch |err| { - this.manager.onGitCheckoutComplete(ctx.task_id, err) catch {}; - checkout_runner.deinit(); - return; - }; + must_deinit = false; } else { // Second step completed (checkout), clean up and read package.json const folder_name = PackageManager.cachedGitFolderNamePrint(&folder_name_buf, ctx.resolved, null); @@ -394,20 +382,8 @@ pub const GitRunner = struct { this.stdout.deinit(); this.stderr.deinit(); - // Free envp - for (this.envp) |env_str| { - if (env_str) |str| { - this.allocator.free(std.mem.span(str)); - } - } - this.allocator.free(this.envp); - - // Free owned argv strings - for (this.argv) |arg| { - this.allocator.free(arg); - } - this.allocator.free(this.argv); - + this.allocator.free(this.argv_bytes); + this.arena.deinit(); this.allocator.destroy(this); } }; diff --git a/src/install/install.zig b/src/install/install.zig index dd6816f1b6..153a9b07aa 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -3827,7 +3827,7 @@ pub const PackageManager = struct { try Repository.findCommit( this, this.allocator, - this.env, + this.env.map, this.log, repo_fd.stdDir(), alias, @@ -9701,7 +9701,7 @@ pub const PackageManager = struct { try Repository.findCommit( this, this.allocator, - this.env, + this.env.map, this.log, repo_dir, this.lockfile.str(&dependency.name), diff --git a/src/install/repository.zig b/src/install/repository.zig index a532792588..a24fcfff56 100644 --- a/src/install/repository.zig +++ b/src/install/repository.zig @@ -137,33 +137,35 @@ pub const Repository = extern struct { pub var shared_env: struct { env: ?DotEnv.Map = null, - pub fn get(this: *@This(), allocator: std.mem.Allocator, other: *DotEnv.Loader) DotEnv.Map { - return this.env orelse brk: { - // Note: currently if the user sets this to some value that causes - // a prompt for a password, the stdout of the prompt will be masked - // by further output of the rest of the install process. - // A value can still be entered, but we need to find a workaround - // so the user can see what is being prompted. By default the settings - // below will cause no prompt and throw instead. - var cloned = other.map.cloneWithAllocator(allocator) catch bun.outOfMemory(); + pub fn get(this: *@This(), allocator: std.mem.Allocator, other: *const DotEnv.Loader) *const DotEnv.Map { + if (this.env) |*env| { + return env; + } - if (cloned.get("GIT_ASKPASS") == null) { - const config = SloppyGlobalGitConfig.get(); - if (!config.has_askpass) { - cloned.put("GIT_ASKPASS", "echo") catch bun.outOfMemory(); - } + // Note: currently if the user sets this to some value that causes + // a prompt for a password, the stdout of the prompt will be masked + // by further output of the rest of the install process. + // A value can still be entered, but we need to find a workaround + // so the user can see what is being prompted. By default the settings + // below will cause no prompt and throw instead. + var cloned = other.map.cloneWithAllocator(allocator) catch bun.outOfMemory(); + + if (cloned.get("GIT_ASKPASS") == null) { + const config = SloppyGlobalGitConfig.get(); + if (!config.has_askpass) { + cloned.put("GIT_ASKPASS", "echo") catch bun.outOfMemory(); } + } - if (cloned.get("GIT_SSH_COMMAND") == null) { - const config = SloppyGlobalGitConfig.get(); - if (!config.has_ssh_command) { - cloned.put("GIT_SSH_COMMAND", "ssh -oStrictHostKeyChecking=accept-new") catch bun.outOfMemory(); - } + if (cloned.get("GIT_SSH_COMMAND") == null) { + const config = SloppyGlobalGitConfig.get(); + if (!config.has_ssh_command) { + cloned.put("GIT_SSH_COMMAND", "ssh -oStrictHostKeyChecking=accept-new") catch bun.outOfMemory(); } + } - this.env = cloned; - break :brk this.env.?; - }; + this.env = cloned; + return &this.env.?; } } = .{}; @@ -414,7 +416,7 @@ pub const Repository = extern struct { pub fn download( pm: *PackageManager, allocator: std.mem.Allocator, - env: DotEnv.Map, + env: *const DotEnv.Map, _: *logger.Log, cache_dir: std.fs.Dir, task_id: u64, @@ -433,7 +435,16 @@ pub const Repository = extern struct { defer bun.PathBufferPool.put(buf); const path = Path.joinAbsStringBuf(PackageManager.get().cache_directory_path, buf, &.{folder_name}, .auto); - const argv = try allocator.dupe([]const u8, &[_][]const u8{ "git", "-C", path, "fetch", "--quiet" }); + const gitpath = bun.PathBufferPool.get(); + defer bun.PathBufferPool.put(gitpath); + const git = bun.which(gitpath, env.get("PATH") orelse "", bun.fs.FileSystem.instance.top_level_dir, "git") orelse "git"; + const argv = &[_][]const u8{ + git, + "-C", + path, + "fetch", + "--quiet", + }; const context = GitRunner.CompletionContext{ .download = .{ @@ -445,7 +456,7 @@ pub const Repository = extern struct { }, }; - const runner = try GitRunner.new(allocator, pm, context, argv, env); + const runner = try GitRunner.init(allocator, pm, context, argv, env); try runner.spawn(); } else |not_found| { if (not_found != error.FileNotFound) { @@ -458,15 +469,19 @@ pub const Repository = extern struct { defer bun.PathBufferPool.put(buf); const target = Path.joinAbsStringBuf(PackageManager.get().cache_directory_path, buf, &.{folder_name}, .auto); - const argv = try allocator.dupe([]const u8, &[_][]const u8{ - "git", + const gitpath = bun.PathBufferPool.get(); + defer bun.PathBufferPool.put(gitpath); + const git = bun.which(gitpath, env.get("PATH") orelse "", bun.fs.FileSystem.instance.top_level_dir, "git") orelse "git"; + const argv = &[_][]const u8{ + git, "clone", - "-c core.longpaths=true", + "-c", + "core.longpaths=true", "--quiet", "--bare", url, target, - }); + }; const context = GitRunner.CompletionContext{ .download = .{ @@ -478,7 +493,13 @@ pub const Repository = extern struct { }, }; - const runner = try GitRunner.new(allocator, pm, context, argv, env); + const runner = try GitRunner.init( + allocator, + pm, + context, + argv, + env, + ); try runner.spawn(); } } @@ -486,7 +507,7 @@ pub const Repository = extern struct { pub fn findCommit( pm: *PackageManager, allocator: std.mem.Allocator, - env: *DotEnv.Loader, + env: *const DotEnv.Map, _: *logger.Log, repo_dir: std.fs.Dir, name: string, @@ -499,11 +520,16 @@ pub const Repository = extern struct { bun.fmt.hexIntLower(task_id), })}, .auto); - const argv = if (committish.len > 0) - try allocator.dupe([]const u8, &[_][]const u8{ "git", "-C", path, "log", "--format=%H", "-1", committish }) - else - try allocator.dupe([]const u8, &[_][]const u8{ "git", "-C", path, "log", "--format=%H", "-1" }); - + const gitpath = bun.PathBufferPool.get(); + defer bun.PathBufferPool.put(gitpath); + const git = bun.which( + gitpath, + env.get("PATH") orelse "", + bun.fs.FileSystem.instance.top_level_dir, + "git", + ) orelse "git"; + const argv_buf = &[_][]const u8{ git, "-C", path, "log", "--format=%H", "-1", committish }; + const argv: []const []const u8 = if (committish.len > 0) argv_buf else argv_buf[0 .. argv_buf.len - 1]; const context = GitRunner.CompletionContext{ .find_commit = .{ .name = try allocator.dupe(u8, name), @@ -513,14 +539,14 @@ pub const Repository = extern struct { }, }; - const runner = try GitRunner.new(allocator, pm, context, argv, shared_env.get(allocator, env)); + const runner = try GitRunner.init(allocator, pm, context, argv, env); try runner.spawn(); } pub fn checkout( pm: *PackageManager, allocator: std.mem.Allocator, - env: DotEnv.Map, + env: *const DotEnv.Map, _: *logger.Log, cache_dir: std.fs.Dir, repo_dir: std.fs.Dir, @@ -557,15 +583,19 @@ pub const Repository = extern struct { const repo_path = try bun.getFdPath(.fromStdDir(repo_dir), &final_path_buf); // First clone without checkout - const clone_argv = try allocator.dupe([]const u8, &[_][]const u8{ - "git", + const gitpath = bun.PathBufferPool.get(); + defer bun.PathBufferPool.put(gitpath); + const git = bun.which(gitpath, env.get("PATH") orelse "", bun.fs.FileSystem.instance.top_level_dir, "git") orelse "git"; + const argv = &[_][]const u8{ + git, "clone", - "-c core.longpaths=true", + "-c", + "core.longpaths=true", "--quiet", "--no-checkout", repo_path, target, - }); + }; // Then we'll need to checkout after clone completes // For now, we'll do both operations in sequence @@ -580,7 +610,7 @@ pub const Repository = extern struct { }, }; - const runner = try GitRunner.new(allocator, pm, context, clone_argv, env); + const runner = try GitRunner.init(allocator, pm, context, argv, env); try runner.spawn(); } } diff --git a/src/string/StringBuilder.zig b/src/string/StringBuilder.zig index 803fdcd4ef..85b7845200 100644 --- a/src/string/StringBuilder.zig +++ b/src/string/StringBuilder.zig @@ -34,6 +34,37 @@ pub fn allocate(this: *StringBuilder, allocator: Allocator) Allocator.Error!void this.len = 0; } +/// Allocates a null-terminated slice in a single allocation. +pub fn createNullDelimited(allocator: Allocator, args: anytype) Allocator.Error!struct { [*:null]?[*:0]const u8, []u8 } { + var this = bun.StringBuilder{}; + for (args) |arg| { + this.countZ(arg); + } + + const byte_len = this.cap + ((args.len + 1) * @sizeOf([*:0]const u8)); + // Slice of: + // ptrs: [*:null][*:0]const u8 + // bytes: [*]u8 + + const raw_bytes = allocator.rawAlloc( + byte_len, + .@"8", + @returnAddress(), + ) orelse return error.OutOfMemory; + const allocated_bytes = raw_bytes[0..byte_len]; + const ptrs: [*:null]?[*:0]const u8 = @alignCast(@ptrCast(allocated_bytes.ptr)); + const bytes: []u8 = allocated_bytes[((args.len + 1) * @sizeOf([*:0]const u8))..]; + + this.ptr = bytes.ptr; + + for (ptrs[0..args.len], args) |*ptr, arg| { + ptr.* = this.appendZ(arg).ptr; + } + ptrs[args.len] = null; + + return .{ ptrs, bytes }; +} + pub fn deinit(this: *StringBuilder, allocator: Allocator) void { if (this.ptr == null or this.cap == 0) return; allocator.free(this.ptr.?[0..this.cap]);