| author | |
| committer | |
| log | 3d785897658fd8b61660649b24d8943bda545982 |
| tree | 9b3d05aedeca757bb399505195f0a565a9e20966 |
| parent | ddabd57743579818a05016e031b4919c47b4428a |
and integrate properly with LazyPath9 files changed, 142 insertions(+), 76 deletions(-)
BRANCH_TODO+15-1| ... | ... | @@ -27,10 +27,13 @@ |
| 27 | 27 | - but artifact install steps also add paths for dyn libs on windows |
| 28 | 28 | * no more "artifact arg" to run step. if you want to run the post-install binary, get the lazy path |
| 29 | 29 | from the install step. |
| 30 | ||
| 30 | * build system fmt step with check=false does not acquire a write lock on source files #35204 | |
| 31 | * fmt step: import zig fmt code directly rather than child proc | |
| 31 | 32 | |
| 32 | 33 | ## Release Notes |
| 33 | 34 | |
| 35 | ### Run Step: Passthru Args | |
| 36 | ||
| 34 | 37 | In the Run step, passthru args are all together now, not observable in |
| 35 | 38 | configure phase whether run args are provided. |
| 36 | 39 | |
| ... | ... | @@ -51,3 +54,14 @@ those arguments. In exchange, it means that when changing those arguments, |
| 51 | 54 | build scripts no longer must be rebuilt from source. |
| 52 | 55 | |
| 53 | 56 | closes #31397 |
| 57 | ||
| 58 | ### Fmt Step: Options | |
| 59 | ||
| 60 | `paths` and `exclude_paths` are now LazyPath lists. There is a convenience method to create them: `b.pathList`. | |
| 61 | ||
| 62 | ```diff | |
| 63 | - const fmt_include_paths = &.{ "lib", "src", "test", "tools", "build.zig", "build.zig.zon" }; | |
| 64 | - const fmt_exclude_paths = &.{ "test/cases", "test/behavior/zon" }; | |
| 65 | + const fmt_include_paths = b.pathList(&.{ "lib", "src", "test", "tools", "build.zig", "build.zig.zon" }); | |
| 66 | + const fmt_exclude_paths = b.pathList(&.{ "test/cases", "test/behavior/zon" }); | |
| 67 | ``` |
build.zig+2-2| ... | ... | @@ -427,8 +427,8 @@ pub fn build(b: *std.Build) !void { |
| 427 | 427 | else |
| 428 | 428 | null; |
| 429 | 429 | |
| 430 | const fmt_include_paths = &.{ "lib", "src", "test", "tools", "build.zig", "build.zig.zon" }; | |
| 431 | const fmt_exclude_paths = &.{ "test/cases", "test/behavior/zon" }; | |
| 430 | const fmt_include_paths = b.pathList(&.{ "lib", "src", "test", "tools", "build.zig", "build.zig.zon" }); | |
| 431 | const fmt_exclude_paths = b.pathList(&.{ "test/cases", "test/behavior/zon" }); | |
| 432 | 432 | const do_fmt = b.addFmt(.{ |
| 433 | 433 | .paths = fmt_include_paths, |
| 434 | 434 | .exclude_paths = fmt_exclude_paths, |
lib/compiler/Maker/Step.zig+1-1| ... | ... | @@ -311,7 +311,7 @@ pub fn captureChildProcess( |
| 311 | 311 | ) !std.process.RunResult { |
| 312 | 312 | const gpa = maker.gpa; |
| 313 | 313 | const graph = maker.graph; |
| 314 | const arena = graph.arena; | |
| 314 | const arena = graph.arena; // TODO stop leaking into process arena | |
| 315 | 315 | const io = graph.io; |
| 316 | 316 | |
| 317 | 317 | // If an error occurs, it's happened in this command: |
lib/compiler/Maker/Step/Fmt.zig created+57| ... | ... | @@ -0,0 +1,57 @@ |
| 1 | const Fmt = @This(); | |
| 2 | ||
| 3 | const std = @import("std"); | |
| 4 | const Configuration = std.Build.Configuration; | |
| 5 | ||
| 6 | const Step = @import("../Step.zig"); | |
| 7 | const Maker = @import("../../Maker.zig"); | |
| 8 | ||
| 9 | /// Persisted to reuse memory on subsequent calls to `make`. | |
| 10 | argv: std.ArrayList([]const u8) = .empty, | |
| 11 | ||
| 12 | pub fn make( | |
| 13 | fmt: *Fmt, | |
| 14 | step_index: Configuration.Step.Index, | |
| 15 | maker: *Maker, | |
| 16 | progress_node: std.Progress.Node, | |
| 17 | ) Step.ExtendedMakeError!void { | |
| 18 | const graph = maker.graph; | |
| 19 | const step = maker.stepByIndex(step_index); | |
| 20 | const gpa = maker.gpa; | |
| 21 | const arena = graph.arena; // TODO don't leak into the process arena | |
| 22 | const argv = &fmt.argv; | |
| 23 | const conf = &maker.scanned_config.configuration; | |
| 24 | const conf_step = step_index.ptr(conf); | |
| 25 | const conf_fmt = conf_step.extended.get(conf.extra).fmt; | |
| 26 | const paths = conf_fmt.paths.slice; | |
| 27 | const exclude_paths = conf_fmt.paths.exclude_paths; | |
| 28 | ||
| 29 | argv.clearRetainingCapacity(); | |
| 30 | try argv.ensureUnusedCapacity(gpa, 2 + 1 + paths.len + 2 * exclude_paths.len); | |
| 31 | ||
| 32 | argv.appendAssumeCapacity(graph.zig_exe); | |
| 33 | argv.appendAssumeCapacity("fmt"); | |
| 34 | ||
| 35 | if (fmt.check) | |
| 36 | argv.appendAssumeCapacity("--check"); | |
| 37 | ||
| 38 | for (fmt.paths) |lp| | |
| 39 | argv.appendAssumeCapacity(try maker.resolveLazyPathIndexAbs(arena, lp, step_index)); | |
| 40 | ||
| 41 | for (fmt.exclude_paths) |lp| { | |
| 42 | argv.appendAssumeCapacity("--exclude"); | |
| 43 | argv.appendAssumeCapacity(try maker.resolveLazyPathIndexAbs(arena, lp, step_index)); | |
| 44 | } | |
| 45 | ||
| 46 | const run_result = try step.captureChildProcess(maker, progress_node, argv.items); | |
| 47 | if (fmt.check) switch (run_result.term) { | |
| 48 | .exited => |code| if (code != 0 and run_result.stdout.len != 0) { | |
| 49 | var it = std.mem.tokenizeScalar(u8, run_result.stdout, '\n'); | |
| 50 | while (it.next()) |bad_file_name| { | |
| 51 | try step.addError("{s}: non-conforming formatting", .{bad_file_name}); | |
| 52 | } | |
| 53 | }, | |
| 54 | else => {}, | |
| 55 | }; | |
| 56 | try step.handleChildProcessTerm(maker, run_result.term); | |
| 57 | } |
lib/compiler/Maker/Step/InstallDir.zig+1-1| ... | ... | @@ -11,7 +11,7 @@ pub fn make( |
| 11 | 11 | step_index: Configuration.Step.Index, |
| 12 | 12 | maker: *Maker, |
| 13 | 13 | progress_node: std.Progress.Node, |
| 14 | ) !void { | |
| 14 | ) Step.ExtendedMakeError!void { | |
| 15 | 15 | const graph = maker.graph; |
| 16 | 16 | const arena = maker.graph.arena; // TODO don't leak into process arena |
| 17 | 17 | const io = graph.io; |
lib/compiler/Maker/Step/Options.zig+2-2| ... | ... | @@ -7,12 +7,12 @@ const Step = @import("../Step.zig"); |
| 7 | 7 | const Maker = @import("../../Maker.zig"); |
| 8 | 8 | |
| 9 | 9 | |
| 10 | fn make( | |
| 10 | pub fn make( | |
| 11 | 11 | options: *Options, |
| 12 | 12 | step_index: Configuration.Step.Index, |
| 13 | 13 | maker: *Maker, |
| 14 | 14 | progress_node: std.Progress.Node, |
| 15 | ) !void { | |
| 15 | ) Step.ExtendedMakeError!void { | |
| 16 | 16 | // This step completes so quickly that no progress reporting is necessary. |
| 17 | 17 | _ = progress_node; |
| 18 | 18 |
lib/std/Build.zig+37-13| ... | ... | @@ -115,8 +115,7 @@ pub const Graph = struct { |
| 115 | 115 | } |
| 116 | 116 | |
| 117 | 117 | pub fn dupeStrings(graph: *const Graph, strings: []const []const u8) []const []const u8 { |
| 118 | const arena = graph.arena; | |
| 119 | const array = arena.alloc([]const u8, strings.len) catch @panic("OOM"); | |
| 118 | const array = graph.alloc([]const u8, strings.len); | |
| 120 | 119 | for (array, strings) |*dest, source| dest.* = dupeString(graph, source); |
| 121 | 120 | return array; |
| 122 | 121 | } |
| ... | ... | @@ -134,6 +133,21 @@ pub const Graph = struct { |
| 134 | 133 | }, |
| 135 | 134 | }; |
| 136 | 135 | } |
| 136 | ||
| 137 | /// Allocates using the global process arena, failing the build on | |
| 138 | /// allocation failure. | |
| 139 | pub fn alloc(graph: *const Graph, comptime T: type, n: usize) []T { | |
| 140 | return graph.arena.allocAdvancedWithRetAddr(T, null, n, @returnAddress()) catch @panic("OOM"); | |
| 141 | } | |
| 142 | ||
| 143 | /// Allocates using the global process arena, failing the build on | |
| 144 | /// allocation failure. | |
| 145 | pub fn create(graph: *const Graph, comptime T: type) *T { | |
| 146 | return if (@sizeOf(T) == 0) | |
| 147 | comptime @ptrFromInt(mem.alignBackward(usize, std.math.maxInt(usize), @alignOf(T))) | |
| 148 | else | |
| 149 | @ptrCast(graph.arena.allocBytesWithAlignment(.of(T), @sizeOf(T), @returnAddress()) catch @panic("OOM")); | |
| 150 | } | |
| 137 | 151 | }; |
| 138 | 152 | |
| 139 | 153 | const AvailableDeps = []const struct { []const u8, []const u8 }; |
| ... | ... | @@ -953,9 +967,10 @@ pub fn getUninstallStep(b: *Build) *Step { |
| 953 | 967 | /// these options when calling the dependency's build.zig script as a function. |
| 954 | 968 | /// `null` is returned when an option is left to default. |
| 955 | 969 | pub fn option(b: *Build, comptime T: type, name_raw: []const u8, description_raw: []const u8) ?T { |
| 956 | const arena = b.allocator; | |
| 957 | const name = b.dupe(name_raw); | |
| 958 | const description = b.dupe(description_raw); | |
| 970 | const graph = b.graph; | |
| 971 | const arena = graph.arena; | |
| 972 | const name = graph.dupeString(name_raw); | |
| 973 | const description = graph.dupeString(description_raw); | |
| 959 | 974 | const type_id = comptime typeToEnum(T); |
| 960 | 975 | const enum_options = if (type_id == .@"enum" or type_id == .enum_list) blk: { |
| 961 | 976 | const EnumType = if (type_id == .enum_list) @typeInfo(T).pointer.child else T; |
| ... | ... | @@ -1105,7 +1120,7 @@ pub fn option(b: *Build, comptime T: type, name_raw: []const u8, description_raw |
| 1105 | 1120 | }, |
| 1106 | 1121 | .list => |lst| { |
| 1107 | 1122 | const Child = @typeInfo(T).pointer.child; |
| 1108 | const new_list = arena.alloc(Child, lst.items.len) catch @panic("OOM"); | |
| 1123 | const new_list = graph.alloc(Child, lst.items.len); | |
| 1109 | 1124 | for (new_list, lst.items) |*new_item, str| { |
| 1110 | 1125 | new_item.* = std.meta.stringToEnum(Child, str) orelse { |
| 1111 | 1126 | log.err("expected -D{s} to be of type {s}", .{ name, @typeName(Child) }); |
| ... | ... | @@ -1130,7 +1145,7 @@ pub fn option(b: *Build, comptime T: type, name_raw: []const u8, description_raw |
| 1130 | 1145 | .scalar => |s| return arena.dupe(LazyPath, &[_]LazyPath{.{ .cwd_relative = s }}) catch @panic("OOM"), |
| 1131 | 1146 | .lazy_path => |lp| return arena.dupe(LazyPath, &[_]LazyPath{lp}) catch @panic("OOM"), |
| 1132 | 1147 | .list => |lst| { |
| 1133 | const new_list = arena.alloc(LazyPath, lst.items.len) catch @panic("OOM"); | |
| 1148 | const new_list = graph.alloc(LazyPath, lst.items.len); | |
| 1134 | 1149 | for (new_list, lst.items) |*new_item, str| { |
| 1135 | 1150 | new_item.* = .{ .cwd_relative = str }; |
| 1136 | 1151 | } |
| ... | ... | @@ -1553,7 +1568,7 @@ pub fn truncateFile(b: *Build, dest_path: []const u8) (Io.Dir.CreateDirError || |
| 1553 | 1568 | /// References a file or directory relative to the source root. |
| 1554 | 1569 | pub fn path(b: *Build, sub_path: []const u8) LazyPath { |
| 1555 | 1570 | if (fs.path.isAbsolute(sub_path)) { |
| 1556 | panic("sub_path is expected to be relative to the build root, but was this absolute path: '{s}'. It is best avoid absolute paths, but if you must, it is supported by LazyPath.cwd_relative", .{ | |
| 1571 | panic("sub_path is expected to be relative to the build root, but was this absolute path: '{s}'. Absolute paths can cause problems but can be created via Graph.cwdRelativePath", .{ | |
| 1557 | 1572 | sub_path, |
| 1558 | 1573 | }); |
| 1559 | 1574 | } |
| ... | ... | @@ -1563,6 +1578,14 @@ pub fn path(b: *Build, sub_path: []const u8) LazyPath { |
| 1563 | 1578 | } }; |
| 1564 | 1579 | } |
| 1565 | 1580 | |
| 1581 | /// Creates a list of files and/or directories relative to the source root. | |
| 1582 | pub fn pathList(b: *Build, sub_paths: []const []const u8) []const LazyPath { | |
| 1583 | const graph = b.graph; | |
| 1584 | const result = graph.alloc(LazyPath, sub_paths.len); | |
| 1585 | for (result, sub_paths) |*d, s| d.* = path(b, s); | |
| 1586 | return result; | |
| 1587 | } | |
| 1588 | ||
| 1566 | 1589 | pub fn pathJoin(b: *Build, paths: []const []const u8) []u8 { |
| 1567 | 1590 | return fs.path.join(b.allocator, paths) catch @panic("OOM"); |
| 1568 | 1591 | } |
| ... | ... | @@ -2022,10 +2045,11 @@ fn dependencyInner( |
| 2022 | 2045 | pkg_deps: AvailableDeps, |
| 2023 | 2046 | args: anytype, |
| 2024 | 2047 | ) *Dependency { |
| 2025 | const io = b.graph.io; | |
| 2026 | const arena = b.graph.arena; | |
| 2048 | const graph = b.graph; | |
| 2049 | const io = graph.io; | |
| 2050 | const arena = graph.arena; | |
| 2027 | 2051 | const user_input_options = userInputOptionsFromArgs(arena, args); |
| 2028 | if (b.graph.dependency_cache.getContext(.{ | |
| 2052 | if (graph.dependency_cache.getContext(.{ | |
| 2029 | 2053 | .build_root_string = build_root_string, |
| 2030 | 2054 | .user_input_options = user_input_options, |
| 2031 | 2055 | }, .{ .allocator = arena })) |dep| return dep; |
| ... | ... | @@ -2048,10 +2072,10 @@ fn dependencyInner( |
| 2048 | 2072 | } |
| 2049 | 2073 | } |
| 2050 | 2074 | |
| 2051 | const dep = arena.create(Dependency) catch @panic("OOM"); | |
| 2075 | const dep = graph.create(Dependency); | |
| 2052 | 2076 | dep.* = .{ .builder = sub_builder }; |
| 2053 | 2077 | |
| 2054 | b.graph.dependency_cache.putContext(b.graph.arena, .{ | |
| 2078 | graph.dependency_cache.putContext(arena, .{ | |
| 2055 | 2079 | .build_root_string = build_root_string, |
| 2056 | 2080 | .user_input_options = user_input_options, |
| 2057 | 2081 | }, dep, .{ .allocator = arena }) catch @panic("OOM"); |
lib/std/Build/Configuration.zig+6-1| ... | ... | @@ -1038,10 +1038,15 @@ pub const Step = extern struct { |
| 1038 | 1038 | |
| 1039 | 1039 | pub const Fmt = struct { |
| 1040 | 1040 | flags: @This().Flags, |
| 1041 | paths: Storage.FlagLengthPrefixedList(.flags, .paths, LazyPath.Index), | |
| 1042 | exclude_paths: Storage.FlagLengthPrefixedList(.flags, .exclude_paths, LazyPath.Index), | |
| 1041 | 1043 | |
| 1042 | 1044 | pub const Flags = packed struct(u32) { |
| 1043 | 1045 | tag: Tag = .fmt, |
| 1044 | _: u27 = 0, | |
| 1046 | paths: bool, | |
| 1047 | exclude_paths: bool, | |
| 1048 | check: bool, | |
| 1049 | _: u24 = 0, | |
| 1045 | 1050 | }; |
| 1046 | 1051 | }; |
| 1047 | 1052 |
lib/std/Build/Step/Fmt.zig+21-55| ... | ... | @@ -1,81 +1,47 @@ |
| 1 | 1 | //! This step has two modes: |
| 2 | 2 | //! * Modify mode: directly modify source files, formatting them in place. |
| 3 | 3 | //! * Check mode: fail the step if a non-conforming file is found. |
| 4 | const Fmt = @This(); | |
| 5 | ||
| 4 | 6 | const std = @import("std"); |
| 5 | 7 | const Step = std.Build.Step; |
| 6 | const Fmt = @This(); | |
| 8 | const LazyPath = std.Build.LazyPath; | |
| 9 | const Configuration = std.Build.Configuration; | |
| 7 | 10 | |
| 8 | 11 | step: Step, |
| 9 | paths: []const []const u8, | |
| 10 | exclude_paths: []const []const u8, | |
| 12 | /// Intended to be read-only after the `Fmt` step is created. | |
| 13 | paths: []const LazyPath, | |
| 14 | /// Intended to be read-only after the `Fmt` step is created. | |
| 15 | exclude_paths: []const LazyPath, | |
| 11 | 16 | check: bool, |
| 12 | 17 | |
| 13 | 18 | pub const base_tag: Step.Tag = .fmt; |
| 14 | 19 | |
| 15 | 20 | pub const Options = struct { |
| 16 | paths: []const []const u8 = &.{}, | |
| 17 | exclude_paths: []const []const u8 = &.{}, | |
| 21 | paths: []const LazyPath = &.{}, | |
| 22 | exclude_paths: []const LazyPath = &.{}, | |
| 18 | 23 | /// If true, fails the build step when any non-conforming files are encountered. |
| 19 | 24 | check: bool = false, |
| 20 | 25 | }; |
| 21 | 26 | |
| 22 | 27 | pub fn create(owner: *std.Build, options: Options) *Fmt { |
| 23 | const fmt = owner.allocator.create(Fmt) catch @panic("OOM"); | |
| 24 | const name = if (options.check) "zig fmt --check" else "zig fmt"; | |
| 28 | const graph = owner.graph; | |
| 29 | const arena = graph.arena; | |
| 30 | const fmt = arena.create(Fmt) catch @panic("OOM"); | |
| 31 | ||
| 25 | 32 | fmt.* = .{ |
| 26 | .step = Step.init(.{ | |
| 33 | .step = .init(.{ | |
| 27 | 34 | .tag = base_tag, |
| 28 | .name = name, | |
| 35 | .name = if (options.check) "zig fmt --check" else "zig fmt", | |
| 29 | 36 | .owner = owner, |
| 30 | .makeFn = make, | |
| 31 | 37 | }), |
| 32 | .paths = owner.dupeStrings(options.paths), | |
| 33 | .exclude_paths = owner.dupeStrings(options.exclude_paths), | |
| 38 | .paths = options.paths, | |
| 39 | .exclude_paths = options.exclude_paths, | |
| 34 | 40 | .check = options.check, |
| 35 | 41 | }; |
| 36 | return fmt; | |
| 37 | } | |
| 38 | ||
| 39 | fn make(step: *Step, options: Step.MakeOptions) !void { | |
| 40 | const prog_node = options.progress_node; | |
| 41 | ||
| 42 | // TODO: if check=false, this means we are modifying source files in place, which | |
| 43 | // is an operation that could race against other operations also modifying source files | |
| 44 | // in place. In this case, this step should obtain a write lock while making those | |
| 45 | // modifications. | |
| 46 | 42 | |
| 47 | const b = step.owner; | |
| 48 | const arena = b.allocator; | |
| 49 | const fmt: *Fmt = @fieldParentPtr("step", step); | |
| 43 | for (options.paths) |lp| lp.addStepDependencies(&fmt.step); | |
| 44 | for (options.exclude_paths) |lp| lp.addStepDependencies(&fmt.step); | |
| 50 | 45 | |
| 51 | var argv: std.ArrayList([]const u8) = .empty; | |
| 52 | try argv.ensureUnusedCapacity(arena, 2 + 1 + fmt.paths.len + 2 * fmt.exclude_paths.len); | |
| 53 | ||
| 54 | argv.appendAssumeCapacity(b.graph.zig_exe); | |
| 55 | argv.appendAssumeCapacity("fmt"); | |
| 56 | ||
| 57 | if (fmt.check) { | |
| 58 | argv.appendAssumeCapacity("--check"); | |
| 59 | } | |
| 60 | ||
| 61 | for (fmt.paths) |p| { | |
| 62 | argv.appendAssumeCapacity(b.pathFromRoot(p)); | |
| 63 | } | |
| 64 | ||
| 65 | for (fmt.exclude_paths) |p| { | |
| 66 | argv.appendAssumeCapacity("--exclude"); | |
| 67 | argv.appendAssumeCapacity(b.pathFromRoot(p)); | |
| 68 | } | |
| 69 | ||
| 70 | const run_result = try step.captureChildProcess(options.gpa, prog_node, argv.items); | |
| 71 | if (fmt.check) switch (run_result.term) { | |
| 72 | .exited => |code| if (code != 0 and run_result.stdout.len != 0) { | |
| 73 | var it = std.mem.tokenizeScalar(u8, run_result.stdout, '\n'); | |
| 74 | while (it.next()) |bad_file_name| { | |
| 75 | try step.addError("{s}: non-conforming formatting", .{bad_file_name}); | |
| 76 | } | |
| 77 | }, | |
| 78 | else => {}, | |
| 79 | }; | |
| 80 | try step.handleChildProcessTerm(run_result.term); | |
| 46 | return fmt; | |
| 81 | 47 | } |