yonaskolb · brentleyjones · Jul 31, 2018 · Jul 24, 2018 · Jul 24, 2018 · Jul 24, 2018
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@
 - Use the latest `xcdatamodel` when sorted by version [341](https://github.com/yonaskolb/XcodeGen/pull/341) @rohitpal440
 - Fixed compiler flags being set on non source files in mixed build phase target sources [347](https://github.com/yonaskolb/XcodeGen/pull/347) @brentleyjones
 - Fixed `options.xcodeVersion` not being parsed [348](https://github.com/yonaskolb/XcodeGen/pull/348) @brentleyjones
+- Fixed non-application targets using `carthage copy-frameworks` [361](https://github.com/yonaskolb/XcodeGen/pull/361) @brentleyjones
 
 #### Changed
 - Improved linking for `static.library` targets [352](https://github.com/yonaskolb/XcodeGen/pull/352) @brentleyjones

diff --git a/Docs/ProjectSpec.md b/Docs/ProjectSpec.md
@@ -172,6 +172,7 @@ Settings are merged in the following order: groups, base, configs.
 - [ ] **dependencies**: **[[Dependency](#dependency)]** - Dependencies for the target
 - [ ] **templates**: **[String]** - A list of target templates that will be merged in order
 - [ ] **transitivelyLinkDependencies**: **Bool** - If this is not specified the value from the project set in [Options](#options)`.transitivelyLinkDependencies` will be used.
+- [ ] **directlyEmbedCarthageDependencies**: **Bool** - If this is `true` Carthage dependencies will be embedded using an `Embed Frameworks` build phase instead of the `copy-frameworks` script. Defaults to `true` for all targets except iOS/tvOS/watchOS Applications.
 - [ ] **requiresObjCLinking**: **Bool** - If this is `true` any targets that link to this target will have `-ObjC` added to their `OTHER_LDFLAGS`. This is required if a static library has any catagories or extensions on Objective-C code. See [this guide](https://pewpewthespells.com/blog/objc_linker_flags.html#objc) for more details. Defaults to `true` if `type` is `library.static`. If you are 100% sure you don't have catagories or extensions on Objective-C code (pure Swift with no use of Foundation/UIKit) you can set this to `false`, otherwise it's best to leave it alone.
 - [ ] **prebuildScripts**: **[[Build Script](#build-script)]** - Build scripts that run *before* any other build phases
 - [ ] **postbuildScripts**: **[[Build Script](#build-script)]** - Build scripts that run *after* any other build phases

diff --git a/Sources/ProjectSpec/Target.swift b/Sources/ProjectSpec/Target.swift
@@ -29,6 +29,7 @@ public struct Target: ProjectTarget {
     public var sources: [TargetSource]
     public var dependencies: [Dependency]
     public var transitivelyLinkDependencies: Bool?
+    public var directlyEmbedCarthageDependencies: Bool?
     public var requiresObjCLinking: Bool?
     public var prebuildScripts: [BuildScript]
     public var postbuildScripts: [BuildScript]
@@ -62,6 +63,7 @@ public struct Target: ProjectTarget {
         sources: [TargetSource] = [],
         dependencies: [Dependency] = [],
         transitivelyLinkDependencies: Bool? = nil,
+        directlyEmbedCarthageDependencies: Bool? = nil,
         requiresObjCLinking: Bool? = nil,
         prebuildScripts: [BuildScript] = [],
         postbuildScripts: [BuildScript] = [],
@@ -79,6 +81,7 @@ public struct Target: ProjectTarget {
         self.sources = sources
         self.dependencies = dependencies
         self.transitivelyLinkDependencies = transitivelyLinkDependencies
+        self.directlyEmbedCarthageDependencies = directlyEmbedCarthageDependencies
         self.requiresObjCLinking = requiresObjCLinking
         self.prebuildScripts = prebuildScripts
         self.postbuildScripts = postbuildScripts
@@ -200,6 +203,7 @@ extension Target: Equatable {
             lhs.deploymentTarget == rhs.deploymentTarget &&
             lhs.transitivelyLinkDependencies == rhs.transitivelyLinkDependencies &&
             lhs.requiresObjCLinking == rhs.requiresObjCLinking &&
+            lhs.directlyEmbedCarthageDependencies == rhs.directlyEmbedCarthageDependencies &&
             lhs.settings == rhs.settings &&
             lhs.configFiles == rhs.configFiles &&
             lhs.sources == rhs.sources &&
@@ -272,6 +276,7 @@ extension Target: NamedJSONDictionaryConvertible {
             dependencies = try jsonDictionary.json(atKeyPath: "dependencies", invalidItemBehaviour: .fail)
         }
         transitivelyLinkDependencies = jsonDictionary.json(atKeyPath: "transitivelyLinkDependencies")
+        directlyEmbedCarthageDependencies = jsonDictionary.json(atKeyPath: "directlyEmbedCarthageDependencies")
         requiresObjCLinking = jsonDictionary.json(atKeyPath: "requiresObjCLinking")
 
         prebuildScripts = jsonDictionary.json(atKeyPath: "prebuildScripts") ?? []

diff --git a/Sources/XcodeGenKit/PBXProjGenerator.swift b/Sources/XcodeGenKit/PBXProjGenerator.swift
@@ -432,25 +432,28 @@ public class PBXProjGenerator {
         var copyResourcesReferences: [String] = []
         var copyWatchReferences: [String] = []
         var extensions: [String] = []
+        var carthageFrameworksToEmbed: [String] = []
 
         let targetDependencies = (target.transitivelyLinkDependencies ?? project.options.transitivelyLinkDependencies) ?
             getAllDependenciesPlusTransitiveNeedingEmbedding(target: target) : target.dependencies
+
+        let directlyEmbedCarthage = target.directlyEmbedCarthageDependencies ?? !(target.platform.requiresSimulatorStripping && target.type.isApp)
+
+        func getEmbedSettings(dependency: Dependency, codeSign: Bool) -> [String: Any] {
+            var embedAttributes: [String] = []
+            if codeSign {
+                embedAttributes.append("CodeSignOnCopy")
+            }
+            if dependency.removeHeaders {
+                embedAttributes.append("RemoveHeadersOnCopy")
+            }
+            return ["ATTRIBUTES": embedAttributes]
+        }
 
         for dependency in targetDependencies {
 
             let embed = dependency.embed ?? target.shouldEmbedDependencies
 
-            func getEmbedSettings(codeSign: Bool) -> [String: Any] {
-                var embedAttributes: [String] = []
-                if codeSign {
-                    embedAttributes.append("CodeSignOnCopy")
-                }
-                if dependency.removeHeaders {
-                    embedAttributes.append("RemoveHeadersOnCopy")
-                }
-                return ["ATTRIBUTES": embedAttributes]
-            }
-
             switch dependency.type {
             case .target:
                 let dependencyTargetName = dependency.reference
@@ -485,7 +488,7 @@ public class PBXProjGenerator {
                         id: dependencyFileReference + target.name,
                         PBXBuildFile(
                             fileRef: dependencyFileReference,
-                            settings: getEmbedSettings(codeSign: dependency.codeSign ?? !dependencyTarget.type.isExecutable)
+                            settings: getEmbedSettings(dependency: dependency, codeSign: dependency.codeSign ?? !dependencyTarget.type.isExecutable)
                         )
                     )
 
@@ -531,7 +534,7 @@ public class PBXProjGenerator {
                 if embed {
                     let embedFile = createObject(
                         id: fileReference + target.name,
-                        PBXBuildFile(fileRef: fileReference, settings: getEmbedSettings(codeSign: dependency.codeSign ?? true))
+                        PBXBuildFile(fileRef: fileReference, settings: getEmbedSettings(dependency: dependency, codeSign: dependency.codeSign ?? true))
                     )
                     copyFrameworksReferences.append(embedFile.reference)
                 }
@@ -554,12 +557,32 @@ public class PBXProjGenerator {
                 carthageFrameworksByPlatform[target.platform.carthageDirectoryName, default: []].insert(fileReference)
 
                 targetFrameworkBuildFiles.append(buildFile.reference)
-                if target.platform == .macOS && embed {
+
+                // Embedding handled by iterating over `carthageDependencies` below
+            }
+        }
+
+        for dependency in carthageDependencies {
+            guard target.type != .staticLibrary else { break }
+
+            let embed = dependency.embed ?? target.shouldEmbedDependencies
+
+            var platformPath = Path(getCarthageBuildPath(platform: target.platform))
+            var frameworkPath = platformPath + dependency.reference
+            if frameworkPath.extension == nil {
+                frameworkPath = Path(frameworkPath.string + ".framework")
+            }
+            let fileReference = sourceGenerator.getFileReference(path: frameworkPath, inPath: platformPath)
+
+            if embed {
+                if directlyEmbedCarthage {
                     let embedFile = createObject(
                         id: fileReference + target.name,
-                        PBXBuildFile(fileRef: fileReference, settings: getEmbedSettings(codeSign: dependency.codeSign ?? true))
+                        PBXBuildFile(fileRef: fileReference, settings: getEmbedSettings(dependency: dependency, codeSign: dependency.codeSign ?? true))
                     )
                     copyFrameworksReferences.append(embedFile.reference)
+                } else  {
+                    carthageFrameworksToEmbed.append(dependency.reference)
                 }
             }
         }
@@ -652,15 +675,8 @@ public class PBXProjGenerator {
 
             buildPhases.append(copyFilesPhase.reference)
         }
-
-        let carthageFrameworksToEmbed = Array(Set(
-            carthageDependencies
-                .filter { $0.embed ?? target.shouldEmbedDependencies }
-                .map { $0.reference }
-        ))
-            .sorted()
-
-        if !carthageFrameworksToEmbed.isEmpty && target.platform != .macOS {
+
+        if !carthageFrameworksToEmbed.isEmpty {
 
             let inputPaths = carthageFrameworksToEmbed
                 .map { "$(SRCROOT)/\(carthageBuildPath)/\(target.platform)/\($0)\($0.contains(".") ? "" : ".framework")" }
@@ -820,7 +836,7 @@ public class PBXProjGenerator {
     func getAllCarthageDependencies(target: Target) -> [Dependency] {
         // this is used to resolve cyclical target dependencies
         var visitedTargets: Set<String> = []
-        var frameworks: [Dependency] = []
+        var frameworks: [String: Dependency] = [:]
 
         var queue: [Target] = [target]
         while !queue.isEmpty {
@@ -830,9 +846,14 @@ public class PBXProjGenerator {
             }
 
             for dependency in target.dependencies {
+                // don't overwrite frameworks, to allow top level ones to rule
+                if frameworks.contains(reference: dependency.reference) {
+                    continue
+                }
+
                 switch dependency.type {
                 case .carthage:
-                    frameworks.append(dependency)
+                    frameworks[dependency.reference] = dependency
                 case .target:
                     if let target = project.getTarget(dependency.reference) {
                         queue.append(target)
@@ -845,7 +866,7 @@ public class PBXProjGenerator {
             visitedTargets.update(with: target.name)
         }
 
-        return frameworks
+        return frameworks.sorted(by: { $0.key < $1.key }).map { $0.value }
     }
 
     func getAllDependenciesPlusTransitiveNeedingEmbedding(target topLevelTarget: Target) -> [Dependency] {
@@ -901,6 +922,18 @@ extension Target {
     }
 }
 
+extension Platform {
+    /// - returns: `true` for platforms that the app store requires simulator slices to be stripped.
+    public var requiresSimulatorStripping: Bool {
+        switch self {
+        case .iOS, .tvOS, .watchOS:
+            return true
+        case .macOS:
+            return false
+        }
+    }
+}
+
 extension PBXFileElement {
 
     public func getSortOrder(groupSortPosition: SpecOptions.GroupSortPosition) -> Int {

diff --git a/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj b/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj
@@ -25,6 +25,7 @@
 /* Begin PBXBuildFile section */
 		BF_130062884703 /* Alamofire.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = FR_257516580010 /* Alamofire.framework */; };
 		BF_138356261076 /* InterfaceController.swift in Sources */ = {isa = PBXBuildFile; fileRef = FR_363921640403 /* InterfaceController.swift */; };
+		BF_144945303317 /* Alamofire.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = FR_410645050443 /* Alamofire.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		BF_167705969896 /* TestProjectUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FR_145531354566 /* TestProjectUITests.swift */; };
 		BF_182635022050 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = FR_830053537293 /* Assets.xcassets */; };
 		BF_184447863946 = {isa = PBXBuildFile; fileRef = FR_935153865209 /* iMessageApp.app */; };
@@ -50,6 +51,7 @@
 		BF_337656089700 = {isa = PBXBuildFile; fileRef = FR_399755008402 /* StaticLibrary_ObjC.a */; };
 		BF_360196406184 /* TestProjectTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FR_722239415598 /* TestProjectTests.swift */; };
 		BF_425679397292 /* MyFramework.h in Headers */ = {isa = PBXBuildFile; fileRef = FR_183521624014 /* MyFramework.h */; settings = {ATTRIBUTES = (Public, ); }; };
+		BF_438950173632 /* Alamofire.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = FR_410645050443 /* Alamofire.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		BF_441538117869 /* App_watchOS Extension.appex in Embed App Extensions */ = {isa = PBXBuildFile; fileRef = FR_507023492251 /* App_watchOS Extension.appex */; settings = {ATTRIBUTES = (RemoveHeadersOnCopy, ); }; };
 		BF_447782698339 /* Alamofire.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = FR_752394658615 /* Alamofire.framework */; };
 		BF_456457948943 /* FrameworkFile.swift in Sources */ = {isa = PBXBuildFile; fileRef = FR_172952167809 /* FrameworkFile.swift */; };
@@ -204,6 +206,28 @@
 			name = "Embed Watch Content";
 			runOnlyForDeploymentPostprocessing = 0;
 		};
+		CFBP_4082141876 /* Embed Frameworks */ = {
+			isa = PBXCopyFilesBuildPhase;
+			buildActionMask = 2147483647;
+			dstPath = "";
+			dstSubfolderSpec = 10;
+			files = (
+				BF_438950173632 /* Alamofire.framework in Embed Frameworks */,
+			);
+			name = "Embed Frameworks";
+			runOnlyForDeploymentPostprocessing = 0;
+		};
+		CFBP_4626438721 /* Embed Frameworks */ = {
+			isa = PBXCopyFilesBuildPhase;
+			buildActionMask = 2147483647;
+			dstPath = "";
+			dstSubfolderSpec = 10;
+			files = (
+				BF_144945303317 /* Alamofire.framework in Embed Frameworks */,
+			);
+			name = "Embed Frameworks";
+			runOnlyForDeploymentPostprocessing = 0;
+		};
 		CFBP_4684049960 /* Embed App Extensions */ = {
 			isa = PBXCopyFilesBuildPhase;
 			buildActionMask = 2147483647;
@@ -671,7 +695,7 @@
 			buildConfigurationList = CL_123503999387 /* Build configuration list for PBXNativeTarget "App_iOS_UITests" */;
 			buildPhases = (
 				SBP_12350399938 /* Sources */,
-				SSBP_3150067808 /* Carthage */,
+				CFBP_4082141876 /* Embed Frameworks */,
 			);
 			buildRules = (
 			);
@@ -832,7 +856,7 @@
 			buildConfigurationList = CL_783122899910 /* Build configuration list for PBXNativeTarget "App_iOS_Tests" */;
 			buildPhases = (
 				SBP_78312289991 /* Sources */,
-				SSBP_6451525835 /* Carthage */,
+				CFBP_4626438721 /* Embed Frameworks */,
 			);
 			buildRules = (
 			);
@@ -1006,22 +1030,6 @@
 			shellPath = /bin/sh;
 			shellScript = "echo \"You ran a script\"\n";
 		};
-		SSBP_3150067808 /* Carthage */ = {
-			isa = PBXShellScriptBuildPhase;
-			buildActionMask = 2147483647;
-			files = (
-			);
-			inputPaths = (
-				"$(SRCROOT)/Carthage/Build/iOS/Alamofire.framework",
-			);
-			name = Carthage;
-			outputPaths = (
-				"$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Alamofire.framework",
-			);
-			runOnlyForDeploymentPostprocessing = 0;
-			shellPath = /bin/sh;
-			shellScript = "carthage copy-frameworks\n";
-		};
 		SSBP_3886691194 /* MyScript */ = {
 			isa = PBXShellScriptBuildPhase;
 			buildActionMask = 2147483647;
@@ -1080,22 +1088,6 @@
 			shellPath = /bin/sh;
 			shellScript = "echo \"You ran a script\"\n";
 		};
-		SSBP_6451525835 /* Carthage */ = {
-			isa = PBXShellScriptBuildPhase;
-			buildActionMask = 2147483647;
-			files = (
-			);
-			inputPaths = (
-				"$(SRCROOT)/Carthage/Build/iOS/Alamofire.framework",
-			);
-			name = Carthage;
-			outputPaths = (
-				"$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/Alamofire.framework",
-			);
-			runOnlyForDeploymentPostprocessing = 0;
-			shellPath = /bin/sh;
-			shellScript = "carthage copy-frameworks\n";
-		};
 		SSBP_8106229290 /* Carthage */ = {
 			isa = PBXShellScriptBuildPhase;
 			buildActionMask = 2147483647;

diff --git a/Tests/XcodeGenKitTests/ProjectGeneratorTests.swift b/Tests/XcodeGenKitTests/ProjectGeneratorTests.swift
@@ -467,6 +467,7 @@ class ProjectGeneratorTests: XCTestCase {
                 ])
                 expectedEmbeddedFrameworks[iosFrameworkB.name] = Set([
                     "FrameworkE.framework",
+                    "CarthageC.framework",
                 ])
 
                 let appTest = Target(
@@ -477,7 +478,8 @@ class ProjectGeneratorTests: XCTestCase {
                         Dependency(type: .target, reference: app.name),
                         Dependency(type: .target, reference: iosFrameworkB.name),
                         Dependency(type: .carthage, reference: "CarthageD"),
-                    ]
+                    ],
+                    directlyEmbedCarthageDependencies: false
                 )
                 expectedResourceFiles[appTest.name] = Set([
                     resourceBundle.filename,