Skip to content

Commit

Permalink
fix build logic due to state mutations and break the API accordingly
Browse files Browse the repository at this point in the history 
 * 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.
  • Loading branch information
@andrewrk
andrewrk committed Apr 11, 2023
1 parent d5eab33 commit 3c3cee2
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 68 deletions.
4 changes: 4 additions & 0 deletions lib/std/Build.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
});
}

Expand Down
95 changes: 37 additions & 58 deletions lib/std/Build/CompileStep.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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}),
Expand All @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions test/standalone/issue_13970/build.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/standalone/test_runner_module_imports/build.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(.{
Expand Down
12 changes: 6 additions & 6 deletions test/tests.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down

0 comments on commit 3c3cee2

Please sign in to comment.