From a3c20dffaed77727494d34f7b4b03c0d10771270 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 11 Jul 2024 16:26:04 -0700 Subject: [PATCH] integrate Compile steps with file watching Updates the build runner to unconditionally require a zig lib directory parameter. This parameter is needed in order to correctly understand file system inputs from zig compiler subprocesses, since they will refer to "the zig lib directory", and the build runner needs to place file system watches on directories in there. The build runner's fanotify file watching implementation now accounts for when two or more Cache.Path instances compare unequal but ultimately refer to the same directory in the file system. Breaking change: std.Build no longer has a zig_lib_dir field. Instead, there is the Graph zig_lib_directory field, and individual Compile steps can still have their zig lib directories overridden. I think this is unlikely to break anyone's build in practice. The compiler now sends a "file_system_inputs" message to the build runner which shares the full set of files that were added to the cache system with the build system, so that the build runner can watch properly and redo the Compile step. This is implemented for whole cache mode but not yet for incremental cache mode. --- build.zig | 9 +++--- lib/compiler/build_runner.zig | 55 +++++++++++++++++++---------------- lib/std/Build.zig | 16 +++++----- lib/std/Build/Cache.zig | 16 ++++++++++ lib/std/Build/Step.zig | 38 ++++++++++++++++++++++++ lib/std/zig/Server.zig | 16 +++++++++- src/Compilation.zig | 14 +++++++++ src/main.zig | 32 +++++++++++++++++--- 8 files changed, 152 insertions(+), 44 deletions(-) diff --git a/build.zig b/build.zig index a364982ce9..d234c68c8e 100644 --- a/build.zig +++ b/build.zig @@ -1261,7 +1261,9 @@ fn generateLangRef(b: *std.Build) std.Build.LazyPath { }); var dir = b.build_root.handle.openDir("doc/langref", .{ .iterate = true }) catch |err| { - std.debug.panic("unable to open 'doc/langref' directory: {s}", .{@errorName(err)}); + std.debug.panic("unable to open '{}doc/langref' directory: {s}", .{ + b.build_root, @errorName(err), + }); }; defer dir.close(); @@ -1280,10 +1282,7 @@ fn generateLangRef(b: *std.Build) std.Build.LazyPath { // in a temporary directory "--cache-root", b.cache_root.path orelse ".", }); - if (b.zig_lib_dir) |p| { - cmd.addArg("--zig-lib-dir"); - cmd.addDirectoryArg(p); - } + cmd.addArgs(&.{ "--zig-lib-dir", b.fmt("{}", .{b.graph.zig_lib_directory}) }); cmd.addArgs(&.{"-i"}); cmd.addFileArg(b.path(b.fmt("doc/langref/{s}", .{entry.name}))); diff --git a/lib/compiler/build_runner.zig b/lib/compiler/build_runner.zig index b3bdd8804d..384c164380 100644 --- a/lib/compiler/build_runner.zig +++ b/lib/compiler/build_runner.zig @@ -31,21 +31,15 @@ pub fn main() !void { // skip my own exe name var arg_idx: usize = 1; - const zig_exe = nextArg(args, &arg_idx) orelse { - std.debug.print("Expected path to zig compiler\n", .{}); - return error.InvalidArgs; - }; - const build_root = nextArg(args, &arg_idx) orelse { - std.debug.print("Expected build root directory path\n", .{}); - return error.InvalidArgs; - }; - const cache_root = nextArg(args, &arg_idx) orelse { - std.debug.print("Expected cache root directory path\n", .{}); - return error.InvalidArgs; - }; - const global_cache_root = nextArg(args, &arg_idx) orelse { - std.debug.print("Expected global cache root directory path\n", .{}); - return error.InvalidArgs; + const zig_exe = nextArg(args, &arg_idx) orelse fatal("missing zig compiler path", .{}); + const zig_lib_dir = nextArg(args, &arg_idx) orelse fatal("missing zig lib directory path", .{}); + const build_root = nextArg(args, &arg_idx) orelse fatal("missing build root directory path", .{}); + const cache_root = nextArg(args, &arg_idx) orelse fatal("missing cache root directory path", .{}); + const global_cache_root = nextArg(args, &arg_idx) orelse fatal("missing global cache root directory path", .{}); + + const zig_lib_directory: std.Build.Cache.Directory = .{ + .path = zig_lib_dir, + .handle = try std.fs.cwd().openDir(zig_lib_dir, .{}), }; const build_root_directory: std.Build.Cache.Directory = .{ @@ -72,6 +66,7 @@ pub fn main() !void { .zig_exe = zig_exe, .env_map = try process.getEnvMap(arena), .global_cache_root = global_cache_directory, + .zig_lib_directory = zig_lib_directory, .host = .{ .query = .{}, .result = try std.zig.system.resolveTargetQuery(.{}), @@ -189,8 +184,6 @@ pub fn main() !void { arg, next_arg, }); }; - } else if (mem.eql(u8, arg, "--zig-lib-dir")) { - builder.zig_lib_dir = .{ .cwd_relative = nextArgOrFatal(args, &arg_idx) }; } else if (mem.eql(u8, arg, "--seed")) { const next_arg = nextArg(args, &arg_idx) orelse fatalWithHint("expected u32 after '{s}'", .{arg}); @@ -416,15 +409,27 @@ pub fn main() !void { const reaction_set = rs: { const gop = try w.dir_table.getOrPut(gpa, path); if (!gop.found_existing) { - std.posix.fanotify_mark(w.fan_fd, .{ - .ADD = true, - .ONLYDIR = true, - }, Watch.fan_mask, path.root_dir.handle.fd, path.subPathOrDot()) catch |err| { - fatal("unable to watch {}: {s}", .{ path, @errorName(err) }); - }; - const dir_handle = try Watch.getDirHandle(gpa, path); - try w.handle_table.putNoClobber(gpa, dir_handle, .{}); + // `dir_handle` may already be present in the table in + // the case that we have multiple Cache.Path instances + // that compare inequal but ultimately point to the same + // directory on the file system. + // In such case, we must revert adding this directory, but keep + // the additions to the step set. + const dh_gop = try w.handle_table.getOrPut(gpa, dir_handle); + if (dh_gop.found_existing) { + _ = w.dir_table.pop(); + } else { + assert(dh_gop.index == gop.index); + dh_gop.value_ptr.* = .{}; + std.posix.fanotify_mark(w.fan_fd, .{ + .ADD = true, + .ONLYDIR = true, + }, Watch.fan_mask, path.root_dir.handle.fd, path.subPathOrDot()) catch |err| { + fatal("unable to watch {}: {s}", .{ path, @errorName(err) }); + }; + } + break :rs dh_gop.value_ptr; } break :rs &w.handle_table.values()[gop.index]; }; diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 556ed89e8d..36f7396c8e 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -54,7 +54,6 @@ libc_file: ?[]const u8 = null, /// Path to the directory containing build.zig. build_root: Cache.Directory, cache_root: Cache.Directory, -zig_lib_dir: ?LazyPath, pkg_config_pkg_list: ?(PkgConfigError![]const PkgConfigPkg) = null, args: ?[]const []const u8 = null, debug_log_scopes: []const []const u8 = &.{}, @@ -117,6 +116,7 @@ pub const Graph = struct { zig_exe: [:0]const u8, env_map: EnvMap, global_cache_root: Cache.Directory, + zig_lib_directory: Cache.Directory, needed_lazy_dependencies: std.StringArrayHashMapUnmanaged(void) = .{}, /// Information about the native target. Computed before build() is invoked. host: ResolvedTarget, @@ -293,7 +293,6 @@ pub fn create( }), .description = "Remove build artifacts from prefix path", }, - .zig_lib_dir = null, .install_path = undefined, .args = null, .host = graph.host, @@ -379,7 +378,6 @@ fn createChildOnly( .libc_file = parent.libc_file, .build_root = build_root, .cache_root = parent.cache_root, - .zig_lib_dir = parent.zig_lib_dir, .debug_log_scopes = parent.debug_log_scopes, .debug_compile_errors = parent.debug_compile_errors, .debug_pkg_config = parent.debug_pkg_config, @@ -687,7 +685,7 @@ pub fn addExecutable(b: *Build, options: ExecutableOptions) *Step.Compile { .max_rss = options.max_rss, .use_llvm = options.use_llvm, .use_lld = options.use_lld, - .zig_lib_dir = options.zig_lib_dir orelse b.zig_lib_dir, + .zig_lib_dir = options.zig_lib_dir, .win32_manifest = options.win32_manifest, }); } @@ -735,7 +733,7 @@ pub fn addObject(b: *Build, options: ObjectOptions) *Step.Compile { .max_rss = options.max_rss, .use_llvm = options.use_llvm, .use_lld = options.use_lld, - .zig_lib_dir = options.zig_lib_dir orelse b.zig_lib_dir, + .zig_lib_dir = options.zig_lib_dir, }); } @@ -791,7 +789,7 @@ pub fn addSharedLibrary(b: *Build, options: SharedLibraryOptions) *Step.Compile .max_rss = options.max_rss, .use_llvm = options.use_llvm, .use_lld = options.use_lld, - .zig_lib_dir = options.zig_lib_dir orelse b.zig_lib_dir, + .zig_lib_dir = options.zig_lib_dir, .win32_manifest = options.win32_manifest, }); } @@ -842,7 +840,7 @@ pub fn addStaticLibrary(b: *Build, options: StaticLibraryOptions) *Step.Compile .max_rss = options.max_rss, .use_llvm = options.use_llvm, .use_lld = options.use_lld, - .zig_lib_dir = options.zig_lib_dir orelse b.zig_lib_dir, + .zig_lib_dir = options.zig_lib_dir, }); } @@ -905,7 +903,7 @@ pub fn addTest(b: *Build, options: TestOptions) *Step.Compile { .test_runner = options.test_runner, .use_llvm = options.use_llvm, .use_lld = options.use_lld, - .zig_lib_dir = options.zig_lib_dir orelse b.zig_lib_dir, + .zig_lib_dir = options.zig_lib_dir, }); } @@ -929,7 +927,7 @@ pub fn addAssembly(b: *Build, options: AssemblyOptions) *Step.Compile { .optimize = options.optimize, }, .max_rss = options.max_rss, - .zig_lib_dir = options.zig_lib_dir orelse b.zig_lib_dir, + .zig_lib_dir = options.zig_lib_dir, }); obj_step.addAssemblyFile(options.source_file); return obj_step; diff --git a/lib/std/Build/Cache.zig b/lib/std/Build/Cache.zig index e78353fa45..8ba2f9f128 100644 --- a/lib/std/Build/Cache.zig +++ b/lib/std/Build/Cache.zig @@ -1007,6 +1007,22 @@ pub const Manifest = struct { } self.files.deinit(self.cache.gpa); } + + pub fn populateFileSystemInputs(man: *Manifest, buf: *std.ArrayListUnmanaged(u8)) Allocator.Error!void { + assert(@typeInfo(std.zig.Server.Message.PathPrefix).Enum.fields.len == man.cache.prefixes_len); + const gpa = man.cache.gpa; + const files = man.files.keys(); + if (files.len > 0) { + for (files) |file| { + try buf.ensureUnusedCapacity(gpa, file.prefixed_path.sub_path.len + 2); + buf.appendAssumeCapacity(file.prefixed_path.prefix + 1); + buf.appendSliceAssumeCapacity(file.prefixed_path.sub_path); + buf.appendAssumeCapacity(0); + } + // The null byte is a separator, not a terminator. + buf.items.len -= 1; + } + } }; /// On operating systems that support symlinks, does a readlink. On other operating systems, diff --git a/lib/std/Build/Step.zig b/lib/std/Build/Step.zig index 13cd47981b..3c6cd660ff 100644 --- a/lib/std/Build/Step.zig +++ b/lib/std/Build/Step.zig @@ -435,6 +435,44 @@ pub fn evalZigProcess( s.result_cached = ebp_hdr.flags.cache_hit; result = try arena.dupe(u8, body[@sizeOf(EbpHdr)..]); }, + .file_system_inputs => { + s.clearWatchInputs(); + var it = std.mem.splitScalar(u8, body, 0); + while (it.next()) |prefixed_path| { + const prefix_index: std.zig.Server.Message.PathPrefix = @enumFromInt(prefixed_path[0] - 1); + const sub_path = try arena.dupe(u8, prefixed_path[1..]); + const sub_path_dirname = std.fs.path.dirname(sub_path) orelse ""; + switch (prefix_index) { + .cwd => { + const path: Build.Cache.Path = .{ + .root_dir = Build.Cache.Directory.cwd(), + .sub_path = sub_path_dirname, + }; + try addWatchInputFromPath(s, path, std.fs.path.basename(sub_path)); + }, + .zig_lib => zl: { + if (s.cast(Step.Compile)) |compile| { + if (compile.zig_lib_dir) |lp| { + try addWatchInput(s, lp); + break :zl; + } + } + const path: Build.Cache.Path = .{ + .root_dir = s.owner.graph.zig_lib_directory, + .sub_path = sub_path_dirname, + }; + try addWatchInputFromPath(s, path, std.fs.path.basename(sub_path)); + }, + .local_cache => { + const path: Build.Cache.Path = .{ + .root_dir = b.cache_root, + .sub_path = sub_path_dirname, + }; + try addWatchInputFromPath(s, path, std.fs.path.basename(sub_path)); + }, + } + } + }, else => {}, // ignore other messages } diff --git a/lib/std/zig/Server.zig b/lib/std/zig/Server.zig index 7f8de00b4a..f1d3bc7b61 100644 --- a/lib/std/zig/Server.zig +++ b/lib/std/zig/Server.zig @@ -20,10 +20,24 @@ pub const Message = struct { test_metadata, /// Body is a TestResults test_results, + /// Body is a series of strings, delimited by null bytes. + /// Each string is a prefixed file path. + /// The first byte indicates the file prefix path (see prefixes fields + /// of Cache). This byte is sent over the wire incremented so that null + /// bytes are not confused with string terminators. + /// The remaining bytes is the file path relative to that prefix. + /// The prefixes are hard-coded in Compilation.create (cwd, zig lib dir, local cache dir) + file_system_inputs, _, }; + pub const PathPrefix = enum(u8) { + cwd, + zig_lib, + local_cache, + }; + /// Trailing: /// * extra: [extra_len]u32, /// * string_bytes: [string_bytes_len]u8, @@ -58,7 +72,7 @@ pub const Message = struct { }; /// Trailing: - /// * the file system path the emitted binary can be found + /// * file system path where the emitted binary can be found pub const EmitBinPath = extern struct { flags: Flags, diff --git a/src/Compilation.zig b/src/Compilation.zig index cc5fd1a9eb..49d4b041ae 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -235,6 +235,8 @@ astgen_wait_group: WaitGroup = .{}, llvm_opt_bisect_limit: c_int, +file_system_inputs: ?*std.ArrayListUnmanaged(u8), + pub const Emit = struct { /// Where the output will go. directory: Directory, @@ -1157,6 +1159,9 @@ pub const CreateOptions = struct { error_limit: ?Zcu.ErrorInt = null, global_cc_argv: []const []const u8 = &.{}, + /// Tracks all files that can cause the Compilation to be invalidated and need a rebuild. + file_system_inputs: ?*std.ArrayListUnmanaged(u8) = null, + pub const Entry = link.File.OpenOptions.Entry; }; @@ -1332,6 +1337,7 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil .gpa = gpa, .manifest_dir = try options.local_cache_directory.handle.makeOpenPath("h", .{}), }; + // These correspond to std.zig.Server.Message.PathPrefix. cache.addPrefix(.{ .path = null, .handle = std.fs.cwd() }); cache.addPrefix(options.zig_lib_directory); cache.addPrefix(options.local_cache_directory); @@ -1508,6 +1514,7 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil .force_undefined_symbols = options.force_undefined_symbols, .link_eh_frame_hdr = link_eh_frame_hdr, .global_cc_argv = options.global_cc_argv, + .file_system_inputs = options.file_system_inputs, }; // Prevent some footguns by making the "any" fields of config reflect @@ -2044,6 +2051,8 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void { ); }; if (is_hit) { + if (comp.file_system_inputs) |buf| try man.populateFileSystemInputs(buf); + comp.last_update_was_cache_hit = true; log.debug("CacheMode.whole cache hit for {s}", .{comp.root_name}); const digest = man.final(); @@ -2170,6 +2179,11 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void { try comp.performAllTheWork(main_progress_node); + switch (comp.cache_use) { + .whole => if (comp.file_system_inputs) |buf| try man.populateFileSystemInputs(buf), + .incremental => {}, + } + if (comp.module) |zcu| { const pt: Zcu.PerThread = .{ .zcu = zcu, .tid = .main }; diff --git a/src/main.zig b/src/main.zig index e00442f399..2fb49b74bc 100644 --- a/src/main.zig +++ b/src/main.zig @@ -3227,6 +3227,9 @@ fn buildOutputType( process.raiseFileDescriptorLimit(); + var file_system_inputs: std.ArrayListUnmanaged(u8) = .{}; + defer file_system_inputs.deinit(gpa); + const comp = Compilation.create(gpa, arena, .{ .zig_lib_directory = zig_lib_directory, .local_cache_directory = local_cache_directory, @@ -3350,6 +3353,7 @@ fn buildOutputType( // than to any particular module. This feature can greatly reduce CLI // noise when --search-prefix and --mod are combined. .global_cc_argv = try cc_argv.toOwnedSlice(arena), + .file_system_inputs = &file_system_inputs, }) catch |err| switch (err) { error.LibCUnavailable => { const triple_name = try target.zigTriple(arena); @@ -3433,7 +3437,7 @@ fn buildOutputType( defer root_prog_node.end(); if (arg_mode == .translate_c) { - return cmdTranslateC(comp, arena, null, root_prog_node); + return cmdTranslateC(comp, arena, null, null, root_prog_node); } updateModule(comp, color, root_prog_node) catch |err| switch (err) { @@ -4059,6 +4063,7 @@ fn serve( var child_pid: ?std.process.Child.Id = null; const main_progress_node = std.Progress.start(.{}); + const file_system_inputs = comp.file_system_inputs.?; while (true) { const hdr = try server.receiveMessage(); @@ -4067,14 +4072,16 @@ fn serve( .exit => return cleanExit(), .update => { tracy.frameMark(); + file_system_inputs.clearRetainingCapacity(); if (arg_mode == .translate_c) { var arena_instance = std.heap.ArenaAllocator.init(gpa); defer arena_instance.deinit(); const arena = arena_instance.allocator(); var output: Compilation.CImportResult = undefined; - try cmdTranslateC(comp, arena, &output, main_progress_node); + try cmdTranslateC(comp, arena, &output, file_system_inputs, main_progress_node); defer output.deinit(gpa); + try server.serveStringMessage(.file_system_inputs, file_system_inputs.items); if (output.errors.errorMessageCount() != 0) { try server.serveErrorBundle(output.errors); } else { @@ -4116,6 +4123,7 @@ fn serve( }, .hot_update => { tracy.frameMark(); + file_system_inputs.clearRetainingCapacity(); if (child_pid) |pid| { try comp.hotCodeSwap(main_progress_node, pid); try serveUpdateResults(&server, comp); @@ -4147,6 +4155,12 @@ fn serve( fn serveUpdateResults(s: *Server, comp: *Compilation) !void { const gpa = comp.gpa; + + if (comp.file_system_inputs) |file_system_inputs| { + assert(file_system_inputs.items.len > 0); + try s.serveStringMessage(.file_system_inputs, file_system_inputs.items); + } + var error_bundle = try comp.getAllErrorsAlloc(); defer error_bundle.deinit(gpa); if (error_bundle.errorMessageCount() > 0) { @@ -4434,6 +4448,7 @@ fn cmdTranslateC( comp: *Compilation, arena: Allocator, fancy_output: ?*Compilation.CImportResult, + file_system_inputs: ?*std.ArrayListUnmanaged(u8), prog_node: std.Progress.Node, ) !void { if (build_options.only_core_functionality) @panic("@translate-c is not available in a zig2.c build"); @@ -4454,7 +4469,10 @@ fn cmdTranslateC( }; if (fancy_output) |p| p.cache_hit = true; - const digest = if (try man.hit()) man.final() else digest: { + const digest = if (try man.hit()) digest: { + if (file_system_inputs) |buf| try man.populateFileSystemInputs(buf); + break :digest man.final(); + } else digest: { if (fancy_output) |p| p.cache_hit = false; var argv = std.ArrayList([]const u8).init(arena); switch (comp.config.c_frontend) { @@ -4566,6 +4584,8 @@ fn cmdTranslateC( @errorName(err), }); + if (file_system_inputs) |buf| try man.populateFileSystemInputs(buf); + break :digest digest; }; @@ -4678,6 +4698,9 @@ fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !void { const self_exe_path = try introspect.findZigExePath(arena); try child_argv.append(self_exe_path); + const argv_index_zig_lib_dir = child_argv.items.len; + _ = try child_argv.addOne(); + const argv_index_build_file = child_argv.items.len; _ = try child_argv.addOne(); @@ -4727,7 +4750,6 @@ fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !void { if (i + 1 >= args.len) fatal("expected argument after '{s}'", .{arg}); i += 1; override_lib_dir = args[i]; - try child_argv.appendSlice(&.{ arg, args[i] }); continue; } else if (mem.eql(u8, arg, "--build-runner")) { if (i + 1 >= args.len) fatal("expected argument after '{s}'", .{arg}); @@ -4865,6 +4887,8 @@ fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !void { defer zig_lib_directory.handle.close(); const cwd_path = try process.getCwdAlloc(arena); + child_argv.items[argv_index_zig_lib_dir] = zig_lib_directory.path orelse cwd_path; + const build_root = try findBuildRoot(arena, .{ .cwd_path = cwd_path, .build_file = build_file,