From 56d5a67c42a1cf16d573b3d7bab192e43b3b6ffc Mon Sep 17 00:00:00 2001 From: yuhan-chen Date: Fri, 18 Sep 2026 00:48:39 +0900 Subject: [PATCH 1/3] Add group parent membership regression tests The first two tests reproduce multiple parents: a source pointing at the spec directory, and a directory reached first through a file source. The third preserves a skipped subdirectory at the project root when a later source populates it. `expectSingleParents` lives in TestSupport so the fixtures are covered too. Co-Authored-By: Claude Opus 5 (1M context) --- Sources/TestSupport/TestHelpers.swift | 22 ++++++ Tests/FixtureTests/FixtureTests.swift | 1 + .../SourceGeneratorTests.swift | 75 +++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/Sources/TestSupport/TestHelpers.swift b/Sources/TestSupport/TestHelpers.swift index 757856dff..50de984b9 100644 --- a/Sources/TestSupport/TestHelpers.swift +++ b/Sources/TestSupport/TestHelpers.swift @@ -22,6 +22,28 @@ public func unwrap(_ value: T?, file: String = #file, line: Int = #line) thro } } +public func expectSingleParents(_ pbxProj: PBXProj, function: String = #function, file: String = #file, line: Int = #line) throws { + func label(_ element: PBXFileElement) -> String { + element.name ?? element.path ?? "" + } + + var parentsByElement: [ObjectIdentifier: (element: PBXFileElement, groups: [String])] = [:] + let allGroups: [PBXGroup] = pbxProj.groups + pbxProj.variantGroups + pbxProj.versionGroups + for group in allGroups { + for child in group.children { + parentsByElement[ObjectIdentifier(child), default: (child, [])].groups.append(label(group)) + } + } + let duplicates = parentsByElement.values.filter { $0.groups.count > 1 } + if !duplicates.isEmpty { + let description = duplicates + .map { "\(label($0.element).quoted) is a child of \($0.groups.map(\.quoted).joined(separator: " and "))" } + .sorted() + .joined(separator: "\n") + throw failure("Elements with more than one parent group:\n\(description)", function: function, file: file, line: line) + } +} + public func expectError(_ expectedError: T, function: String = #function, file: String = #file, line: Int = #line, _ closure: () throws -> Void) throws where T: CustomStringConvertible { do { try closure() diff --git a/Tests/FixtureTests/FixtureTests.swift b/Tests/FixtureTests/FixtureTests.swift index c638098f7..8367633fc 100644 --- a/Tests/FixtureTests/FixtureTests.swift +++ b/Tests/FixtureTests/FixtureTests.swift @@ -30,6 +30,7 @@ private func generateXcodeProject(specPath: Path, file: String = #file, line: In let generator = ProjectGenerator(project: project) let writer = FileWriter(project: project) let xcodeProject = try generator.generateXcodeProject(userName: "someUser") + try expectSingleParents(xcodeProject.pbxproj, file: file, line: line) try writer.writeXcodeProject(xcodeProject) try writer.writePlists() } diff --git a/Tests/XcodeGenKitTests/SourceGeneratorTests.swift b/Tests/XcodeGenKitTests/SourceGeneratorTests.swift index f24ad71e4..87c18b628 100644 --- a/Tests/XcodeGenKitTests/SourceGeneratorTests.swift +++ b/Tests/XcodeGenKitTests/SourceGeneratorTests.swift @@ -84,6 +84,81 @@ class SourceGeneratorTests: XCTestCase { try pbxProj.expectFile(paths: ["Sources", "A", "C2.0", "c.swift"], buildPhase: .sources) } + $0.it("generates a single parent for groups when a source is the base path") { + let directories = """ + Module: + - Extension: + - a.swift + - Model: + - b.swift + - c.swift + """ + try createDirectories(directories) + + let target = Target(name: "Module", type: .framework, platform: .iOS, sources: ["../Module"]) + let project = Project(basePath: directoryPath + "Module", name: "Module", targets: [target]) + + let generator = PBXProjGenerator(project: project, projectDirectory: directoryPath) + let pbxProj = try generator.generate() + + try pbxProj.expectFile(paths: ["Module", "Extension", "a.swift"], buildPhase: .sources) + try pbxProj.expectFile(paths: ["Module", "Model", "b.swift"], buildPhase: .sources) + try pbxProj.expectFile(paths: ["Module", "c.swift"], buildPhase: .sources) + try expectSingleParents(pbxProj) + + let mainGroupChildren = try pbxProj.getMainGroup().children.map(\.nameOrPath) + try expect(mainGroupChildren.contains("Extension")) == false + try expect(mainGroupChildren.contains("Model")) == false + } + + $0.it("generates a single parent for a group first referenced as a top level file source") { + let directories = """ + Sources: + - Pages: + - a.swift + - b.swift + """ + try createDirectories(directories) + + // The file source creates Pages at the top level before the directory source reparents it. + let fileTarget = Target(name: "A", type: .application, platform: .iOS, sources: ["Sources/Pages/a.swift"]) + let directoryTarget = Target(name: "B", type: .application, platform: .iOS, sources: ["Sources"]) + let project = Project(basePath: directoryPath, name: "Test", targets: [fileTarget, directoryTarget]) + + let pbxProj = try project.generatePbxProj() + + try pbxProj.expectFile(paths: ["Sources", "Pages", "a.swift"], buildPhase: .sources) + try pbxProj.expectFile(paths: ["Sources", "b.swift"], buildPhase: .sources) + try expectSingleParents(pbxProj) + + let mainGroupChildren = try pbxProj.getMainGroup().children.map(\.nameOrPath) + try expect(mainGroupChildren.contains("Sources/Pages")) == false + } + + $0.it("keeps a subdirectory reachable when the base source excludes it and another source adds it") { + let directories = """ + Module: + - Resources: + - a.json + - b.swift + """ + try createDirectories(directories) + + // The excluded directory is skipped by its parent, then populated through a later source. + let target = Target(name: "Module", type: .framework, platform: .iOS, sources: [ + TargetSource(path: "../Module", excludes: ["Resources/**"]), + TargetSource(path: "Resources", buildPhase: .resources), + ]) + let project = Project(basePath: directoryPath + "Module", name: "Module", targets: [target]) + + let generator = PBXProjGenerator(project: project, projectDirectory: directoryPath) + let pbxProj = try generator.generate() + + try pbxProj.expectFile(paths: ["Module", "b.swift"], buildPhase: .sources) + try pbxProj.expectFile(paths: ["Module/Resources", "a.json"], names: ["Resources", "a.json"], buildPhase: .resources) + try expectSingleParents(pbxProj) + } + $0.it("generates synced folder") { let directories = """ Sources: From 8118ff59df4f2fe571369bae270a2a8667f28064 Mon Sep 17 00:00:00 2001 From: yuhan-chen Date: Fri, 18 Sep 2026 00:48:39 +0900 Subject: [PATCH 2/3] Keep file elements in a single parent group `getGroup` infers "top level" from the shape of the path, which stops holding once the same directory is also reached as a child. Drop anything that gets attached to a group from `rootGroups`, and rewrite its path, project relative until now, to be relative to the new parent. The TestProject fixture loses the duplicate `Folder` entry in the main group. Co-Authored-By: Claude Opus 5 (1M context) --- Sources/XcodeGenKit/SourceGenerator.swift | 16 ++++++++++++++++ .../Project.xcodeproj/project.pbxproj | 1 - 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Sources/XcodeGenKit/SourceGenerator.swift b/Sources/XcodeGenKit/SourceGenerator.swift index 10313f47f..e1d460b5e 100644 --- a/Sources/XcodeGenKit/SourceGenerator.swift +++ b/Sources/XcodeGenKit/SourceGenerator.swift @@ -335,6 +335,9 @@ class SourceGenerator { private func getGroup(path: Path, name: String? = nil, mergingChildren children: [PBXFileElement], createIntermediateGroups: Bool, hasCustomParent: Bool, isBaseGroup: Bool) -> PBXGroup { let groupReference: PBXGroup + // A child may already have been registered as top level before being attached to its parent. + removeRootGroupMembership(from: children) + if let cachedGroup = groupsByPath[path] { var cachedGroupChildren = cachedGroup.children for child in children { @@ -382,6 +385,19 @@ class SourceGenerator { return groupReference } + private func removeRootGroupMembership(from elements: [PBXFileElement]) { + for element in elements where rootGroups.contains(element) { + rootGroups.remove(element) + // Top-level group paths are relative to the project; nested group paths are relative to their parent. + guard element is PBXGroup, element.sourceTree == .group, let elementPath = element.path else { continue } + let relativePath = Path(elementPath).lastComponent + element.path = relativePath + if element.name == relativePath { + element.name = nil + } + } + } + /// Creates a variant group or returns an existing one at the path private func getVariantGroup(path: Path, inPath: Path) -> PBXVariantGroup { let variantGroup: PBXVariantGroup diff --git a/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj b/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj index bbfb8e41c..80d4f7ca0 100644 --- a/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj +++ b/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj @@ -1090,7 +1090,6 @@ 4C7F5EB7D6F3E0E9B426AB4A /* Utilities */, 3FEA12CF227D41EF50E5C2DB /* Vendor */, 80C3A0E524EC1ABCB9149EA2 /* XPC Service */, - DAA7880242A9DE61E68026CC /* Folder */, 2E1E747C7BC434ADB80CC269 /* Headers */, 6B1603BA83AA0C7B94E45168 /* ResourceFolder */, 6BBE762F36D94AB6FFBFE834 /* SomeFile */, From e2025b1affe7c979e5664810963d7d9fef7a146b Mon Sep 17 00:00:00 2001 From: yuhan-chen Date: Fri, 18 Sep 2026 00:56:14 +0900 Subject: [PATCH 3/3] Add changelog entry for #1652 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45ff4ff89..70c4cdfb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### Fixed - Fix `syncedFolder` source paths being relative to the spec directory instead of the project directory when they differ, which caused Xcode to treat the synced folder as empty #1636 @Ckitakishi - Fix nested target attributes (e.g. `attributes.SystemCapabilities`) being serialized as a stringified Swift `Dictionary` description instead of a proper nested plist dictionary, which also caused non-deterministic key ordering in generated `project.pbxproj` files across runs #1639 @imadaan @sergeyospanov +- Fix file elements being added to both the main group and another group, which prevented Xcode 27.2 from opening the generated project. Subdirectory groups of a source that points at the spec directory now appear under that source's group instead of at the project root #1652 @Ckitakishi ### Internal - Use a dedicated local package in the SPM fixture so generated fixtures don't depend on the checkout directory name, such as when running tests from a git worktree @yonaskolb