tweaks to --build-id

 * build.zig: the result of b.option() can be assigned directly in many
   cases thanks to the return type being an optional
 * std.Build: make the build system aware of the
   std.Build.Step.Compile.BuildId type when used as an option.
   - remove extraneous newlines in error logs
 * simplify caching logic
 * simplify hexstring parsing tests and use a doc test
 * simplify hashing logic. don't use an optional when the `none` tag
   already provides this meaning.
 * CLI: fix incorrect linker arg parsing

5 files changed, 63 insertions, 56 deletions

diff --git a/src/Compilation.zig b/src/Compilation.zig
index 5a547346a5..de09a78c77 100644
--- a/src/Compilation.zig
+++ b/src/Compilation.zig
@@ -798,6 +798,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
         const unwind_tables = options.want_unwind_tables orelse
             (link_libunwind or target_util.needUnwindTables(options.target));
         const link_eh_frame_hdr = options.link_eh_frame_hdr or unwind_tables;
+        const build_id = options.build_id orelse .none;
 
         // Make a decision on whether to use LLD or our own linker.
         const use_lld = options.use_lld orelse blk: {
@@ -828,7 +829,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
                 options.output_mode == .Lib or
                 options.linker_script != null or options.version_script != null or
                 options.emit_implib != null or
-                options.build_id != null or
+                build_id != .none or
                 options.symbol_wrap_set.count() > 0)
             {
                 break :blk true;
@@ -1514,7 +1515,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
             .skip_linker_dependencies = options.skip_linker_dependencies,
             .parent_compilation_link_libc = options.parent_compilation_link_libc,
             .each_lib_rpath = options.each_lib_rpath orelse options.is_native_os,
-            .build_id = options.build_id,
+            .build_id = build_id,
             .cache_mode = cache_mode,
             .disable_lld_caching = options.disable_lld_caching or cache_mode == .whole,
             .subsystem = options.subsystem,
@@ -2269,9 +2270,7 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
     man.hash.addListOfBytes(comp.bin_file.options.rpath_list);
     man.hash.addListOfBytes(comp.bin_file.options.symbol_wrap_set.keys());
     man.hash.add(comp.bin_file.options.each_lib_rpath);
-    if (comp.bin_file.options.build_id) |build_id| {
-        build_id.hash(&man.hash.hasher);
-    }
+    man.hash.add(comp.bin_file.options.build_id);
     man.hash.add(comp.bin_file.options.skip_linker_dependencies);
     man.hash.add(comp.bin_file.options.z_nodelete);
     man.hash.add(comp.bin_file.options.z_notext);
diff --git a/src/link.zig b/src/link.zig
index eccf389d05..79ac33b892 100644
--- a/src/link.zig
+++ b/src/link.zig
@@ -158,7 +158,7 @@ pub const Options = struct {
     skip_linker_dependencies: bool,
     parent_compilation_link_libc: bool,
     each_lib_rpath: bool,
-    build_id: ?BuildId,
+    build_id: BuildId,
     disable_lld_caching: bool,
     is_test: bool,
     hash_style: HashStyle,
diff --git a/src/link/Elf.zig b/src/link/Elf.zig
index 289c687270..f90f4ebd46 100644
--- a/src/link/Elf.zig
+++ b/src/link/Elf.zig
@@ -1399,8 +1399,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
         man.hash.add(self.base.options.each_lib_rpath);
         if (self.base.options.output_mode == .Exe) {
             man.hash.add(stack_size);
-            if (self.base.options.build_id) |build_id|
-                build_id.hash(&man.hash.hasher);
+            man.hash.add(self.base.options.build_id);
         }
         man.hash.addListOfBytes(self.base.options.symbol_wrap_set.keys());
         man.hash.add(self.base.options.skip_linker_dependencies);
@@ -1543,12 +1542,18 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
             try argv.append("-z");
             try argv.append(try std.fmt.allocPrint(arena, "stack-size={d}", .{stack_size}));
 
-            if (self.base.options.build_id) |build_id| {
-                const fmt_str = "--build-id={s}{s}";
-                try argv.append(switch (build_id) {
-                    .hexstring => |str| try std.fmt.allocPrint(arena, fmt_str, .{ "0x", str }),
-                    .none, .fast, .uuid, .sha1, .md5 => try std.fmt.allocPrint(arena, fmt_str, .{ "", @tagName(build_id) }),
-                });
+            switch (self.base.options.build_id) {
+                .none => {},
+                .fast, .uuid, .sha1, .md5 => {
+                    try argv.append(try std.fmt.allocPrint(arena, "--build-id={s}", .{
+                        @tagName(self.base.options.build_id),
+                    }));
+                },
+                .hexstring => |hs| {
+                    try argv.append(try std.fmt.allocPrint(arena, "--build-id=0x{s}", .{
+                        std.fmt.fmtSliceHexLower(hs.toSlice()),
+                    }));
+                },
             }
         }
 
diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig
index 9396377c73..cd9c44d656 100644
--- a/src/link/Wasm.zig
+++ b/src/link/Wasm.zig
@@ -3163,8 +3163,7 @@ fn linkWithZld(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) l
         try man.addOptionalFile(compiler_rt_path);
         man.hash.addOptionalBytes(options.entry);
         man.hash.addOptional(options.stack_size_override);
-        if (wasm.base.options.build_id) |build_id|
-            build_id.hash(&man.hash.hasher);
+        man.hash.add(wasm.base.options.build_id);
         man.hash.add(options.import_memory);
         man.hash.add(options.import_table);
         man.hash.add(options.export_table);
@@ -3798,27 +3797,29 @@ fn writeToFile(
     if (!wasm.base.options.strip) {
         // The build id must be computed on the main sections only,
         // so we have to do it now, before the debug sections.
-        if (wasm.base.options.build_id) |build_id| {
-            switch (build_id) {
-                .none => {},
-                .fast => {
-                    var id: [16]u8 = undefined;
-                    std.crypto.hash.sha3.TurboShake128(null).hash(binary_bytes.items, &id, .{});
-                    var uuid: [36]u8 = undefined;
-                    _ = try std.fmt.bufPrint(&uuid, "{s}-{s}-{s}-{s}-{s}", .{
-                        std.fmt.fmtSliceHexLower(id[0..4]),
-                        std.fmt.fmtSliceHexLower(id[4..6]),
-                        std.fmt.fmtSliceHexLower(id[6..8]),
-                        std.fmt.fmtSliceHexLower(id[8..10]),
-                        std.fmt.fmtSliceHexLower(id[10..]),
-                    });
-                    try emitBuildIdSection(&binary_bytes, &uuid);
-                },
-                .hexstring => |str| {
-                    try emitBuildIdSection(&binary_bytes, str);
-                },
-                else => |mode| log.err("build-id '{s}' is not supported for WASM", .{@tagName(mode)}),
-            }
+        switch (wasm.base.options.build_id) {
+            .none => {},
+            .fast => {
+                var id: [16]u8 = undefined;
+                std.crypto.hash.sha3.TurboShake128(null).hash(binary_bytes.items, &id, .{});
+                var uuid: [36]u8 = undefined;
+                _ = try std.fmt.bufPrint(&uuid, "{s}-{s}-{s}-{s}-{s}", .{
+                    std.fmt.fmtSliceHexLower(id[0..4]),
+                    std.fmt.fmtSliceHexLower(id[4..6]),
+                    std.fmt.fmtSliceHexLower(id[6..8]),
+                    std.fmt.fmtSliceHexLower(id[8..10]),
+                    std.fmt.fmtSliceHexLower(id[10..]),
+                });
+                try emitBuildIdSection(&binary_bytes, &uuid);
+            },
+            .hexstring => |hs| {
+                var buffer: [32 * 2]u8 = undefined;
+                const str = std.fmt.bufPrint(&buffer, "{s}", .{
+                    std.fmt.fmtSliceHexLower(hs.toSlice()),
+                }) catch unreachable;
+                try emitBuildIdSection(&binary_bytes, str);
+            },
+            else => |mode| log.err("build-id '{s}' is not supported for WASM", .{@tagName(mode)}),
         }
 
         // if (wasm.dwarf) |*dwarf| {
@@ -4211,8 +4212,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
         try man.addOptionalFile(compiler_rt_path);
         man.hash.addOptionalBytes(wasm.base.options.entry);
         man.hash.addOptional(wasm.base.options.stack_size_override);
-        if (wasm.base.options.build_id) |build_id|
-            build_id.hash(&man.hash.hasher);
+        man.hash.add(wasm.base.options.build_id);
         man.hash.add(wasm.base.options.import_memory);
         man.hash.add(wasm.base.options.import_table);
         man.hash.add(wasm.base.options.export_table);
diff --git a/src/main.zig b/src/main.zig
index 93199f9566..aa7d587983 100644
--- a/src/main.zig
+++ b/src/main.zig
@@ -494,7 +494,10 @@ const usage_build_generic =
     \\  -fno-each-lib-rpath            Prevent adding rpath for each used dynamic library
     \\  -fallow-shlib-undefined        Allows undefined symbols in shared libraries
     \\  -fno-allow-shlib-undefined     Disallows undefined symbols in shared libraries
-    \\  --build-id[=style]             Generate a build ID note
+    \\  --build-id[=style]             At a minor link-time expense, coordinates stripped binaries
+    \\      fast, uuid, sha1, md5      with debug symbols via a '.note.gnu.build-id' section
+    \\      0x[hexstring]              Maximum 32 bytes
+    \\      none                       (default) Disable build-id
     \\  --eh-frame-hdr                 Enable C++ exception handling by passing --eh-frame-hdr to linker
     \\  --emit-relocs                  Enable output of relocation sections for post build tools
     \\  -z [arg]                       Set linker extension flags
@@ -1445,11 +1448,11 @@ fn buildOutputType(
                     } else if (mem.eql(u8, arg, "--build-id")) {
                         build_id = .fast;
                     } else if (mem.startsWith(u8, arg, "--build-id=")) {
-                        const value = arg["--build-id=".len..];
-                        build_id = BuildId.parse(arena, value) catch |err| switch (err) {
-                            error.InvalidHexInt => fatal("failed to parse hex value {s}", .{value}),
-                            error.InvalidBuildId => fatal("invalid --build-id={s}", .{value}),
-                            error.OutOfMemory => fatal("OOM", .{}),
+                        const style = arg["--build-id=".len..];
+                        build_id = BuildId.parse(style) catch |err| {
+                            fatal("unable to parse --build-id style '{s}': {s}", .{
+                                style, @errorName(err),
+                            });
                         };
                     } else if (mem.eql(u8, arg, "--debug-compile-errors")) {
                         if (!crash_report.is_enabled) {
@@ -1689,7 +1692,14 @@ fn buildOutputType(
                                 if (mem.indexOfScalar(u8, linker_arg, '=')) |equals_pos| {
                                     const key = linker_arg[0..equals_pos];
                                     const value = linker_arg[equals_pos + 1 ..];
-                                    if (mem.eql(u8, key, "--sort-common")) {
+                                    if (mem.eql(u8, key, "--build-id")) {
+                                        build_id = BuildId.parse(value) catch |err| {
+                                            fatal("unable to parse --build-id style '{s}': {s}", .{
+                                                value, @errorName(err),
+                                            });
+                                        };
+                                        continue;
+                                    } else if (mem.eql(u8, key, "--sort-common")) {
                                         // this ignores --sort=common=<anything>; ignoring plain --sort-common
                                         // is done below.
                                         continue;
@@ -1699,7 +1709,9 @@ fn buildOutputType(
                                     continue;
                                 }
                             }
-                            if (mem.eql(u8, linker_arg, "--as-needed")) {
+                            if (mem.eql(u8, linker_arg, "--build-id")) {
+                                build_id = .fast;
+                            } else if (mem.eql(u8, linker_arg, "--as-needed")) {
                                 needed = false;
                             } else if (mem.eql(u8, linker_arg, "--no-as-needed")) {
                                 needed = true;
@@ -1731,15 +1743,6 @@ fn buildOutputType(
                                 search_strategy = .paths_first;
                             } else if (mem.eql(u8, linker_arg, "-search_dylibs_first")) {
                                 search_strategy = .dylibs_first;
-                            } else if (mem.eql(u8, linker_arg, "--build-id")) {
-                                build_id = .fast;
-                            } else if (mem.startsWith(u8, linker_arg, "--build-id=")) {
-                                const value = linker_arg["--build-id=".len..];
-                                build_id = BuildId.parse(arena, value) catch |err| switch (err) {
-                                    error.InvalidHexInt => fatal("failed to parse hex value {s}", .{value}),
-                                    error.InvalidBuildId => fatal("invalid --build-id={s}", .{value}),
-                                    error.OutOfMemory => fatal("OOM", .{}),
-                                };
                             } else {
                                 try linker_args.append(linker_arg);
                             }