Handle imports with same relative path (#1262)

* Added test for when includes contain relative paths with the same name.

* Updated mergedDictionary to handle includes with the same file path.

* Removed the need to multiple places to pass in cachedSpecFiles.

* Converts the projectRoot into a absolute path before loading a project.

* Updated changelog.

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Next Version
 
+### Fixed
+
+- Fix includes when the projectRoot is a realtive path #1262 @CraigSiemens
+
 ## 2.33.0
 
 ### Added

Sources/ProjectSpec/Project.swift

@@ -155,14 +155,7 @@ extension Project: Equatable {
 extension Project {
 
     public init(path: Path) throws {
-        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)
+        let spec = try SpecFile(path: path)
         try self.init(spec: spec)
     }
 

Sources/ProjectSpec/SpecFile.swift

@@ -4,11 +4,17 @@ import PathKit
 import Yams
 
 public struct SpecFile {
+    /// For the root spec, this is the folder containing the SpecFile. For subSpecs this is the path
+    /// to the folder of the parent spec that is including this SpecFile.
     public let basePath: Path
-    public let relativePath: Path
     public let jsonDictionary: JSONDictionary
     public let subSpecs: [SpecFile]
 
+    /// The relative path to use when resolving paths in the json dictionary. Is an empty path when
+    /// included with relativePaths disabled.
+    private let relativePath: Path
+    
+    /// The path to the file relative to the basePath.
     private let filePath: Path
 
     fileprivate struct Include {
@@ -47,11 +53,21 @@ public struct SpecFile {
             dictionary[key] as? Bool ?? (dictionary[key] as? NSString)?.boolValue
         }
     }
-
-    public init(path: Path, cachedSpecFiles: inout [Path: SpecFile], variables: [String: String] = [:]) throws {
-        try self.init(filePath: path, basePath: path.parent(), cachedSpecFiles: &cachedSpecFiles, variables: variables)
+    
+    /// Create a SpecFile for a Project
+    /// - Parameters:
+    ///   - path: The absolute path to the spec file
+    ///   - projectRoot: The root of the project to use as the base path. When nil, uses the parent
+    ///     of the path.
+    public init(path: Path, projectRoot: Path? = nil, variables: [String: String] = [:]) throws {
+        let basePath = projectRoot ?? path.parent()
+        let filePath = try path.relativePath(from: basePath)
+        var cachedSpecFiles: [Path: SpecFile] = [:]
+        
+        try self.init(filePath: filePath, basePath: basePath, cachedSpecFiles: &cachedSpecFiles, variables: variables)
     }
-
+    
+    /// Memberwise initializer for SpecFile
     public init(filePath: Path, jsonDictionary: JSONDictionary, basePath: Path = "", relativePath: Path = "", subSpecs: [SpecFile] = []) {
         self.basePath = basePath
         self.relativePath = relativePath
@@ -63,26 +79,28 @@ public struct SpecFile {
     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: includePath, basePath: basePath, cachedSpecFiles: &cachedSpecFiles, variables: variables, relativePath: relativePath)
+        try self.init(filePath: include.path, basePath: basePath, cachedSpecFiles: &cachedSpecFiles, variables: variables, relativePath: relativePath)
     }
 
-    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)
+    private init(filePath: Path, basePath: Path, cachedSpecFiles: inout [Path: SpecFile], variables: [String: String], relativePath: Path = "") throws {
+        let path = basePath + filePath
+        if let specFile = cachedSpecFiles[path] {
+            self = specFile
+            return
+        }
+
+        let jsonDictionary = try SpecFile.loadDictionary(path: path).expand(variables: variables)
+
         let includes = Include.parse(json: jsonDictionary["include"])
         let subSpecs: [SpecFile] = try includes
             .filter(\.enable)
             .map { include in
-                if let specFile = cachedSpecFiles[filePath] {
-                    return specFile
-                } else {
-                    return try SpecFile(include: include, basePath: basePath, relativePath: relativePath, cachedSpecFiles: &cachedSpecFiles, variables: variables)
-                }
+                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
+        cachedSpecFiles[path] = self
     }
 
     static func loadDictionary(path: Path) throws -> JSONDictionary {
@@ -107,24 +125,24 @@ public struct SpecFile {
         var cachedSpecFiles: [Path: SpecFile] = [:]
         let resolvedSpec = resolvingPaths(cachedSpecFiles: &cachedSpecFiles)
 
-        var value = Set<String>()
-        return resolvedSpec.mergedDictionary(set: &value)
+        var mergedSpecPaths = Set<Path>()
+        return resolvedSpec.mergedDictionary(set: &mergedSpecPaths)
     }
 
-    func mergedDictionary(set mergedTargets: inout Set<String>) -> JSONDictionary {
-        let name = filePath.description
+    private func mergedDictionary(set mergedSpecPaths: inout Set<Path>) -> JSONDictionary {
+        let path = basePath + filePath
 
-        guard !mergedTargets.contains(name) else { return [:] }
-        mergedTargets.insert(name)
+        guard mergedSpecPaths.insert(path).inserted else { return [:] }
 
         return jsonDictionary.merged(onto:
             subSpecs
-                .map { $0.mergedDictionary(set: &mergedTargets) }
+                .map { $0.mergedDictionary(set: &mergedSpecPaths) }
                 .reduce([:]) { $1.merged(onto: $0) })
     }
 
-    func resolvingPaths(cachedSpecFiles: inout [Path: SpecFile], relativeTo basePath: Path = Path()) -> SpecFile {
-        if let cachedSpecFile = cachedSpecFiles[filePath] {
+    private func resolvingPaths(cachedSpecFiles: inout [Path: SpecFile], relativeTo basePath: Path = Path()) -> SpecFile {
+        let path = basePath + filePath
+        if let cachedSpecFile = cachedSpecFiles[path] {
             return cachedSpecFile
         }
 
@@ -137,10 +155,11 @@ public struct SpecFile {
         let specFile = SpecFile(
             filePath: filePath,
             jsonDictionary: jsonDictionary,
+            basePath: self.basePath,
             relativePath: self.relativePath,
             subSpecs: subSpecs.map { $0.resolvingPaths(cachedSpecFiles: &cachedSpecFiles, relativeTo: relativePath) }
         )
-        cachedSpecFiles[filePath] = specFile
+        cachedSpecFiles[path] = specFile
         return specFile
     }
 }

Sources/ProjectSpec/SpecLoader.swift

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

Tests/Fixtures/paths_test/included_paths_test.yml

@@ -1,4 +1,6 @@
-include: recursive_test/recursive_test.yml
+include:
+  - recursive_test/recursive_test.yml
+  - same_relative_path_test/same_relative_path_test.yml
 configFiles:
   IncludedConfig: config
 projectReferences:

Tests/Fixtures/paths_test/same_relative_path_test/parent1/parent1.yml

@@ -0,0 +1,2 @@
+include:
+- same/same.yml

Tests/Fixtures/paths_test/same_relative_path_test/parent1/same/same.yml

@@ -0,0 +1,2 @@
+include:
+- target1/target1.yml

Tests/Fixtures/paths_test/same_relative_path_test/parent1/same/target1/target1.yml

@@ -0,0 +1,6 @@
+targets:
+  target1:
+    type: framework
+    platform: macOS
+    sources:
+    - source

Tests/Fixtures/paths_test/same_relative_path_test/parent2/parent2.yml

@@ -0,0 +1,2 @@
+include:
+- same/same.yml

Tests/Fixtures/paths_test/same_relative_path_test/parent2/same/same.yml

@@ -0,0 +1,2 @@
+include:
+- target2/target2.yml

Tests/Fixtures/paths_test/same_relative_path_test/parent2/same/target2/target2.yml

@@ -0,0 +1,6 @@
+targets:
+  target2:
+    type: framework
+    platform: macOS
+    sources:
+    - source

Tests/Fixtures/paths_test/same_relative_path_test/same_relative_path_test.yml

@@ -0,0 +1,12 @@
+include:
+- parent1/parent1.yml
+- parent2/parent2.yml
+targets:
+  app:
+    type: application
+    platform: macOS
+    sources:
+    - source
+    dependencies:
+    - target: target1
+    - target: target2

Tests/PerformanceTests/PerformanceTests.swift

@@ -13,11 +13,9 @@ 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,
-                                     cachedSpecFiles: &cachedSpecFiles,
                                      variables: ProcessInfo.processInfo.environment)
             _ = spec.resolvedDictionary()
         }

Tests/ProjectSpecTests/SpecLoadingTests.swift

@@ -162,6 +162,28 @@ class SpecLoadingTests: XCTestCase {
                         postCompileScripts: [BuildScript(script: .path("paths_test/recursive_test/postCompileScript"))],
                         postBuildScripts: [BuildScript(script: .path("paths_test/recursive_test/postBuildScript"))]
                     ),
+                    Target(
+                        name: "app",
+                        type: .application,
+                        platform: .macOS,
+                        sources: ["paths_test/same_relative_path_test/source"],
+                        dependencies: [
+                            Dependency(type: .target, reference: "target1"),
+                            Dependency(type: .target, reference: "target2")
+                        ]
+                    ),
+                    Target(
+                        name: "target1",
+                        type: .framework,
+                        platform: .macOS,
+                        sources: ["paths_test/same_relative_path_test/parent1/same/target1/source"]
+                    ),
+                    Target(
+                        name: "target2",
+                        type: .framework,
+                        platform: .macOS,
+                        sources: ["paths_test/same_relative_path_test/parent2/same/target2/source"]
+                    )
                 ]
 
                 try expect(project.schemes) == [