From 3c3cee2cfabc5d03bb83cf6cc6a8ed80f13ee1df Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 10 Apr 2023 18:09:39 -0700 Subject: [PATCH] fix build logic due to state mutations and break the API accordingly * remove setName, setFilter, and setTestRunner. Please set these options directly when creating the CompileStep. * removed unused field * remove computeOutFileNames and inline the logic, making clear the goal of avoiding state mutations after the build step is created. --- lib/std/Build.zig | 4 + lib/std/Build/CompileStep.zig | 95 ++++++++----------- test/standalone/issue_13970/build.zig | 6 +- .../test_runner_module_imports/build.zig | 2 +- test/tests.zig | 12 +-- 5 files changed, 51 insertions(+), 68 deletions(-) diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 5b974bb8165d..7faa7780b22a 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -539,6 +539,8 @@ pub const TestOptions = struct { optimize: std.builtin.Mode = .Debug, version: ?std.builtin.Version = null, max_rss: usize = 0, + filter: ?[]const u8 = null, + test_runner: ?[]const u8 = null, }; pub fn addTest(b: *Build, options: TestOptions) *CompileStep { @@ -549,6 +551,8 @@ pub fn addTest(b: *Build, options: TestOptions) *CompileStep { .target = options.target, .optimize = options.optimize, .max_rss = options.max_rss, + .filter = options.filter, + .test_runner = options.test_runner, }); } diff --git a/lib/std/Build/CompileStep.zig b/lib/std/Build/CompileStep.zig index ceacc87dca14..5c3b166282e5 100644 --- a/lib/std/Build/CompileStep.zig +++ b/lib/std/Build/CompileStep.zig @@ -97,8 +97,6 @@ out_lib_filename: []const u8, out_pdb_filename: []const u8, modules: std.StringArrayHashMap(*Module), -object_src: []const u8, - link_objects: ArrayList(LinkObject), include_dirs: ArrayList(IncludeDir), c_macros: ArrayList([]const u8), @@ -287,6 +285,8 @@ pub const Options = struct { linkage: ?Linkage = null, version: ?std.builtin.Version = null, max_rss: usize = 0, + filter: ?[]const u8 = null, + test_runner: ?[]const u8 = null, }; pub const Kind = enum { @@ -339,6 +339,23 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep { options.target.zigTriple(owner.allocator) catch @panic("OOM"), }); + const target_info = NativeTargetInfo.detect(options.target) catch @panic("unhandled error"); + + const out_filename = std.zig.binNameAlloc(owner.allocator, .{ + .root_name = name, + .target = target_info.target, + .output_mode = switch (options.kind) { + .lib => .Lib, + .obj => .Obj, + .exe, .@"test" => .Exe, + }, + .link_mode = if (options.linkage) |some| @as(std.builtin.LinkMode, switch (some) { + .dynamic => .Dynamic, + .static => .Static, + }) else null, + .version = options.version, + }) catch @panic("OOM"); + const self = owner.allocator.create(CompileStep) catch @panic("OOM"); self.* = CompileStep{ .strip = null, @@ -360,7 +377,7 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep { .max_rss = options.max_rss, }), .version = options.version, - .out_filename = undefined, + .out_filename = out_filename, .out_h_filename = owner.fmt("{s}.h", .{name}), .out_lib_filename = undefined, .out_pdb_filename = owner.fmt("{s}.pdb", .{name}), @@ -374,13 +391,12 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep { .rpaths = ArrayList(FileSource).init(owner.allocator), .framework_dirs = ArrayList(FileSource).init(owner.allocator), .installed_headers = ArrayList(*Step).init(owner.allocator), - .object_src = undefined, .c_std = std.Build.CStd.C99, .zig_lib_dir = null, .main_pkg_path = null, .exec_cmd_args = null, - .filter = null, - .test_runner = null, + .filter = options.filter, + .test_runner = options.test_runner, .disable_stack_probing = false, .disable_sanitize_c = false, .sanitize_thread = false, @@ -395,60 +411,41 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep { .output_pdb_path_source = GeneratedFile{ .step = &self.step }, .output_dirname_source = GeneratedFile{ .step = &self.step }, - .target_info = NativeTargetInfo.detect(self.target) catch @panic("unhandled error"), + .target_info = target_info, }; - self.computeOutFileNames(); - if (root_src) |rs| rs.addStepDependencies(&self.step); - return self; -} - -fn computeOutFileNames(self: *CompileStep) void { - const b = self.step.owner; - const target = self.target_info.target; - - self.out_filename = std.zig.binNameAlloc(b.allocator, .{ - .root_name = self.name, - .target = target, - .output_mode = switch (self.kind) { - .lib => .Lib, - .obj => .Obj, - .exe, .@"test" => .Exe, - }, - .link_mode = if (self.linkage) |some| @as(std.builtin.LinkMode, switch (some) { - .dynamic => .Dynamic, - .static => .Static, - }) else null, - .version = self.version, - }) catch @panic("OOM"); if (self.kind == .lib) { if (self.linkage != null and self.linkage.? == .static) { self.out_lib_filename = self.out_filename; } else if (self.version) |version| { - if (target.isDarwin()) { - self.major_only_filename = b.fmt("lib{s}.{d}.dylib", .{ + if (target_info.target.isDarwin()) { + self.major_only_filename = owner.fmt("lib{s}.{d}.dylib", .{ self.name, version.major, }); - self.name_only_filename = b.fmt("lib{s}.dylib", .{self.name}); + self.name_only_filename = owner.fmt("lib{s}.dylib", .{self.name}); self.out_lib_filename = self.out_filename; - } else if (target.os.tag == .windows) { - self.out_lib_filename = b.fmt("{s}.lib", .{self.name}); + } else if (target_info.target.os.tag == .windows) { + self.out_lib_filename = owner.fmt("{s}.lib", .{self.name}); } else { - self.major_only_filename = b.fmt("lib{s}.so.{d}", .{ self.name, version.major }); - self.name_only_filename = b.fmt("lib{s}.so", .{self.name}); + self.major_only_filename = owner.fmt("lib{s}.so.{d}", .{ self.name, version.major }); + self.name_only_filename = owner.fmt("lib{s}.so", .{self.name}); self.out_lib_filename = self.out_filename; } } else { - if (target.isDarwin()) { + if (target_info.target.isDarwin()) { self.out_lib_filename = self.out_filename; - } else if (target.os.tag == .windows) { - self.out_lib_filename = b.fmt("{s}.lib", .{self.name}); + } else if (target_info.target.os.tag == .windows) { + self.out_lib_filename = owner.fmt("{s}.lib", .{self.name}); } else { self.out_lib_filename = self.out_filename; } } } + + if (root_src) |rs| rs.addStepDependencies(&self.step); + + return self; } pub fn installHeader(cs: *CompileStep, src_path: []const u8, dest_rel_path: []const u8) void { @@ -841,24 +838,6 @@ fn linkSystemLibraryInner(self: *CompileStep, name: []const u8, opts: struct { }) catch @panic("OOM"); } -pub fn setName(self: *CompileStep, text: []const u8) void { - const b = self.step.owner; - assert(self.kind == .@"test"); - self.name = b.dupe(text); -} - -pub fn setFilter(self: *CompileStep, text: ?[]const u8) void { - const b = self.step.owner; - assert(self.kind == .@"test"); - self.filter = if (text) |t| b.dupe(t) else null; -} - -pub fn setTestRunner(self: *CompileStep, path: ?[]const u8) void { - const b = self.step.owner; - assert(self.kind == .@"test"); - self.test_runner = if (path) |p| b.dupePath(p) else null; -} - /// Handy when you have many C/C++ source files and want them all to have the same flags. pub fn addCSourceFiles(self: *CompileStep, files: []const []const u8, flags: []const []const u8) void { const b = self.step.owner; diff --git a/test/standalone/issue_13970/build.zig b/test/standalone/issue_13970/build.zig index 7d2799e29e90..cc70144596df 100644 --- a/test/standalone/issue_13970/build.zig +++ b/test/standalone/issue_13970/build.zig @@ -6,16 +6,16 @@ pub fn build(b: *std.Build) void { const test1 = b.addTest(.{ .root_source_file = .{ .path = "test_root/empty.zig" }, + .test_runner = "src/main.zig", }); const test2 = b.addTest(.{ .root_source_file = .{ .path = "src/empty.zig" }, + .test_runner = "src/main.zig", }); const test3 = b.addTest(.{ .root_source_file = .{ .path = "empty.zig" }, + .test_runner = "src/main.zig", }); - test1.setTestRunner("src/main.zig"); - test2.setTestRunner("src/main.zig"); - test3.setTestRunner("src/main.zig"); test_step.dependOn(&b.addRunArtifact(test1).step); test_step.dependOn(&b.addRunArtifact(test2).step); diff --git a/test/standalone/test_runner_module_imports/build.zig b/test/standalone/test_runner_module_imports/build.zig index 307957faeeac..f27771889827 100644 --- a/test/standalone/test_runner_module_imports/build.zig +++ b/test/standalone/test_runner_module_imports/build.zig @@ -3,8 +3,8 @@ const std = @import("std"); pub fn build(b: *std.Build) void { const t = b.addTest(.{ .root_source_file = .{ .path = "src/main.zig" }, + .test_runner = "test_runner/main.zig", }); - t.setTestRunner("test_runner/main.zig"); const module1 = b.createModule(.{ .source_file = .{ .path = "module1/main.zig" } }); const module2 = b.createModule(.{ diff --git a/test/tests.zig b/test/tests.zig index a9bed635e2d4..18af132992b1 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -977,11 +977,11 @@ pub fn addModuleTests(b: *std.Build, options: ModuleTestOptions) *Step { .optimize = test_target.optimize_mode, .target = test_target.target, .max_rss = max_rss, + .filter = options.test_filter, }); const single_threaded_txt = if (test_target.single_threaded) "single" else "multi"; const backend_txt = if (test_target.backend) |backend| @tagName(backend) else "default"; these_tests.single_threaded = test_target.single_threaded; - these_tests.setFilter(options.test_filter); if (test_target.link_libc) { these_tests.linkSystemLibrary("c"); } @@ -1037,10 +1037,15 @@ pub fn addCAbiTests(b: *std.Build, skip_non_native: bool, skip_release: bool) *S continue; } + const triple_prefix = c_abi_target.zigTriple(b.allocator) catch @panic("OOM"); + const test_step = b.addTest(.{ .root_source_file = .{ .path = "test/c_abi/main.zig" }, .optimize = optimize_mode, .target = c_abi_target, + .name = b.fmt("test-c-abi-{s}-{s}", .{ + triple_prefix, @tagName(optimize_mode), + }), }); if (c_abi_target.abi != null and c_abi_target.abi.?.isMusl()) { // TODO NativeTargetInfo insists on dynamically linking musl @@ -1057,11 +1062,6 @@ pub fn addCAbiTests(b: *std.Build, skip_non_native: bool, skip_release: bool) *S test_step.want_lto = false; } - const triple_prefix = c_abi_target.zigTriple(b.allocator) catch @panic("OOM"); - test_step.setName(b.fmt("test-c-abi-{s}-{s} ", .{ - triple_prefix, @tagName(optimize_mode), - })); - const run = b.addRunArtifact(test_step); run.skip_foreign_checks = true; step.dependOn(&run.step);