Commit a966eee090

Andrew Kelley <andrew@ziglang.org>
2024-07-11 00:09:46
std.Build.Step.WriteFile: fix handling of directories
and add file system watching integration. `addDirectoryWatchInput` now returns a `bool` which helps remind the caller to 1. call addDirectoryWatchInputFromPath on any derived paths 2. but only if the dependency is not already captured by a step dependency edge. The make function now recursively walks all directories and adds the found files to the cache hash rather than incorrectly only adding the directory name to the cache hash. closes #20571
1 parent f285640
Changed files (3)
lib/std/Build/Step/InstallDir.zig
@@ -63,8 +63,8 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
     const arena = b.allocator;
     const dest_prefix = b.getInstallPath(install_dir.options.install_dir, install_dir.options.install_subdir);
     const src_dir_path = install_dir.options.source_dir.getPath3(b, step);
-    try step.addDirectoryWatchInput(install_dir.options.source_dir);
-    var src_dir = src_dir_path.root_dir.handle.openDir(src_dir_path.subPathOpt() orelse ".", .{ .iterate = true }) catch |err| {
+    const need_derived_inputs = try step.addDirectoryWatchInput(install_dir.options.source_dir);
+    var src_dir = src_dir_path.root_dir.handle.openDir(src_dir_path.subPathOrDot(), .{ .iterate = true }) catch |err| {
         return step.fail("unable to open source directory '{}': {s}", .{
             src_dir_path, @errorName(err),
         });
@@ -96,8 +96,7 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
 
         switch (entry.kind) {
             .directory => {
-                const subdir_path = try src_dir_path.join(arena, entry.path);
-                try step.addDirectoryWatchInputFromPath(subdir_path);
+                if (need_derived_inputs) try step.addDirectoryWatchInputFromPath(src_sub_path);
                 try cwd.makePath(dest_path);
                 // TODO: set result_cached=false if the directory did not already exist.
             },
lib/std/Build/Step/WriteFile.zig
@@ -40,6 +40,22 @@ pub const Directory = struct {
                 .include_extensions = if (opts.include_extensions) |incs| b.dupeStrings(incs) else null,
             };
         }
+
+        pub fn pathIncluded(opts: Options, path: []const u8) bool {
+            for (opts.exclude_extensions) |ext| {
+                if (std.mem.endsWith(u8, path, ext))
+                    return false;
+            }
+            if (opts.include_extensions) |incs| {
+                for (incs) |inc| {
+                    if (std.mem.endsWith(u8, path, inc))
+                        return true;
+                } else {
+                    return false;
+                }
+            }
+            return true;
+        }
     };
 };
 
@@ -158,7 +174,10 @@ fn maybeUpdateName(write_file: *WriteFile) void {
 fn make(step: *Step, prog_node: std.Progress.Node) !void {
     _ = prog_node;
     const b = step.owner;
+    const arena = b.allocator;
+    const gpa = arena;
     const write_file: *WriteFile = @fieldParentPtr("step", step);
+    step.clearWatchInputs();
 
     // The cache is used here not really as a way to speed things up - because writing
     // the data to a file would probably be very fast - but as a way to find a canonical
@@ -173,29 +192,67 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
     // Random bytes to make WriteFile unique. Refresh this with
     // new random bytes when WriteFile implementation is modified
     // in a non-backwards-compatible way.
-    man.hash.add(@as(u32, 0xd767ee59));
+    man.hash.add(@as(u32, 0xc2a287d0));
 
     for (write_file.files.items) |file| {
         man.hash.addBytes(file.sub_path);
+
         switch (file.contents) {
             .bytes => |bytes| {
                 man.hash.addBytes(bytes);
             },
-            .copy => |file_source| {
-                _ = try man.addFile(file_source.getPath2(b, step), null);
+            .copy => |lazy_path| {
+                const path = lazy_path.getPath3(b, step);
+                _ = try man.addFilePath(path, null);
+                try step.addWatchInput(lazy_path);
             },
         }
     }
-    for (write_file.directories.items) |dir| {
-        man.hash.addBytes(dir.source.getPath2(b, step));
+
+    const open_dir_cache = try arena.alloc(fs.Dir, write_file.directories.items.len);
+    var open_dirs_count: usize = 0;
+    defer closeDirs(open_dir_cache[0..open_dirs_count]);
+
+    for (write_file.directories.items, open_dir_cache) |dir, *open_dir_cache_elem| {
         man.hash.addBytes(dir.sub_path);
         for (dir.options.exclude_extensions) |ext| man.hash.addBytes(ext);
         if (dir.options.include_extensions) |incs| for (incs) |inc| man.hash.addBytes(inc);
+
+        const need_derived_inputs = try step.addDirectoryWatchInput(dir.source);
+        const src_dir_path = dir.source.getPath3(b, step);
+
+        var src_dir = src_dir_path.root_dir.handle.openDir(src_dir_path.subPathOrDot(), .{ .iterate = true }) catch |err| {
+            return step.fail("unable to open source directory '{}': {s}", .{
+                src_dir_path, @errorName(err),
+            });
+        };
+        open_dir_cache_elem.* = src_dir;
+        open_dirs_count += 1;
+
+        var it = try src_dir.walk(gpa);
+        defer it.deinit();
+        while (try it.next()) |entry| {
+            if (!dir.options.pathIncluded(entry.path)) continue;
+
+            switch (entry.kind) {
+                .directory => {
+                    if (need_derived_inputs) {
+                        const entry_path = try src_dir_path.join(arena, entry.path);
+                        try step.addDirectoryWatchInputFromPath(entry_path);
+                    }
+                },
+                .file => {
+                    const entry_path = try src_dir_path.join(arena, entry.path);
+                    _ = try man.addFilePath(entry_path, null);
+                },
+                else => continue,
+            }
+        }
     }
 
     if (try step.cacheHit(&man)) {
         const digest = man.final();
-        write_file.generated_directory.path = try b.cache_root.join(b.allocator, &.{ "o", &digest });
+        write_file.generated_directory.path = try b.cache_root.join(arena, &.{ "o", &digest });
         step.result_cached = true;
         return;
     }
@@ -203,7 +260,7 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
     const digest = man.final();
     const cache_path = "o" ++ fs.path.sep_str ++ digest;
 
-    write_file.generated_directory.path = try b.cache_root.join(b.allocator, &.{ "o", &digest });
+    write_file.generated_directory.path = try b.cache_root.join(arena, &.{ "o", &digest });
 
     var cache_dir = b.cache_root.handle.makeOpenPath(cache_path, .{}) catch |err| {
         return step.fail("unable to make path '{}{s}': {s}", .{
@@ -256,8 +313,9 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
             },
         }
     }
-    for (write_file.directories.items) |dir| {
-        const full_src_dir_path = dir.source.getPath2(b, step);
+
+    for (write_file.directories.items, open_dir_cache) |dir, already_open_dir| {
+        const src_dir_path = dir.source.getPath3(b, step);
         const dest_dirname = dir.sub_path;
 
         if (dest_dirname.len != 0) {
@@ -268,44 +326,25 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
             };
         }
 
-        var src_dir = b.build_root.handle.openDir(full_src_dir_path, .{ .iterate = true }) catch |err| {
-            return step.fail("unable to open source directory '{s}': {s}", .{
-                full_src_dir_path, @errorName(err),
-            });
-        };
-        defer src_dir.close();
+        var it = try already_open_dir.walk(gpa);
+        defer it.deinit();
+        while (try it.next()) |entry| {
+            if (!dir.options.pathIncluded(entry.path)) continue;
 
-        var it = try src_dir.walk(b.allocator);
-        next_entry: while (try it.next()) |entry| {
-            for (dir.options.exclude_extensions) |ext| {
-                if (std.mem.endsWith(u8, entry.path, ext)) continue :next_entry;
-            }
-            if (dir.options.include_extensions) |incs| {
-                for (incs) |inc| {
-                    if (std.mem.endsWith(u8, entry.path, inc)) break;
-                } else {
-                    continue :next_entry;
-                }
-            }
-            const full_src_entry_path = b.pathJoin(&.{ full_src_dir_path, entry.path });
+            const src_entry_path = try src_dir_path.join(arena, entry.path);
             const dest_path = b.pathJoin(&.{ dest_dirname, entry.path });
             switch (entry.kind) {
                 .directory => try cache_dir.makePath(dest_path),
                 .file => {
                     const prev_status = fs.Dir.updateFile(
-                        cwd,
-                        full_src_entry_path,
+                        src_entry_path.root_dir.handle,
+                        src_entry_path.sub_path,
                         cache_dir,
                         dest_path,
                         .{},
                     ) catch |err| {
-                        return step.fail("unable to update file from '{s}' to '{}{s}{c}{s}': {s}", .{
-                            full_src_entry_path,
-                            b.cache_root,
-                            cache_path,
-                            fs.path.sep,
-                            dest_path,
-                            @errorName(err),
+                        return step.fail("unable to update file from '{}' to '{}{s}{c}{s}': {s}", .{
+                            src_entry_path, b.cache_root, cache_path, fs.path.sep, dest_path, @errorName(err),
                         });
                     };
                     _ = prev_status;
@@ -317,3 +356,7 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
 
     try step.writeManifest(&man);
 }
+
+fn closeDirs(dirs: []fs.Dir) void {
+    for (dirs) |*d| d.close();
+}
lib/std/Build/Step.zig
@@ -634,7 +634,11 @@ pub fn addWatchInput(step: *Step, lazy_file: Build.LazyPath) Allocator.Error!voi
 /// Any changes inside the directory will trigger invalidation.
 ///
 /// See also `addDirectoryWatchInputFromPath` which takes a `Build.Cache.Path` instead.
-pub fn addDirectoryWatchInput(step: *Step, lazy_directory: Build.LazyPath) Allocator.Error!void {
+///
+/// Paths derived from this directory should also be manually added via
+/// `addDirectoryWatchInputFromPath` if and only if this function returns
+/// `true`.
+pub fn addDirectoryWatchInput(step: *Step, lazy_directory: Build.LazyPath) Allocator.Error!bool {
     switch (lazy_directory) {
         .src_path => |src_path| try addDirectoryWatchInputFromBuilder(step, src_path.owner, src_path.sub_path),
         .dependency => |d| try addDirectoryWatchInputFromBuilder(step, d.dependency.builder, d.sub_path),
@@ -648,13 +652,19 @@ pub fn addDirectoryWatchInput(step: *Step, lazy_directory: Build.LazyPath) Alloc
             });
         },
         // Nothing to watch because this dependency edge is modeled instead via `dependants`.
-        .generated => {},
+        .generated => return false,
     }
+    return true;
 }
 
 /// Any changes inside the directory will trigger invalidation.
 ///
 /// See also `addDirectoryWatchInput` which takes a `Build.LazyPath` instead.
+///
+/// This function should only be called when it has been verified that the
+/// dependency on `path` is not already accounted for by a `Step` dependency.
+/// In other words, before calling this function, first check that the
+/// `Build.LazyPath` which this `path` is derived from is not `generated`.
 pub fn addDirectoryWatchInputFromPath(step: *Step, path: Build.Cache.Path) !void {
     return addWatchInputFromPath(step, path, ".");
 }