Skip to content

Commit 31b24a5

Browse files
committed
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.
1 parent 562a71b commit 31b24a5

File tree

5 files changed

+51
-68
lines changed

5 files changed

+51
-68
lines changed

lib/std/Build.zig

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,8 @@ pub const TestOptions = struct {
539539
optimize: std.builtin.Mode = .Debug,
540540
version: ?std.builtin.Version = null,
541541
max_rss: usize = 0,
542+
filter: ?[]const u8 = null,
543+
test_runner: ?[]const u8 = null,
542544
};
543545

544546
pub fn addTest(b: *Build, options: TestOptions) *CompileStep {
@@ -549,6 +551,8 @@ pub fn addTest(b: *Build, options: TestOptions) *CompileStep {
549551
.target = options.target,
550552
.optimize = options.optimize,
551553
.max_rss = options.max_rss,
554+
.filter = options.filter,
555+
.test_runner = options.test_runner,
552556
});
553557
}
554558

lib/std/Build/CompileStep.zig

Lines changed: 37 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,6 @@ out_lib_filename: []const u8,
9797
out_pdb_filename: []const u8,
9898
modules: std.StringArrayHashMap(*Module),
9999

100-
object_src: []const u8,
101-
102100
link_objects: ArrayList(LinkObject),
103101
include_dirs: ArrayList(IncludeDir),
104102
c_macros: ArrayList([]const u8),
@@ -288,6 +286,8 @@ pub const Options = struct {
288286
linkage: ?Linkage = null,
289287
version: ?std.builtin.Version = null,
290288
max_rss: usize = 0,
289+
filter: ?[]const u8 = null,
290+
test_runner: ?[]const u8 = null,
291291
};
292292

293293
pub const Kind = enum {
@@ -340,6 +340,23 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep {
340340
options.target.zigTriple(owner.allocator) catch @panic("OOM"),
341341
});
342342

343+
const target_info = NativeTargetInfo.detect(options.target) catch @panic("unhandled error");
344+
345+
const out_filename = std.zig.binNameAlloc(owner.allocator, .{
346+
.root_name = name,
347+
.target = target_info.target,
348+
.output_mode = switch (options.kind) {
349+
.lib => .Lib,
350+
.obj => .Obj,
351+
.exe, .@"test" => .Exe,
352+
},
353+
.link_mode = if (options.linkage) |some| @as(std.builtin.LinkMode, switch (some) {
354+
.dynamic => .Dynamic,
355+
.static => .Static,
356+
}) else null,
357+
.version = options.version,
358+
}) catch @panic("OOM");
359+
343360
const self = owner.allocator.create(CompileStep) catch @panic("OOM");
344361
self.* = CompileStep{
345362
.strip = null,
@@ -361,7 +378,7 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep {
361378
.max_rss = options.max_rss,
362379
}),
363380
.version = options.version,
364-
.out_filename = undefined,
381+
.out_filename = out_filename,
365382
.out_h_filename = owner.fmt("{s}.h", .{name}),
366383
.out_lib_filename = undefined,
367384
.out_pdb_filename = owner.fmt("{s}.pdb", .{name}),
@@ -375,13 +392,12 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep {
375392
.rpaths = ArrayList(FileSource).init(owner.allocator),
376393
.framework_dirs = ArrayList(FileSource).init(owner.allocator),
377394
.installed_headers = ArrayList(*Step).init(owner.allocator),
378-
.object_src = undefined,
379395
.c_std = std.Build.CStd.C99,
380396
.zig_lib_dir = null,
381397
.main_pkg_path = null,
382398
.exec_cmd_args = null,
383-
.filter = null,
384-
.test_runner = null,
399+
.filter = options.filter,
400+
.test_runner = options.test_runner,
385401
.disable_stack_probing = false,
386402
.disable_sanitize_c = false,
387403
.sanitize_thread = false,
@@ -397,60 +413,41 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep {
397413
.output_pdb_path_source = GeneratedFile{ .step = &self.step },
398414
.output_dirname_source = GeneratedFile{ .step = &self.step },
399415

400-
.target_info = NativeTargetInfo.detect(self.target) catch @panic("unhandled error"),
416+
.target_info = target_info,
401417
};
402-
self.computeOutFileNames();
403-
if (root_src) |rs| rs.addStepDependencies(&self.step);
404-
return self;
405-
}
406-
407-
fn computeOutFileNames(self: *CompileStep) void {
408-
const b = self.step.owner;
409-
const target = self.target_info.target;
410-
411-
self.out_filename = std.zig.binNameAlloc(b.allocator, .{
412-
.root_name = self.name,
413-
.target = target,
414-
.output_mode = switch (self.kind) {
415-
.lib => .Lib,
416-
.obj => .Obj,
417-
.exe, .@"test" => .Exe,
418-
},
419-
.link_mode = if (self.linkage) |some| @as(std.builtin.LinkMode, switch (some) {
420-
.dynamic => .Dynamic,
421-
.static => .Static,
422-
}) else null,
423-
.version = self.version,
424-
}) catch @panic("OOM");
425418

426419
if (self.kind == .lib) {
427420
if (self.linkage != null and self.linkage.? == .static) {
428421
self.out_lib_filename = self.out_filename;
429422
} else if (self.version) |version| {
430-
if (target.isDarwin()) {
431-
self.major_only_filename = b.fmt("lib{s}.{d}.dylib", .{
423+
if (target_info.target.isDarwin()) {
424+
self.major_only_filename = owner.fmt("lib{s}.{d}.dylib", .{
432425
self.name,
433426
version.major,
434427
});
435-
self.name_only_filename = b.fmt("lib{s}.dylib", .{self.name});
428+
self.name_only_filename = owner.fmt("lib{s}.dylib", .{self.name});
436429
self.out_lib_filename = self.out_filename;
437-
} else if (target.os.tag == .windows) {
438-
self.out_lib_filename = b.fmt("{s}.lib", .{self.name});
430+
} else if (target_info.target.os.tag == .windows) {
431+
self.out_lib_filename = owner.fmt("{s}.lib", .{self.name});
439432
} else {
440-
self.major_only_filename = b.fmt("lib{s}.so.{d}", .{ self.name, version.major });
441-
self.name_only_filename = b.fmt("lib{s}.so", .{self.name});
433+
self.major_only_filename = owner.fmt("lib{s}.so.{d}", .{ self.name, version.major });
434+
self.name_only_filename = owner.fmt("lib{s}.so", .{self.name});
442435
self.out_lib_filename = self.out_filename;
443436
}
444437
} else {
445-
if (target.isDarwin()) {
438+
if (target_info.target.isDarwin()) {
446439
self.out_lib_filename = self.out_filename;
447-
} else if (target.os.tag == .windows) {
448-
self.out_lib_filename = b.fmt("{s}.lib", .{self.name});
440+
} else if (target_info.target.os.tag == .windows) {
441+
self.out_lib_filename = owner.fmt("{s}.lib", .{self.name});
449442
} else {
450443
self.out_lib_filename = self.out_filename;
451444
}
452445
}
453446
}
447+
448+
if (root_src) |rs| rs.addStepDependencies(&self.step);
449+
450+
return self;
454451
}
455452

456453
pub fn install(self: *CompileStep) void {
@@ -847,24 +844,6 @@ fn linkSystemLibraryInner(self: *CompileStep, name: []const u8, opts: struct {
847844
}) catch @panic("OOM");
848845
}
849846

850-
pub fn setName(self: *CompileStep, text: []const u8) void {
851-
const b = self.step.owner;
852-
assert(self.kind == .@"test");
853-
self.name = b.dupe(text);
854-
}
855-
856-
pub fn setFilter(self: *CompileStep, text: ?[]const u8) void {
857-
const b = self.step.owner;
858-
assert(self.kind == .@"test");
859-
self.filter = if (text) |t| b.dupe(t) else null;
860-
}
861-
862-
pub fn setTestRunner(self: *CompileStep, path: ?[]const u8) void {
863-
const b = self.step.owner;
864-
assert(self.kind == .@"test");
865-
self.test_runner = if (path) |p| b.dupePath(p) else null;
866-
}
867-
868847
/// Handy when you have many C/C++ source files and want them all to have the same flags.
869848
pub fn addCSourceFiles(self: *CompileStep, files: []const []const u8, flags: []const []const u8) void {
870849
const b = self.step.owner;

test/standalone/issue_13970/build.zig

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,16 @@ pub fn build(b: *std.Build) void {
66

77
const test1 = b.addTest(.{
88
.root_source_file = .{ .path = "test_root/empty.zig" },
9+
.test_runner = "src/main.zig",
910
});
1011
const test2 = b.addTest(.{
1112
.root_source_file = .{ .path = "src/empty.zig" },
13+
.test_runner = "src/main.zig",
1214
});
1315
const test3 = b.addTest(.{
1416
.root_source_file = .{ .path = "empty.zig" },
17+
.test_runner = "src/main.zig",
1518
});
16-
test1.setTestRunner("src/main.zig");
17-
test2.setTestRunner("src/main.zig");
18-
test3.setTestRunner("src/main.zig");
1919

2020
test_step.dependOn(&test1.run().step);
2121
test_step.dependOn(&test2.run().step);

test/standalone/test_runner_module_imports/build.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ const std = @import("std");
33
pub fn build(b: *std.Build) void {
44
const t = b.addTest(.{
55
.root_source_file = .{ .path = "src/main.zig" },
6+
.test_runner = "test_runner/main.zig",
67
});
7-
t.setTestRunner("test_runner/main.zig");
88

99
const module1 = b.createModule(.{ .source_file = .{ .path = "module1/main.zig" } });
1010
const module2 = b.createModule(.{

test/tests.zig

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -976,11 +976,11 @@ pub fn addModuleTests(b: *std.Build, options: ModuleTestOptions) *Step {
976976
.optimize = test_target.optimize_mode,
977977
.target = test_target.target,
978978
.max_rss = max_rss,
979+
.filter = options.test_filter,
979980
});
980981
const single_threaded_txt = if (test_target.single_threaded) "single" else "multi";
981982
const backend_txt = if (test_target.backend) |backend| @tagName(backend) else "default";
982983
these_tests.single_threaded = test_target.single_threaded;
983-
these_tests.setFilter(options.test_filter);
984984
if (test_target.link_libc) {
985985
these_tests.linkSystemLibrary("c");
986986
}
@@ -1036,10 +1036,15 @@ pub fn addCAbiTests(b: *std.Build, skip_non_native: bool, skip_release: bool) *S
10361036
continue;
10371037
}
10381038

1039+
const triple_prefix = c_abi_target.zigTriple(b.allocator) catch @panic("OOM");
1040+
10391041
const test_step = b.addTest(.{
10401042
.root_source_file = .{ .path = "test/c_abi/main.zig" },
10411043
.optimize = optimize_mode,
10421044
.target = c_abi_target,
1045+
.name = b.fmt("test-c-abi-{s}-{s}", .{
1046+
triple_prefix, @tagName(optimize_mode),
1047+
}),
10431048
});
10441049
if (c_abi_target.abi != null and c_abi_target.abi.?.isMusl()) {
10451050
// TODO NativeTargetInfo insists on dynamically linking musl
@@ -1056,11 +1061,6 @@ pub fn addCAbiTests(b: *std.Build, skip_non_native: bool, skip_release: bool) *S
10561061
test_step.want_lto = false;
10571062
}
10581063

1059-
const triple_prefix = c_abi_target.zigTriple(b.allocator) catch @panic("OOM");
1060-
test_step.setName(b.fmt("test-c-abi-{s}-{s} ", .{
1061-
triple_prefix, @tagName(optimize_mode),
1062-
}));
1063-
10641064
const run = test_step.run();
10651065
run.skip_foreign_checks = true;
10661066
step.dependOn(&run.step);

0 commit comments

Comments
 (0)