Skip to content

Commit

Permalink
std.Build: improve debugging of misconfigured steps
Browse files Browse the repository at this point in the history
 * Step.init() now takes an options struct
 * Step.init() now captures a small stack trace and stores it in the
   Step so that it can be accessed when printing user-friendly debugging
   information, including the lines of code that created the step in
   question.
  • Loading branch information
andrewrk committed Mar 15, 2023
1 parent 9580fbc commit 02381c0
Show file tree
Hide file tree
Showing 20 changed files with 209 additions and 68 deletions.
88 changes: 67 additions & 21 deletions lib/std/Build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -227,12 +227,19 @@ pub fn create(
.h_dir = undefined,
.dest_dir = env_map.get("DESTDIR"),
.installed_files = ArrayList(InstalledFile).init(allocator),
.install_tls = TopLevelStep{
.step = Step.initNoOp(.top_level, "install", allocator),
.install_tls = .{
.step = Step.init(allocator, .{
.id = .top_level,
.name = "install",
}),
.description = "Copy build artifacts to prefix path",
},
.uninstall_tls = TopLevelStep{
.step = Step.init(.top_level, "uninstall", allocator, makeUninstall),
.uninstall_tls = .{
.step = Step.init(allocator, .{
.id = .top_level,
.name = "uninstall",
.makeFn = makeUninstall,
}),
.description = "Remove build artifacts from prefix path",
},
.zig_lib_dir = null,
Expand Down Expand Up @@ -264,11 +271,18 @@ fn createChildOnly(parent: *Build, dep_name: []const u8, build_root: Cache.Direc
child.* = .{
.allocator = allocator,
.install_tls = .{
.step = Step.initNoOp(.top_level, "install", allocator),
.step = Step.init(allocator, .{
.id = .top_level,
.name = "install",
}),
.description = "Copy build artifacts to prefix path",
},
.uninstall_tls = .{
.step = Step.init(.top_level, "uninstall", allocator, makeUninstall),
.step = Step.init(allocator, .{
.id = .top_level,
.name = "uninstall",
.makeFn = makeUninstall,
}),
.description = "Remove build artifacts from prefix path",
},
.user_input_options = UserInputOptionsMap.init(allocator),
Expand Down Expand Up @@ -634,7 +648,11 @@ pub fn addConfigHeader(
options: ConfigHeaderStep.Options,
values: anytype,
) *ConfigHeaderStep {
const config_header_step = ConfigHeaderStep.create(b, options);
var options_copy = options;
if (options_copy.first_ret_addr == null)
options_copy.first_ret_addr = @returnAddress();

const config_header_step = ConfigHeaderStep.create(b, options_copy);
config_header_step.addValues(values);
return config_header_step;
}
Expand Down Expand Up @@ -858,7 +876,10 @@ pub fn option(self: *Build, comptime T: type, name_raw: []const u8, description_
pub fn step(self: *Build, name: []const u8, description: []const u8) *Step {
const step_info = self.allocator.create(TopLevelStep) catch @panic("OOM");
step_info.* = TopLevelStep{
.step = Step.initNoOp(.top_level, name, self.allocator),
.step = Step.init(self.allocator, .{
.id = .top_level,
.name = name,
}),
.description = self.dupe(description),
};
self.top_level_steps.put(self.allocator, step_info.step.name, step_info) catch @panic("OOM");
Expand Down Expand Up @@ -1153,7 +1174,7 @@ pub fn spawnChildEnvMap(self: *Build, cwd: ?[]const u8, env_map: *const EnvMap,
printCmd(self.allocator, cwd, argv);
}

if (!std.process.can_spawn)
if (!process.can_spawn)
return error.ExecNotSupported;

var child = std.ChildProcess.init(argv, self.allocator);
Expand Down Expand Up @@ -1355,7 +1376,7 @@ pub fn execAllowFail(
) ExecError![]u8 {
assert(argv.len != 0);

if (!std.process.can_spawn)
if (!process.can_spawn)
return error.ExecNotSupported;

const max_output_size = 400 * 1024;
Expand Down Expand Up @@ -1395,7 +1416,7 @@ pub fn execFromStep(b: *Build, argv: []const []const u8, s: *Step) ![]u8 {
printCmd(b.allocator, null, argv);
}

if (!std.process.can_spawn) {
if (!process.can_spawn) {
s.result.stderr = b.fmt("Unable to spawn the following command: cannot spawn child processes\n{s}", .{
try allocPrintCmd(b.allocator, null, argv),
});
Expand Down Expand Up @@ -1458,7 +1479,7 @@ fn unwrapExecResult(
/// inside step make() functions. If any errors occur, it fails the build with
/// a helpful message.
pub fn exec(b: *Build, argv: []const []const u8) []u8 {
if (!std.process.can_spawn) {
if (!process.can_spawn) {
std.debug.print("unable to spawn the following command: cannot spawn child process\n{s}", .{
try allocPrintCmd(b.allocator, null, argv),
});
Expand Down Expand Up @@ -1539,7 +1560,7 @@ pub fn dependency(b: *Build, name: []const u8, args: anytype) *Dependency {

const full_path = b.pathFromRoot("build.zig.zon");
std.debug.print("no dependency named '{s}' in '{s}'. All packages used in build.zig must be declared in this file.\n", .{ name, full_path });
std.process.exit(1);
process.exit(1);
}

fn dependencyInner(
Expand All @@ -1555,7 +1576,7 @@ fn dependencyInner(
std.debug.print("unable to open '{s}': {s}\n", .{
build_root_string, @errorName(err),
});
std.process.exit(1);
process.exit(1);
},
};
const sub_builder = b.createChild(name, build_root, args) catch @panic("unhandled error");
Expand Down Expand Up @@ -1599,7 +1620,7 @@ pub const GeneratedFile = struct {

pub fn getPath(self: GeneratedFile) []const u8 {
return self.path orelse std.debug.panic(
"getPath() was called on a GeneratedFile that wasn't build yet. Is there a missing Step dependency on step '{s}'?",
"getPath() was called on a GeneratedFile that wasn't built yet. Is there a missing Step dependency on step '{s}'?",
.{self.step.name},
);
}
Expand Down Expand Up @@ -1639,12 +1660,16 @@ pub const FileSource = union(enum) {
}

/// Should only be called during make(), returns a path relative to the build root or absolute.
pub fn getPath(self: FileSource, builder: *Build) []const u8 {
const path = switch (self) {
.path => |p| builder.pathFromRoot(p),
.generated => |gen| gen.getPath(),
};
return path;
pub fn getPath(self: FileSource, src_builder: *Build) []const u8 {
switch (self) {
.path => |p| return src_builder.pathFromRoot(p),
.generated => |gen| return gen.path orelse {
std.debug.getStderrMutex().lock();
const stderr = std.io.getStdErr();
dumpBadGetPathHelp(gen.step, stderr, src_builder) catch {};
@panic("unable to get path");
},
}
}

/// Duplicates the file source for a given builder.
Expand All @@ -1656,6 +1681,27 @@ pub const FileSource = union(enum) {
}
};

fn dumpBadGetPathHelp(s: *Step, stderr: fs.File, src_builder: *Build) anyerror!void {
try stderr.writer().print(
\\getPath() was called on a GeneratedFile that wasn't built yet.
\\ source package path: {s}
\\ Is there a missing Step dependency on step '{s}'?
\\ The step was created by this stack trace:
\\
, .{
src_builder.build_root.path orelse ".",
s.name,
});
const debug_info = std.debug.getSelfDebugInfo() catch |err| {
try stderr.writer().print("Unable to dump stack trace: Unable to open debug info: {s}\n", .{@errorName(err)});
return;
};
std.debug.writeStackTrace(s.getStackTrace(), stderr.writer(), debug_info.allocator, debug_info, std.debug.detectTTYConfig(stderr)) catch |err| {
try stderr.writer().print("Unable to dump stack trace: {s}\n", .{@errorName(err)});
return;
};
}

/// Allocates a new string for assigning a value to a named macro.
/// If the value is omitted, it is set to 1.
/// `name` and `value` need not live longer than the function call.
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/CheckFileStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ pub fn create(
const self = builder.allocator.create(CheckFileStep) catch @panic("OOM");
self.* = CheckFileStep{
.builder = builder,
.step = Step.init(.check_file, "CheckFile", builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = .check_file,
.name = "CheckFile",
.makeFn = make,
}),
.source = source.dupe(builder),
.expected_matches = builder.dupeStrings(expected_matches),
};
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/CheckObjectStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ pub fn create(builder: *std.Build, source: std.Build.FileSource, obj_format: std
const self = gpa.create(CheckObjectStep) catch @panic("OOM");
self.* = .{
.builder = builder,
.step = Step.init(.check_file, "CheckObject", gpa, make),
.step = Step.init(gpa, .{
.id = .check_file,
.name = "CheckObject",
.makeFn = make,
}),
.source = source.dupe(builder),
.checks = std.ArrayList(Check).init(gpa),
.obj_format = obj_format,
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/CompileStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,11 @@ pub fn create(builder: *std.Build, options: Options) *CompileStep {
.root_src = root_src,
.name = name,
.frameworks = StringHashMap(FrameworkLinkInfo).init(builder.allocator),
.step = Step.init(base_id, name, builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = base_id,
.name = name,
.makeFn = make,
}),
.version = options.version,
.out_filename = undefined,
.out_h_filename = builder.fmt("{s}.h", .{name}),
Expand Down
8 changes: 7 additions & 1 deletion lib/std/Build/ConfigHeaderStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub const Options = struct {
style: Style = .blank,
max_bytes: usize = 2 * 1024 * 1024,
include_path: ?[]const u8 = null,
first_ret_addr: ?usize = null,
};

pub fn create(builder: *std.Build, options: Options) *ConfigHeaderStep {
Expand All @@ -56,7 +57,12 @@ pub fn create(builder: *std.Build, options: Options) *ConfigHeaderStep {
builder.fmt("configure {s} header", .{@tagName(options.style)});
self.* = .{
.builder = builder,
.step = Step.init(base_id, name, builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = base_id,
.name = name,
.makeFn = make,
.first_ret_addr = options.first_ret_addr orelse @returnAddress(),
}),
.style = options.style,
.values = std.StringArrayHashMap(Value).init(builder.allocator),

Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/EmulatableRunStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ pub fn create(builder: *std.Build, name: []const u8, artifact: *CompileStep) *Em

self.* = .{
.builder = builder,
.step = Step.init(.emulatable_run, name, builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = .emulatable_run,
.name = name,
.makeFn = make,
}),
.exe = artifact,
.env_map = null,
.cwd = null,
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/FmtStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ pub fn create(builder: *std.Build, paths: []const []const u8) *FmtStep {
const self = builder.allocator.create(FmtStep) catch @panic("OOM");
const name = "zig fmt";
self.* = FmtStep{
.step = Step.init(.fmt, name, builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = .fmt,
.name = name,
.makeFn = make,
}),
.builder = builder,
.argv = builder.allocator.alloc([]u8, paths.len + 2) catch @panic("OOM"),
};
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/InstallArtifactStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ pub fn create(builder: *std.Build, artifact: *CompileStep) *InstallArtifactStep
const self = builder.allocator.create(InstallArtifactStep) catch @panic("OOM");
self.* = InstallArtifactStep{
.builder = builder,
.step = Step.init(.install_artifact, builder.fmt("install {s}", .{artifact.step.name}), builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = base_id,
.name = builder.fmt("install {s}", .{artifact.step.name}),
.makeFn = make,
}),
.artifact = artifact,
.dest_dir = artifact.override_dest_dir orelse switch (artifact.kind) {
.obj => @panic("Cannot install a .obj build artifact."),
Expand Down
8 changes: 6 additions & 2 deletions lib/std/Build/InstallDirStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,13 @@ pub fn init(
options: Options,
) InstallDirStep {
builder.pushInstalledFile(options.install_dir, options.install_subdir);
return InstallDirStep{
return .{
.builder = builder,
.step = Step.init(.install_dir, builder.fmt("install {s}/", .{options.source_dir}), builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = .install_dir,
.name = builder.fmt("install {s}/", .{options.source_dir}),
.makeFn = make,
}),
.options = options.dupe(builder),
};
}
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/InstallFileStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ pub fn init(
builder.pushInstalledFile(dir, dest_rel_path);
return InstallFileStep{
.builder = builder,
.step = Step.init(.install_file, builder.fmt("install {s} to {s}", .{ source.getDisplayName(), dest_rel_path }), builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = .install_file,
.name = builder.fmt("install {s} to {s}", .{ source.getDisplayName(), dest_rel_path }),
.makeFn = make,
}),
.source = source.dupe(builder),
.dir = dir.dupe(builder),
.dest_rel_path = builder.dupePath(dest_rel_path),
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/LogStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ data: []const u8,
pub fn init(builder: *std.Build, data: []const u8) LogStep {
return LogStep{
.builder = builder,
.step = Step.init(.log, builder.fmt("log {s}", .{data}), builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = .log,
.name = builder.fmt("log {s}", .{data}),
.makeFn = make,
}),
.data = builder.dupe(data),
};
}
Expand Down
11 changes: 5 additions & 6 deletions lib/std/Build/ObjCopyStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@ pub fn create(
) *ObjCopyStep {
const self = builder.allocator.create(ObjCopyStep) catch @panic("OOM");
self.* = ObjCopyStep{
.step = Step.init(
base_id,
builder.fmt("objcopy {s}", .{file_source.getDisplayName()}),
builder.allocator,
make,
),
.step = Step.init(builder.allocator, .{
.id = base_id,
.name = builder.fmt("objcopy {s}", .{file_source.getDisplayName()}),
.makeFn = make,
}),
.builder = builder,
.file_source = file_source,
.basename = options.basename orelse file_source.getDisplayName(),
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/OptionsStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ pub fn create(builder: *std.Build) *OptionsStep {
const self = builder.allocator.create(OptionsStep) catch @panic("OOM");
self.* = .{
.builder = builder,
.step = Step.init(.options, "options", builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = base_id,
.name = "options",
.makeFn = make,
}),
.generated_file = undefined,
.contents = std.ArrayList(u8).init(builder.allocator),
.artifact_args = std.ArrayList(OptionArtifactArg).init(builder.allocator),
Expand Down
6 changes: 5 additions & 1 deletion lib/std/Build/RemoveDirStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ dir_path: []const u8,
pub fn init(builder: *std.Build, dir_path: []const u8) RemoveDirStep {
return RemoveDirStep{
.builder = builder,
.step = Step.init(.remove_dir, builder.fmt("RemoveDir {s}", .{dir_path}), builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = .remove_dir,
.name = builder.fmt("RemoveDir {s}", .{dir_path}),
.makeFn = make,
}),
.dir_path = builder.dupePath(dir_path),
};
}
Expand Down
8 changes: 6 additions & 2 deletions lib/std/Build/RunStep.zig
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,13 @@ pub const Arg = union(enum) {

pub fn create(builder: *std.Build, name: []const u8) *RunStep {
const self = builder.allocator.create(RunStep) catch @panic("OOM");
self.* = RunStep{
self.* = .{
.builder = builder,
.step = Step.init(base_id, name, builder.allocator, make),
.step = Step.init(builder.allocator, .{
.id = base_id,
.name = name,
.makeFn = make,
}),
.argv = ArrayList(Arg).init(builder.allocator),
.cwd = null,
.env_map = null,
Expand Down
Loading

0 comments on commit 02381c0

Please sign in to comment.