Fixed issue when linking and embeding static frameworks: they should …

…be linked and NOT embed (#820)
yonaskolb · Apr 7, 2020 · 7afcf1f · 7afcf1f
1 parent 0328fa0
commit 7afcf1f
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,9 @@
 - Improve speed of metadata parsing and dependency resolution. [#803](https://github.com/yonaskolb/XcodeGen/pull/803) @michaeleisel
 - Improve support for iOS sticker packs and add support for `launchAutomaticallySubstyle` to run schemes. [#824](https://github.com/yonaskolb/XcodeGen/pull/824) @scelis
 
+#### Fixed
+- Fixed issue when linking and embeding static frameworks: they should be linked and NOT embed. [#820](https://github.com/yonaskolb/XcodeGen/pull/820) @acecilia
+
 ## 2.15.1
 
 #### Fixed

diff --git a/Sources/ProjectSpec/XCProjExtensions.swift b/Sources/ProjectSpec/XCProjExtensions.swift
@@ -52,6 +52,27 @@ extension PBXProductType {
             return false
         }
     }
+
+    /// Function to determine when a dependendency should be embedded into the target
+    public func shouldEmbed(_ dependencyType: PBXProductType) -> Bool {
+        switch dependencyType {
+        case .staticLibrary, .staticFramework:
+            // Some dependendencies should not be embed, independently of the target type
+            return false
+
+        default:
+            if isApp {
+                // If target is an app, all dependencies should be embed (except for the ones mentioned above)
+                return true
+            } else if isTest, [.framework, .bundle].contains(dependencyType) {
+                // If target is test, some dependencies should be embed (depending on their type)
+                return true
+            } else {
+                // If none of the above, do not embed the dependency
+                return false
+            }
+        }
+    }
 }
 
 extension Platform {

diff --git a/Sources/XcodeGenKit/PBXProjGenerator.swift b/Sources/XcodeGenKit/PBXProjGenerator.swift
@@ -643,10 +643,7 @@ public class PBXProjGenerator {
                 }
             }
 
-            let embed = dependency.embed ?? (!dependencyTarget.type.isLibrary && (
-                target.type.isApp
-                    || (target.type.isTest && (dependencyTarget.type.isFramework || dependencyTarget.type == .bundle))
-            ))
+            let embed = dependency.embed ?? target.type.shouldEmbed(dependencyTarget.type)
             if embed {
                 let embedFile = addObject(
                     PBXBuildFile(
@@ -1240,19 +1237,20 @@ public class PBXProjGenerator {
 
                 // don't want a dependency if it's going to be embedded or statically linked in a non-top level target
                 // in .target check we filter out targets that will embed all of their dependencies
+                // For some more context about the `dependency.embed != true` lines, refer to https://github.com/yonaskolb/XcodeGen/pull/820
                 switch dependency.type {
                 case .sdk:
                     dependencies[dependency.reference] = dependency
                 case .framework, .carthage, .package:
-                    if isTopLevel || dependency.embed == nil {
+                    if isTopLevel || dependency.embed != true {
                         dependencies[dependency.reference] = dependency
                     }
                 case .target:
                     let dependencyTargetReference = try! TargetReference(dependency.reference)
 
                     switch dependencyTargetReference.location {
                     case .local:
-                        if isTopLevel || dependency.embed == nil {
+                        if isTopLevel || dependency.embed != true {
                             if let dependencyTarget = project.getTarget(dependency.reference) {
                                 dependencies[dependency.reference] = dependency
                                 if !dependencyTarget.shouldEmbedDependencies {
@@ -1265,7 +1263,7 @@ public class PBXProjGenerator {
                             }
                         }
                     case .project:
-                        if isTopLevel || dependency.embed == nil {
+                        if isTopLevel || dependency.embed != true {
                             dependencies[dependency.reference] = dependency
                         }
                     }

diff --git a/Tests/XcodeGenKitTests/ProjectGeneratorTests.swift b/Tests/XcodeGenKitTests/ProjectGeneratorTests.swift
@@ -594,6 +594,7 @@ class ProjectGeneratorTests: XCTestCase {
                     "FrameworkE.framework",
                     "FrameworkF.framework",
                     "CarthageA.framework",
+                    "CarthageB.framework",
                     "CarthageC.framework",
                 ])
                 expectedEmbeddedFrameworks[iosFrameworkB.name] = Set([
@@ -623,10 +624,12 @@ class ProjectGeneratorTests: XCTestCase {
                     "FrameworkZ.framework",
                     "FrameworkX.framework",
                     "CarthageZ.framework",
+                    "FrameworkF.framework",
                     "FrameworkC.framework",
                     iosFrameworkB.filename,
                     "FrameworkD.framework",
                     "CarthageA.framework",
+                    "CarthageB.framework",
                     "CarthageD.framework",
                 ])
                 expectedEmbeddedFrameworks[appTest.name] = Set([
@@ -710,7 +713,7 @@ class ProjectGeneratorTests: XCTestCase {
                     if !expectedLinkedFiles.isEmpty {
                         let linkFrameworks = (frameworkPhases[0].files ?? [])
                             .compactMap { $0.file?.nameOrPath }
-                        try expect(Set(linkFrameworks)) == expectedLinkedFiles
+                        try expect(Array(Set(linkFrameworks)).sorted()) == Array(expectedLinkedFiles).sorted()
                     }
 
                     var expectedCopyFilesPhasesCount = 0