Skip to content

Commit

Permalink
std.Build.Step.WriteFile: fix handling of directories
Browse files Browse the repository at this point in the history
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
  • Loading branch information
andrewrk committed Jul 12, 2024
1 parent f285640 commit a966eee
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 43 deletions.
14 changes: 12 additions & 2 deletions lib/std/Build/Step.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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, ".");
}
Expand Down
7 changes: 3 additions & 4 deletions lib/std/Build/Step/InstallDir.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
Expand Down Expand Up @@ -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.
},
Expand Down
117 changes: 80 additions & 37 deletions lib/std/Build/Step/WriteFile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};
};

Expand Down Expand Up @@ -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
Expand All @@ -173,37 +192,75 @@ 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;
}

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}", .{
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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();
}

0 comments on commit a966eee

Please sign in to comment.