Fix missing platform-specific settings for SPM packages (tuist#6386)

* Fix missing platform-specific settings for SPM packages * Respect maccatalyst setting condition
hiltonc · Jun 10, 2024 · a0aa54a · a0aa54a
1 parent aeeb19b
commit a0aa54a
Show file tree

Hide file tree

Showing 11 changed files with 113 additions and 72 deletions.
diff --git a/Sources/TuistLoader/SwiftPackageManager/PackageInfo+graphPlatform.swift b/Sources/TuistLoader/SwiftPackageManager/PackageInfo+graphPlatform.swift
@@ -4,23 +4,6 @@ import TuistSupport
 import XcodeGraph
 
 extension PackageInfo.Platform {
-    func graphPlatform() throws -> XcodeGraph.Platform {
-        switch platformName.lowercased() {
-        case "ios", "maccatalyst":
-            return .iOS
-        case "macos":
-            return .macOS
-        case "tvos":
-            return .tvOS
-        case "watchos":
-            return .watchOS
-        case "visionos":
-            return .visionOS
-        default:
-            throw PackageInfoMapperError.unknownPlatform(platformName)
-        }
-    }
-
     func destinations() throws -> ProjectDescription.Destinations {
         switch platformName.lowercased() {
         case "ios":

diff --git a/Sources/TuistLoader/SwiftPackageManager/PackageInfoMapper.swift b/Sources/TuistLoader/SwiftPackageManager/PackageInfoMapper.swift
@@ -593,7 +593,6 @@ public final class PackageInfoMapper: PackageInfoMapping {
             packageFolder: packageFolder,
             packageName: packageInfo.name,
             settings: target.settings,
-            platforms: packageInfo.platforms,
             moduleMap: moduleMap,
             baseSettings: baseSettings,
             targetSettings: targetSettings
@@ -943,7 +942,6 @@ extension ProjectDescription.Settings {
         packageFolder: AbsolutePath,
         packageName _: String,
         settings: [PackageInfo.Target.TargetBuildSettingDescription.Setting],
-        platforms: [PackageInfo.Platform],
         moduleMap: ModuleMap?,
         baseSettings: XcodeGraph.Settings,
         targetSettings: [String: XcodeGraph.SettingsDictionary]
@@ -980,7 +978,7 @@ extension ProjectDescription.Settings {
             settings: settings
         )
 
-        let resolvedSettings = try mapper.settingsForPlatforms(platforms)
+        let resolvedSettings = try mapper.mapSettings()
 
         settingsDictionary.merge(resolvedSettings) { $1 }
 

diff --git a/Sources/TuistLoader/SwiftPackageManager/SettingsMapper.swift b/Sources/TuistLoader/SwiftPackageManager/SettingsMapper.swift
@@ -19,12 +19,12 @@ struct SettingsMapper {
     private let mainRelativePath: RelativePath
     private let settings: [PackageInfo.Target.TargetBuildSettingDescription.Setting]
 
-    func settingsForPlatforms(_ platforms: [PackageInfo.Platform]) throws -> XcodeGraph.SettingsDictionary {
+    func mapSettings() throws -> XcodeGraph.SettingsDictionary {
         var resolvedSettings = try settingsDictionary()
 
-        for platform in platforms.sorted(by: { $0.platformName < $1.platformName }) {
+        for platform in XcodeGraph.Platform.allCases.sorted(by: { $0.rawValue < $1.rawValue }) {
             let platformSettings = try settingsDictionary(for: platform)
-            resolvedSettings.overlay(with: platformSettings, for: try platform.graphPlatform())
+            resolvedSettings.overlay(with: platformSettings, for: platform)
         }
 
         return resolvedSettings
@@ -41,8 +41,8 @@ struct SettingsMapper {
     }
 
     // swiftlint:disable:next function_body_length
-    func settingsDictionary(for platform: PackageInfo.Platform? = nil) throws -> XcodeGraph.SettingsDictionary {
-        let platformSettings = try settings(for: platform?.platformName)
+    func settingsDictionary(for platform: XcodeGraph.Platform? = nil) throws -> XcodeGraph.SettingsDictionary {
+        let platformSettings = try settings(for: platform?.rawValue)
 
         return try map(
             settings: platformSettings,
@@ -141,7 +141,14 @@ struct SettingsMapper {
     {
         settings.filter { setting in
             if let platformName, setting.hasConditions {
-                return setting.condition?.platformNames.contains(platformName) == true
+                let hasMacCatalystPlatform = setting.condition?.platformNames.contains("maccatalyst") == true
+                let platformNames: [String]
+                if hasMacCatalystPlatform {
+                    platformNames = (setting.condition?.platformNames ?? []) + ["ios"]
+                } else {
+                    platformNames = setting.condition?.platformNames ?? []
+                }
+                return platformNames.contains(platformName)
             } else {
                 return !setting.hasConditions
             }

diff --git a/Tests/TuistAcceptanceTests/RunAcceptanceTests.swift b/Tests/TuistAcceptanceTests/RunAcceptanceTests.swift
@@ -8,6 +8,7 @@ import XCTest
 final class RunAcceptanceTestCommandLineToolBasic: TuistAcceptanceTestCase {
     func test_command_line_tool_basic() async throws {
         try setUpFixture(.commandLineToolBasic)
+        try await run(InstallCommand.self)
         try await run(RunCommand.self, "CommandLineTool")
     }
 }
diff --git a/Tests/TuistGeneratorAcceptanceTests/GenerateAcceptanceTests.swift b/Tests/TuistGeneratorAcceptanceTests/GenerateAcceptanceTests.swift
@@ -634,6 +634,7 @@ final class GenerateAcceptanceTestiOSAppWithAppClip: TuistAcceptanceTestCase {
 final class GenerateAcceptanceTestCommandLineToolBase: TuistAcceptanceTestCase {
     func test_command_line_tool_basic() async throws {
         try setUpFixture(.commandLineToolBasic)
+        try await run(InstallCommand.self)
         try await run(GenerateCommand.self)
         try await run(BuildCommand.self, "CommandLineTool")
     }

diff --git a/Tests/TuistLoaderTests/SwiftPackageManager/PackageInfoGraphPlatformTests.swift b/Tests/TuistLoaderTests/SwiftPackageManager/PackageInfoGraphPlatformTests.swift
diff --git a/Tests/TuistLoaderTests/SwiftPackageManager/PackageInfoMapperTests.swift b/Tests/TuistLoaderTests/SwiftPackageManager/PackageInfoMapperTests.swift
@@ -2300,7 +2300,7 @@ final class PackageInfoMapperTests: TuistUnitTestCase {
         )
     }
 
-    func testMap_whenConditionalSetting_ignoresByPlatform() throws {
+    func testMap_whenConditionalSetting() throws {
         let basePath = try temporaryPath()
         let sourcesPath = basePath.appending(try RelativePath(validating: "Package/Sources/Target1"))
         try fileHandler.createFolder(sourcesPath)
@@ -2348,7 +2348,19 @@ final class PackageInfoMapperTests: TuistUnitTestCase {
                     .test(
                         "Target1",
                         basePath: basePath,
-                        customSettings: ["HEADER_SEARCH_PATHS": ["$(SRCROOT)/Sources/Target1/otherValue"]]
+                        customSettings: [
+                            "HEADER_SEARCH_PATHS": ["$(SRCROOT)/Sources/Target1/otherValue"],
+                            "HEADER_SEARCH_PATHS[sdk=appletvos*]": [
+                                "$(inherited)",
+                                "$(SRCROOT)/Sources/Target1/value",
+                                "$(SRCROOT)/Sources/Target1/otherValue",
+                            ],
+                            "HEADER_SEARCH_PATHS[sdk=appletvsimulator*]": [
+                                "$(inherited)",
+                                "$(SRCROOT)/Sources/Target1/value",
+                                "$(SRCROOT)/Sources/Target1/otherValue",
+                            ],
+                        ]
                     ),
                 ]
             )

diff --git a/Tests/TuistLoaderTests/SwiftPackageManager/SettingsMapperTests.swift b/Tests/TuistLoaderTests/SwiftPackageManager/SettingsMapperTests.swift
@@ -211,14 +211,14 @@ final class SettingsMapperTests: XCTestCase {
             .string("$(inherited) SWIFT_PACKAGE Define1")
         )
 
-        let iosPlatformSettings = try mapper.settingsDictionary(for: .ios)
+        let iosPlatformSettings = try mapper.settingsDictionary(for: .iOS)
 
         XCTAssertEqual(
             iosPlatformSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS"],
             .string("$(inherited) SWIFT_PACKAGE Define1 Define2")
         )
 
-        let combinedSettings = try mapper.settingsForPlatforms([.ios, .macos, .tvos])
+        let combinedSettings = try mapper.mapSettings()
 
         XCTAssertEqual(
             combinedSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS[sdk=iphoneos*]"],
@@ -245,6 +245,55 @@ final class SettingsMapperTests: XCTestCase {
             .string("$(inherited) SWIFT_PACKAGE Define1")
         )
     }
+
+    func test_set_maccatalyst() throws {
+        let settings: [PackageInfo.Target.TargetBuildSettingDescription.Setting] = [
+            .init(tool: .swift, name: .define, condition: nil, value: ["Define1"]),
+            .init(
+                tool: .swift,
+                name: .define,
+                condition: PackageInfo.PackageConditionDescription(platformNames: ["maccatalyst"], config: nil),
+                value: ["Define2"]
+            ),
+        ]
+
+        let mapper = SettingsMapper(
+            headerSearchPaths: [],
+            mainRelativePath: try RelativePath(validating: "path"),
+            settings: settings
+        )
+
+        let allPlatformSettings = try mapper.settingsDictionary()
+
+        XCTAssertEqual(
+            allPlatformSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS"],
+            .string("$(inherited) SWIFT_PACKAGE Define1")
+        )
+
+        let iosPlatformSettings = try mapper.settingsDictionary(for: .iOS)
+
+        XCTAssertEqual(
+            iosPlatformSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS"],
+            .string("$(inherited) SWIFT_PACKAGE Define1 Define2")
+        )
+
+        let combinedSettings = try mapper.mapSettings()
+
+        XCTAssertEqual(
+            combinedSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS[sdk=iphoneos*]"],
+            .string("$(inherited) SWIFT_PACKAGE Define1 Define2")
+        )
+
+        XCTAssertEqual(
+            combinedSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS[sdk=iphonesimulator*]"],
+            .string("$(inherited) SWIFT_PACKAGE Define1 Define2")
+        )
+
+        XCTAssertEqual(
+            combinedSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS"],
+            .string("$(inherited) SWIFT_PACKAGE Define1")
+        )
+    }
 }
 
 // OTHER_LDFLAGS

diff --git a/fixtures/command_line_tool_basic/Project.swift b/fixtures/command_line_tool_basic/Project.swift
@@ -9,7 +9,10 @@ let project = Project(
             product: .commandLineTool,
             bundleId: "com.example.commandlinetool",
             infoPlist: .default,
-            sources: ["main.swift"]
+            sources: ["main.swift"],
+            dependencies: [
+                .external(name: "SystemPackage"),
+            ]
         ),
     ]
 )
diff --git a/fixtures/command_line_tool_basic/Tuist/Package.resolved b/fixtures/command_line_tool_basic/Tuist/Package.resolved
@@ -0,0 +1,15 @@
+{
+  "originHash" : "1d9037060b1f5bba948714e32c2f1311d4e0412b80099aa57b7e530425cb4cea",
+  "pins" : [
+    {
+      "identity" : "swift-system",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/apple/swift-system.git",
+      "state" : {
+        "revision" : "f9266c85189c2751589a50ea5aec72799797e471",
+        "version" : "1.3.0"
+      }
+    }
+  ],
+  "version" : 3
+}
diff --git a/fixtures/command_line_tool_basic/Tuist/Package.swift b/fixtures/command_line_tool_basic/Tuist/Package.swift
@@ -0,0 +1,13 @@
+// swift-tools-version:5.10
+
+import PackageDescription
+
+let package = Package(
+    name: "workspace-dependencies",
+    dependencies: [
+        .package(
+            url: "https://github.com/apple/swift-system.git",
+            from: "1.3.0"
+        ),
+    ]
+)