diff --git a/Sources/XcodeProj/XCProj/XCProjEncoder.swift b/Sources/XcodeProj/XCProj/XCProjEncoder.swift index c9c41ecc9..d0c64abbb 100644 --- a/Sources/XcodeProj/XCProj/XCProjEncoder.swift +++ b/Sources/XcodeProj/XCProj/XCProjEncoder.swift @@ -33,8 +33,8 @@ final class XCProjEncoder { /// dictionary on every access. private var targetAttributes: [PBXTarget: [String: ProjectAttribute]] = [:] - /// Build phases that can only be referred to by identifier, because their target has more than - /// one phase of the same kind and name. + /// Build phases that can only be referred to by identifier, because a name based reference + /// would not pick them out. private var ambiguousPhases: Set = [] /// The target that owns each build phase. @@ -176,6 +176,20 @@ extension XCProjEncoder { for (phase, key) in keyed where counts[key, default: 0] > 1 { ambiguousPhases.insert(phase.reference) } + for (phase, key) in keyed where key.name == nil && Self.isRepeatableKind(key.kind) { + ambiguousPhases.insert(phase.reference) + } + } + } + + /// Whether a target can hold more than one build phase of this kind. + /// + /// Xcode resolves `"App/resources"` against the single phase of that kind, but rejects + /// `"App/copy"` and `"App/script"` even when the target holds one, so those need an identifier. + private static func isRepeatableKind(_ kind: BuildPhase) -> Bool { + switch kind { + case .copyFiles, .runScript: true + case .sources, .frameworks, .resources, .headers, .carbonResources: false } } diff --git a/Tests/XcodeProjTests/XCProj/XCProjEncoderTests.swift b/Tests/XcodeProjTests/XCProj/XCProjEncoderTests.swift index eca044925..a96f237d4 100644 --- a/Tests/XcodeProjTests/XCProj/XCProjEncoderTests.swift +++ b/Tests/XcodeProjTests/XCProj/XCProjEncoderTests.swift @@ -173,6 +173,56 @@ import XcodeProjectFormat ) } + /// Xcode rejects `"App/copy"` and `"App/script"` with "Could not uniquely resolve the build + /// phase name" even when the target holds one, because it can hold many. Apple's schema + /// validator accepts both spellings, so only an identifier makes such a phase addressable. + @Test(arguments: ["CopyFiles", "Run Script", "Sources", "Frameworks", "Resources", "Headers"]) + func namelessRepeatablePhasesCarryAnIdentifier(rawKind: String) throws { + let kind = try #require(BuildPhase(rawValue: rawKind)) + let phase: PBXBuildPhase = switch kind { + case .copyFiles: PBXCopyFilesBuildPhase(dstPath: "include", dstSubfolderSpec: .productsDirectory) + case .runScript: PBXShellScriptBuildPhase(shellScript: "echo hi") + case .sources: PBXSourcesBuildPhase() + case .frameworks: PBXFrameworksBuildPhase() + case .resources: PBXResourcesBuildPhase() + default: PBXHeadersBuildPhase() + } + let file = PBXFileReference(sourceTree: .group, path: "Main.swift") + let buildFile = PBXBuildFile(file: file) + phase.files = [buildFile] + + let configuration = XCBuildConfiguration(name: "Release") + let targetList = XCConfigurationList(buildConfigurations: [configuration], defaultConfigurationName: "Release") + let target = PBXNativeTarget(name: "App", buildConfigurationList: targetList, buildPhases: [phase]) + let mainGroup = PBXGroup(children: [file], sourceTree: .group) + let proj = try Self.makeProj( + mainGroup: mainGroup, + target: target, + extraObjects: [phase, file, buildFile, configuration] + ) + + let schemaKind = switch kind { + case .copyFiles: "copy" + case .runScript: "script" + case .sources: "compile-sources" + default: rawKind.lowercased() + } + let data = try proj.xcprojData() + let emittedByName = String(decoding: data, as: UTF8.self).contains("\"App/\(schemaKind)\"") + let repeatable = kind == .copyFiles || kind == .runScript + + #expect( + emittedByName == !repeatable, + repeatable + ? "a nameless \(schemaKind) phase is not addressable by name and needs an identifier" + : "a target holds one \(schemaKind) phase, so the name is enough" + ) + + // Whichever spelling it is, the membership has to survive the trip back. + let decoded = try PBXProj(xcprojData: data, projectName: "App") + #expect(decoded.rootObject?.targets.first?.buildPhases.first?.files?.count == 1) + } + // MARK: - Ambiguity @Test func usesIdentifiersForAmbiguousNames() throws {