rename std.Build.InstallRawStep to ObjCopyStep

And make it not do any installation, only objcopying. We already have
install steps for doing installation.

This commit also makes ObjCopyStep properly integrate with caching.

lib/std/Build.zig

@@ -37,7 +37,7 @@ pub const FmtStep = @import("Build/FmtStep.zig");
 pub const InstallArtifactStep = @import("Build/InstallArtifactStep.zig");
 pub const InstallDirStep = @import("Build/InstallDirStep.zig");
 pub const InstallFileStep = @import("Build/InstallFileStep.zig");
-pub const InstallRawStep = @import("Build/InstallRawStep.zig");
+pub const ObjCopyStep = @import("Build/ObjCopyStep.zig");
 pub const CompileStep = @import("Build/CompileStep.zig");
 pub const LogStep = @import("Build/LogStep.zig");
 pub const OptionsStep = @import("Build/OptionsStep.zig");
@@ -1254,11 +1254,8 @@ pub fn installLibFile(self: *Build, src_path: []const u8, dest_rel_path: []const
     self.getInstallStep().dependOn(&self.addInstallFileWithDir(.{ .path = src_path }, .lib, dest_rel_path).step);
 }
 
-/// Output format (BIN vs Intel HEX) determined by filename
-pub fn installRaw(self: *Build, artifact: *CompileStep, dest_filename: []const u8, options: InstallRawStep.CreateOptions) *InstallRawStep {
-    const raw = self.addInstallRaw(artifact, dest_filename, options);
-    self.getInstallStep().dependOn(&raw.step);
-    return raw;
+pub fn addObjCopy(b: *Build, source: FileSource, options: ObjCopyStep.Options) *ObjCopyStep {
+    return ObjCopyStep.create(b, source, options);
 }
 
 ///`dest_rel_path` is relative to install prefix path
@@ -1280,10 +1277,6 @@ pub fn addInstallHeaderFile(b: *Build, src_path: []const u8, dest_rel_path: []co
     return b.addInstallFileWithDir(.{ .path = src_path }, .header, dest_rel_path);
 }
 
-pub fn addInstallRaw(self: *Build, artifact: *CompileStep, dest_filename: []const u8, options: InstallRawStep.CreateOptions) *InstallRawStep {
-    return InstallRawStep.create(self, artifact, dest_filename, options);
-}
-
 pub fn addInstallFileWithDir(
     self: *Build,
     source: FileSource,
@@ -1771,7 +1764,7 @@ test {
     _ = InstallArtifactStep;
     _ = InstallDirStep;
     _ = InstallFileStep;
-    _ = InstallRawStep;
+    _ = ObjCopyStep;
     _ = CompileStep;
     _ = LogStep;
     _ = OptionsStep;

lib/std/Build/CompileStep.zig

@@ -21,7 +21,7 @@ const VcpkgRoot = std.Build.VcpkgRoot;
 const InstallDir = std.Build.InstallDir;
 const InstallArtifactStep = std.Build.InstallArtifactStep;
 const GeneratedFile = std.Build.GeneratedFile;
-const InstallRawStep = std.Build.InstallRawStep;
+const ObjCopyStep = std.Build.ObjCopyStep;
 const EmulatableRunStep = std.Build.EmulatableRunStep;
 const CheckObjectStep = std.Build.CheckObjectStep;
 const RunStep = std.Build.RunStep;
@@ -432,10 +432,6 @@ pub fn install(self: *CompileStep) void {
     self.builder.installArtifact(self);
 }
 
-pub fn installRaw(self: *CompileStep, dest_filename: []const u8, options: InstallRawStep.CreateOptions) *InstallRawStep {
-    return self.builder.installRaw(self, dest_filename, options);
-}
-
 pub fn installHeader(a: *CompileStep, src_path: []const u8, dest_rel_path: []const u8) void {
     const install_file = a.builder.addInstallHeaderFile(src_path, dest_rel_path);
     a.builder.getInstallStep().dependOn(&install_file.step);
@@ -506,6 +502,18 @@ pub fn installLibraryHeaders(a: *CompileStep, l: *CompileStep) void {
     a.installed_headers.appendSlice(l.installed_headers.items) catch @panic("OOM");
 }
 
+pub fn addObjCopy(cs: *CompileStep, options: ObjCopyStep.Options) *ObjCopyStep {
+    var copy = options;
+    if (copy.basename == null) {
+        if (options.format) |f| {
+            copy.basename = cs.builder.fmt("{s}.{s}", .{ cs.name, @tagName(f) });
+        } else {
+            copy.basename = cs.name;
+        }
+    }
+    return cs.builder.addObjCopy(cs.getOutputSource(), copy);
+}
+
 /// Deprecated: use `std.Build.addRunArtifact`
 /// This function will run in the context of the package that created the executable,
 /// which is undesirable when running an executable provided by a dependency package.

lib/std/Build/InstallRawStep.zig

@@ -1,110 +0,0 @@
-//! TODO: Rename this to ObjCopyStep now that it invokes the `zig objcopy`
-//! subcommand rather than containing an implementation directly.
-
-const std = @import("std");
-const InstallRawStep = @This();
-
-const Allocator = std.mem.Allocator;
-const ArenaAllocator = std.heap.ArenaAllocator;
-const ArrayListUnmanaged = std.ArrayListUnmanaged;
-const File = std.fs.File;
-const InstallDir = std.Build.InstallDir;
-const CompileStep = std.Build.CompileStep;
-const Step = std.Build.Step;
-const elf = std.elf;
-const fs = std.fs;
-const io = std.io;
-const sort = std.sort;
-
-pub const base_id = .install_raw;
-
-pub const RawFormat = enum {
-    bin,
-    hex,
-};
-
-step: Step,
-builder: *std.Build,
-artifact: *CompileStep,
-dest_dir: InstallDir,
-dest_filename: []const u8,
-options: CreateOptions,
-output_file: std.Build.GeneratedFile,
-
-pub const CreateOptions = struct {
-    format: ?RawFormat = null,
-    dest_dir: ?InstallDir = null,
-    only_section: ?[]const u8 = null,
-    pad_to: ?u64 = null,
-};
-
-pub fn create(
-    builder: *std.Build,
-    artifact: *CompileStep,
-    dest_filename: []const u8,
-    options: CreateOptions,
-) *InstallRawStep {
-    const self = builder.allocator.create(InstallRawStep) catch @panic("OOM");
-    self.* = InstallRawStep{
-        .step = Step.init(.install_raw, builder.fmt("install raw binary {s}", .{artifact.step.name}), builder.allocator, make),
-        .builder = builder,
-        .artifact = artifact,
-        .dest_dir = if (options.dest_dir) |d| d else switch (artifact.kind) {
-            .obj => unreachable,
-            .@"test" => unreachable,
-            .exe, .test_exe => .bin,
-            .lib => unreachable,
-        },
-        .dest_filename = dest_filename,
-        .options = options,
-        .output_file = std.Build.GeneratedFile{ .step = &self.step },
-    };
-    self.step.dependOn(&artifact.step);
-
-    builder.pushInstalledFile(self.dest_dir, dest_filename);
-    return self;
-}
-
-pub fn getOutputSource(self: *const InstallRawStep) std.Build.FileSource {
-    return std.Build.FileSource{ .generated = &self.output_file };
-}
-
-fn make(step: *Step) !void {
-    const self = @fieldParentPtr(InstallRawStep, "step", step);
-    const b = self.builder;
-
-    if (self.artifact.target.getObjectFormat() != .elf) {
-        std.debug.print("InstallRawStep only works with ELF format.\n", .{});
-        return error.InvalidObjectFormat;
-    }
-
-    const full_src_path = self.artifact.getOutputSource().getPath(b);
-    const full_dest_path = b.getInstallPath(self.dest_dir, self.dest_filename);
-    self.output_file.path = full_dest_path;
-
-    try fs.cwd().makePath(b.getInstallPath(self.dest_dir, ""));
-
-    var argv_list = std.ArrayList([]const u8).init(b.allocator);
-    try argv_list.appendSlice(&.{ b.zig_exe, "objcopy" });
-
-    if (self.options.only_section) |only_section| {
-        try argv_list.appendSlice(&.{ "-j", only_section });
-    }
-    if (self.options.pad_to) |pad_to| {
-        try argv_list.appendSlice(&.{
-            "--pad-to",
-            b.fmt("{d}", .{pad_to}),
-        });
-    }
-    if (self.options.format) |format| switch (format) {
-        .bin => try argv_list.appendSlice(&.{ "-O", "binary" }),
-        .hex => try argv_list.appendSlice(&.{ "-O", "hex" }),
-    };
-
-    try argv_list.appendSlice(&.{ full_src_path, full_dest_path });
-    _ = try self.builder.execFromStep(argv_list.items, &self.step);
-}
-
-test {
-    std.testing.refAllDecls(InstallRawStep);
-}

lib/std/Build/ObjCopyStep.zig

@@ -0,0 +1,138 @@
+const std = @import("std");
+const ObjCopyStep = @This();
+
+const Allocator = std.mem.Allocator;
+const ArenaAllocator = std.heap.ArenaAllocator;
+const ArrayListUnmanaged = std.ArrayListUnmanaged;
+const File = std.fs.File;
+const InstallDir = std.Build.InstallDir;
+const CompileStep = std.Build.CompileStep;
+const Step = std.Build.Step;
+const elf = std.elf;
+const fs = std.fs;
+const io = std.io;
+const sort = std.sort;
+
+pub const base_id: Step.Id = .objcopy;
+
+pub const RawFormat = enum {
+    bin,
+    hex,
+};
+
+step: Step,
+builder: *std.Build,
+file_source: std.Build.FileSource,
+basename: []const u8,
+output_file: std.Build.GeneratedFile,
+
+format: ?RawFormat,
+only_section: ?[]const u8,
+pad_to: ?u64,
+
+pub const Options = struct {
+    basename: ?[]const u8 = null,
+    format: ?RawFormat = null,
+    only_section: ?[]const u8 = null,
+    pad_to: ?u64 = null,
+};
+
+pub fn create(
+    builder: *std.Build,
+    file_source: std.Build.FileSource,
+    options: Options,
+) *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,
+        ),
+        .builder = builder,
+        .file_source = file_source,
+        .basename = options.basename orelse file_source.getDisplayName(),
+        .output_file = std.Build.GeneratedFile{ .step = &self.step },
+
+        .format = options.format,
+        .only_section = options.only_section,
+        .pad_to = options.pad_to,
+    };
+    file_source.addStepDependencies(&self.step);
+    return self;
+}
+
+pub fn getOutputSource(self: *const ObjCopyStep) std.Build.FileSource {
+    return .{ .generated = &self.output_file };
+}
+
+fn make(step: *Step) !void {
+    const self = @fieldParentPtr(ObjCopyStep, "step", step);
+    const b = self.builder;
+
+    var man = b.cache.obtain();
+    defer man.deinit();
+
+    // Random bytes to make ObjCopyStep unique. Refresh this with new random
+    // bytes when ObjCopyStep implementation is modified incompatibly.
+    man.hash.add(@as(u32, 0xe18b7baf));
+
+    const full_src_path = self.file_source.getPath(b);
+    _ = try man.addFile(full_src_path, null);
+    man.hash.addOptionalBytes(self.only_section);
+    man.hash.addOptional(self.pad_to);
+    man.hash.addOptional(self.format);
+
+    if (man.hit() catch |err| failWithCacheError(man, err)) {
+        // Cache hit, skip subprocess execution.
+        const digest = man.final();
+        self.output_file.path = try b.cache_root.join(b.allocator, &.{
+            "o", &digest, self.basename,
+        });
+        return;
+    }
+
+    const digest = man.final();
+    const full_dest_path = try b.cache_root.join(b.allocator, &.{ "o", &digest, self.basename });
+    const cache_path = "o" ++ fs.path.sep_str ++ digest;
+    b.cache_root.handle.makePath(cache_path) catch |err| {
+        std.debug.print("unable to make path {s}: {s}\n", .{ cache_path, @errorName(err) });
+        return err;
+    };
+
+    var argv = std.ArrayList([]const u8).init(b.allocator);
+    try argv.appendSlice(&.{ b.zig_exe, "objcopy" });
+
+    if (self.only_section) |only_section| {
+        try argv.appendSlice(&.{ "-j", only_section });
+    }
+    if (self.pad_to) |pad_to| {
+        try argv.appendSlice(&.{ "--pad-to", b.fmt("{d}", .{pad_to}) });
+    }
+    if (self.format) |format| switch (format) {
+        .bin => try argv.appendSlice(&.{ "-O", "binary" }),
+        .hex => try argv.appendSlice(&.{ "-O", "hex" }),
+    };
+
+    try argv.appendSlice(&.{ full_src_path, full_dest_path });
+    _ = try self.builder.execFromStep(argv.items, &self.step);
+
+    self.output_file.path = full_dest_path;
+    try man.writeManifest();
+}
+
+/// TODO consolidate this with the same function in RunStep?
+/// Also properly deal with concurrency (see open PR)
+fn failWithCacheError(man: std.Build.Cache.Manifest, err: anyerror) noreturn {
+    const i = man.failed_file_index orelse failWithSimpleError(err);
+    const pp = man.files.items[i].prefixed_path orelse failWithSimpleError(err);
+    const prefix = man.cache.prefixes()[pp.prefix].path orelse "";
+    std.debug.print("{s}: {s}/{s}\n", .{ @errorName(err), prefix, pp.sub_path });
+    std.process.exit(1);
+}
+
+fn failWithSimpleError(err: anyerror) noreturn {
+    std.debug.print("{s}\n", .{@errorName(err)});
+    std.process.exit(1);
+}

lib/std/Build/Step.zig

@@ -21,7 +21,7 @@ pub const Id = enum {
     check_file,
     check_object,
     config_header,
-    install_raw,
+    objcopy,
     options,
     custom,
 
@@ -42,7 +42,7 @@ pub const Id = enum {
             .check_file => Build.CheckFileStep,
             .check_object => Build.CheckObjectStep,
             .config_header => Build.ConfigHeaderStep,
-            .install_raw => Build.InstallRawStep,
+            .objcopy => Build.ObjCopyStep,
             .options => Build.OptionsStep,
             .custom => @compileError("no type available for custom step"),
         };

test/standalone/install_raw_hex/build.zig

@@ -3,6 +3,9 @@ const std = @import("std");
 const CheckFileStep = std.Build.CheckFileStep;
 
 pub fn build(b: *std.Build) void {
+    const test_step = b.step("test", "Test the program");
+    b.default_step.dependOn(test_step);
+
     const target = .{
         .cpu_arch = .thumb,
         .cpu_model = .{ .explicit = &std.Target.arm.cpu.cortex_m4 },
@@ -19,12 +22,14 @@ pub fn build(b: *std.Build) void {
         .optimize = optimize,
     });
 
-    const test_step = b.step("test", "Test the program");
-    b.default_step.dependOn(test_step);
-
-    const hex_step = b.addInstallRaw(elf, "hello.hex", .{});
+    const hex_step = elf.addObjCopy(.{
+        .basename = "hello.hex",
+    });
     test_step.dependOn(&hex_step.step);
 
-    const explicit_format_hex_step = b.addInstallRaw(elf, "hello.foo", .{ .format = .hex });
+    const explicit_format_hex_step = elf.addObjCopy(.{
+        .basename = "hello.foo",
+        .format = .hex,
+    });
     test_step.dependOn(&explicit_format_hex_step.step);
 }