From a17505c71109361b7b4bd13125871299de53e4e0 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 2 Feb 2024 20:12:28 -0700 Subject: [PATCH] zig build: avoid using stdout for communication with runner Pass the required lazy dependencies from the build runner to the parent process via a tmp file instead of stdout. I'll reproduce this comment to explain it: The `zig build` parent process needs a way to obtain results from the configuration phase of the child process. In the future, the make phase will be executed in a separate process than the configure phase, and we can then use stdout from the configuration phase for this purpose. However, currently, both phases are in the same process, and Run Step provides API for making the runned subprocesses inherit stdout and stderr which means these streams are not available for passing metadata back to the parent. Until make and configure phases are separated into different processes, the strategy is to choose a temporary file name ahead of time, and then read this file in the parent to obtain the results, in the case the child exits with code 3. This commit also extracts some common logic from the loop that rebuilds the build runner so that it does not run again when the build runner is rebuilt. --- lib/build_runner.zig | 18 +++++- src/main.zig | 132 +++++++++++++++++++++++++------------------ 2 files changed, 93 insertions(+), 57 deletions(-) diff --git a/lib/build_runner.zig b/lib/build_runner.zig index 8875acb335c20758cb0eabf62abdec67eed6ce19..5c1daaaf426ab2251146656bc390c8718ebe22b9 100644 --- a/lib/build_runner.zig +++ b/lib/build_runner.zig @@ -99,9 +99,13 @@ pub fn main() !void { var prominent_compile_errors: bool = false; var help_menu: bool = false; var steps_menu: bool = false; + var output_tmp_nonce: ?[16]u8 = null; while (nextArg(args, &arg_idx)) |arg| { - if (mem.startsWith(u8, arg, "-D")) { + if (mem.startsWith(u8, arg, "-Z")) { + if (arg.len != 18) fatalWithHint("bad argument: '{s}'", .{arg}); + output_tmp_nonce = arg[2..18].*; + } else if (mem.startsWith(u8, arg, "-D")) { const option_contents = arg[2..]; if (option_contents.len == 0) fatalWithHint("expected option name after '-D'", .{}); @@ -312,7 +316,17 @@ pub fn main() !void { try buffer.appendSlice(arena, k); try buffer.append(arena, '\n'); } - try io.getStdOut().writeAll(buffer.items); + const s = std.fs.path.sep_str; + const tmp_sub_path = "tmp" ++ s ++ (output_tmp_nonce orelse fatal("missing -Z arg", .{})); + local_cache_directory.handle.writeFile2(.{ + .sub_path = tmp_sub_path, + .data = buffer.items, + .flags = .{ .exclusive = true }, + }) catch |err| { + fatal("unable to write configuration results to '{}{s}': {s}", .{ + local_cache_directory, tmp_sub_path, @errorName(err), + }); + }; process.exit(3); // Indicate configure phase failed with meaningful stdout. } diff --git a/src/main.zig b/src/main.zig index 48d3996a9a838677ec4e77d612b71f00eaceea19..18a29b6233574fd4c7d20dae8e18018ddceeb94a 100644 --- a/src/main.zig +++ b/src/main.zig @@ -5195,6 +5195,23 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi }); const argv_index_seed = child_argv.items.len - 1; + // This parent process needs a way to obtain results from the configuration + // phase of the child process. In the future, the make phase will be + // executed in a separate process than the configure phase, and we can then + // use stdout from the configuration phase for this purpose. + // + // However, currently, both phases are in the same process, and Run Step + // provides API for making the runned subprocesses inherit stdout and stderr + // which means these streams are not available for passing metadata back + // to the parent. + // + // Until make and configure phases are separated into different processes, + // the strategy is to choose a temporary file name ahead of time, and then + // read this file in the parent to obtain the results, in the case the child + // exits with code 3. + const results_tmp_file_nonce = Package.Manifest.hex64(std.crypto.random.int(u64)); + try child_argv.append("-Z" ++ results_tmp_file_nonce); + { var i: usize = 0; while (i < args.len) : (i += 1) { @@ -5311,6 +5328,53 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi .basename = exe_basename, }; + gimmeMoreOfThoseSweetSweetFileDescriptors(); + + var zig_lib_directory: Compilation.Directory = if (override_lib_dir) |lib_dir| .{ + .path = lib_dir, + .handle = fs.cwd().openDir(lib_dir, .{}) catch |err| { + fatal("unable to open zig lib directory from 'zig-lib-dir' argument: '{s}': {s}", .{ lib_dir, @errorName(err) }); + }, + } else introspect.findZigLibDirFromSelfExe(arena, self_exe_path) catch |err| { + fatal("unable to find zig installation directory '{s}': {s}", .{ self_exe_path, @errorName(err) }); + }; + defer zig_lib_directory.handle.close(); + + const cwd_path = try process.getCwdAlloc(arena); + const build_root = try findBuildRoot(arena, .{ + .cwd_path = cwd_path, + .build_file = build_file, + }); + child_argv.items[argv_index_build_file] = build_root.directory.path orelse cwd_path; + + var global_cache_directory: Compilation.Directory = l: { + const p = override_global_cache_dir orelse try introspect.resolveGlobalCacheDir(arena); + break :l .{ + .handle = try fs.cwd().makeOpenPath(p, .{}), + .path = p, + }; + }; + defer global_cache_directory.handle.close(); + + child_argv.items[argv_index_global_cache_dir] = global_cache_directory.path orelse cwd_path; + + var local_cache_directory: Compilation.Directory = l: { + if (override_local_cache_dir) |local_cache_dir_path| { + break :l .{ + .handle = try fs.cwd().makeOpenPath(local_cache_dir_path, .{}), + .path = local_cache_dir_path, + }; + } + const cache_dir_path = try build_root.directory.join(arena, &[_][]const u8{"zig-cache"}); + break :l .{ + .handle = try build_root.directory.handle.makeOpenPath("zig-cache", .{}), + .path = cache_dir_path, + }; + }; + defer local_cache_directory.handle.close(); + + child_argv.items[argv_index_cache_dir] = local_cache_directory.path orelse cwd_path; + var thread_pool: ThreadPool = undefined; try thread_pool.init(.{ .allocator = gpa }); defer thread_pool.deinit(); @@ -5318,8 +5382,6 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi var http_client: std.http.Client = .{ .allocator = gpa }; defer http_client.deinit(); - gimmeMoreOfThoseSweetSweetFileDescriptors(); - var unlazy_set: Package.Fetch.JobQueue.UnlazySet = .{}; // This loop is re-evaluated when the build script exits with an indication that it @@ -5328,51 +5390,6 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi // We want to release all the locks before executing the child process, so we make a nice // big block here to ensure the cleanup gets run when we extract out our argv. { - var zig_lib_directory: Compilation.Directory = if (override_lib_dir) |lib_dir| .{ - .path = lib_dir, - .handle = fs.cwd().openDir(lib_dir, .{}) catch |err| { - fatal("unable to open zig lib directory from 'zig-lib-dir' argument: '{s}': {s}", .{ lib_dir, @errorName(err) }); - }, - } else introspect.findZigLibDirFromSelfExe(arena, self_exe_path) catch |err| { - fatal("unable to find zig installation directory '{s}': {s}", .{ self_exe_path, @errorName(err) }); - }; - defer zig_lib_directory.handle.close(); - - const cwd_path = try process.getCwdAlloc(arena); - const build_root = try findBuildRoot(arena, .{ - .cwd_path = cwd_path, - .build_file = build_file, - }); - child_argv.items[argv_index_build_file] = build_root.directory.path orelse cwd_path; - - var global_cache_directory: Compilation.Directory = l: { - const p = override_global_cache_dir orelse try introspect.resolveGlobalCacheDir(arena); - break :l .{ - .handle = try fs.cwd().makeOpenPath(p, .{}), - .path = p, - }; - }; - defer global_cache_directory.handle.close(); - - child_argv.items[argv_index_global_cache_dir] = global_cache_directory.path orelse cwd_path; - - var local_cache_directory: Compilation.Directory = l: { - if (override_local_cache_dir) |local_cache_dir_path| { - break :l .{ - .handle = try fs.cwd().makeOpenPath(local_cache_dir_path, .{}), - .path = local_cache_dir_path, - }; - } - const cache_dir_path = try build_root.directory.join(arena, &[_][]const u8{"zig-cache"}); - break :l .{ - .handle = try build_root.directory.handle.makeOpenPath("zig-cache", .{}), - .path = cache_dir_path, - }; - }; - defer local_cache_directory.handle.close(); - - child_argv.items[argv_index_cache_dir] = local_cache_directory.path orelse cwd_path; - const main_mod_paths: Package.Module.CreateOptions.Paths = if (override_build_runner) |runner| .{ .root = .{ .root_dir = Cache.Directory.cwd(), @@ -5626,14 +5643,10 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi if (process.can_spawn) { var child = std.ChildProcess.init(child_argv.items, gpa); child.stdin_behavior = .Inherit; - child.stdout_behavior = .Pipe; + child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - try child.spawn(); - // Since only one output stream is piped, we can simply do a blocking - // read until the stream is finished. - const stdout = try child.stdout.?.readToEndAlloc(arena, 50 * 1024 * 1024); - const term = try child.wait(); + const term = try child.spawnAndWait(); switch (term) { .Exited => |code| { if (code == 0) return cleanExit(); @@ -5646,6 +5659,15 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi // Indicates the configure phase failed due to missing lazy // dependencies and stdout contains the hashes of the ones // that are missing. + const s = fs.path.sep_str; + const tmp_sub_path = "tmp" ++ s ++ results_tmp_file_nonce; + const stdout = local_cache_directory.handle.readFileAlloc(arena, tmp_sub_path, 50 * 1024 * 1024) catch |err| { + fatal("unable to read results of configure phase from '{}{s}': {s}", .{ + local_cache_directory, tmp_sub_path, @errorName(err), + }); + }; + local_cache_directory.handle.deleteFile(tmp_sub_path) catch {}; + var it = mem.splitScalar(u8, stdout, '\n'); var any_errors = false; while (it.next()) |hash| { @@ -5665,8 +5687,8 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi // In this mode, the system needs to provide these packages; they // cannot be fetched by Zig. for (unlazy_set.keys()) |hash| { - std.log.err("lazy dependency package not found: {s}{c}{s}", .{ - p, fs.path.sep, hash, + std.log.err("lazy dependency package not found: {s}" ++ s ++ "{s}", .{ + p, hash, }); } std.log.info("remote package fetching disabled due to --system mode", .{}); -- 2.54.0