Commit 3c3cee2cfabc5d03bb83cf6cc6a8ed80f13ee1df | zig

author	Andrew Kelley <andrew@ziglang.org> 2023-04-10 18:09:39-07:00
committer	Andrew Kelley <andrew@ziglang.org> 2023-04-11 08:42:14-07:00
log	3c3cee2cfabc5d03bb83cf6cc6a8ed80f13ee1df
tree	b440fbaf013ed472c39a60381928c0224e694f5c
parent	d5eab33fd23351a8606cb69a846b1b753b4c7c4a

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.

5 files changed, 51 insertions(+), 68 deletions(-)

lib
- std
  - Build.zig
  - Build
    CompileStep.zig
test
- standalone
  - issue_13970
    build.zig
  - test_runner_module_imports
    build.zig
- tests.zig

lib/std/Build.zig+4

...	@@ -539,6 +539,8 @@ pub const TestOptions = struct {	...	@@ -539,6 +539,8 @@ pub const TestOptions = struct {
539	optimize: std.builtin.Mode = .Debug,	539	optimize: std.builtin.Mode = .Debug,
540	version: ?std.builtin.Version = null,	540	version: ?std.builtin.Version = null,
541	max_rss: usize = 0,	541	max_rss: usize = 0,
		542	filter: ?[]const u8 = null,
		543	test_runner: ?[]const u8 = null,
542	};	544	};
543		545
544	pub fn addTest(b: Build, options: TestOptions) CompileStep {	546	pub fn addTest(b: Build, options: TestOptions) CompileStep {
...	@@ -549,6 +551,8 @@ pub fn addTest(b: Build, options: TestOptions) CompileStep {	...	@@ -549,6 +551,8 @@ pub fn addTest(b: Build, options: TestOptions) CompileStep {
549	.target = options.target,	551	.target = options.target,
550	.optimize = options.optimize,	552	.optimize = options.optimize,
551	.max_rss = options.max_rss,	553	.max_rss = options.max_rss,
		554	.filter = options.filter,
		555	.test_runner = options.test_runner,
552	});	556	});
553	}	557	}
554		558

lib/std/Build/CompileStep.zig+37-58

 @@ -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,
+        .filter = options.filter,
-        .test_runner = null,
+        .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()) {
+            if (target_info.target.isDarwin()) {
-                self.major_only_filename = b.fmt("lib{s}.{d}.dylib", .{
+                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) {
+            } else if (target_info.target.os.tag == .windows) {
-                self.out_lib_filename = b.fmt("{s}.lib", .{self.name});
+                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.major_only_filename = owner.fmt("lib{s}.so.{d}", .{ self.name, version.major });
-                self.name_only_filename = b.fmt("lib{s}.so", .{self.name});
+                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) {
+            } else if (target_info.target.os.tag == .windows) {
-                self.out_lib_filename = b.fmt("{s}.lib", .{self.name});
+                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;

test/standalone/issue_13970/build.zig+3-3

 @@ -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);

test/standalone/test_runner_module_imports/build.zig+1-1

 @@ -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(.{

test/tests.zig+6-6

 @@ -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);