Commit a3c20dffae

Andrew Kelley <andrew@ziglang.org>
2024-07-12 01:26:04
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.
1 parent fd4d366
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];
                 };
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,
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
         }
 
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,
 
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;
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 };
 
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,
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})));