Fix missing external target settings with config conditions (#6170)

* Fix external target settings with config condition * Address PR feedback * Update Package.resolved of app_with_spm_dependencies
tuist · Apr 16, 2024 · 88a1763 · 88a1763
1 parent 895ef48
commit 88a1763
Show file tree

Hide file tree

Showing 8 changed files with 130 additions and 58 deletions.
diff --git a/Sources/TuistGraph/Models/Settings.swift b/Sources/TuistGraph/Models/Settings.swift
@@ -76,8 +76,8 @@ extension SettingsDictionary {
 public struct Configuration: Equatable, Codable {
     // MARK: - Attributes
 
-    public let settings: SettingsDictionary
-    public let xcconfig: AbsolutePath?
+    public var settings: SettingsDictionary
+    public var xcconfig: AbsolutePath?
 
     // MARK: - Init
 

diff --git a/Sources/TuistLoader/SwiftPackageManager/PackageInfoMapper.swift b/Sources/TuistLoader/SwiftPackageManager/PackageInfoMapper.swift
@@ -997,7 +997,26 @@ extension ProjectDescription.Settings {
             mappedSettingsDictionary.merge(projectDescriptionSettingsToOverride)
         }
 
-        return .from(settings: baseSettings, adding: mappedSettingsDictionary, packageFolder: packageFolder)
+        let configurations: [ProjectDescription.Configuration] = try baseSettings.configurations
+            .map { buildConfiguration, configuration in
+                var configuration = configuration ?? Configuration(settings: [:])
+                configuration.settings = configuration.settings.merging(
+                    try mapper.settingsForBuildConfiguration(buildConfiguration.name),
+                    uniquingKeysWith: { $1 }
+                )
+                return .from(
+                    buildConfiguration: buildConfiguration,
+                    configuration: configuration,
+                    packageFolder: packageFolder
+                )
+            }
+
+        return .settings(
+            base: .from(settingsDictionary: baseSettings.base).merging(mappedSettingsDictionary, uniquingKeysWith: { $1 }),
+            configurations: configurations
+                .sorted { $0.name.rawValue < $1.name.rawValue },
+            defaultSettings: .from(defaultSettings: baseSettings.defaultSettings)
+        )
     }
 
     fileprivate struct PackageTarget: Hashable {
@@ -1085,24 +1104,6 @@ extension ProjectDescription.SettingsDictionary {
     }
 }
 
-extension ProjectDescription.Settings {
-    public static func from(
-        settings: TuistGraph.Settings,
-        adding: ProjectDescription.SettingsDictionary,
-        packageFolder: AbsolutePath
-    ) -> Self {
-        .settings(
-            base: .from(settingsDictionary: settings.base).merging(adding, uniquingKeysWith: { $1 }),
-            configurations: settings.configurations
-                .map { buildConfiguration, configuration in
-                    .from(buildConfiguration: buildConfiguration, configuration: configuration, packageFolder: packageFolder)
-                }
-                .sorted { $0.name.rawValue < $1.name.rawValue },
-            defaultSettings: .from(defaultSettings: settings.defaultSettings)
-        )
-    }
-}
-
 extension ProjectDescription.Configuration {
     public static func from(
         buildConfiguration: BuildConfiguration,

diff --git a/Sources/TuistLoader/SwiftPackageManager/SettingsMapper.swift b/Sources/TuistLoader/SwiftPackageManager/SettingsMapper.swift
@@ -19,33 +19,55 @@ struct SettingsMapper {
     private let mainRelativePath: RelativePath
     private let settings: [PackageInfo.Target.TargetBuildSettingDescription.Setting]
 
-    // `nil` means settings without a condition
-    private func settingsForPlatform(_ platformName: String?) throws
-        -> [PackageInfo.Target.TargetBuildSettingDescription.Setting]
-    {
-        settings.filter { setting in
-            if let platformName, setting.hasConditions {
-                return setting.condition?.platformNames.contains(platformName) == true
-            } else {
-                return !setting.hasConditions
-            }
+    func settingsForPlatforms(_ platforms: [PackageInfo.Platform]) throws -> TuistGraph.SettingsDictionary {
+        var resolvedSettings = try settingsDictionary()
+
+        for platform in platforms.sorted(by: { $0.platformName < $1.platformName }) {
+            let platformSettings = try settingsDictionary(for: platform)
+            resolvedSettings.overlay(with: platformSettings, for: try platform.graphPlatform())
         }
+
+        return resolvedSettings
+    }
+
+    func settingsForBuildConfiguration(
+        _ buildConfiguration: String
+    ) throws -> TuistGraph.SettingsDictionary {
+        try map(
+            settings: settings.filter { setting in
+                return setting.hasConditions && setting.condition?.config?.uppercasingFirst == buildConfiguration
+            }
+        )
     }
 
     // swiftlint:disable:next function_body_length
-    func settingsDictionaryForPlatform(_ platform: PackageInfo.Platform?) throws -> TuistGraph.SettingsDictionary {
+    func settingsDictionary(for platform: PackageInfo.Platform? = nil) throws -> TuistGraph.SettingsDictionary {
+        let platformSettings = try settings(for: platform?.platformName)
+
+        return try map(
+            settings: platformSettings,
+            headerSearchPaths: headerSearchPaths,
+            defines: ["SWIFT_PACKAGE": "1"],
+            swiftDefines: "SWIFT_PACKAGE"
+        )
+    }
+
+    private func map(
+        settings: [PackageInfo.Target.TargetBuildSettingDescription.Setting],
+        headerSearchPaths: [String] = [],
+        defines: [String: String] = [:],
+        swiftDefines: String = ""
+    ) throws -> TuistGraph.SettingsDictionary {
         var headerSearchPaths = headerSearchPaths
-        var defines = ["SWIFT_PACKAGE": "1"]
-        var swiftDefines = "SWIFT_PACKAGE"
+        var defines = defines
+        var swiftDefines = swiftDefines
         var cFlags: [String] = []
         var cxxFlags: [String] = []
         var swiftFlags: [String] = []
         var linkerFlags: [String] = []
 
         var settingsDictionary = TuistGraph.SettingsDictionary()
-        let platformSettings = try settingsForPlatform(platform?.platformName)
-
-        for setting in platformSettings {
+        for setting in settings {
             switch (setting.tool, setting.name) {
             case (.c, .headerSearchPath), (.cxx, .headerSearchPath):
                 headerSearchPaths.append("$(SRCROOT)/\(mainRelativePath.pathString)/\(setting.value[0])")
@@ -113,15 +135,17 @@ struct SettingsMapper {
         return settingsDictionary
     }
 
-    func settingsForPlatforms(_ platforms: [PackageInfo.Platform]) throws -> TuistGraph.SettingsDictionary {
-        var resolvedSettings = try settingsDictionaryForPlatform(nil)
-
-        for platform in platforms.sorted(by: { $0.platformName < $1.platformName }) {
-            let platformSettings = try settingsDictionaryForPlatform(platform)
-            resolvedSettings.overlay(with: platformSettings, for: try platform.graphPlatform())
+    // `nil` means settings without a condition
+    private func settings(for platformName: String?) throws
+        -> [PackageInfo.Target.TargetBuildSettingDescription.Setting]
+    {
+        settings.filter { setting in
+            if let platformName, setting.hasConditions {
+                return setting.condition?.platformNames.contains(platformName) == true
+            } else {
+                return !setting.hasConditions
+            }
         }
-
-        return resolvedSettings
     }
 }
 

diff --git a/Tests/TuistLoaderTests/SwiftPackageManager/PackageInfoMapperTests.swift b/Tests/TuistLoaderTests/SwiftPackageManager/PackageInfoMapperTests.swift
@@ -611,6 +611,15 @@ final class PackageInfoMapperTests: TuistUnitTestCase {
                                 .init(tool: .c, name: .define, condition: nil, value: ["FOO1=\"BAR1\""]),
                                 .init(tool: .cxx, name: .define, condition: nil, value: ["FOO2=\"BAR2\""]),
                                 .init(tool: .cxx, name: .define, condition: nil, value: ["FOO3=3"]),
+                                .init(
+                                    tool: .cxx,
+                                    name: .define,
+                                    condition: PackageInfo.PackageConditionDescription(
+                                        platformNames: [],
+                                        config: "debug"
+                                    ),
+                                    value: ["FOO_DEBUG=1"]
+                                ),
                             ]
                         ),
                     ],
@@ -621,14 +630,31 @@ final class PackageInfoMapperTests: TuistUnitTestCase {
                 ),
             ]
         )
-        XCTAssertEqual(
+        XCTAssertBetterEqual(
             project,
             .testWithDefaultConfigs(
                 name: "Package",
                 targets: [
                     .test(
                         "Target1",
                         basePath: basePath,
+                        baseSettings: .settings(
+                            configurations: [
+                                .debug(
+                                    name: .debug,
+                                    settings: [
+                                        "GCC_PREPROCESSOR_DEFINITIONS": [
+                                            "$(inherited)",
+                                            "FOO_DEBUG=1",
+                                        ],
+                                    ]
+                                ),
+                                .release(
+                                    name: .release,
+                                    settings: [:]
+                                ),
+                            ]
+                        ),
                         customSettings: [
                             "GCC_PREPROCESSOR_DEFINITIONS": [
                                 // Escaped
@@ -1064,7 +1090,7 @@ final class PackageInfoMapperTests: TuistUnitTestCase {
                 ),
             ]
         )
-        XCTAssertEqual(
+        XCTAssertBetterEqual(
             project,
             .testWithDefaultConfigs(
                 name: "Package",

diff --git a/Tests/TuistLoaderTests/SwiftPackageManager/SettingsMapperTests.swift b/Tests/TuistLoaderTests/SwiftPackageManager/SettingsMapperTests.swift
@@ -20,7 +20,7 @@ final class SettingsMapperTests: XCTestCase {
             settings: []
         )
 
-        let resolvedSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let resolvedSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(resolvedSettings, [
             "GCC_PREPROCESSOR_DEFINITIONS": .array(["$(inherited)",
@@ -44,7 +44,7 @@ final class SettingsMapperTests: XCTestCase {
             settings: settings
         )
 
-        let resolvedSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let resolvedSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(
             resolvedSettings["GCC_PREPROCESSOR_DEFINITIONS"],
@@ -72,7 +72,7 @@ final class SettingsMapperTests: XCTestCase {
             settings: settings
         )
 
-        let resolvedSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let resolvedSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(
             resolvedSettings["HEADER_SEARCH_PATHS"],
@@ -93,7 +93,7 @@ final class SettingsMapperTests: XCTestCase {
             settings: settings
         )
 
-        let resolvedSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let resolvedSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(
             resolvedSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS"],
@@ -112,7 +112,7 @@ final class SettingsMapperTests: XCTestCase {
             settings: settings
         )
 
-        let resolvedSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let resolvedSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(
             resolvedSettings["OTHER_CFLAGS"],
@@ -131,7 +131,7 @@ final class SettingsMapperTests: XCTestCase {
             settings: settings
         )
 
-        let resolvedSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let resolvedSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(
             resolvedSettings["OTHER_CPLUSPLUSFLAGS"],
@@ -152,7 +152,7 @@ final class SettingsMapperTests: XCTestCase {
             settings: settings
         )
 
-        let resolvedSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let resolvedSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(
             resolvedSettings["OTHER_SWIFT_FLAGS"],
@@ -176,7 +176,7 @@ final class SettingsMapperTests: XCTestCase {
             settings: settings
         )
 
-        let resolvedSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let resolvedSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(
             resolvedSettings["OTHER_LDFLAGS"],
@@ -204,14 +204,14 @@ final class SettingsMapperTests: XCTestCase {
             settings: settings
         )
 
-        let allPlatformSettings = try mapper.settingsDictionaryForPlatform(nil)
+        let allPlatformSettings = try mapper.settingsDictionary()
 
         XCTAssertEqual(
             allPlatformSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS"],
             .string("$(inherited) SWIFT_PACKAGE Define1")
         )
 
-        let iosPlatformSettings = try mapper.settingsDictionaryForPlatform(.ios)
+        let iosPlatformSettings = try mapper.settingsDictionary(for: .ios)
 
         XCTAssertEqual(
             iosPlatformSettings["SWIFT_ACTIVE_COMPILATION_CONDITIONS"],

diff --git a/fixtures/app_with_spm_dependencies/App/Project.swift b/fixtures/app_with_spm_dependencies/App/Project.swift
@@ -44,6 +44,7 @@ let project = Project(
                 .external(name: "AirshipPreferenceCenter"),
                 .external(name: "MarkdownUI"),
                 .external(name: "GoogleMobileAds"),
+                .external(name: "LookinServer"),
             ],
             settings: .targetSettings
         ),

diff --git a/fixtures/app_with_spm_dependencies/Tuist/Package.resolved b/fixtures/app_with_spm_dependencies/Tuist/Package.resolved
@@ -1,4 +1,5 @@
 {
+  "originHash" : "d97ce5778061e5c442f34dc995c6b958648d4f6c778b322c7334b33b5231fd13",
   "pins" : [
     {
       "identity" : "alamofire",
@@ -45,6 +46,15 @@
         "version" : "1.0.0"
       }
     },
+    {
+      "identity" : "cuckoo",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/Brightify/Cuckoo.git",
+      "state" : {
+        "revision" : "0cfbb729d0aced6494c19c58c0c31ea16d1cbf66",
+        "version" : "1.10.4"
+      }
+    },
     {
       "identity" : "cwlcatchexception",
       "kind" : "remoteSourceControl",
@@ -99,6 +109,15 @@
         "version" : "17.7.3"
       }
     },
+    {
+      "identity" : "lookinserver",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/QMUI/LookinServer",
+      "state" : {
+        "revision" : "e553d1b689d147817dc54ad5c28fcff71e860101",
+        "version" : "1.2.8"
+      }
+    },
     {
       "identity" : "networkimage",
       "kind" : "remoteSourceControl",
@@ -370,5 +389,5 @@
       }
     }
   ],
-  "version" : 2
+  "version" : 3
 }
diff --git a/fixtures/app_with_spm_dependencies/Tuist/Package.swift b/fixtures/app_with_spm_dependencies/Tuist/Package.swift
@@ -41,6 +41,7 @@ let package = Package(
         .package(path: "../LocalSwiftPackage"),
         .package(path: "../StringifyMacro"),
         .package(url: "https://github.com/kishikawakatsumi/UICKeyChainStore", exact: "2.2.1"),
+        .package(url: "https://github.com/QMUI/LookinServer", from: "1.2.8"),
         // Has XCTest API in a non-test target. Tuist will add Test Search path to support it
         .package(url: "https://github.com/Brightify/Cuckoo.git", exact: "1.10.4"),
     ]