From a966eee090d55c7d61484333af675c80115bf188 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 10 Jul 2024 15:09:46 -0700 Subject: [PATCH] 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 --- lib/std/Build/Step.zig | 14 +++- lib/std/Build/Step/InstallDir.zig | 7 +- lib/std/Build/Step/WriteFile.zig | 117 ++++++++++++++++++++---------- 3 files changed, 95 insertions(+), 43 deletions(-) diff --git a/lib/std/Build/Step.zig b/lib/std/Build/Step.zig index a2640492ba8d0c20aed97f6cc18d328866c2e347..5c77bd3367b61248b899fcd05990cd673905d411 100644 --- a/lib/std/Build/Step.zig +++ b/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, "."); } diff --git a/lib/std/Build/Step/InstallDir.zig b/lib/std/Build/Step/InstallDir.zig index 8cb06641ec5f9bc6f9be8f2bc0e9c2bf556b3275..78281e56d9df7c70972ffccf933e63f0e29cf54c 100644 --- a/lib/std/Build/Step/InstallDir.zig +++ b/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. }, diff --git a/lib/std/Build/Step/WriteFile.zig b/lib/std/Build/Step/WriteFile.zig index f35bf09b7e832d33319cac3af0275a95d1f81cf0..6c0770e45866f1b4b50335bce13741b5bfc19396 100644 --- a/lib/std/Build/Step/WriteFile.zig +++ b/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(); +} -- 2.54.0