Skip to content

Commit b324ff7

Browse files
committed
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.
1 parent f91f762 commit b324ff7

File tree

2 files changed

+93
-57
lines changed

2 files changed

+93
-57
lines changed

lib/build_runner.zig

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,13 @@ pub fn main() !void {
9999
var prominent_compile_errors: bool = false;
100100
var help_menu: bool = false;
101101
var steps_menu: bool = false;
102+
var output_tmp_nonce: ?[16]u8 = null;
102103

103104
while (nextArg(args, &arg_idx)) |arg| {
104-
if (mem.startsWith(u8, arg, "-D")) {
105+
if (mem.startsWith(u8, arg, "-Z")) {
106+
if (arg.len != 18) fatalWithHint("bad argument: '{s}'", .{arg});
107+
output_tmp_nonce = arg[2..18].*;
108+
} else if (mem.startsWith(u8, arg, "-D")) {
105109
const option_contents = arg[2..];
106110
if (option_contents.len == 0)
107111
fatalWithHint("expected option name after '-D'", .{});
@@ -312,7 +316,17 @@ pub fn main() !void {
312316
try buffer.appendSlice(arena, k);
313317
try buffer.append(arena, '\n');
314318
}
315-
try io.getStdOut().writeAll(buffer.items);
319+
const s = std.fs.path.sep_str;
320+
const tmp_sub_path = "tmp" ++ s ++ (output_tmp_nonce orelse fatal("missing -Z arg", .{}));
321+
local_cache_directory.handle.writeFile2(.{
322+
.sub_path = tmp_sub_path,
323+
.data = buffer.items,
324+
.flags = .{ .exclusive = true },
325+
}) catch |err| {
326+
fatal("unable to write configuration results to '{}{s}': {s}", .{
327+
local_cache_directory, tmp_sub_path, @errorName(err),
328+
});
329+
};
316330
process.exit(3); // Indicate configure phase failed with meaningful stdout.
317331
}
318332

src/main.zig

Lines changed: 77 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -5195,6 +5195,23 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
51955195
});
51965196
const argv_index_seed = child_argv.items.len - 1;
51975197

5198+
// This parent process needs a way to obtain results from the configuration
5199+
// phase of the child process. In the future, the make phase will be
5200+
// executed in a separate process than the configure phase, and we can then
5201+
// use stdout from the configuration phase for this purpose.
5202+
//
5203+
// However, currently, both phases are in the same process, and Run Step
5204+
// provides API for making the runned subprocesses inherit stdout and stderr
5205+
// which means these streams are not available for passing metadata back
5206+
// to the parent.
5207+
//
5208+
// Until make and configure phases are separated into different processes,
5209+
// the strategy is to choose a temporary file name ahead of time, and then
5210+
// read this file in the parent to obtain the results, in the case the child
5211+
// exits with code 3.
5212+
const results_tmp_file_nonce = Package.Manifest.hex64(std.crypto.random.int(u64));
5213+
try child_argv.append("-Z" ++ results_tmp_file_nonce);
5214+
51985215
{
51995216
var i: usize = 0;
52005217
while (i < args.len) : (i += 1) {
@@ -5311,15 +5328,60 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
53115328
.basename = exe_basename,
53125329
};
53135330

5331+
gimmeMoreOfThoseSweetSweetFileDescriptors();
5332+
5333+
var zig_lib_directory: Compilation.Directory = if (override_lib_dir) |lib_dir| .{
5334+
.path = lib_dir,
5335+
.handle = fs.cwd().openDir(lib_dir, .{}) catch |err| {
5336+
fatal("unable to open zig lib directory from 'zig-lib-dir' argument: '{s}': {s}", .{ lib_dir, @errorName(err) });
5337+
},
5338+
} else introspect.findZigLibDirFromSelfExe(arena, self_exe_path) catch |err| {
5339+
fatal("unable to find zig installation directory '{s}': {s}", .{ self_exe_path, @errorName(err) });
5340+
};
5341+
defer zig_lib_directory.handle.close();
5342+
5343+
const cwd_path = try process.getCwdAlloc(arena);
5344+
const build_root = try findBuildRoot(arena, .{
5345+
.cwd_path = cwd_path,
5346+
.build_file = build_file,
5347+
});
5348+
child_argv.items[argv_index_build_file] = build_root.directory.path orelse cwd_path;
5349+
5350+
var global_cache_directory: Compilation.Directory = l: {
5351+
const p = override_global_cache_dir orelse try introspect.resolveGlobalCacheDir(arena);
5352+
break :l .{
5353+
.handle = try fs.cwd().makeOpenPath(p, .{}),
5354+
.path = p,
5355+
};
5356+
};
5357+
defer global_cache_directory.handle.close();
5358+
5359+
child_argv.items[argv_index_global_cache_dir] = global_cache_directory.path orelse cwd_path;
5360+
5361+
var local_cache_directory: Compilation.Directory = l: {
5362+
if (override_local_cache_dir) |local_cache_dir_path| {
5363+
break :l .{
5364+
.handle = try fs.cwd().makeOpenPath(local_cache_dir_path, .{}),
5365+
.path = local_cache_dir_path,
5366+
};
5367+
}
5368+
const cache_dir_path = try build_root.directory.join(arena, &[_][]const u8{"zig-cache"});
5369+
break :l .{
5370+
.handle = try build_root.directory.handle.makeOpenPath("zig-cache", .{}),
5371+
.path = cache_dir_path,
5372+
};
5373+
};
5374+
defer local_cache_directory.handle.close();
5375+
5376+
child_argv.items[argv_index_cache_dir] = local_cache_directory.path orelse cwd_path;
5377+
53145378
var thread_pool: ThreadPool = undefined;
53155379
try thread_pool.init(.{ .allocator = gpa });
53165380
defer thread_pool.deinit();
53175381

53185382
var http_client: std.http.Client = .{ .allocator = gpa };
53195383
defer http_client.deinit();
53205384

5321-
gimmeMoreOfThoseSweetSweetFileDescriptors();
5322-
53235385
var unlazy_set: Package.Fetch.JobQueue.UnlazySet = .{};
53245386

53255387
// 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
53285390
// We want to release all the locks before executing the child process, so we make a nice
53295391
// big block here to ensure the cleanup gets run when we extract out our argv.
53305392
{
5331-
var zig_lib_directory: Compilation.Directory = if (override_lib_dir) |lib_dir| .{
5332-
.path = lib_dir,
5333-
.handle = fs.cwd().openDir(lib_dir, .{}) catch |err| {
5334-
fatal("unable to open zig lib directory from 'zig-lib-dir' argument: '{s}': {s}", .{ lib_dir, @errorName(err) });
5335-
},
5336-
} else introspect.findZigLibDirFromSelfExe(arena, self_exe_path) catch |err| {
5337-
fatal("unable to find zig installation directory '{s}': {s}", .{ self_exe_path, @errorName(err) });
5338-
};
5339-
defer zig_lib_directory.handle.close();
5340-
5341-
const cwd_path = try process.getCwdAlloc(arena);
5342-
const build_root = try findBuildRoot(arena, .{
5343-
.cwd_path = cwd_path,
5344-
.build_file = build_file,
5345-
});
5346-
child_argv.items[argv_index_build_file] = build_root.directory.path orelse cwd_path;
5347-
5348-
var global_cache_directory: Compilation.Directory = l: {
5349-
const p = override_global_cache_dir orelse try introspect.resolveGlobalCacheDir(arena);
5350-
break :l .{
5351-
.handle = try fs.cwd().makeOpenPath(p, .{}),
5352-
.path = p,
5353-
};
5354-
};
5355-
defer global_cache_directory.handle.close();
5356-
5357-
child_argv.items[argv_index_global_cache_dir] = global_cache_directory.path orelse cwd_path;
5358-
5359-
var local_cache_directory: Compilation.Directory = l: {
5360-
if (override_local_cache_dir) |local_cache_dir_path| {
5361-
break :l .{
5362-
.handle = try fs.cwd().makeOpenPath(local_cache_dir_path, .{}),
5363-
.path = local_cache_dir_path,
5364-
};
5365-
}
5366-
const cache_dir_path = try build_root.directory.join(arena, &[_][]const u8{"zig-cache"});
5367-
break :l .{
5368-
.handle = try build_root.directory.handle.makeOpenPath("zig-cache", .{}),
5369-
.path = cache_dir_path,
5370-
};
5371-
};
5372-
defer local_cache_directory.handle.close();
5373-
5374-
child_argv.items[argv_index_cache_dir] = local_cache_directory.path orelse cwd_path;
5375-
53765393
const main_mod_paths: Package.Module.CreateOptions.Paths = if (override_build_runner) |runner| .{
53775394
.root = .{
53785395
.root_dir = Cache.Directory.cwd(),
@@ -5626,14 +5643,10 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
56265643
if (process.can_spawn) {
56275644
var child = std.ChildProcess.init(child_argv.items, gpa);
56285645
child.stdin_behavior = .Inherit;
5629-
child.stdout_behavior = .Pipe;
5646+
child.stdout_behavior = .Inherit;
56305647
child.stderr_behavior = .Inherit;
56315648

5632-
try child.spawn();
5633-
// Since only one output stream is piped, we can simply do a blocking
5634-
// read until the stream is finished.
5635-
const stdout = try child.stdout.?.readToEndAlloc(arena, 50 * 1024 * 1024);
5636-
const term = try child.wait();
5649+
const term = try child.spawnAndWait();
56375650
switch (term) {
56385651
.Exited => |code| {
56395652
if (code == 0) return cleanExit();
@@ -5646,6 +5659,15 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
56465659
// Indicates the configure phase failed due to missing lazy
56475660
// dependencies and stdout contains the hashes of the ones
56485661
// that are missing.
5662+
const s = fs.path.sep_str;
5663+
const tmp_sub_path = "tmp" ++ s ++ results_tmp_file_nonce;
5664+
const stdout = local_cache_directory.handle.readFileAlloc(arena, tmp_sub_path, 50 * 1024 * 1024) catch |err| {
5665+
fatal("unable to read results of configure phase from '{}{s}': {s}", .{
5666+
local_cache_directory, tmp_sub_path, @errorName(err),
5667+
});
5668+
};
5669+
local_cache_directory.handle.deleteFile(tmp_sub_path) catch {};
5670+
56495671
var it = mem.splitScalar(u8, stdout, '\n');
56505672
var any_errors = false;
56515673
while (it.next()) |hash| {
@@ -5665,8 +5687,8 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
56655687
// In this mode, the system needs to provide these packages; they
56665688
// cannot be fetched by Zig.
56675689
for (unlazy_set.keys()) |hash| {
5668-
std.log.err("lazy dependency package not found: {s}{c}{s}", .{
5669-
p, fs.path.sep, hash,
5690+
std.log.err("lazy dependency package not found: {s}" ++ s ++ "{s}", .{
5691+
p, hash,
56705692
});
56715693
}
56725694
std.log.info("remote package fetching disabled due to --system mode", .{});

0 commit comments

Comments
 (0)