From 26196be344e971c26ed044a39b68d0420cd94b90 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 24 Feb 2023 16:02:01 -0700 Subject: [PATCH] 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 | 15 +-- lib/std/Build/CompileStep.zig | 18 ++- lib/std/Build/InstallRawStep.zig | 110 ----------------- lib/std/Build/ObjCopyStep.zig | 138 ++++++++++++++++++++++ lib/std/Build/Step.zig | 4 +- test/standalone/install_raw_hex/build.zig | 15 ++- 6 files changed, 167 insertions(+), 133 deletions(-) delete mode 100644 lib/std/Build/InstallRawStep.zig create mode 100644 lib/std/Build/ObjCopyStep.zig diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 678120847f..26919962e3 100644 --- a/lib/std/Build.zig +++ b/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; diff --git a/lib/std/Build/CompileStep.zig b/lib/std/Build/CompileStep.zig index 6477c20f6b..db663fc767 100644 --- a/lib/std/Build/CompileStep.zig +++ b/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. diff --git a/lib/std/Build/InstallRawStep.zig b/lib/std/Build/InstallRawStep.zig deleted file mode 100644 index 014c44f287..0000000000 --- a/lib/std/Build/InstallRawStep.zig +++ /dev/null @@ -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); -} diff --git a/lib/std/Build/ObjCopyStep.zig b/lib/std/Build/ObjCopyStep.zig new file mode 100644 index 0000000000..aea5b8975c --- /dev/null +++ b/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); +} diff --git a/lib/std/Build/Step.zig b/lib/std/Build/Step.zig index ff0ceb2a51..82c39ac2cc 100644 --- a/lib/std/Build/Step.zig +++ b/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"), }; diff --git a/test/standalone/install_raw_hex/build.zig b/test/standalone/install_raw_hex/build.zig index b0f938a344..6ed515e381 100644 --- a/test/standalone/install_raw_hex/build.zig +++ b/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); }