Merge pull request #383 from yonaskolb/transitive-aggragate-targets

Fix aggregate target dependencies with `transitivelyLinkDependencies`
yonaskolb · Aug 22, 2018 · 6a0c8c8 · 6a0c8c8
2 parents 8b0cd5d + 5693717
commit 6a0c8c8
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 
 #### Fixed
 - Fixed `FRAMEWORK_SEARCH_PATHS` for `framework` dependency paths with spaces [382](https://github.com/yonaskolb/XcodeGen/pull/382) @brentleyjones
+- Fixed aggregate targets not being found with `transitivelyLinkDependencies` [383](https://github.com/yonaskolb/XcodeGen/pull/383) @brentleyjones
 
 [Commits](https://github.com/yonaskolb/XcodeGen/compare/1.11.0...1.11.1)
 

diff --git a/Sources/ProjectSpec/Project.swift b/Sources/ProjectSpec/Project.swift
@@ -14,7 +14,11 @@ public struct Project: BuildSettingsContainer {
         }
     }
 
-    public var aggregateTargets: [AggregateTarget]
+    public var aggregateTargets: [AggregateTarget] {
+        didSet {
+            aggregateTargetsMap = Dictionary(uniqueKeysWithValues: aggregateTargets.map { ($0.name, $0) })
+        }
+    }
 
     public var settings: Settings
     public var settingGroups: [String: Settings]
@@ -25,7 +29,9 @@ public struct Project: BuildSettingsContainer {
     public var fileGroups: [String]
     public var configFiles: [String: String]
     public var include: [String] = []
+
     private var targetsMap: [String: Target]
+    private var aggregateTargetsMap: [String: AggregateTarget]
 
     public init(
         basePath: Path,
@@ -46,6 +52,7 @@ public struct Project: BuildSettingsContainer {
         self.targets = targets
         targetsMap = Dictionary(uniqueKeysWithValues: self.targets.map { ($0.name, $0) })
         self.aggregateTargets = aggregateTargets
+        aggregateTargetsMap = Dictionary(uniqueKeysWithValues: self.aggregateTargets.map { ($0.name, $0) })
         self.configs = configs
         self.settings = settings
         self.settingGroups = settingGroups
@@ -59,9 +66,13 @@ public struct Project: BuildSettingsContainer {
     public func getTarget(_ targetName: String) -> Target? {
         return targetsMap[targetName]
     }
+
+    public func getAggregateTarget(_ targetName: String) -> AggregateTarget? {
+        return aggregateTargetsMap[targetName]
+    }
 
     public func getProjectTarget(_ targetName: String) -> ProjectTarget? {
-        return targetsMap[targetName] ?? aggregateTargets.first { $0.name == targetName }
+        return targetsMap[targetName] ?? aggregateTargetsMap[targetName]
     }
 
     public func getConfig(_ configName: String) -> Config? {
@@ -138,6 +149,7 @@ extension Project {
             options = SpecOptions()
         }
         targetsMap = Dictionary(uniqueKeysWithValues: targets.map { ($0.name, $0) })
+        aggregateTargetsMap = Dictionary(uniqueKeysWithValues: aggregateTargets.map { ($0.name, $0) })
     }
 
     static func resolveProject(jsonDictionary: JSONDictionary) throws -> JSONDictionary {

diff --git a/Sources/XcodeGenKit/PBXProjGenerator.swift b/Sources/XcodeGenKit/PBXProjGenerator.swift
@@ -910,37 +910,45 @@ public class PBXProjGenerator {
         return "\(carthagePath)/\(platformName)"
     }
 
-    func getAllCarthageDependencies(target: Target) -> [Dependency] {
+    func getAllCarthageDependencies(target topLevelTarget: Target) -> [Dependency] {
         // this is used to resolve cyclical target dependencies
         var visitedTargets: Set<String> = []
         var frameworks: [String: Dependency] = [:]
 
-        var queue: [Target] = [target]
+        var queue: [ProjectTarget] = [topLevelTarget]
         while !queue.isEmpty {
-            let target = queue.removeFirst()
-            if visitedTargets.contains(target.name) {
+            let projectTarget = queue.removeFirst()
+            if visitedTargets.contains(projectTarget.name) {
                 continue
             }
-
-            for dependency in target.dependencies {
-                // don't overwrite frameworks, to allow top level ones to rule
-                if frameworks.contains(reference: dependency.reference) {
-                    continue
+
+            if let target = projectTarget as? Target {
+                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[dependency.reference] = dependency
+                    case .target:
+                        if let projectTarget = project.getProjectTarget(dependency.reference) {
+                            queue.append(projectTarget)
+                        }
+                    default:
+                        break
+                    }
                 }
-
-                switch dependency.type {
-                case .carthage:
-                    frameworks[dependency.reference] = dependency
-                case .target:
-                    if let target = project.getTarget(dependency.reference) {
-                        queue.append(target)
+            } else if let aggregateTarget = projectTarget as? AggregateTarget {
+                for dependencyName in aggregateTarget.targets {
+                    if let projectTarget = project.getProjectTarget(dependencyName) {
+                        queue.append(projectTarget)
                     }
-                default:
-                    break
                 }
             }
 
-            visitedTargets.update(with: target.name)
+            visitedTargets.update(with: projectTarget.name)
         }
 
         return frameworks.sorted(by: { $0.key < $1.key }).map { $0.value }
@@ -973,13 +981,16 @@ public class PBXProjGenerator {
                         dependencies[dependency.reference] = dependency
                     }
                 case .target:
-                    if let dependencyTarget = project.getTarget(dependency.reference) {
-                        if isTopLevel || dependency.embed == nil {
+                    if isTopLevel || dependency.embed == nil {
+                        if let dependencyTarget = project.getTarget(dependency.reference) {
                             dependencies[dependency.reference] = dependency
                             if !dependencyTarget.shouldEmbedDependencies {
                                 // traverse target's dependencies if it doesn't embed them itself
                                 queue.append(dependencyTarget)
                             }
+                        } else if project.getAggregateTarget(dependency.reference) != nil {
+                            // Aggregate targets should be included
+                            dependencies[dependency.reference] = dependency
                         }
                     }
                 }

diff --git a/Tests/XcodeGenKitTests/ProjectGeneratorTests.swift b/Tests/XcodeGenKitTests/ProjectGeneratorTests.swift
@@ -191,16 +191,17 @@ class ProjectGeneratorTests: XCTestCase {
         describe {
 
             let otherTarget = Target(name: "Other", type: .framework, platform: .iOS, dependencies: [Dependency(type: .target, reference: "AggregateTarget")])
+            let otherTarget2 = Target(name: "Other2", type: .framework, platform: .iOS, dependencies: [Dependency(type: .target, reference: "Other")], transitivelyLinkDependencies: true)
             let aggregateTarget = AggregateTarget(name: "AggregateTarget", targets: ["MyApp", "MyFramework"])
             let aggregateTarget2 = AggregateTarget(name: "AggregateTarget2", targets: ["AggregateTarget"])
-            let project = Project(basePath: "", name: "test", targets: [app, framework, otherTarget], aggregateTargets: [aggregateTarget, aggregateTarget2])
+            let project = Project(basePath: "", name: "test", targets: [app, framework, otherTarget, otherTarget2], aggregateTargets: [aggregateTarget, aggregateTarget2])
 
             $0.it("generates aggregate targets") {
                 let pbxProject = try project.generatePbxProj()
                 let nativeTargets = pbxProject.objects.nativeTargets.referenceValues
                 let aggregateTargets = pbxProject.objects.aggregateTargets.referenceValues
 
-                try expect(nativeTargets.count) == 3
+                try expect(nativeTargets.count) == 4
                 try expect(aggregateTargets.count) == 2
 
                 try expect(aggregateTargets[0].name) == "AggregateTarget"
@@ -209,10 +210,14 @@ class ProjectGeneratorTests: XCTestCase {
                 try expect(aggregateTargets[1].name) == "AggregateTarget2"
                 try expect(aggregateTargets[1].dependencies.count) == 1
 
+                try expect(nativeTargets[2].name) == "Other"
                 try expect(nativeTargets[2].dependencies.count) == 1
+
+                try expect(nativeTargets[3].name) == "Other2"
+                try expect(nativeTargets[3].dependencies.count) == 2
 
                 let targetDependencies = pbxProject.objects.targetDependencies.referenceValues
-                try expect(targetDependencies.count) == 5
+                try expect(targetDependencies.count) == 7
             }
         }
     }