Make sure to create parent group structure for local packages (#1417)

* Make sure to create parent group structure for local packages * Remove redundant localPackageGroup variable --------- Co-authored-by: Jaap Manenschijn <[email protected]>
yonaskolb · Feb 14, 2024 · 5413909 · 5413909
1 parent 19109ac
commit 5413909
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@
 
 - Added support for String Catalogs (`.xcstrings`) #1421 @nicolasbosi95
 
+- Fixed custom local package groups not being created #1416 @JaapManenschijn
+
 ## 2.38.0
 
 ### Added

diff --git a/Sources/XcodeGenKit/SourceGenerator.swift b/Sources/XcodeGenKit/SourceGenerator.swift
@@ -18,7 +18,6 @@ class SourceGenerator {
     private var fileReferencesByPath: [String: PBXFileElement] = [:]
     private var groupsByPath: [Path: PBXGroup] = [:]
     private var variantGroupsByPath: [Path: PBXVariantGroup] = [:]
-    private var localPackageGroup: PBXGroup?
 
     private let project: Project
     let pbxProj: PBXProj
@@ -54,19 +53,11 @@ class SourceGenerator {
     }
 
     func createLocalPackage(path: Path, group: Path?) throws {
-        var pbxGroup: PBXGroup?
-
-        if let location = group {
-            let fullLocationPath = project.basePath + location
-            pbxGroup = getGroup(path: fullLocationPath, mergingChildren: [], createIntermediateGroups: true, hasCustomParent: false, isBaseGroup: true)
-        }
-
-        if localPackageGroup == nil && group == nil {
-            let groupName = project.options.localPackagesGroup ?? "Packages"
-            localPackageGroup = addObject(PBXGroup(sourceTree: .sourceRoot, name: groupName))
-            rootGroups.insert(localPackageGroup!)
+        var parentGroup: String = project.options.localPackagesGroup ?? "Packages"
+        if let group {
+          parentGroup = group.string
         }
-        
+
         let absolutePath = project.basePath + path.normalize()
 
         // Get the local package's relative path from the project root
@@ -80,11 +71,9 @@ class SourceGenerator {
                 path: fileReferencePath
             )
         )
-        if let pbxGroup = pbxGroup {
-            pbxGroup.children.append(fileReference)
-        } else {
-            localPackageGroup!.children.append(fileReference)
-        }
+
+        let parentGroups = parentGroup.components(separatedBy: "/")
+        createParentGroups(parentGroups, for: fileReference)
     }
 
     /// Collects an array complete of all `SourceFile` objects that make up the target based on the provided `TargetSource` definitions.

diff --git a/Tests/Fixtures/SPM/SPM.xcodeproj/project.pbxproj b/Tests/Fixtures/SPM/SPM.xcodeproj/project.pbxproj
@@ -143,7 +143,7 @@
 				979AE1767E2AF6B3B9D7F13D /* FooFeature */,
 			);
 			name = Packages;
-			sourceTree = SOURCE_ROOT;
+			sourceTree = "<group>";
 		};
 		CF3BD77AEAA56553289456BA /* SPMTests */ = {
 			isa = PBXGroup;

diff --git a/Tests/XcodeGenKitTests/ProjectGeneratorTests.swift b/Tests/XcodeGenKitTests/ProjectGeneratorTests.swift
@@ -1610,7 +1610,8 @@ class ProjectGeneratorTests: XCTestCase {
                     ]
                 )
 
-                let project = Project(name: "test", targets: [app], packages: ["XcodeGen": .local(path: "../XcodeGen", group: "Packages/Feature")])
+                let customLocalPackageGroup = "Packages/Feature"
+                let project = Project(name: "test", targets: [app], packages: ["XcodeGen": .local(path: "../XcodeGen", group: customLocalPackageGroup)])
 
                 let pbxProject = try project.generatePbxProj(specValidate: false)
                 let nativeTarget = try unwrap(pbxProject.nativeTargets.first(where: { $0.name == app.name }))
@@ -1619,6 +1620,17 @@ class ProjectGeneratorTests: XCTestCase {
 
                 let frameworkPhases = nativeTarget.buildPhases.compactMap { $0 as? PBXFrameworksBuildPhase }
 
+                let packagesGroup = try unwrap(pbxProject.groups.first(where: { $0.name == "Packages" }))
+                let featureGroup = try unwrap(pbxProject.groups.first(where: { $0.name == "Feature" }))
+
+                guard featureGroup.parent?.uuid == packagesGroup.uuid else {
+                  return XCTFail("Packages group should be parent of Feature group")
+                }
+
+                guard localPackageFile.parent?.uuid == featureGroup.uuid else {
+                  return XCTFail("Packages group should be parent of Feature group")
+                }
+
                 guard let frameworkPhase = frameworkPhases.first else {
                     return XCTFail("frameworkPhases should have more than one")
                 }