darwin: move inference of SDK version into the linker

kubkon · kubkon · commit 3b2b9fcbc5e1 · 2023-08-29T06:43:41.000+02:00
`std.zig.system.darwin.getSdk` now pulls only the SDK path
so we execute a child process only once and not twice as it was
until now since we parse the SDK version directly from the pulled path.
This is actually how `ld64` does it too.
diff --git a/lib/std/zig/system/NativePaths.zig b/lib/std/zig/system/NativePaths.zig
@@ -81,9 +81,9 @@ pub fn detect(arena: Allocator, native_info: NativeTargetInfo) !NativePaths {
     if (comptime builtin.target.isDarwin()) {
         if (std.zig.system.darwin.isSdkInstalled(arena)) sdk: {
             const sdk = std.zig.system.darwin.getSdk(arena, native_target) orelse break :sdk;
-            try self.addLibDir(try std.fs.path.join(arena, &.{ sdk.path, "usr/lib" }));
-            try self.addFrameworkDir(try std.fs.path.join(arena, &.{ sdk.path, "System/Library/Frameworks" }));
-            try self.addIncludeDir(try std.fs.path.join(arena, &.{ sdk.path, "usr/include" }));
+            try self.addLibDir(try std.fs.path.join(arena, &.{ sdk, "usr/lib" }));
+            try self.addFrameworkDir(try std.fs.path.join(arena, &.{ sdk, "System/Library/Frameworks" }));
+            try self.addIncludeDir(try std.fs.path.join(arena, &.{ sdk, "usr/include" }));
             return self;
         }
         return self;
diff --git a/lib/std/zig/system/darwin.zig b/lib/std/zig/system/darwin.zig
@@ -30,12 +30,11 @@ pub fn isSdkInstalled(allocator: Allocator) bool {
 }
 
 /// Detect SDK on Darwin.
-/// Calls `xcrun --sdk <target_sdk> --show-sdk-path` which fetches the path to the SDK sysroot (if any).
-/// Subsequently calls `xcrun --sdk <target_sdk> --show-sdk-version` which fetches version of the SDK.
-/// The caller needs to deinit the resulting struct.
+/// Calls `xcrun --sdk <target_sdk> --show-sdk-path` which fetches the path to the SDK.
+/// Caller owns the memory.
 /// stderr from xcrun is ignored.
 /// If error.OutOfMemory occurs in Allocator, this function returns null.
-pub fn getSdk(allocator: Allocator, target: Target) ?Sdk {
+pub fn getSdk(allocator: Allocator, target: Target) ?[]const u8 {
     const is_simulator_abi = target.abi == .simulator;
     const sdk = switch (target.os.tag) {
         .macos => "macosx",
@@ -44,91 +43,19 @@ pub fn getSdk(allocator: Allocator, target: Target) ?Sdk {
         .tvos => if (is_simulator_abi) "appletvsimulator" else "appletvos",
         else => return null,
     };
-    const path = path: {
-        const argv = &[_][]const u8{ "/usr/bin/xcrun", "--sdk", sdk, "--show-sdk-path" };
-        const result = std.process.Child.exec(.{ .allocator = allocator, .argv = argv }) catch return null;
-        defer {
-            allocator.free(result.stderr);
-            allocator.free(result.stdout);
-        }
-        switch (result.term) {
-            .Exited => |code| if (code != 0) return null,
-            else => return null,
-        }
-        const path = allocator.dupe(u8, mem.trimRight(u8, result.stdout, "\r\n")) catch return null;
-        break :path path;
-    };
-    const version = version: {
-        const argv = &[_][]const u8{ "/usr/bin/xcrun", "--sdk", sdk, "--show-sdk-version" };
-        const result = std.process.Child.exec(.{ .allocator = allocator, .argv = argv }) catch return null;
-        defer {
-            allocator.free(result.stderr);
-            allocator.free(result.stdout);
-        }
-        switch (result.term) {
-            .Exited => |code| if (code != 0) return null,
-            else => return null,
-        }
-        const raw_version = mem.trimRight(u8, result.stdout, "\r\n");
-        const version = parseSdkVersion(raw_version) orelse Version{
-            .major = 0,
-            .minor = 0,
-            .patch = 0,
-        };
-        break :version version;
-    };
-    return Sdk{
-        .path = path,
-        .version = version,
-    };
-}
-
-// Versions reported by Apple aren't exactly semantically valid as they usually omit
-// the patch component. Hence, we do a simple check for the number of components and
-// add the missing patch value if needed.
-fn parseSdkVersion(raw: []const u8) ?Version {
-    var buffer: [128]u8 = undefined;
-    if (raw.len > buffer.len) return null;
-    @memcpy(buffer[0..raw.len], raw);
-    const dots_count = mem.count(u8, raw, ".");
-    if (dots_count < 1) return null;
-    const len = if (dots_count < 2) blk: {
-        const patch_suffix = ".0";
-        buffer[raw.len..][0..patch_suffix.len].* = patch_suffix.*;
-        break :blk raw.len + patch_suffix.len;
-    } else raw.len;
-    return Version.parse(buffer[0..len]) catch null;
-}
-
-pub const Sdk = struct {
-    path: []const u8,
-    version: Version,
-
-    pub fn deinit(self: Sdk, allocator: Allocator) void {
-        allocator.free(self.path);
+    const argv = &[_][]const u8{ "/usr/bin/xcrun", "--sdk", sdk, "--show-sdk-path" };
+    const result = std.process.Child.exec(.{ .allocator = allocator, .argv = argv }) catch return null;
+    defer {
+        allocator.free(result.stderr);
+        allocator.free(result.stdout);
+    }
+    switch (result.term) {
+        .Exited => |code| if (code != 0) return null,
+        else => return null,
     }
-};
+    return allocator.dupe(u8, mem.trimRight(u8, result.stdout, "\r\n")) catch null;
+}
 
 test {
     _ = macos;
 }
-
-const expect = std.testing.expect;
-const expectEqual = std.testing.expectEqual;
-
-fn testParseSdkVersionSuccess(exp: Version, raw: []const u8) !void {
-    const maybe_ver = parseSdkVersion(raw);
-    try expect(maybe_ver != null);
-    const ver = maybe_ver.?;
-    try expectEqual(exp.major, ver.major);
-    try expectEqual(exp.minor, ver.minor);
-    try expectEqual(exp.patch, ver.patch);
-}
-
-test "parseSdkVersion" {
-    try testParseSdkVersionSuccess(.{ .major = 13, .minor = 4, .patch = 0 }, "13.4");
-    try testParseSdkVersionSuccess(.{ .major = 13, .minor = 4, .patch = 1 }, "13.4.1");
-    try testParseSdkVersionSuccess(.{ .major = 11, .minor = 15, .patch = 0 }, "11.15");
-
-    try expect(parseSdkVersion("11") == null);
-}
diff --git a/src/libc_installation.zig b/src/libc_installation.zig
@@ -187,13 +187,13 @@ pub const LibCInstallation = struct {
                 return error.DarwinSdkNotFound;
             const sdk = std.zig.system.darwin.getSdk(args.allocator, args.target) orelse
                 return error.DarwinSdkNotFound;
-            defer args.allocator.free(sdk.path);
+            defer args.allocator.free(sdk);
 
             self.include_dir = try fs.path.join(args.allocator, &.{
-                sdk.path, "usr/include",
+                sdk, "usr/include",
             });
             self.sys_include_dir = try fs.path.join(args.allocator, &.{
-                sdk.path, "usr/include",
+                sdk, "usr/include",
             });
             return self;
         } else if (is_windows) {
diff --git a/src/link/MachO/load_commands.zig b/src/link/MachO/load_commands.zig
@@ -278,7 +278,11 @@ pub fn writeBuildVersionLC(options: *const link.Options, lc_writer: anytype) !vo
         const platform_version = @as(u32, @intCast(ver.major << 16 | ver.minor << 8));
         break :blk platform_version;
     };
-    const sdk_version: u32 = if (options.darwin_sdk_version) |ver|
+    const sdk_version: ?std.SemanticVersion = options.darwin_sdk_version orelse blk: {
+        if (options.sysroot) |path| break :blk inferSdkVersionFromSdkPath(path);
+        break :blk null;
+    };
+    const sdk_version_value: u32 = if (sdk_version) |ver|
         @intCast(ver.major << 16 | ver.minor << 8)
     else
         platform_version;
@@ -293,7 +297,7 @@ pub fn writeBuildVersionLC(options: *const link.Options, lc_writer: anytype) !vo
             else => unreachable,
         },
         .minos = platform_version,
-        .sdk = sdk_version,
+        .sdk = sdk_version_value,
         .ntools = 1,
     });
     try lc_writer.writeAll(mem.asBytes(&macho.build_tool_version{
@@ -315,3 +319,58 @@ pub fn writeLoadDylibLCs(dylibs: []const Dylib, referenced: []u16, lc_writer: an
         }, lc_writer);
     }
 }
+
+fn inferSdkVersionFromSdkPath(path: []const u8) ?std.SemanticVersion {
+    const stem = std.fs.path.stem(path);
+    const start = for (stem, 0..) |c, i| {
+        if (std.ascii.isDigit(c)) break i;
+    } else stem.len;
+    const end = for (stem[start..], start..) |c, i| {
+        if (std.ascii.isDigit(c) or c == '.') continue;
+        break i;
+    } else stem.len;
+    return parseSdkVersion(stem[start..end]);
+}
+
+// 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 {
+    var parsed: std.SemanticVersion = .{
+        .major = 0,
+        .minor = 0,
+        .patch = 0,
+    };
+
+    const parseNext = struct {
+        fn parseNext(it: anytype) ?u16 {
+            const nn = it.next() orelse return null;
+            return std.fmt.parseInt(u16, nn, 10) catch null;
+        }
+    }.parseNext;
+
+    var it = std.mem.splitAny(u8, raw, ".");
+    parsed.major = parseNext(&it) orelse return null;
+    parsed.minor = parseNext(&it) orelse return null;
+    parsed.patch = parseNext(&it) orelse 0;
+    return parsed;
+}
+
+const expect = std.testing.expect;
+const expectEqual = std.testing.expectEqual;
+
+fn testParseSdkVersionSuccess(exp: std.SemanticVersion, raw: []const u8) !void {
+    const maybe_ver = parseSdkVersion(raw);
+    try expect(maybe_ver != null);
+    const ver = maybe_ver.?;
+    try expectEqual(exp.major, ver.major);
+    try expectEqual(exp.minor, ver.minor);
+    try expectEqual(exp.patch, ver.patch);
+}
+
+test "parseSdkVersion" {
+    try testParseSdkVersionSuccess(.{ .major = 13, .minor = 4, .patch = 0 }, "13.4");
+    try testParseSdkVersionSuccess(.{ .major = 13, .minor = 4, .patch = 1 }, "13.4.1");
+    try testParseSdkVersionSuccess(.{ .major = 11, .minor = 15, .patch = 0 }, "11.15");
+
+    try expect(parseSdkVersion("11") == null);
+}
diff --git a/test/link/macho/bugs/13056/build.zig b/test/link/macho/bugs/13056/build.zig
@@ -23,13 +23,13 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.Optimize
         .name = "test",
         .optimize = optimize,
     });
-    exe.addSystemIncludePath(.{ .path = b.pathJoin(&.{ sdk.path, "/usr/include" }) });
-    exe.addIncludePath(.{ .path = b.pathJoin(&.{ sdk.path, "/usr/include/c++/v1" }) });
+    exe.addSystemIncludePath(.{ .path = b.pathJoin(&.{ sdk, "/usr/include" }) });
+    exe.addIncludePath(.{ .path = b.pathJoin(&.{ sdk, "/usr/include/c++/v1" }) });
     exe.addCSourceFile(.{ .file = .{ .path = "test.cpp" }, .flags = &.{
         "-nostdlib++",
         "-nostdinc++",
     } });
-    exe.addObjectFile(.{ .path = b.pathJoin(&.{ sdk.path, "/usr/lib/libc++.tbd" }) });
+    exe.addObjectFile(.{ .path = b.pathJoin(&.{ sdk, "/usr/lib/libc++.tbd" }) });
 
     const run_cmd = b.addRunArtifact(exe);
     run_cmd.expectStdErrEqual("x: 5\n");
diff --git a/test/standalone/ios/build.zig b/test/standalone/ios/build.zig
@@ -14,17 +14,17 @@ pub fn build(b: *std.Build) void {
     };
     const target_info = std.zig.system.NativeTargetInfo.detect(target) catch @panic("couldn't detect native target");
     const sdk = std.zig.system.darwin.getSdk(b.allocator, target_info.target) orelse @panic("no iOS SDK found");
-    b.sysroot = sdk.path;
+    b.sysroot = sdk;
 
     const exe = b.addExecutable(.{
         .name = "main",
         .optimize = optimize,
         .target = target,
     });
     exe.addCSourceFile(.{ .file = .{ .path = "main.m" }, .flags = &.{} });
-    exe.addSystemIncludePath(.{ .path = b.pathJoin(&.{ sdk.path, "/usr/include" }) });
-    exe.addSystemFrameworkPath(.{ .path = b.pathJoin(&.{ sdk.path, "/System/Library/Frameworks" }) });
-    exe.addLibraryPath(.{ .path = b.pathJoin(&.{ sdk.path, "/usr/lib" }) });
+    exe.addSystemIncludePath(.{ .path = b.pathJoin(&.{ sdk, "/usr/include" }) });
+    exe.addSystemFrameworkPath(.{ .path = b.pathJoin(&.{ sdk, "/System/Library/Frameworks" }) });
+    exe.addLibraryPath(.{ .path = b.pathJoin(&.{ sdk, "/usr/lib" }) });
     exe.linkFramework("Foundation");
     exe.linkFramework("UIKit");
     exe.linkLibC();
