From d582575aba5264aaa02a8af0cdb7da7c4f4c6220 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Sat, 4 May 2024 14:49:38 -0400 Subject: [PATCH] Run: add lazy path file inputs This replaces `extra_file_dependencies` with support for lazy paths. Also assert output file basenames are not empty, avoid improper use of field default values, ensure stored strings are duplicated, and prefer `ArrayListUnmanaged` to discourage misuse of direct field access which wouldn't add step dependencies. --- lib/std/Build/Step/Run.zig | 101 ++++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 34 deletions(-) diff --git a/lib/std/Build/Step/Run.zig b/lib/std/Build/Step/Run.zig index 25c01ca8aa..7a9eca681c 100644 --- a/lib/std/Build/Step/Run.zig +++ b/lib/std/Build/Step/Run.zig @@ -5,7 +5,6 @@ const Step = Build.Step; const fs = std.fs; const mem = std.mem; const process = std.process; -const ArrayList = std.ArrayList; const EnvMap = process.EnvMap; const assert = std.debug.assert; @@ -16,7 +15,7 @@ pub const base_id: Step.Id = .run; step: Step, /// See also addArg and addArgs to modifying this directly -argv: ArrayList(Arg), +argv: std.ArrayListUnmanaged(Arg), /// Use `setCwd` to set the initial current working directory cwd: ?Build.LazyPath, @@ -32,22 +31,26 @@ env_map: ?*EnvMap, /// If the Run step is determined to not have side-effects, then execution will /// be skipped if all output files are up-to-date and input files are /// unchanged. -stdio: StdIo = .infer_from_args, +stdio: StdIo, /// This field must be `.none` if stdio is `inherit`. /// It should be only set using `setStdIn`. -stdin: StdIn = .none, +stdin: StdIn, -/// Additional file paths relative to build.zig that, when modified, indicate -/// that the Run step should be re-executed. -/// If the Run step is determined to have side-effects, this field is ignored -/// and the Run step is always executed when it appears in the build graph. -extra_file_dependencies: []const []const u8 = &.{}, +/// Deprecated: use `addFileInput` +extra_file_dependencies: []const []const u8, + +/// Additional input files that, when modified, indicate that the Run step +/// should be re-executed. +/// If the Run step is determined to have side-effects, the Run step is always +/// executed when it appears in the build graph, regardless of whether these +/// files have been modified. +file_inputs: std.ArrayListUnmanaged(std.Build.LazyPath), /// After adding an output argument, this step will by default rename itself /// for a better display name in the build summary. /// This can be disabled by setting this to false. -rename_step_with_output_arg: bool = true, +rename_step_with_output_arg: bool, /// If this is true, a Run step which is configured to check the output of the /// executed binary will not fail the build if the binary cannot be executed @@ -58,25 +61,25 @@ rename_step_with_output_arg: bool = true, /// Rosetta (macOS) and binfmt_misc (Linux). /// If this Run step is considered to have side-effects, then this flag does /// nothing. -skip_foreign_checks: bool = false, +skip_foreign_checks: bool, /// If this is true, failing to execute a foreign binary will be considered an /// error. However if this is false, the step will be skipped on failure instead. /// /// This allows for a Run step to attempt to execute a foreign binary using an /// external executor (such as qemu) but not fail if the executor is unavailable. -failing_to_execute_foreign_is_an_error: bool = true, +failing_to_execute_foreign_is_an_error: bool, /// If stderr or stdout exceeds this amount, the child process is killed and /// the step fails. -max_stdio_size: usize = 10 * 1024 * 1024, +max_stdio_size: usize, -captured_stdout: ?*Output = null, -captured_stderr: ?*Output = null, +captured_stdout: ?*Output, +captured_stderr: ?*Output, -dep_output_file: ?*Output = null, +dep_output_file: ?*Output, -has_side_effects: bool = false, +has_side_effects: bool, pub const StdIn = union(enum) { none, @@ -103,7 +106,7 @@ pub const StdIo = union(enum) { /// conditions. /// Note that an explicit check for exit code 0 needs to be added to this /// list if such a check is desirable. - check: std.ArrayList(Check), + check: std.ArrayListUnmanaged(Check), /// This Run step is running a zig unit test binary and will communicate /// extra metadata over the IPC protocol. zig_test, @@ -145,9 +148,21 @@ pub fn create(owner: *std.Build, name: []const u8) *Run { .owner = owner, .makeFn = make, }), - .argv = ArrayList(Arg).init(owner.allocator), + .argv = .{}, .cwd = null, .env_map = null, + .stdio = .infer_from_args, + .stdin = .none, + .extra_file_dependencies = &.{}, + .file_inputs = .{}, + .rename_step_with_output_arg = true, + .skip_foreign_checks = false, + .failing_to_execute_foreign_is_an_error = true, + .max_stdio_size = 10 * 1024 * 1024, + .captured_stdout = null, + .captured_stderr = null, + .dep_output_file = null, + .has_side_effects = false, }; return self; } @@ -163,9 +178,10 @@ pub fn enableTestRunnerMode(self: *Run) void { } pub fn addArtifactArg(self: *Run, artifact: *Step.Compile) void { + const b = self.step.owner; const bin_file = artifact.getEmittedBin(); bin_file.addStepDependencies(&self.step); - self.argv.append(Arg{ .artifact = artifact }) catch @panic("OOM"); + self.argv.append(b.allocator, Arg{ .artifact = artifact }) catch @panic("OOM"); } /// Provides a file path as a command line argument to the command being run. @@ -181,6 +197,7 @@ pub fn addOutputFileArg(self: *Run, basename: []const u8) std.Build.LazyPath { } /// Provides a file path as a command line argument to the command being run. +/// Asserts `basename` is not empty. /// /// For example, a prefix of "-o" and basename of "output.txt" will result in /// the child process seeing something like this: "-ozig-cache/.../output.txt" @@ -200,14 +217,15 @@ pub fn addPrefixedOutputFileArg( basename: []const u8, ) std.Build.LazyPath { const b = self.step.owner; + if (basename.len == 0) @panic("basename must not be empty"); const output = b.allocator.create(Output) catch @panic("OOM"); output.* = .{ - .prefix = prefix, - .basename = basename, + .prefix = b.dupe(prefix), + .basename = b.dupe(basename), .generated_file = .{ .step = &self.step }, }; - self.argv.append(.{ .output = output }) catch @panic("OOM"); + self.argv.append(b.allocator, .{ .output = output }) catch @panic("OOM"); if (self.rename_step_with_output_arg) { self.setName(b.fmt("{s} ({s})", .{ self.step.name, basename })); @@ -248,7 +266,7 @@ pub fn addPrefixedFileArg(self: *Run, prefix: []const u8, lp: std.Build.LazyPath .prefix = b.dupe(prefix), .lazy_path = lp.dupe(b), }; - self.argv.append(.{ .lazy_path = prefixed_file_source }) catch @panic("OOM"); + self.argv.append(b.allocator, .{ .lazy_path = prefixed_file_source }) catch @panic("OOM"); lp.addStepDependencies(&self.step); } @@ -269,7 +287,7 @@ pub fn addPrefixedDirectoryArg(self: *Run, prefix: []const u8, directory_source: .prefix = b.dupe(prefix), .lazy_path = directory_source.dupe(b), }; - self.argv.append(.{ .directory_source = prefixed_directory_source }) catch @panic("OOM"); + self.argv.append(b.allocator, .{ .directory_source = prefixed_directory_source }) catch @panic("OOM"); directory_source.addStepDependencies(&self.step); } @@ -284,9 +302,8 @@ pub fn addDepFileOutputArg(self: *Run, basename: []const u8) std.Build.LazyPath /// write its discovered additional dependencies. /// Only one dep file argument is allowed by instance. pub fn addPrefixedDepFileOutputArg(self: *Run, prefix: []const u8, basename: []const u8) std.Build.LazyPath { - assert(self.dep_output_file == null); - const b = self.step.owner; + assert(self.dep_output_file == null); const dep_file = b.allocator.create(Output) catch @panic("OOM"); dep_file.* = .{ @@ -297,13 +314,14 @@ pub fn addPrefixedDepFileOutputArg(self: *Run, prefix: []const u8, basename: []c self.dep_output_file = dep_file; - self.argv.append(.{ .output = dep_file }) catch @panic("OOM"); + self.argv.append(b.allocator, .{ .output = dep_file }) catch @panic("OOM"); return .{ .generated = &dep_file.generated_file }; } pub fn addArg(self: *Run, arg: []const u8) void { - self.argv.append(.{ .bytes = self.step.owner.dupe(arg) }) catch @panic("OOM"); + const b = self.step.owner; + self.argv.append(b.allocator, .{ .bytes = self.step.owner.dupe(arg) }) catch @panic("OOM"); } pub fn addArgs(self: *Run, args: []const []const u8) void { @@ -401,12 +419,14 @@ pub fn hasTermCheck(self: Run) bool { } pub fn addCheck(self: *Run, new_check: StdIo.Check) void { + const b = self.step.owner; + switch (self.stdio) { .infer_from_args => { - self.stdio = .{ .check = std.ArrayList(StdIo.Check).init(self.step.owner.allocator) }; - self.stdio.check.append(new_check) catch @panic("OOM"); + self.stdio = .{ .check = .{} }; + self.stdio.check.append(b.allocator, new_check) catch @panic("OOM"); }, - .check => |*checks| checks.append(new_check) catch @panic("OOM"), + .check => |*checks| checks.append(b.allocator, new_check) catch @panic("OOM"), else => @panic("illegal call to addCheck: conflicting helper method calls. Suggest to directly set stdio field of Run instead"), } } @@ -441,6 +461,16 @@ pub fn captureStdOut(self: *Run) std.Build.LazyPath { return .{ .generated = &output.generated_file }; } +/// Adds an additional input files that, when modified, indicates that this Run +/// step should be re-executed. +/// If the Run step is determined to have side-effects, the Run step is always +/// executed when it appears in the build graph, regardless of whether this +/// file has been modified. +pub fn addFileInput(self: *Run, file_input: std.Build.LazyPath) void { + file_input.addStepDependencies(&self.step); + self.file_inputs.append(self.step.owner.allocator, file_input.dupe(self.step.owner)) catch @panic("OOM"); +} + /// Returns whether the Run step has side effects *other than* updating the output arguments. fn hasSideEffects(self: Run) bool { if (self.has_side_effects) return true; @@ -500,8 +530,8 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { const self: *Run = @fieldParentPtr("step", step); const has_side_effects = self.hasSideEffects(); - var argv_list = ArrayList([]const u8).init(arena); - var output_placeholders = ArrayList(IndexedOutput).init(arena); + var argv_list = std.ArrayList([]const u8).init(arena); + var output_placeholders = std.ArrayList(IndexedOutput).init(arena); var man = b.graph.cache.obtain(); defer man.deinit(); @@ -579,6 +609,9 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { for (self.extra_file_dependencies) |file_path| { _ = try man.addFile(b.pathFromRoot(file_path), null); } + for (self.file_inputs.items) |lazy_path| { + _ = try man.addFile(lazy_path.getPath2(b, step), null); + } if (try step.cacheHit(&man)) { // cache hit, skip running command