From 09a84c8384dffc7884528947b879f32d93c1bd90 Mon Sep 17 00:00:00 2001 From: mlugg Date: Fri, 17 Feb 2023 06:20:52 +0000 Subject: Update std.Build to new module CLI, update zig1 and CMakeLists Resolves: #14667 --- lib/std/Build/CompileStep.zig | 127 +++++++++++++++++++++++++++++++++++------- 1 file changed, 107 insertions(+), 20 deletions(-) (limited to 'lib/std/Build/CompileStep.zig') diff --git a/lib/std/Build/CompileStep.zig b/lib/std/Build/CompileStep.zig index a916de0fc6..6477c20f6b 100644 --- a/lib/std/Build/CompileStep.zig +++ b/lib/std/Build/CompileStep.zig @@ -955,7 +955,10 @@ pub fn addFrameworkPath(self: *CompileStep, dir_path: []const u8) void { /// package's module table using `name`. pub fn addModule(cs: *CompileStep, name: []const u8, module: *Module) void { cs.modules.put(cs.builder.dupe(name), module) catch @panic("OOM"); - cs.addRecursiveBuildDeps(module); + + var done = std.AutoHashMap(*Module, void).init(cs.builder.allocator); + defer done.deinit(); + cs.addRecursiveBuildDeps(module, &done) catch @panic("OOM"); } /// Adds a module to be used with `@import` without exposing it in the current @@ -969,10 +972,12 @@ pub fn addOptions(cs: *CompileStep, module_name: []const u8, options: *OptionsSt addModule(cs, module_name, options.createModule()); } -fn addRecursiveBuildDeps(cs: *CompileStep, module: *Module) void { +fn addRecursiveBuildDeps(cs: *CompileStep, module: *Module, done: *std.AutoHashMap(*Module, void)) !void { + if (done.contains(module)) return; + try done.put(module, {}); module.source_file.addStepDependencies(&cs.step); for (module.dependencies.values()) |dep| { - cs.addRecursiveBuildDeps(dep); + try cs.addRecursiveBuildDeps(dep, done); } } @@ -1031,22 +1036,110 @@ fn linkLibraryOrObject(self: *CompileStep, other: *CompileStep) void { fn appendModuleArgs( cs: *CompileStep, zig_args: *ArrayList([]const u8), - name: []const u8, - module: *Module, ) error{OutOfMemory}!void { - try zig_args.append("--pkg-begin"); - try zig_args.append(name); - try zig_args.append(module.builder.pathFromRoot(module.source_file.getPath(module.builder))); + // First, traverse the whole dependency graph and give every module a unique name, ideally one + // named after what it's called somewhere in the graph. It will help here to have both a mapping + // from module to name and a set of all the currently-used names. + var mod_names = std.AutoHashMap(*Module, []const u8).init(cs.builder.allocator); + var names = std.StringHashMap(void).init(cs.builder.allocator); + + var to_name = std.ArrayList(struct { + name: []const u8, + mod: *Module, + }).init(cs.builder.allocator); + { + var it = cs.modules.iterator(); + while (it.next()) |kv| { + // While we're traversing the root dependencies, let's make sure that no module names + // have colons in them, since the CLI forbids it. We handle this for transitive + // dependencies further down. + if (std.mem.indexOfScalar(u8, kv.key_ptr.*, ':') != null) { + @panic("Module names cannot contain colons"); + } + try to_name.append(.{ + .name = kv.key_ptr.*, + .mod = kv.value_ptr.*, + }); + } + } + + while (to_name.popOrNull()) |dep| { + if (mod_names.contains(dep.mod)) continue; + + // We'll use this buffer to store the name we decide on + var buf = try cs.builder.allocator.alloc(u8, dep.name.len + 32); + // First, try just the exposed dependency name + std.mem.copy(u8, buf, dep.name); + var name = buf[0..dep.name.len]; + var n: usize = 0; + while (names.contains(name)) { + // If that failed, append an incrementing number to the end + name = std.fmt.bufPrint(buf, "{s}{}", .{ dep.name, n }) catch unreachable; + n += 1; + } + + try mod_names.put(dep.mod, name); + try names.put(name, {}); + + var it = dep.mod.dependencies.iterator(); + while (it.next()) |kv| { + // Same colon-in-name check as above, but for transitive dependencies. + if (std.mem.indexOfScalar(u8, kv.key_ptr.*, ':') != null) { + @panic("Module names cannot contain colons"); + } + try to_name.append(.{ + .name = kv.key_ptr.*, + .mod = kv.value_ptr.*, + }); + } + } + // Since the module names given to the CLI are based off of the exposed names, we already know + // that none of the CLI names have colons in them, so there's no need to check that explicitly. + + // Every module in the graph is now named; output their definitions { - const keys = module.dependencies.keys(); - for (module.dependencies.values(), 0..) |sub_module, i| { - const sub_name = keys[i]; - try cs.appendModuleArgs(zig_args, sub_name, sub_module); + var it = mod_names.iterator(); + while (it.next()) |kv| { + const mod = kv.key_ptr.*; + const name = kv.value_ptr.*; + + const deps_str = try constructDepString(cs.builder.allocator, mod_names, mod.dependencies); + const src = mod.builder.pathFromRoot(mod.source_file.getPath(mod.builder)); + try zig_args.append("--mod"); + try zig_args.append(try std.fmt.allocPrint(cs.builder.allocator, "{s}:{s}:{s}", .{ name, deps_str, src })); } } - try zig_args.append("--pkg-end"); + // Lastly, output the root dependencies + const deps_str = try constructDepString(cs.builder.allocator, mod_names, cs.modules); + if (deps_str.len > 0) { + try zig_args.append("--deps"); + try zig_args.append(deps_str); + } +} + +fn constructDepString( + allocator: std.mem.Allocator, + mod_names: std.AutoHashMap(*Module, []const u8), + deps: std.StringArrayHashMap(*Module), +) ![]const u8 { + var deps_str = std.ArrayList(u8).init(allocator); + var it = deps.iterator(); + while (it.next()) |kv| { + const expose = kv.key_ptr.*; + const name = mod_names.get(kv.value_ptr.*).?; + if (std.mem.eql(u8, expose, name)) { + try deps_str.writer().print("{s},", .{name}); + } else { + try deps_str.writer().print("{s}={s},", .{ expose, name }); + } + } + if (deps_str.items.len > 0) { + return deps_str.items[0 .. deps_str.items.len - 1]; // omit trailing comma + } else { + return ""; + } } fn make(step: *Step) !void { @@ -1573,13 +1666,7 @@ fn make(step: *Step) !void { try zig_args.append("--test-no-exec"); } - { - const keys = self.modules.keys(); - for (self.modules.values(), 0..) |module, i| { - const name = keys[i]; - try self.appendModuleArgs(&zig_args, name, module); - } - } + try self.appendModuleArgs(&zig_args); for (self.include_dirs.items) |include_dir| { switch (include_dir) { -- cgit v1.2.3 From 26196be344e971c26ed044a39b68d0420cd94b90 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 24 Feb 2023 16:02:01 -0700 Subject: 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 (limited to 'lib/std/Build/CompileStep.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); } -- cgit v1.2.3 From f6c934677315665c140151b8dd28a56f948205e2 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 25 Feb 2023 15:21:18 -0700 Subject: std.Build.CompileStep.installConfigHeader: add missing step dependency --- lib/std/Build/CompileStep.zig | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/std/Build/CompileStep.zig') diff --git a/lib/std/Build/CompileStep.zig b/lib/std/Build/CompileStep.zig index db663fc767..ea2320cc89 100644 --- a/lib/std/Build/CompileStep.zig +++ b/lib/std/Build/CompileStep.zig @@ -454,6 +454,7 @@ pub fn installConfigHeader( options.install_dir, dest_rel_path, ); + install_file.step.dependOn(&config_header.step); cs.builder.getInstallStep().dependOn(&install_file.step); cs.installed_headers.append(&install_file.step) catch @panic("OOM"); } -- cgit v1.2.3