Merge pull request #20321 from ziglang/build-system-fmt

enhance `std.Build.Step.Fmt` and use it more

build.zig

@@ -428,18 +428,21 @@ pub fn build(b: *std.Build) !void {
     }
     const optimization_modes = chosen_opt_modes_buf[0..chosen_mode_index];
 
-    const fmt_include_paths = &.{ "doc", "lib", "src", "test", "tools", "build.zig" };
+    const fmt_include_paths = &.{ "lib", "src", "test", "tools", "build.zig", "build.zig.zon" };
     const fmt_exclude_paths = &.{"test/cases"};
     const do_fmt = b.addFmt(.{
         .paths = fmt_include_paths,
         .exclude_paths = fmt_exclude_paths,
     });
+    b.step("fmt", "Modify source files in place to have conforming formatting").dependOn(&do_fmt.step);
 
-    b.step("test-fmt", "Check source files having conforming formatting").dependOn(&b.addFmt(.{
+    const check_fmt = b.step("test-fmt", "Check source files having conforming formatting");
+    check_fmt.dependOn(&b.addFmt(.{
         .paths = fmt_include_paths,
         .exclude_paths = fmt_exclude_paths,
         .check = true,
     }).step);
+    test_step.dependOn(check_fmt);
 
     const test_cases_step = b.step("test-cases", "Run the main compiler test cases");
     try tests.addCases(b, test_cases_step, test_filters, check_case_exe, target, .{
@@ -534,9 +537,6 @@ pub fn build(b: *std.Build) !void {
 
     try addWasiUpdateStep(b, version);
 
-    b.step("fmt", "Modify source files in place to have conforming formatting")
-        .dependOn(&do_fmt.step);
-
     const update_mingw_step = b.step("update-mingw", "Update zig's bundled mingw");
     const opt_mingw_src_path = b.option([]const u8, "mingw-src", "path to mingw-w64 source directory");
     const update_mingw_exe = b.addExecutable(.{

ci/aarch64-linux-debug.sh

@@ -49,13 +49,6 @@ unset CXX
 
 ninja install
 
-# TODO: move this to a build.zig step (check-fmt)
-echo "Looking for non-conforming code formatting..."
-stage3-debug/bin/zig fmt --check .. \
-  --exclude ../test/cases/ \
-  --exclude ../doc/ \
-  --exclude ../build-debug
-
 # simultaneously test building self-hosted without LLVM and with 32-bit arm
 stage3-debug/bin/zig build \
   -Dtarget=arm-linux-musleabihf \

ci/aarch64-linux-release.sh

@@ -49,13 +49,6 @@ unset CXX
 
 ninja install
 
-# TODO: move this to a build.zig step (check-fmt)
-echo "Looking for non-conforming code formatting..."
-stage3-release/bin/zig fmt --check .. \
-  --exclude ../test/cases/ \
-  --exclude ../doc/ \
-  --exclude ../build-release
-
 # simultaneously test building self-hosted without LLVM and with 32-bit arm
 stage3-release/bin/zig build \
   -Dtarget=arm-linux-musleabihf \

ci/x86_64-linux-debug.sh

@@ -57,13 +57,6 @@ unset CXX
 
 ninja install
 
-# TODO: move this to a build.zig step (check-fmt)
-echo "Looking for non-conforming code formatting..."
-stage3-debug/bin/zig fmt --check .. \
-  --exclude ../test/cases/ \
-  --exclude ../doc/ \
-  --exclude ../build-debug
-
 # simultaneously test building self-hosted without LLVM and with 32-bit arm
 stage3-debug/bin/zig build \
   -Dtarget=arm-linux-musleabihf \

ci/x86_64-linux-release.sh

@@ -57,14 +57,6 @@ unset CXX
 
 ninja install
 
-# TODO: move this to a build.zig step (check-fmt)
-echo "Looking for non-conforming code formatting..."
-stage3-release/bin/zig fmt --check .. \
-  --exclude ../test/cases/ \
-  --exclude ../doc/ \
-  --exclude ../build-debug \
-  --exclude ../build-release
-
 # simultaneously test building self-hosted without LLVM and with 32-bit arm
 stage3-release/bin/zig build \
   -Dtarget=arm-linux-musleabihf \

lib/std/Build/Step.zig

@@ -272,7 +272,17 @@ const Allocator = std.mem.Allocator;
 const assert = std.debug.assert;
 const builtin = @import("builtin");
 
-pub fn evalChildProcess(s: *Step, argv: []const []const u8) !void {
+pub fn evalChildProcess(s: *Step, argv: []const []const u8) ![]u8 {
+    const run_result = try captureChildProcess(s, std.Progress.Node.none, argv);
+    try handleChildProcessTerm(s, run_result.term, null, argv);
+    return run_result.stdout;
+}
+
+pub fn captureChildProcess(
+    s: *Step,
+    progress_node: std.Progress.Node,
+    argv: []const []const u8,
+) !std.process.Child.RunResult {
     const arena = s.owner.allocator;
 
     try handleChildProcUnsupported(s, null, argv);
@@ -281,13 +291,14 @@ pub fn evalChildProcess(s: *Step, argv: []const []const u8) !void {
     const result = std.process.Child.run(.{
         .allocator = arena,
         .argv = argv,
+        .progress_node = progress_node,
     }) catch |err| return s.fail("unable to spawn {s}: {s}", .{ argv[0], @errorName(err) });
 
     if (result.stderr.len > 0) {
         try s.result_error_msgs.append(arena, result.stderr);
     }
 
-    try handleChildProcessTerm(s, result.term, null, argv);
+    return result;
 }
 
 pub fn fail(step: *Step, comptime fmt: []const u8, args: anytype) error{ OutOfMemory, MakeFailed } {

lib/std/Build/Step/Fmt.zig

@@ -37,9 +37,6 @@ pub fn create(owner: *std.Build, options: Options) *Fmt {
 }
 
 fn make(step: *Step, prog_node: std.Progress.Node) !void {
-    // zig fmt is fast enough that no progress is needed.
-    _ = prog_node;
-
     // TODO: if check=false, this means we are modifying source files in place, which
     // is an operation that could race against other operations also modifying source files
     // in place. In this case, this step should obtain a write lock while making those
@@ -68,5 +65,15 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
         argv.appendAssumeCapacity(b.pathFromRoot(p));
     }
 
-    return step.evalChildProcess(argv.items);
+    const run_result = try step.captureChildProcess(prog_node, argv.items);
+    if (fmt.check) switch (run_result.term) {
+        .Exited => |code| if (code != 0 and run_result.stdout.len != 0) {
+            var it = std.mem.tokenizeScalar(u8, run_result.stdout, '\n');
+            while (it.next()) |bad_file_name| {
+                try step.addError("{s}: non-conforming formatting", .{bad_file_name});
+            }
+        },
+        else => {},
+    };
+    try step.handleChildProcessTerm(run_result.term, null, argv.items);
 }

lib/std/Progress.zig

@@ -80,6 +80,8 @@ pub const Options = struct {
 pub const Node = struct {
     index: OptionalIndex,
 
+    pub const none: Node = .{ .index = .none };
+
     pub const max_name_len = 40;
 
     const Storage = extern struct {
@@ -177,9 +179,9 @@ pub const Node = struct {
     pub fn start(node: Node, name: []const u8, estimated_total_items: usize) Node {
         if (noop_impl) {
             assert(node.index == .none);
-            return .{ .index = .none };
+            return Node.none;
         }
-        const node_index = node.index.unwrap() orelse return .{ .index = .none };
+        const node_index = node.index.unwrap() orelse return Node.none;
         const parent = node_index.toParent();
 
         const freelist_head = &global_progress.node_freelist_first;
@@ -196,7 +198,7 @@ pub const Node = struct {
         if (free_index >= global_progress.node_storage.len) {
             // Ran out of node storage memory. Progress for this node will not be tracked.
             _ = @atomicRmw(u32, &global_progress.node_end_index, .Sub, 1, .monotonic);
-            return .{ .index = .none };
+            return Node.none;
         }
 
         return init(@enumFromInt(free_index), parent, name, estimated_total_items);
@@ -357,7 +359,7 @@ pub fn start(options: Options) Node {
     global_progress.initial_delay_ns = options.initial_delay_ns;
 
     if (noop_impl)
-        return .{ .index = .none };
+        return Node.none;
 
     if (std.process.parseEnvVarInt("ZIG_PROGRESS", u31, 10)) |ipc_fd| {
         global_progress.update_thread = std.Thread.spawn(.{}, ipcThreadRun, .{
@@ -368,12 +370,12 @@ pub fn start(options: Options) Node {
             }),
         }) catch |err| {
             std.log.warn("failed to spawn IPC thread for communicating progress to parent: {s}", .{@errorName(err)});
-            return .{ .index = .none };
+            return Node.none;
         };
     } else |env_err| switch (env_err) {
         error.EnvironmentVariableNotFound => {
             if (options.disable_printing) {
-                return .{ .index = .none };
+                return Node.none;
             }
             const stderr = std.io.getStdErr();
             global_progress.terminal = stderr;
@@ -386,7 +388,7 @@ pub fn start(options: Options) Node {
             }
 
             if (global_progress.terminal_mode == .off) {
-                return .{ .index = .none };
+                return Node.none;
             }
 
             if (have_sigwinch) {
@@ -408,12 +410,12 @@ pub fn start(options: Options) Node {
                 global_progress.update_thread = thread;
             } else |err| {
                 std.log.warn("unable to spawn thread for printing progress to terminal: {s}", .{@errorName(err)});
-                return .{ .index = .none };
+                return Node.none;
             }
         },
         else => |e| {
             std.log.warn("invalid ZIG_PROGRESS file descriptor integer: {s}", .{@errorName(e)});
-            return .{ .index = .none };
+            return Node.none;
         },
     }
 

lib/std/process/Child.zig

@@ -101,7 +101,7 @@ resource_usage_statistics: ResourceUsageStatistics = .{},
 ///
 /// The child's progress tree will be grafted into the parent's progress tree,
 /// by substituting this node with the child's root node.
-progress_node: std.Progress.Node = .{ .index = .none },
+progress_node: std.Progress.Node = std.Progress.Node.none,
 
 pub const ResourceUsageStatistics = struct {
     rusage: @TypeOf(rusage_init) = rusage_init,
@@ -376,6 +376,7 @@ pub fn run(args: struct {
     env_map: ?*const EnvMap = null,
     max_output_bytes: usize = 50 * 1024,
     expand_arg0: Arg0Expand = .no_expand,
+    progress_node: std.Progress.Node = std.Progress.Node.none,
 }) RunError!RunResult {
     var child = ChildProcess.init(args.argv, args.allocator);
     child.stdin_behavior = .Ignore;
@@ -385,6 +386,7 @@ pub fn run(args: struct {
     child.cwd_dir = args.cwd_dir;
     child.env_map = args.env_map;
     child.expand_arg0 = args.expand_arg0;
+    child.progress_node = args.progress_node;
 
     var stdout = std.ArrayList(u8).init(args.allocator);
     var stderr = std.ArrayList(u8).init(args.allocator);