kubkon review changes: 1

general:
- rename `DarwinSdkLayout` → `DarwinSdkLayout`
- drop `DarwinSdkLayout.installation` (not needed for darwin)
- document struct

inferSdkVersion:
- use explicit allocator
- avoid trying to infer SDK ver from vendored path

5 files changed, 43 insertions, 33 deletions

diff --git a/src/Compilation.zig b/src/Compilation.zig
index e9f6be89fd..65e433627f 100644
--- a/src/Compilation.zig
+++ b/src/Compilation.zig
@@ -1554,10 +1554,10 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
             .use_lld = use_lld,
             .use_llvm = use_llvm,
             .use_lib_llvm = use_lib_llvm,
-            .libc_provider = libc_dirs.provider,
             .link_libc = link_libc,
             .link_libcpp = link_libcpp,
             .link_libunwind = link_libunwind,
+            .darwinSdkLayout = libc_dirs.darwinSdkLayout,
             .objects = options.link_objects,
             .frameworks = options.frameworks,
             .framework_dirs = options.framework_dirs,
@@ -5287,6 +5287,7 @@ fn detectWin32ResourceIncludeDirs(arena: Allocator, options: InitOptions) !LibCD
                 .libc_installation = null,
                 .libc_framework_dir_list = &.{},
                 .sysroot = null,
+                .darwinSdkLayout = .none,
             },
         }
     }
@@ -5655,7 +5656,7 @@ const LibCDirs = struct {
     libc_installation: ?*const LibCInstallation,
     libc_framework_dir_list: []const []const u8,
     sysroot: ?[]const u8,
-    provider: link.LibCProvider,
+    darwinSdkLayout: link.DarwinSdkLayout,
 };
 
 fn getZigShippedLibCIncludeDirsDarwin(arena: Allocator, zig_lib_dir: []const u8) !LibCDirs {
@@ -5671,7 +5672,7 @@ fn getZigShippedLibCIncludeDirsDarwin(arena: Allocator, zig_lib_dir: []const u8)
         .libc_installation = null,
         .libc_framework_dir_list = &.{},
         .sysroot = null,
-        .provider = .vendored,
+        .darwinSdkLayout = .vendored,
     };
 }
 
@@ -5689,7 +5690,7 @@ pub fn detectLibCIncludeDirs(
             .libc_installation = null,
             .libc_framework_dir_list = &.{},
             .sysroot = null,
-            .provider = .none,
+            .darwinSdkLayout = .none,
         };
     }
 
@@ -5747,7 +5748,7 @@ pub fn detectLibCIncludeDirs(
         .libc_installation = null,
         .libc_framework_dir_list = &.{},
         .sysroot = null,
-        .provider = .none,
+        .darwinSdkLayout = .none,
     };
 }
 
@@ -5802,7 +5803,7 @@ fn detectLibCFromLibCInstallation(arena: Allocator, target: Target, lci: *const
         .libc_installation = lci,
         .libc_framework_dir_list = framework_list.items,
         .sysroot = sysroot,
-        .provider = if (sysroot == null) .installation else .sysroot,
+        .darwinSdkLayout = if (sysroot == null) .none else .sdk,
     };
 }
 
@@ -5864,7 +5865,7 @@ fn detectLibCFromBuilding(
         .libc_installation = null,
         .libc_framework_dir_list = &.{},
         .sysroot = null,
-        .provider = .vendored,
+        .darwinSdkLayout = .vendored,
     };
 }
 
diff --git a/src/link.zig b/src/link.zig
index 94b6517f0c..efd02e982e 100644
--- a/src/link.zig
+++ b/src/link.zig
@@ -134,10 +134,10 @@ pub const Options = struct {
     /// Otherwise (depending on `use_lld`) this link code directly outputs and updates the final binary.
     use_llvm: bool,
     use_lib_llvm: bool,
-    libc_provider: LibCProvider,
     link_libc: bool,
     link_libcpp: bool,
     link_libunwind: bool,
+    darwinSdkLayout: DarwinSdkLayout,
     function_sections: bool,
     no_builtin: bool,
     eh_frame_hdr: bool,
@@ -283,10 +283,13 @@ pub const HashStyle = enum { sysv, gnu, both };
 
 pub const CompressDebugSections = enum { none, zlib };
 
-pub const LibCProvider = enum {
+/// The filesystem layout of darwin SDK elements.
+pub const DarwinSdkLayout = enum {
+    /// Does not apply to the target.
     none,
-    installation,
-    sysroot,
+    /// macOS SDK layout: TOP { /usr/include, /usr/lib, /System/Library/Frameworks }.
+    sdk,
+    /// Shipped libc layout: TOP { /lib/libc/include,  /lib/libc/darwin, <NONE> }.
     vendored,
 };
 
diff --git a/src/link/MachO.zig b/src/link/MachO.zig
index 893c181f48..0110ff81ce 100644
--- a/src/link/MachO.zig
+++ b/src/link/MachO.zig
@@ -558,7 +558,7 @@ pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.No
     });
     {
         const platform = Platform.fromTarget(self.base.options.target);
-        const sdk_version: ?std.SemanticVersion = load_commands.inferSdkVersion(comp);
+        const sdk_version: ?std.SemanticVersion = load_commands.inferSdkVersion(arena, comp);
         if (platform.isBuildVersionCompatible()) {
             try load_commands.writeBuildVersionLC(platform, sdk_version, lc_writer);
         } else if (platform.isVersionMinCompatible()) {
@@ -652,10 +652,9 @@ pub fn resolveLibSystem(
             "libSystem",
         )) break :success;
 
-        switch (self.base.options.libc_provider) {
+        switch (self.base.options.darwinSdkLayout) {
             .none => unreachable,
-            .installation => unreachable,
-            .sysroot => {
+            .sdk => {
                 const dir = try fs.path.join(tmp_arena, &[_][]const u8{ self.base.options.sysroot.?, "usr", "lib" });
                 if (try accessLibPath(tmp_arena, &test_path, &checked_paths, dir, "libSystem")) break :success;
             },
diff --git a/src/link/MachO/load_commands.zig b/src/link/MachO/load_commands.zig
index 47cd0a54e3..a00aec46fa 100644
--- a/src/link/MachO/load_commands.zig
+++ b/src/link/MachO/load_commands.zig
@@ -467,31 +467,27 @@ pub inline fn appleVersionToSemanticVersion(version: u32) std.SemanticVersion {
     };
 }
 
-fn readSdkVersionString(arena: Allocator, dir: []const u8) ![]const u8 {
-    const sdk_path = try std.fs.path.join(arena, &.{ dir, "SDKSettings.json" });
-    const contents = try std.fs.cwd().readFileAlloc(arena, sdk_path, std.math.maxInt(u16));
-    const parsed = try std.json.parseFromSlice(std.json.Value, arena, contents, .{});
-    if (parsed.value.object.get("MinimalDisplayName")) |ver| return ver.string;
-    return error.SdkVersionFailure;
-}
-
-pub fn inferSdkVersion(comp: *const Compilation) ?std.SemanticVersion {
-    var arena_allocator = std.heap.ArenaAllocator.init(comp.gpa);
+pub fn inferSdkVersion(gpa: Allocator, comp: *const Compilation) ?std.SemanticVersion {
+    var arena_allocator = std.heap.ArenaAllocator.init(gpa);
     defer arena_allocator.deinit();
     const arena = arena_allocator.allocator();
+
     const options = comp.bin_file.options;
 
-    const sdk_dir = switch (options.libc_provider) {
+    const sdk_dir = switch (options.darwinSdkLayout) {
         .none => unreachable,
-        .installation => unreachable,
-        .sysroot => options.sysroot.?,
+        .sdk => options.sysroot.?,
         .vendored => std.fs.path.join(arena, &.{ comp.zig_lib_directory.path.?, "libc", "darwin" }) catch return null,
     };
 
-    // prefer meta information if available
-    if (readSdkVersionString(arena, sdk_dir)) |ver| {
+    if (readSdkVersionFromSettings(arena, sdk_dir)) |ver| {
         return parseSdkVersion(ver);
-    } else |_| {}
+    } else |_| {
+        if (options.darwinSdkLayout == .vendored) {
+            // vendored layout does not have versioned pathname
+            return null;
+        }
+    }
 
     // infer from pathname
     const stem = std.fs.path.stem(sdk_dir);
@@ -505,6 +501,17 @@ pub fn inferSdkVersion(comp: *const Compilation) ?std.SemanticVersion {
     return parseSdkVersion(stem[start..end]);
 }
 
+// Official Apple SDKs ship with a `SDKSettings.json` located at the top of SDK fs layout.
+// Use property `MinimalDisplayName` to determine version.
+// The file/property is also available with vendored libc.
+fn readSdkVersionFromSettings(arena: Allocator, dir: []const u8) ![]const u8 {
+    const sdk_path = try std.fs.path.join(arena, &.{ dir, "SDKSettings.json" });
+    const contents = try std.fs.cwd().readFileAlloc(arena, sdk_path, std.math.maxInt(u16));
+    const parsed = try std.json.parseFromSlice(std.json.Value, arena, contents, .{});
+    if (parsed.value.object.get("MinimalDisplayName")) |ver| return ver.string;
+    return error.SdkVersionFailure;
+}
+
 // Versions reported by Apple aren't exactly semantically valid as they usually omit
 // the patch component, so we parse SDK value by hand.
 fn parseSdkVersion(raw: []const u8) ?std.SemanticVersion {
diff --git a/src/link/MachO/zld.zig b/src/link/MachO/zld.zig
index ad9c59cb97..7768deecdf 100644
--- a/src/link/MachO/zld.zig
+++ b/src/link/MachO/zld.zig
@@ -241,7 +241,7 @@ pub fn linkWithZld(
                 try argv.append(@tagName(platform.os_tag));
                 try argv.append(try std.fmt.allocPrint(arena, "{}", .{platform.version}));
 
-                const sdk_version: ?std.SemanticVersion = load_commands.inferSdkVersion(comp);
+                const sdk_version: ?std.SemanticVersion = load_commands.inferSdkVersion(arena, comp);
                 if (sdk_version) |ver| {
                     try argv.append(try std.fmt.allocPrint(arena, "{d}.{d}", .{ ver.major, ver.minor }));
                 } else {
@@ -588,7 +588,7 @@ pub fn linkWithZld(
         });
         {
             const platform = Platform.fromTarget(macho_file.base.options.target);
-            const sdk_version: ?std.SemanticVersion = load_commands.inferSdkVersion(comp);
+            const sdk_version: ?std.SemanticVersion = load_commands.inferSdkVersion(arena, comp);
             if (platform.isBuildVersionCompatible()) {
                 try load_commands.writeBuildVersionLC(platform, sdk_version, lc_writer);
             } else {