Commit 02381c0372

Andrew Kelley <andrew@ziglang.org>
2023-02-14 20:47:45
std.Build: improve debugging of misconfigured steps
* 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.
1 parent 9580fbc
lib/std/Build/CheckFileStep.zig
@@ -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),
     };
lib/std/Build/CheckObjectStep.zig
@@ -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,
lib/std/Build/CompileStep.zig
@@ -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}),
lib/std/Build/ConfigHeaderStep.zig
@@ -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 {
@@ -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),
 
lib/std/Build/EmulatableRunStep.zig
@@ -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,
lib/std/Build/FmtStep.zig
@@ -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"),
     };
lib/std/Build/InstallArtifactStep.zig
@@ -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."),
lib/std/Build/InstallDirStep.zig
@@ -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),
     };
 }
lib/std/Build/InstallFileStep.zig
@@ -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),
lib/std/Build/LogStep.zig
@@ -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),
     };
 }
lib/std/Build/ObjCopyStep.zig
@@ -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(),
lib/std/Build/OptionsStep.zig
@@ -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),
lib/std/Build/RemoveDirStep.zig
@@ -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),
     };
 }
lib/std/Build/RunStep.zig
@@ -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,
lib/std/Build/Step.zig
@@ -11,6 +11,11 @@ result: struct {
     err_code: anyerror,
     stderr: []u8,
 },
+/// The return addresss associated with creation of this step that can be useful
+/// to print along with debugging messages.
+debug_stack_trace: [n_debug_stack_frames]usize,
+
+const n_debug_stack_frames = 4;
 
 pub const State = enum {
     precheck_unstarted,
@@ -66,16 +71,26 @@ pub const Id = enum {
     }
 };
 
-pub fn init(
+pub const Options = struct {
     id: Id,
     name: []const u8,
-    allocator: Allocator,
-    makeFn: *const fn (self: *Step) anyerror!void,
-) Step {
-    return Step{
-        .id = id,
-        .name = allocator.dupe(u8, name) catch @panic("OOM"),
-        .makeFn = makeFn,
+    makeFn: *const fn (self: *Step) anyerror!void = makeNoOp,
+    first_ret_addr: ?usize = null,
+};
+
+pub fn init(allocator: Allocator, options: Options) Step {
+    var addresses = [1]usize{0} ** n_debug_stack_frames;
+    const first_ret_addr = options.first_ret_addr orelse @returnAddress();
+    var stack_trace = std.builtin.StackTrace{
+        .instruction_addresses = &addresses,
+        .index = 0,
+    };
+    std.debug.captureStackTrace(first_ret_addr, &stack_trace);
+
+    return .{
+        .id = options.id,
+        .name = allocator.dupe(u8, options.name) catch @panic("OOM"),
+        .makeFn = options.makeFn,
         .dependencies = std.ArrayList(*Step).init(allocator),
         .dependants = .{},
         .state = .precheck_unstarted,
@@ -83,13 +98,10 @@ pub fn init(
             .err_code = undefined,
             .stderr = &.{},
         },
+        .debug_stack_trace = addresses,
     };
 }
 
-pub fn initNoOp(id: Id, name: []const u8, allocator: Allocator) Step {
-    return init(id, name, allocator, makeNoOp);
-}
-
 pub fn make(self: *Step) !void {
     try self.makeFn(self);
 }
@@ -98,6 +110,18 @@ pub fn dependOn(self: *Step, other: *Step) void {
     self.dependencies.append(other) catch @panic("OOM");
 }
 
+pub fn getStackTrace(s: *Step) std.builtin.StackTrace {
+    const stack_addresses = &s.debug_stack_trace;
+    var len: usize = 0;
+    while (len < n_debug_stack_frames and stack_addresses[len] != 0) {
+        len += 1;
+    }
+    return .{
+        .instruction_addresses = stack_addresses,
+        .index = len,
+    };
+}
+
 fn makeNoOp(self: *Step) anyerror!void {
     _ = self;
 }
lib/std/Build/TranslateCStep.zig
@@ -30,7 +30,11 @@ pub fn create(builder: *std.Build, options: Options) *TranslateCStep {
     const self = builder.allocator.create(TranslateCStep) catch @panic("OOM");
     const source = options.source_file.dupe(builder);
     self.* = TranslateCStep{
-        .step = Step.init(.translate_c, "translate-c", builder.allocator, make),
+        .step = Step.init(builder.allocator, .{
+            .id = .translate_c,
+            .name = "translate-c",
+            .makeFn = make,
+        }),
         .builder = builder,
         .source = source,
         .include_dirs = std.ArrayList([]const u8).init(builder.allocator),
lib/std/Build/WriteFileStep.zig
@@ -37,7 +37,11 @@ pub const Contents = union(enum) {
 pub fn init(builder: *std.Build) WriteFileStep {
     return .{
         .builder = builder,
-        .step = Step.init(.write_file, "writefile", builder.allocator, make),
+        .step = Step.init(builder.allocator, .{
+            .id = .write_file,
+            .name = "writefile",
+            .makeFn = make,
+        }),
         .files = .{},
         .output_source_files = .{},
     };
lib/std/Build.zig
@@ -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,
@@ -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),
@@ -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;
 }
@@ -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");
@@ -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);
@@ -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;
@@ -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),
         });
@@ -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),
         });
@@ -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(
@@ -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");
@@ -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},
         );
     }
@@ -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.
@@ -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.
test/link/macho/uuid/build.zig
@@ -90,10 +90,13 @@ const InstallWithRename = struct {
         const self = builder.allocator.create(InstallWithRename) catch @panic("OOM");
         self.* = InstallWithRename{
             .builder = builder,
-            .step = Step.init(.custom, builder.fmt("install and rename: {s} -> {s}", .{
-                source.getDisplayName(),
-                name,
-            }), builder.allocator, make),
+            .step = Step.init(builder.allocator, .{
+                .id = .custom,
+                .name = builder.fmt("install and rename: {s} -> {s}", .{
+                    source.getDisplayName(), name,
+                }),
+                .makeFn = make,
+            }),
             .source = source,
             .name = builder.dupe(name),
         };
@@ -123,15 +126,14 @@ const CompareUuid = struct {
         const self = builder.allocator.create(CompareUuid) catch @panic("OOM");
         self.* = CompareUuid{
             .builder = builder,
-            .step = Step.init(
-                .custom,
-                builder.fmt("compare uuid: {s} and {s}", .{
+            .step = Step.init(builder.allocator, .{
+                .id = .custom,
+                .name = builder.fmt("compare uuid: {s} and {s}", .{
                     lhs,
                     rhs,
                 }),
-                builder.allocator,
-                make,
-            ),
+                .makeFn = make,
+            }),
             .lhs = lhs,
             .rhs = rhs,
         };
test/tests.zig
@@ -858,7 +858,11 @@ pub const StackTracesContext = struct {
             const allocator = context.b.allocator;
             const ptr = allocator.create(RunAndCompareStep) catch unreachable;
             ptr.* = RunAndCompareStep{
-                .step = Step.init(.custom, "StackTraceCompareOutputStep", allocator, make),
+                .step = Step.init(allocator, .{
+                    .id = .custom,
+                    .name = "StackTraceCompareOutputStep",
+                    .makeFn = make,
+                }),
                 .context = context,
                 .exe = exe,
                 .name = name,
@@ -1194,7 +1198,11 @@ pub const GenHContext = struct {
             const allocator = context.b.allocator;
             const ptr = allocator.create(GenHCmpOutputStep) catch unreachable;
             ptr.* = GenHCmpOutputStep{
-                .step = Step.init(.Custom, "ParseCCmpOutput", allocator, make),
+                .step = Step.init(allocator, .{
+                    .id = .custom,
+                    .name = "ParseCCmpOutput",
+                    .makeFn = make,
+                }),
                 .context = context,
                 .obj = obj,
                 .name = name,