Improve Destination.sdkPlatformFrameworkPaths() (#5876)

We should not ignore errors or an empty SDK platform path since that means XCTest imports and test execution might silently fail on macOS with no indication to what the problem is.

Author: neonichu

Sources/Commands/Utilities/TestingSupport.swift

@@ -82,13 +82,13 @@ enum TestingSupport {
                 ),
                 sanitizers: sanitizers
             )
-            // Add the sdk platform path if we have it. If this is not present, we
-            // might always end up failing.
-            if let sdkPlatformFrameworksPath = try Destination.sdkPlatformFrameworkPaths() {
-                // appending since we prefer the user setting (if set) to the one we inject
-                env.appendPath("DYLD_FRAMEWORK_PATH", value: sdkPlatformFrameworksPath.fwk.pathString)
-                env.appendPath("DYLD_LIBRARY_PATH", value: sdkPlatformFrameworksPath.lib.pathString)
-            }
+
+            // Add the sdk platform path if we have it. If this is not present, we might always end up failing.
+            let sdkPlatformFrameworksPath = try Destination.sdkPlatformFrameworkPaths()
+            // appending since we prefer the user setting (if set) to the one we inject
+            env.appendPath("DYLD_FRAMEWORK_PATH", value: sdkPlatformFrameworksPath.fwk.pathString)
+            env.appendPath("DYLD_LIBRARY_PATH", value: sdkPlatformFrameworksPath.lib.pathString)
+
             try TSCBasic.Process.checkNonZeroExit(arguments: args, environment: env)
             // Read the temporary file's content.
             return try swiftTool.fileSystem.readFileContents(tempFile.path)

Sources/PackageModel/Destination.swift

@@ -195,12 +195,11 @@ public struct Destination: Encodable, Equatable {
         var extraCCFlags: [String] = []
         var extraSwiftCFlags: [String] = []
 #if os(macOS)
-        if let sdkPaths = try Destination.sdkPlatformFrameworkPaths(environment: environment) {
-            extraCCFlags += ["-F", sdkPaths.fwk.pathString]
-            extraSwiftCFlags += ["-F", sdkPaths.fwk.pathString]
-            extraSwiftCFlags += ["-I", sdkPaths.lib.pathString]
-            extraSwiftCFlags += ["-L", sdkPaths.lib.pathString]
-        }
+        let sdkPaths = try Destination.sdkPlatformFrameworkPaths(environment: environment)
+        extraCCFlags += ["-F", sdkPaths.fwk.pathString]
+        extraSwiftCFlags += ["-F", sdkPaths.fwk.pathString]
+        extraSwiftCFlags += ["-I", sdkPaths.lib.pathString]
+        extraSwiftCFlags += ["-L", sdkPaths.lib.pathString]
 #endif
 
 #if !os(Windows)
@@ -217,26 +216,29 @@ public struct Destination: Encodable, Equatable {
     /// Returns macosx sdk platform framework path.
     public static func sdkPlatformFrameworkPaths(
         environment: EnvironmentVariables = .process()
-    ) throws -> (fwk: AbsolutePath, lib: AbsolutePath)? {
+    ) throws -> (fwk: AbsolutePath, lib: AbsolutePath) {
         if let path = _sdkPlatformFrameworkPath {
             return path
         }
-        let platformPath = try? TSCBasic.Process.checkNonZeroExit(
+        let platformPath = try TSCBasic.Process.checkNonZeroExit(
             arguments: ["/usr/bin/xcrun", "--sdk", "macosx", "--show-sdk-platform-path"],
             environment: environment).spm_chomp()
 
-        if let platformPath = platformPath, !platformPath.isEmpty {
-            // For XCTest framework.
-            let fwk = try AbsolutePath(validating: platformPath).appending(
-                components: "Developer", "Library", "Frameworks")
+        guard !platformPath.isEmpty else {
+            throw StringError("could not determine SDK platform path")
+        }
+
+        // For XCTest framework.
+        let fwk = try AbsolutePath(validating: platformPath).appending(
+            components: "Developer", "Library", "Frameworks")
 
-            // For XCTest Swift library.
-            let lib = try AbsolutePath(validating: platformPath).appending(
-                components: "Developer", "usr", "lib")
+        // For XCTest Swift library.
+        let lib = try AbsolutePath(validating: platformPath).appending(
+            components: "Developer", "usr", "lib")
 
-            _sdkPlatformFrameworkPath = (fwk, lib)
-        }
-        return _sdkPlatformFrameworkPath
+        let sdkPlatformFrameworkPath = (fwk, lib)
+        _sdkPlatformFrameworkPath = sdkPlatformFrameworkPath
+        return sdkPlatformFrameworkPath
     }
 
     /// Cache storage for sdk platform path.

Sources/SPMTestSupport/Toolchain.swift

@@ -33,18 +33,6 @@ private func resolveBinDir() throws -> AbsolutePath {
 #endif
 }
 
-extension UserToolchain {
-
-#if os(macOS)
-    public var sdkPlatformFrameworksPath: AbsolutePath {
-        get throws {
-            return try Destination.sdkPlatformFrameworkPaths()!.fwk
-        }
-    }
-#endif
-
-}
-
 extension Destination {
     public static var `default`: Self {
         get throws {

Tests/FunctionalTests/SwiftPMXCTestHelperTests.swift

@@ -49,7 +49,7 @@ class SwiftPMXCTestHelperTests: XCTestCase {
 
     func XCTAssertXCTestHelper(_ bundlePath: AbsolutePath, testCases: NSDictionary) throws {
         #if os(macOS)
-        let env = ["DYLD_FRAMEWORK_PATH": try UserToolchain.default.sdkPlatformFrameworksPath.pathString]
+        let env = ["DYLD_FRAMEWORK_PATH": try Destination.sdkPlatformFrameworkPaths().fwk.pathString]
         let outputFile = bundlePath.parentDirectory.appending(component: "tests.txt")
         let _ = try SwiftPMProduct.XCTestHelper.execute([bundlePath.pathString, outputFile.pathString], env: env)
         guard let data = NSData(contentsOfFile: outputFile.pathString) else {