Commit a17505c71109361b7b4bd13125871299de53e4e0 | zig

author	Andrew Kelley <andrew@ziglang.org> 2024-02-02 20:12:28-07:00
committer	Andrew Kelley <andrew@ziglang.org> 2024-02-02 20:43:01-07:00
log	a17505c71109361b7b4bd13125871299de53e4e0
tree	666d088314d553e1cab96e037f31c28310b599a4
parent	996e61f8137ce468f2997b0220bae32c17ea3a00

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.

2 files changed, 93 insertions(+), 57 deletions(-)

lib
- build_runner.zig
src
- main.zig

lib/build_runner.zig+16-2

 @@ -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.
     }

src/main.zig+77-55

 @@ -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();
+            const term = try child.spawnAndWait();
-            // 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();
             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}", .{
+                                std.log.err("lazy dependency package not found: {s}" ++ s ++ "{s}", .{
-                                    p, fs.path.sep, hash,
+                                    p, hash,
                                 });
                             }
                             std.log.info("remote package fetching disabled due to --system mode", .{});