Skip to content

executor: return untouched output directories from the input root in strict mode #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dzbarsky merged 1 commit into from
Oct 20, 2026
Merged

executor: return untouched output directories from the input root in strict mode #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 121 additions & 15 deletions src/action_executor.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,17 @@ pub fn executeDecodedActionWithOptions(
.actiondfs_overlay
else
.actiondfs_strict;
try validateOutputParentsAgainstInputsWithCancellation(io, allocator, store, input_root_digest, command, options.cancellation);
var input_output_directories: std.ArrayListUnmanaged([]const u8) = .empty;
defer input_output_directories.deinit(allocator);
try validateOutputParentsAgainstInputsWithCancellation(
io,
allocator,
store,
input_root_digest,
command,
if (actiondfs_mode == .actiondfs_strict) &input_output_directories else null,
options.cancellation,
);
try checkExecutionCancellation(options.cancellation);

var cwd_buffer: [std.Io.Dir.max_path_bytes]u8 = undefined;
Expand Down Expand Up @@ -301,6 +311,13 @@ pub fn executeDecodedActionWithOptions(
return err;
},
};
prepareInputOutputDirectoriesWithCancellation(io, actiondfs_stage_dir, input_output_directories.items, options.cancellation) catch |err| switch (err) {
error.ExecutionCancelled => return error.ExecutionCancelled,
else => {
logExecuteSetupError("prepare input output directories", action_digest, err);
return err;
},
};
try checkExecutionCancellation(options.cancellation);

var chroot_cwd_owned: ?[]u8 = null;
Expand Down Expand Up @@ -2001,29 +2018,49 @@ const OutputParentValidator = struct {
return &entry.value_ptr.directory;
}

fn validate(self: *OutputParentValidator, path: []const u8) !void {
// Rejects outputs whose parents are input files or symlinks, and reports
// whether the output path itself is a directory in the input root.
fn validate(self: *OutputParentValidator, path: []const u8) !bool {
try checkExecutionCancellation(self.cancellation);
if (path.len == 0) return;
if (path.len == 0) return false;
try validatePath(path);
const parent_end = std.mem.lastIndexOfScalar(u8, path, '/') orelse return;
if (parent_end == 0) return;

var current_digest = self.root_digest;
var components = std.mem.splitScalar(u8, path[0..parent_end], '/');
var components = std.mem.splitScalar(u8, path, '/');
while (components.next()) |component| {
try checkExecutionCancellation(self.cancellation);
const directory = try self.getDirectory(current_digest);
if (findInputDirectoryEntry(reapi.FileNode, directory.files, component) != null)
return error.OutputParentConflictsWithInputFile;
if (findInputDirectoryEntry(reapi.SymlinkNode, directory.symlinks, component) != null)
return error.OutputParentConflictsWithInputSymlink;
if (components.peek() != null) {
if (findInputDirectoryEntry(reapi.FileNode, directory.files, component) != null)
return error.OutputParentConflictsWithInputFile;
if (findInputDirectoryEntry(reapi.SymlinkNode, directory.symlinks, component) != null)
return error.OutputParentConflictsWithInputSymlink;
}

const child = findInputDirectoryEntry(reapi.DirectoryNode, directory.directories, component) orelse return;
const child = findInputDirectoryEntry(reapi.DirectoryNode, directory.directories, component) orelse return false;
current_digest = try cas.Digest.fromReapi(child.digest orelse return error.MissingInputDirectoryDigest);
}
return true;
}
};

// Bazel adds declared output directories to the input root as empty
// directories (bazelbuild/bazel#15366). Strict actiondfs collects outputs from
// the stage only, so an output directory the action never writes into would be
// reported missing; mirror those input directories into the stage before
// execution.
fn prepareInputOutputDirectoriesWithCancellation(
io: std.Io,
stage_root: std.Io.Dir,
paths: []const []const u8,
cancellation: ?*const std.atomic.Value(bool),
) !void {
for (paths) |path| {
try checkExecutionCancellation(cancellation);
try stage_root.createDirPath(io, path);
}
}

fn findInputDirectoryEntry(comptime Entry: type, entries: []const Entry, name: []const u8) ?*const Entry {
var first: usize = 0;
var remaining = entries.len;
Expand All @@ -2049,15 +2086,19 @@ fn validateOutputParentsAgainstInputs(
root_digest: cas.Digest,
command: reapi.Command,
) !void {
try validateOutputParentsAgainstInputsWithCancellation(io, allocator, store, root_digest, command, null);
try validateOutputParentsAgainstInputsWithCancellation(io, allocator, store, root_digest, command, null, null);
}

// When `input_output_directories` is set, output paths (or legacy output
// directories) that are already directories in the input root are appended to
// it. The appended slices borrow from `command`.
fn validateOutputParentsAgainstInputsWithCancellation(
io: std.Io,
allocator: std.mem.Allocator,
store: cas.Store,
root_digest: cas.Digest,
command: reapi.Command,
input_output_directories: ?*std.ArrayListUnmanaged([]const u8),
cancellation: ?*const std.atomic.Value(bool),
) !void {
try checkExecutionCancellation(cancellation);
Expand All @@ -2071,12 +2112,20 @@ fn validateOutputParentsAgainstInputsWithCancellation(
defer validator.deinit();

if (command.output_paths.len != 0) {
for (command.output_paths) |path| try validator.validate(path);
for (command.output_paths) |path| {
if (try validator.validate(path)) {
if (input_output_directories) |list| try list.append(allocator, path);
}
}
return;
}

for (command.output_files) |path| try validator.validate(path);
for (command.output_directories) |path| try validator.validate(path);
for (command.output_files) |path| _ = try validator.validate(path);
for (command.output_directories) |path| {
if (try validator.validate(path)) {
if (input_output_directories) |list| try list.append(allocator, path);
}
}
}

fn prepareOutputParents(
Expand Down Expand Up @@ -2719,6 +2768,7 @@ test "output parent validation and preparation stop before cancelled filesystem
cas.Store.init(work_dir),
cas.Digest.fromBytes(""),
.{ .output_paths = &.{"generated/out.txt"} },
null,
&cancellation,
));
try std.testing.expectError(error.ExecutionCancelled, prepareOutputParentsWithCancellation(
Expand Down Expand Up @@ -3559,6 +3609,62 @@ test "validateOutputParentsAgainstInputs accepts existing input directories and
);
}

test "validateOutputParentsAgainstInputs reports output paths that are input directories" {
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();

var cas_dir = try tmp.dir.createDirPathOpen(std.testing.io, "cas", .{});
defer cas_dir.close(std.testing.io);
var stage_dir = try tmp.dir.createDirPathOpen(std.testing.io, "stage", .{});
defer stage_dir.close(std.testing.io);
const store = cas.Store.init(cas_dir);
const file_digest = try store.putBytes(std.testing.io, "input");
var file_hash: [64]u8 = undefined;
const empty_digest = try putProto(std.testing.io, std.testing.allocator, store, reapi.Directory{});
var empty_hash: [64]u8 = undefined;
const pkg_digest = try putProto(std.testing.io, std.testing.allocator, store, reapi.Directory{
.files = &.{.{ .name = "input.txt", .digest = file_digest.toReapi(&file_hash) }},
.directories = &.{.{ .name = "tree", .digest = empty_digest.toReapi(&empty_hash) }},
});
var pkg_hash: [64]u8 = undefined;
const root_digest = try putProto(std.testing.io, std.testing.allocator, store, reapi.Directory{
.directories = &.{.{ .name = "pkg", .digest = pkg_digest.toReapi(&pkg_hash) }},
});

var input_output_directories: std.ArrayListUnmanaged([]const u8) = .empty;
defer input_output_directories.deinit(std.testing.allocator);
try validateOutputParentsAgainstInputsWithCancellation(
std.testing.io,
std.testing.allocator,
store,
root_digest,
.{ .output_paths = &.{ "pkg/input.txt", "pkg/missing", "pkg/tree" } },
&input_output_directories,
null,
);
try std.testing.expectEqual(@as(usize, 1), input_output_directories.items.len);
try std.testing.expectEqualStrings("pkg/tree", input_output_directories.items[0]);

try prepareInputOutputDirectoriesWithCancellation(std.testing.io, stage_dir, input_output_directories.items, null);
const stat = try stage_dir.statFile(std.testing.io, "pkg/tree", .{});
try std.testing.expectEqual(std.Io.File.Kind.directory, stat.kind);
try std.testing.expectError(error.FileNotFound, stage_dir.statFile(std.testing.io, "pkg/input.txt", .{}));
try std.testing.expectError(error.FileNotFound, stage_dir.statFile(std.testing.io, "pkg/missing", .{}));

input_output_directories.clearRetainingCapacity();
try validateOutputParentsAgainstInputsWithCancellation(
std.testing.io,
std.testing.allocator,
store,
root_digest,
.{ .output_files = &.{"pkg/input.txt"}, .output_directories = &.{ "pkg/missing", "pkg/tree" } },
&input_output_directories,
null,
);
try std.testing.expectEqual(@as(usize, 1), input_output_directories.items.len);
try std.testing.expectEqualStrings("pkg/tree", input_output_directories.items[0]);
}

test "validateOutputParentsAgainstInputs ignores legacy output conflicts when output_paths is present" {
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
Expand Down
43 changes: 43 additions & 0 deletions test/stress.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,40 @@ stress_tree = rule(
},
)

def _stress_untouched_tree_impl(ctx):
# The tool never receives the declared directory, so the action leaves it
# exactly as Bazel materialized it in the input root (empty).
out_dir = ctx.actions.declare_directory(ctx.label.name + ".tree")
out_file = ctx.actions.declare_file(ctx.label.name + ".manifest.txt")

args = ctx.actions.args()
_add_common_args(args, out_file.path, None, 0, ctx.files.srcs, [], False, False, False)
ctx.actions.run(
executable = ctx.executable.tool,
inputs = ctx.files.srcs,
outputs = [out_file, out_dir],
arguments = [args],
env = _stress_env(ctx),
execution_requirements = ctx.attr.execution_requirements,
mnemonic = "ActiondStressUntouchedTree",
progress_message = "Leaving stress tree untouched %{label}",
)
return DefaultInfo(files = depset([out_file, out_dir]))

stress_untouched_tree = rule(
implementation = _stress_untouched_tree_impl,
attrs = {
"srcs": attr.label_list(allow_files = True),
"stress_case": attr.string(),
"execution_requirements": attr.string_dict(),
"tool": attr.label(
allow_single_file = True,
executable = True,
cfg = "exec",
),
},
)

def _stress_symlinks_impl(ctx):
out_file = ctx.actions.declare_file(ctx.label.name + ".target.txt")
out_dir = ctx.actions.declare_directory(ctx.label.name + ".target.tree")
Expand Down Expand Up @@ -351,6 +385,15 @@ def stress_workload(name, tool):
)
stress_targets.append(":symlink_input_consumer")

stress_untouched_tree(
name = "untouched_output_dir_regression",
execution_requirements = {"libc": "glibc2.35"},
srcs = [":bare_inputs"],
stress_case = "untouched_output_dir_regression",
tool = tool,
)
stress_targets.append(":untouched_output_dir_regression")

stress_tree(
name = "shared_generated_tree",
execution_requirements = {"libc": "glibc2.35"},
Expand Down
Loading