Skip to content
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.

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

enhance std.Build.Step.Fmt and use it more #20321

Merged
merged 5 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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, .{
Expand Down Expand Up @@ -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(.{
Expand Down
7 changes: 0 additions & 7 deletions ci/aarch64-linux-debug.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
7 changes: 0 additions & 7 deletions ci/aarch64-linux-release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
7 changes: 0 additions & 7 deletions ci/x86_64-linux-debug.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
8 changes: 0 additions & 8 deletions ci/x86_64-linux-release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
15 changes: 13 additions & 2 deletions lib/std/Build/Step.zig
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,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);
Expand All @@ -278,13 +288,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 } {
Expand Down
15 changes: 11 additions & 4 deletions lib/std/Build/Step/Fmt.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
20 changes: 11 additions & 9 deletions lib/std/Progress.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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, .{
Expand All @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
},
}

Expand Down
4 changes: 3 additions & 1 deletion lib/std/process/Child.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Loading