Fix includes related issues and improve their performances (#1275)

* Fix recursive include path when relativePath is not set

If relativePath is not set on a particular include, the first level of
include will currently work, but starting at the second level of
iteration, the computed include path will fail as relativePath will be
appended over and over onto the filePath. We're fixing that recursion
problem here and adding the corresponding tests to make sure it doesn't
happen again.

* Include projectRoot in include paths

The projectRoot setting (when specified) is currently ignored when
computing the include paths. We're fixing that in that commit.

* Use memoization during recursive SpecFiles creation

SpecFile objects are created by recursive through includes. On a large
project with programatically generated SpecFile, it is not rare to have
hundreds of SpecFiles, creating a large web of include dependencies.
In such a case, it is not rare either for a particular SpecFile to be
included by multiple other SpecFiles. When that happens, XcodeGen
currently creates a SpecFile object every time a SpecFile gets included,
which can lead to an exponential growth of includes.

I have seen hundreds of files being turned into hundred of thousands of
SpecFile object creations, which leads to an impractical XcodeGen run of
tens of minutes.

This change adds memoization during SpecFile recursion, in order to
reuse the previously created SpecFiles, if available, instead of
re-creating them.

* Update CHANGELOG.md

Add the following changes to the changelog:
* b97bdc4 - Use memoization during recursive SpecFiles creation
* a6b96ad - Include projectRoot in include paths
* 557b074 - Fix recursive include path when relativePath is not set

CHANGELOG.md

@@ -7,10 +7,13 @@
 - Added support for `enableGPUFrameCaptureMode` #1251 @bsudekum
 - Config setting presets can now also be loaded from the main bundle when bundling XcodeGenKit #1135 @SofteqDG
 - Added ability to generate multiple projects in one XcodeGen launch #1270 @skofgar
+- Use memoization during recursive SpecFiles creation. This provides a drastic performance boost with lots of recursive includes #1275 @ma-oli
 
 ### Fixed
 
 - Fix scheme not being generated for aggregate targets #1250 @CraigSiemens
+- Fix recursive include path when relativePath is not set #1275 @ma-oli
+- Include projectRoot in include paths #1275 @ma-oli
 
 ## 2.32.0
 

Sources/ProjectSpec/Project.swift

@@ -155,7 +155,14 @@ extension Project: Equatable {
 extension Project {
 
     public init(path: Path) throws {
-        let spec = try SpecFile(path: path)
+        var cachedSpecFiles: [Path: SpecFile] = [:]
+        let spec = try SpecFile(filePath: path, basePath: path.parent(), cachedSpecFiles: &cachedSpecFiles)
+        try self.init(spec: spec)
+    }
+
+    public init(path: Path, basePath: Path) throws {
+        var cachedSpecFiles: [Path: SpecFile] = [:]
+        let spec = try SpecFile(filePath: path, basePath: basePath, cachedSpecFiles: &cachedSpecFiles)
         try self.init(spec: spec)
     }
 

Sources/ProjectSpec/SpecFile.swift

@@ -48,8 +48,8 @@ public struct SpecFile {
         }
     }
 
-    public init(path: Path, variables: [String: String] = [:]) throws {
-        try self.init(filePath: path, basePath: path.parent(), variables: variables)
+    public init(path: Path, cachedSpecFiles: inout [Path: SpecFile], variables: [String: String] = [:]) throws {
+        try self.init(filePath: path, basePath: path.parent(), cachedSpecFiles: &cachedSpecFiles, variables: variables)
     }
 
     public init(filePath: Path, jsonDictionary: JSONDictionary, basePath: Path = "", relativePath: Path = "", subSpecs: [SpecFile] = []) {
@@ -60,25 +60,29 @@ public struct SpecFile {
         self.filePath = filePath
     }
 
-    private init(include: Include, basePath: Path, relativePath: Path, variables: [String: String]) throws {
-        let basePath = include.relativePaths ? (basePath + relativePath) : (basePath + relativePath + include.path.parent())
+    private init(include: Include, basePath: Path, relativePath: Path, cachedSpecFiles: inout [Path: SpecFile], variables: [String: String]) throws {
+        let basePath = include.relativePaths ? (basePath + relativePath) : basePath
         let relativePath = include.relativePaths ? include.path.parent() : Path()
+        let includePath = include.relativePaths ? basePath + relativePath + include.path.lastComponent : basePath + include.path
 
-        try self.init(filePath: include.path, basePath: basePath, variables: variables, relativePath: relativePath)
+        try self.init(filePath: includePath, basePath: basePath, cachedSpecFiles: &cachedSpecFiles, variables: variables, relativePath: relativePath)
     }
 
-    private init(filePath: Path, basePath: Path, variables: [String: String], relativePath: Path = "") throws {
-        let path = basePath + relativePath + filePath.lastComponent
-        let jsonDictionary = try SpecFile.loadDictionary(path: path).expand(variables: variables)
-
+    public init(filePath: Path, basePath: Path, cachedSpecFiles: inout [Path: SpecFile], variables: [String: String] = [:], relativePath: Path = "") throws {
+        let jsonDictionary = try SpecFile.loadDictionary(path: filePath).expand(variables: variables)
         let includes = Include.parse(json: jsonDictionary["include"])
         let subSpecs: [SpecFile] = try includes
             .filter(\.enable)
             .map { include in
-                try SpecFile(include: include, basePath: basePath, relativePath: relativePath, variables: variables)
+                if let specFile = cachedSpecFiles[filePath] {
+                    return specFile
+                } else {
+                    return try SpecFile(include: include, basePath: basePath, relativePath: relativePath, cachedSpecFiles: &cachedSpecFiles, variables: variables)
+                }
             }
 
         self.init(filePath: filePath, jsonDictionary: jsonDictionary, basePath: basePath, relativePath: relativePath, subSpecs: subSpecs)
+        cachedSpecFiles[filePath] = self
     }
 
     static func loadDictionary(path: Path) throws -> JSONDictionary {
@@ -100,7 +104,8 @@ public struct SpecFile {
     }
 
     private func resolvedDictionaryWithUniqueTargets() -> JSONDictionary {
-        let resolvedSpec = resolvingPaths()
+        var cachedSpecFiles: [Path: SpecFile] = [:]
+        let resolvedSpec = resolvingPaths(cachedSpecFiles: &cachedSpecFiles)
 
         var value = Set<String>()
         return resolvedSpec.mergedDictionary(set: &value)
@@ -118,19 +123,25 @@ public struct SpecFile {
                 .reduce([:]) { $1.merged(onto: $0) })
     }
 
-    func resolvingPaths(relativeTo basePath: Path = Path()) -> SpecFile {
+    func resolvingPaths(cachedSpecFiles: inout [Path: SpecFile], relativeTo basePath: Path = Path()) -> SpecFile {
+        if let cachedSpecFile = cachedSpecFiles[filePath] {
+            return cachedSpecFile
+        }
+
         let relativePath = (basePath + self.relativePath).normalize()
         guard relativePath != Path() else {
             return self
         }
 
         let jsonDictionary = Project.pathProperties.resolvingPaths(in: self.jsonDictionary, relativeTo: relativePath)
-        return SpecFile(
+        let specFile = SpecFile(
             filePath: filePath,
             jsonDictionary: jsonDictionary,
             relativePath: self.relativePath,
-            subSpecs: subSpecs.map { $0.resolvingPaths(relativeTo: relativePath) }
+            subSpecs: subSpecs.map { $0.resolvingPaths(cachedSpecFiles: &cachedSpecFiles, relativeTo: relativePath) }
         )
+        cachedSpecFiles[filePath] = specFile
+        return specFile
     }
 }
 

Sources/ProjectSpec/SpecLoader.swift

@@ -16,7 +16,8 @@ public class SpecLoader {
     }
 
     public func loadProject(path: Path, projectRoot: Path? = nil, variables: [String: String] = [:]) throws -> Project {
-        let spec = try SpecFile(path: path, variables: variables)
+        var cachedSpecFiles: [Path: SpecFile] = [:]
+        let spec = try SpecFile(filePath: path, basePath: projectRoot ?? path.parent(), cachedSpecFiles: &cachedSpecFiles, variables: variables)
         let resolvedDictionary = spec.resolvedDictionary()
         let project = try Project(basePath: projectRoot ?? spec.basePath, jsonDictionary: resolvedDictionary)
 

Tests/Fixtures/legacy_paths_test/legacy_included_paths_test.yml

@@ -1,5 +1,8 @@
 configFiles:
   IncludedConfig: config
+include:
+  - path: legacy_paths_test/recursive_include.yml
+    relativePaths: false
 options:
   carthageBuildPath: carthage_build
   carthageExecutablePath: carthage_executable
@@ -9,8 +12,6 @@ targets:
     platform: tvOS
     configFiles:
       Config: config
-    sources:
-      - source
     dependencies:
       - framework: Framework
     info:

Tests/Fixtures/legacy_paths_test/recursive_include.yml

@@ -0,0 +1,4 @@
+targets:
+  IncludedTarget:
+    sources:
+      - source

Tests/PerformanceTests/PerformanceTests.swift

@@ -13,9 +13,12 @@ class GeneratedPerformanceTests: XCTestCase {
         let project = try Project.testProject(basePath: basePath)
         let specPath = basePath + "project.yaml"
         try dumpYamlDictionary(project.toJSONDictionary(), path: specPath)
+        var cachedSpecFiles: [Path: SpecFile] = [:]
 
         measure {
-            let spec = try! SpecFile(path: specPath, variables: ProcessInfo.processInfo.environment)
+            let spec = try! SpecFile(path: specPath,
+                                     cachedSpecFiles: &cachedSpecFiles,
+                                     variables: ProcessInfo.processInfo.environment)
             _ = spec.resolvedDictionary()
         }
     }