From df284a57476573476ecb26cd477c7daa37f54ddf Mon Sep 17 00:00:00 2001 From: fortmarek Date: Wed, 19 Feb 2025 10:24:45 +0100 Subject: [PATCH] fix: mapping resource build phase with bundle targets --- .../Phases/PBXResourcesBuildPhaseMapper.swift | 92 ++++++++--- .../Mappers/Targets/PBXTargetMapper.swift | 37 ++--- .../PBXResourcesBuildPhaseMapperTests.swift | 83 +++++++++- .../Target/PBXTargetMapperTests.swift | 147 +----------------- 4 files changed, 168 insertions(+), 191 deletions(-) diff --git a/Sources/XcodeGraphMapper/Mappers/Phases/PBXResourcesBuildPhaseMapper.swift b/Sources/XcodeGraphMapper/Mappers/Phases/PBXResourcesBuildPhaseMapper.swift index db89ce32..bb579495 100644 --- a/Sources/XcodeGraphMapper/Mappers/Phases/PBXResourcesBuildPhaseMapper.swift +++ b/Sources/XcodeGraphMapper/Mappers/Phases/PBXResourcesBuildPhaseMapper.swift @@ -11,20 +11,33 @@ protocol PBXResourcesBuildPhaseMapping { /// - xcodeProj: The `XcodeProj` used for path resolution. /// - Returns: An array of `ResourceFileElement`s, representing file paths or grouped variants. /// - Throws: If any file references are missing or paths cannot be resolved. - func map(_ resourcesBuildPhase: PBXResourcesBuildPhase, xcodeProj: XcodeProj) throws -> [ResourceFileElement] + func map( + _ resourcesBuildPhase: PBXResourcesBuildPhase, + xcodeProj: XcodeProj, + projectNativeTargets: [String: ProjectNativeTarget] + ) throws -> (resources: [ResourceFileElement], resourceDependencies: [TargetDependency]) } /// A mapper that converts a `PBXResourcesBuildPhase` into a list of `ResourceFileElement`. struct PBXResourcesBuildPhaseMapper: PBXResourcesBuildPhaseMapping { func map( _ resourcesBuildPhase: PBXResourcesBuildPhase, - xcodeProj: XcodeProj - ) throws -> [ResourceFileElement] { + xcodeProj: XcodeProj, + projectNativeTargets: [String: ProjectNativeTarget] + ) throws -> (resources: [ResourceFileElement], resourceDependencies: [TargetDependency]) { let files = resourcesBuildPhase.files ?? [] - let elements = try files.flatMap { buildFile in - try mapResourceElement(buildFile, xcodeProj: xcodeProj) + let (resources, resourceDependencies): ([ResourceFileElement], [TargetDependency]) = try files.reduce(( + [], + [] + )) { acc, buildFile in + let result = try mapResourceElement(buildFile, xcodeProj: xcodeProj, projectNativeTargets: projectNativeTargets) + return (acc.0 + result.0, acc.1 + result.1) } - return elements.sorted { $0.path < $1.path } + + return ( + resources.sorted(by: { $0.path < $1.path }), + resourceDependencies.sorted(by: { $0.name < $1.name }) + ) } // MARK: - Private Helpers @@ -32,40 +45,83 @@ struct PBXResourcesBuildPhaseMapper: PBXResourcesBuildPhaseMapping { /// Maps a single `PBXBuildFile` to one or more `ResourceFileElement`s. private func mapResourceElement( _ buildFile: PBXBuildFile, - xcodeProj: XcodeProj - ) throws -> [ResourceFileElement] { + xcodeProj: XcodeProj, + projectNativeTargets: [String: ProjectNativeTarget] + ) throws -> ([ResourceFileElement], [TargetDependency]) { let fileElement = try buildFile.file .throwing(PBXResourcesMappingError.missingFileReference) // If it's a PBXVariantGroup, map each child within that group. if let variantGroup = fileElement as? PBXVariantGroup { - return try mapVariantGroup(variantGroup, xcodeProj: xcodeProj) + return try mapVariantGroup(variantGroup, xcodeProj: xcodeProj, projectNativeTargets: projectNativeTargets) } else { // Otherwise, it's a straightforward file or reference. - return try mapFileElement(fileElement, xcodeProj: xcodeProj) + return try mapFileElement( + fileElement, + xcodeProj: xcodeProj, + projectNativeTargets: projectNativeTargets + ) } } /// Maps a simple (non-variant) file element to a list (usually a single entry) of `ResourceFileElement`. private func mapFileElement( _ fileElement: PBXFileElement, - xcodeProj: XcodeProj - ) throws -> [ResourceFileElement] { + xcodeProj: XcodeProj, + projectNativeTargets: [String: ProjectNativeTarget] + ) throws -> ([ResourceFileElement], [TargetDependency]) { + switch fileElement.sourceTree { + case .buildProductsDir: + guard let path = fileElement.path else { break } + let name = path.replacingOccurrences(of: ".bundle", with: "") + if let target = xcodeProj.pbxproj.targets(named: name).first { + return ( + [], + [ + .target( + name: target.name, + status: .required, + condition: nil + ), + ] + ) + } else if let projectNativeTarget = projectNativeTargets[name] { + return ( + [], + [ + .project( + target: projectNativeTarget.nativeTarget.name, + path: projectNativeTarget.project.projectPath.parentDirectory, + status: .required, + condition: nil + ), + ] + ) + } + default: + break + } + let pathString = try fileElement .fullPath(sourceRoot: xcodeProj.srcPathString) .throwing(PBXResourcesMappingError.missingFullPath(fileElement.name ?? "Unknown")) - let absolutePath = try AbsolutePath(validating: pathString) - return [.file(path: absolutePath)] + return ([.file(path: absolutePath)], []) } /// Maps a PBXVariantGroup by expanding each child into a `ResourceFileElement`. private func mapVariantGroup( _ variantGroup: PBXVariantGroup, - xcodeProj: XcodeProj - ) throws -> [ResourceFileElement] { - try variantGroup.children.flatMap { child in - try mapFileElement(child, xcodeProj: xcodeProj) + xcodeProj: XcodeProj, + projectNativeTargets: [String: ProjectNativeTarget] + ) throws -> ([ResourceFileElement], [TargetDependency]) { + try variantGroup.children.reduce(([], [])) { acc, child in + let result = try mapFileElement( + child, + xcodeProj: xcodeProj, + projectNativeTargets: projectNativeTargets + ) + return (acc.0 + result.0, acc.1 + result.1) } } } diff --git a/Sources/XcodeGraphMapper/Mappers/Targets/PBXTargetMapper.swift b/Sources/XcodeGraphMapper/Mappers/Targets/PBXTargetMapper.swift index 5b1d7412..929bbb6b 100644 --- a/Sources/XcodeGraphMapper/Mappers/Targets/PBXTargetMapper.swift +++ b/Sources/XcodeGraphMapper/Mappers/Targets/PBXTargetMapper.swift @@ -124,9 +124,13 @@ struct PBXTargetMapper: PBXTargetMapping { packages: packages ) + sources - var resources = try pbxTarget.resourcesBuildPhase().map { - try resourcesMapper.map($0, xcodeProj: xcodeProj) - } ?? [] + var (resources, resourceDependencies) = try pbxTarget.resourcesBuildPhase().map { + try resourcesMapper.map( + $0, + xcodeProj: xcodeProj, + projectNativeTargets: projectNativeTargets + ) + } ?? ([], []) resources = try await fileSystemSynchronizedGroupsResources( from: pbxTarget, xcodeProj: xcodeProj @@ -202,7 +206,7 @@ struct PBXTargetMapper: PBXTargetMapping { let projectNativeTargets = try pbxTarget.dependencies.compactMap { try dependencyMapper.map($0, xcodeProj: xcodeProj) } - let allDependencies = (projectNativeTargets + frameworks).sorted { $0.name < $1.name } + let allDependencies = (projectNativeTargets + frameworks + resourceDependencies).sorted { $0.name < $1.name } // Construct final Target return Target( @@ -275,8 +279,7 @@ struct PBXTargetMapper: PBXTargetMapping { } else { xcodeProj.srcPath.appending(try RelativePath(validating: pathString)) } - let plistDictionary = try await readPlistAsDictionary(at: path) - return .dictionary(plistDictionary, configuration: config) + return .file(path: path, configuration: config) } return .dictionary([:]) } @@ -334,28 +337,6 @@ struct PBXTargetMapper: PBXTargetMapping { return sources.filter { $0.path.fileExtension == .playground }.map(\.path) } - /// Reads and parses a plist file into a `[String: Plist.Value]` dictionary. - private func readPlistAsDictionary( - at path: AbsolutePath, - fileSystem: FileSysteming = FileSystem() - ) async throws -> [String: Plist.Value] { - var format = PropertyListSerialization.PropertyListFormat.xml - - let data = try await fileSystem.readFile(at: path) - - guard let plist = try? PropertyListSerialization.propertyList( - from: data, - options: .mutableContainersAndLeaves, - format: &format - ) as? [String: Any] else { - throw PBXTargetMappingError.invalidPlist(path: path.pathString) - } - - return try plist.reduce(into: [String: Plist.Value]()) { result, item in - result[item.key] = try convertToPlistValue(item.value) - } - } - /// Converts a raw plist value into a `Plist.Value`. private func convertToPlistValue(_ value: Any) throws -> Plist.Value { switch value { diff --git a/Tests/XcodeGraphMapperTests/MapperTests/Phases/PBXResourcesBuildPhaseMapperTests.swift b/Tests/XcodeGraphMapperTests/MapperTests/Phases/PBXResourcesBuildPhaseMapperTests.swift index 2a3cce15..16e2f49b 100644 --- a/Tests/XcodeGraphMapperTests/MapperTests/Phases/PBXResourcesBuildPhaseMapperTests.swift +++ b/Tests/XcodeGraphMapperTests/MapperTests/Phases/PBXResourcesBuildPhaseMapperTests.swift @@ -34,7 +34,7 @@ struct PBXResourcesBuildPhaseMapperTests { let mapper = PBXResourcesBuildPhaseMapper() // When - let resources = try mapper.map(resourcesPhase, xcodeProj: xcodeProj) + let (resources, _) = try mapper.map(resourcesPhase, xcodeProj: xcodeProj, projectNativeTargets: [:]) // Then #expect(resources.count == 1) @@ -47,6 +47,85 @@ struct PBXResourcesBuildPhaseMapperTests { } } + @Test("Maps resource bundle target dependencies from resources phase") + func testMapResourceBundleTargets() async throws { + // Given + let xcodeProj = try await XcodeProj.test() + let pbxProj = xcodeProj.pbxproj + + let targetABundle = try PBXFileReference( + sourceTree: .buildProductsDir, + path: "TargetA.bundle" + ) + .add(to: pbxProj) + .addToMainGroup(in: pbxProj) + .add(to: pbxProj) + + let buildFile = PBXBuildFile(file: targetABundle).add(to: pbxProj) + + let projectTargetPath = xcodeProj.projectPath.parentDirectory.appending( + components: "AnotherProject", + "AnotherProject.xcodeproj" + ) + let targetBFrameworkRef = PBXFileReference( + sourceTree: .buildProductsDir, + path: "TargetB.bundle" + ) + let targetBFrameworkBuildFile = PBXBuildFile(file: targetBFrameworkRef).add(to: pbxProj) + + let resourcesPhase = PBXResourcesBuildPhase(files: [buildFile, targetBFrameworkBuildFile]).add(to: pbxProj) + + try PBXNativeTarget( + name: "App", + buildPhases: [resourcesPhase], + productType: .application + ) + .add(to: pbxProj) + .add(to: pbxProj.rootObject) + + PBXNativeTarget( + name: "TargetA", + buildPhases: [resourcesPhase], + productType: .bundle + ) + .add(to: pbxProj) + + let mapper = PBXResourcesBuildPhaseMapper() + + // When + let (_, resourceDependencies) = try await mapper.map( + resourcesPhase, + xcodeProj: xcodeProj, + projectNativeTargets: [ + "TargetB": ProjectNativeTarget( + nativeTarget: .test( + name: "TargetB" + ), + project: .test( + path: projectTargetPath + ) + ), + ] + ) + + // Then + #expect( + resourceDependencies == [ + .target( + name: "TargetA", + status: .required, + condition: nil + ), + .project( + target: "TargetB", + path: projectTargetPath.parentDirectory, + status: .required, + condition: nil + ), + ] + ) + } + @Test("Maps localized variant groups from resources") func testMapVariantGroup() async throws { // Given @@ -78,7 +157,7 @@ struct PBXResourcesBuildPhaseMapperTests { let mapper = PBXResourcesBuildPhaseMapper() // When - let resources = try mapper.map(resourcesPhase, xcodeProj: xcodeProj) + let (resources, _) = try mapper.map(resourcesPhase, xcodeProj: xcodeProj, projectNativeTargets: [:]) // Then #expect(resources.count == 2) diff --git a/Tests/XcodeGraphMapperTests/MapperTests/Target/PBXTargetMapperTests.swift b/Tests/XcodeGraphMapperTests/MapperTests/Target/PBXTargetMapperTests.swift index e18de05e..c03abfc8 100644 --- a/Tests/XcodeGraphMapperTests/MapperTests/Target/PBXTargetMapperTests.swift +++ b/Tests/XcodeGraphMapperTests/MapperTests/Target/PBXTargetMapperTests.swift @@ -422,8 +422,8 @@ struct PBXTargetMapperTests: Sendable { } } - @Test("Parses a valid Info.plist successfully") - func testMapTarget_validPlist() async throws { + @Test("Returns a plist path") + func testMapTarget_withPlist() async throws { // Given let xcodeProj = try await XcodeProj.test() @@ -463,153 +463,14 @@ struct PBXTargetMapperTests: Sendable { // Then #expect({ switch mapped.infoPlist { - case let .dictionary(dict, _): - - return dict["CFBundleIdentifier"] == "com.example.app" - && dict["CFBundleName"] == "ExampleApp" + case let .file(path, _): + return path == plistPath default: return false } }() == true) } - @Test("Parses a valid Info.plist referenced with a SRCROOT variable") - func testMapTarget_validPlist_referenced_with_SRCROOT() async throws { - // Given - - let xcodeProj = try await XcodeProj.test() - let srcPath = xcodeProj.srcPath - let relativePath = try RelativePath(validating: "Info.plist") - let plistPath = srcPath.appending(relativePath) - - let plistContent: [String: Any] = [ - "CFBundleIdentifier": "com.example.app", - "CFBundleName": "ExampleApp", - "CFVersion": 1.4, - ] - let data = try PropertyListSerialization.data(fromPropertyList: plistContent, format: .xml, options: 0) - try data.write(to: URL(fileURLWithPath: plistPath.pathString)) - - let target = createTarget( - name: "App", - xcodeProj: xcodeProj, - productType: .application, - buildSettings: [ - "PRODUCT_BUNDLE_IDENTIFIER": "com.example.app", - "INFOPLIST_FILE": "$(SRCROOT)/\(relativePath.pathString)", - ] - ) - - try xcodeProj.write(path: xcodeProj.path!) - let mapper = PBXTargetMapper() - - // When - let mapped = try await mapper.map( - pbxTarget: target, - xcodeProj: xcodeProj, - projectNativeTargets: [:], - packages: [] - ) - - // Then - #expect( - mapped.infoPlist == .dictionary( - [ - "CFBundleIdentifier": "com.example.app", - "CFBundleName": "ExampleApp", - "CFVersion": 1.4, - ], - configuration: .release("Release") - ) - ) - } - - @Test("Parses a valid Info.plist referenced with a PROJECT_DIR variable") - func testMapTarget_validPlist_referenced_with_PROJECT_DIR() async throws { - // Given - let xcodeProj = try await XcodeProj.test() - let srcPath = xcodeProj.srcPath - let relativePath = try RelativePath(validating: "Info.plist") - let plistPath = srcPath.appending(relativePath) - - let plistContent: [String: Any] = [ - "CFBundleIdentifier": "com.example.app", - "CFBundleName": "ExampleApp", - "CFVersion": 1.4, - ] - let data = try PropertyListSerialization.data(fromPropertyList: plistContent, format: .xml, options: 0) - try data.write(to: URL(fileURLWithPath: plistPath.pathString)) - - let target = createTarget( - name: "App", - xcodeProj: xcodeProj, - productType: .application, - buildSettings: [ - "PRODUCT_BUNDLE_IDENTIFIER": "com.example.app", - "INFOPLIST_FILE": "$(PROJECT_DIR)/\(relativePath.pathString)", - ] - ) - - try xcodeProj.write(path: xcodeProj.path!) - let mapper = PBXTargetMapper() - - // When - let mapped = try await mapper.map( - pbxTarget: target, - xcodeProj: xcodeProj, - projectNativeTargets: [:], - packages: [] - ) - - // Then - #expect( - mapped.infoPlist == .dictionary( - [ - "CFBundleIdentifier": "com.example.app", - "CFBundleName": "ExampleApp", - "CFVersion": 1.4, - ], - configuration: .release("Release") - ) - ) - } - - @Test("Throws invalidPlist when Info.plist cannot be parsed") - func testMapTarget_invalidPlist() async throws { - // Given - let xcodeProj = try await XcodeProj.test() - let srcPath = xcodeProj.srcPath - let relativePath = try RelativePath(validating: "Invalid.plist") - let invalidPlistPath = srcPath.appending(relativePath) - try await FileSystem().writeText("Invalid Plist", at: invalidPlistPath) - - let target = createTarget( - name: "App", - xcodeProj: xcodeProj, - productType: .application, - buildSettings: [ - "PRODUCT_BUNDLE_IDENTIFIER": "com.example.app", - "INFOPLIST_FILE": relativePath.pathString, - ] - ) - try xcodeProj.write(path: xcodeProj.path!) - - let mapper = PBXTargetMapper() - - // When / Then - await #expect { - _ = try await mapper.map( - pbxTarget: target, - xcodeProj: xcodeProj, - projectNativeTargets: [:], - packages: [] - ) - } throws: { error in - error.localizedDescription - == "Failed to read a valid plist dictionary from file at: \(invalidPlistPath.pathString)." - } - } - // MARK: - Helper Methods private func createTarget(