From 8320e8cfa66b267b3c6424768e26504bc5cbcdec Mon Sep 17 00:00:00 2001 From: fortmarek Date: Mon, 10 Feb 2025 11:13:38 +0100 Subject: [PATCH] fix: do not map project target dependency to an underlying type --- Package.resolved | 10 +-- .../Mappers/Graph/XcodeGraphMapper.swift | 10 +-- .../TargetDependency+GraphMapping.swift | 71 +--------------- .../TargetDependencyExtensionsTests.swift | 81 +------------------ 4 files changed, 12 insertions(+), 160 deletions(-) diff --git a/Package.resolved b/Package.resolved index e900c7e0..9b9bd94d 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "d85d2800ade4b7815fc45dc86a042a0565bf66be2c0a52d40c24b35d11f9a0be", + "originHash" : "252c1ee702d5bd9c4155485fe13cdca178e46fc497b3f48683ff138bb43d7492", "pins" : [ { "identity" : "aexml", @@ -24,8 +24,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/tuist/Command.git", "state" : { - "revision" : "437e0c0ca18d1a16194c55b4690971b5bfb1f185", - "version" : "0.12.0" + "revision" : "9d03a95faa94b961edc1cf2c5f4379b0108ee97a", + "version" : "0.12.1" } }, { @@ -141,8 +141,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-service-context", "state" : { - "revision" : "0c62c5b4601d6c125050b5c3a97f20cce881d32b", - "version" : "1.1.0" + "revision" : "8946c930cae601452149e45d31d8ddfac973c3c7", + "version" : "1.2.0" } }, { diff --git a/Sources/XcodeGraphMapper/Mappers/Graph/XcodeGraphMapper.swift b/Sources/XcodeGraphMapper/Mappers/Graph/XcodeGraphMapper.swift index 0d6615be..5fe287b5 100644 --- a/Sources/XcodeGraphMapper/Mappers/Graph/XcodeGraphMapper.swift +++ b/Sources/XcodeGraphMapper/Mappers/Graph/XcodeGraphMapper.swift @@ -193,16 +193,11 @@ public struct XcodeGraphMapper: XcodeGraphMapping { private func resolveDependencies( for projects: [AbsolutePath: Project] ) async throws -> ([GraphDependency: Set], [GraphEdge: PlatformCondition]) { - let allTargetsMap = Dictionary( - projects.values.flatMap(\.targets), - uniquingKeysWith: { existing, _ in existing } - ) - return try await buildDependencies(for: projects, using: allTargetsMap) + return try await buildDependencies(for: projects) } private func buildDependencies( - for projects: [AbsolutePath: Project], - using allTargetsMap: [String: Target] + for projects: [AbsolutePath: Project] ) async throws -> ([GraphDependency: Set], [GraphEdge: PlatformCondition]) { var dependencies = [GraphDependency: Set]() var dependencyConditions = [GraphEdge: PlatformCondition]() @@ -219,7 +214,6 @@ public struct XcodeGraphMapper: XcodeGraphMapping { ) in let graphDep = try await dep.graphDependency( sourceDirectory: path, - allTargetsMap: allTargetsMap, target: target ) return (GraphEdge(from: sourceDependency, to: graphDep), dep.condition, graphDep) diff --git a/Sources/XcodeGraphMapper/Mappers/Targets/TargetDependency+GraphMapping.swift b/Sources/XcodeGraphMapper/Mappers/Targets/TargetDependency+GraphMapping.swift index b0d65102..46434e64 100644 --- a/Sources/XcodeGraphMapper/Mappers/Targets/TargetDependency+GraphMapping.swift +++ b/Sources/XcodeGraphMapper/Mappers/Targets/TargetDependency+GraphMapping.swift @@ -7,11 +7,10 @@ import XcodeProj // swiftlint:disable function_body_length extension TargetDependency { /// Maps this `TargetDependency` to a `GraphDependency` by resolving paths, product types, - /// and linking details. Project-based dependencies are resolved using `allTargetsMap`. + /// and linking details. /// /// - Parameters: /// - sourceDirectory: The root directory for resolving relative paths. - /// - allTargetsMap: A map of target names to `Target` models for resolving project-based dependencies. /// - target: The target of this dependency. /// - xcframeworkMetadataProvider: Provides metadata (linking, architectures, etc.) for `.xcframework` dependencies. /// - libraryMetadataProvider: Provides metadata for libraries. @@ -22,7 +21,6 @@ extension TargetDependency { /// - Throws: `TargetDependencyMappingError` if a referenced target is not found or if the dependency type is unknown. func graphDependency( sourceDirectory: AbsolutePath, - allTargetsMap: [String: Target], target: Target, xcframeworkMetadataProvider: XCFrameworkMetadataProviding = XCFrameworkMetadataProvider(), libraryMetadataProvider: LibraryMetadataProviding = LibraryMetadataProvider(), @@ -37,12 +35,7 @@ extension TargetDependency { return .target(name: name, path: sourceDirectory, status: status) case let .project(targetName, projectPath, status, _): - return try mapProjectGraphDependency( - projectPath: projectPath, - targetName: targetName, - status: status, - allTargetsMap: allTargetsMap - ) + return .target(name: targetName, path: projectPath, status: status) // MARK: - Precompiled Binary Cases @@ -131,66 +124,6 @@ extension TargetDependency { ) } } - - /// Resolves a project-based target dependency into a `GraphDependency`. - /// - /// - Parameters: - /// - projectPath: The absolute path of the `.xcodeproj` directory. - /// - targetName: The name of the target within that project. - /// - status: The linking status of the dependency. - /// - allTargetsMap: A dictionary of target names to `Target` models for resolution. - /// - Returns: A `GraphDependency` representing the resolved dependency. - /// - Throws: `TargetDependencyMappingError.targetNotFound` if `targetName` isn't in `allTargetsMap`, - /// `TargetDependencyMappingError.unknownDependencyType` if the product type can't be mapped. - private func mapProjectGraphDependency( - projectPath: AbsolutePath, - targetName: String, - status: LinkingStatus, - allTargetsMap: [String: Target] - ) throws -> GraphDependency { - guard let target = allTargetsMap[targetName] else { - throw TargetDependencyMappingError.targetNotFound( - targetName: targetName, - path: projectPath - ) - } - - let product = target.product - switch product { - case .framework, .staticFramework: - let linking: BinaryLinking = (product == .staticFramework) ? .static : .dynamic - return .framework( - path: projectPath, - binaryPath: projectPath.appending(component: "\(targetName).framework"), - dsymPath: nil, - bcsymbolmapPaths: [], - linking: linking, - architectures: [], - status: status - ) - - case .staticLibrary, .dynamicLibrary: - let linking: BinaryLinking = (product == .staticLibrary) ? .static : .dynamic - let libName = (linking == .static) ? "lib\(targetName).a" : "lib\(targetName).dylib" - let publicHeadersPath = projectPath.appending(component: "include") - return .library( - path: projectPath.appending(component: libName), - publicHeaders: publicHeadersPath, - linking: linking, - architectures: [], - swiftModuleMap: nil - ) - - case .bundle: - return .bundle(path: projectPath.appending(component: "\(targetName).bundle")) - - case .app, .commandLineTool: - return .target(name: targetName, path: projectPath, status: status) - - default: - throw TargetDependencyMappingError.unknownDependencyType(name: product.description) - } - } } /// Resolves the system-provided `XCTest.framework` path, falling back to a standard location diff --git a/Tests/XcodeGraphMapperTests/MapperTests/Target/TargetDependencyExtensionsTests.swift b/Tests/XcodeGraphMapperTests/MapperTests/Target/TargetDependencyExtensionsTests.swift index b7a63b6a..636ee8a8 100644 --- a/Tests/XcodeGraphMapperTests/MapperTests/Target/TargetDependencyExtensionsTests.swift +++ b/Tests/XcodeGraphMapperTests/MapperTests/Target/TargetDependencyExtensionsTests.swift @@ -10,13 +10,6 @@ struct TargetDependencyExtensionsTests { let sourceDirectory = AssertionsTesting.fixturePath() let target = Target.test(platform: .iOS) - // A dummy target map for .project dependencies - let allTargetsMap: [String: Target] = [ - "StaticLibrary": Target.test(name: "libStaticLibrary", product: .staticLibrary), - "MyProjectTarget": Target.test(name: "MyProjectTarget", product: .framework), - "MyProjectDynamicLibrary": Target.test(name: "MyProjectDynamicLibrary", product: .dynamicLibrary), - ] - @Test("Resolves a target dependency into a target graph dependency") func testTargetGraphDependency_Target() async throws { // Given @@ -25,7 +18,6 @@ struct TargetDependencyExtensionsTests { // When let graphDep = try await dependency.graphDependency( sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, target: target ) @@ -33,7 +25,7 @@ struct TargetDependencyExtensionsTests { #expect(graphDep == .target(name: "App", path: sourceDirectory, status: .required)) } - @Test("Resolves a project-based framework dependency to a dynamic framework in the graph") + @Test("Resolves a project-based framework dependency") func testTargetGraphDependencyFramework_Project() async throws { // Given let dependency = TargetDependency.project( @@ -46,54 +38,20 @@ struct TargetDependencyExtensionsTests { // When let graphDep = try await dependency.graphDependency( sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, target: target ) // Then #expect({ switch graphDep { - case let .framework(path, binaryPath, _, _, linking, archs, status): - return path == sourceDirectory - && binaryPath == sourceDirectory.appending(component: "MyProjectTarget.framework") - && linking == .dynamic && archs.isEmpty && status == .required + case .target(name: "MyProjectTarget", path: sourceDirectory, status: .required): + return true default: return false } }() == true) } - @Test("Resolves a project-based dynamic library dependency correctly") - func testTargetGraphDependencyLibrary_Project() async throws { - // Given - let libraryPath = AssertionsTesting.fixturePath(path: try RelativePath(validating: "libStaticLibrary.a")) - - let dependency = TargetDependency.project( - target: "StaticLibrary", - path: sourceDirectory, - status: .required, - condition: nil - ) - - // When - let graphDep = try await dependency.graphDependency( - sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, - target: target - ) - - let expected = GraphDependency.library( - path: libraryPath, - publicHeaders: libraryPath.parentDirectory.appending(component: "include"), - linking: .static, - architectures: [], - swiftModuleMap: nil - ) - - // Then - #expect(expected == graphDep) - } - @Test("Resolves a framework file dependency into a dynamic framework graph dependency") func testTargetGraphDependency_Framework() async throws { // Given @@ -103,7 +61,6 @@ struct TargetDependencyExtensionsTests { // When let graphDep = try await dependency.graphDependency( sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, target: target ) let expectedBinaryPath = frameworkPath.appending(component: frameworkPath.basenameWithoutExt) @@ -131,7 +88,6 @@ struct TargetDependencyExtensionsTests { // When let graphDep = try await dependency.graphDependency( sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, target: target ) @@ -156,7 +112,6 @@ struct TargetDependencyExtensionsTests { // When let graphDep = try await dependency.graphDependency( sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, target: target ) let expected = GraphDependency.library( @@ -179,7 +134,6 @@ struct TargetDependencyExtensionsTests { // When let graphDep = try await dependency.graphDependency( sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, target: target ) // Then @@ -194,7 +148,6 @@ struct TargetDependencyExtensionsTests { // When let graphDep = try await dependency.graphDependency( sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, target: target ) // Then @@ -221,7 +174,6 @@ struct TargetDependencyExtensionsTests { // When let graphDep = try await dependency.graphDependency( sourceDirectory: sourceDirectory, - allTargetsMap: allTargetsMap, target: target ) let expected = GraphDependency.framework( @@ -237,31 +189,4 @@ struct TargetDependencyExtensionsTests { // Then #expect(expected == graphDep) } - - @Test("Throws a MappingError when a project target does not exist in allTargetsMap") - func testMapProjectGraphDependency_TargetNotFound() async throws { - // Given - let dependency = TargetDependency.project( - target: "NonExistentTarget", - path: sourceDirectory, - status: .required, - condition: nil - ) - - // When / Then - do { - _ = try await dependency.graphDependency(sourceDirectory: sourceDirectory, allTargetsMap: [:], target: target) - Issue.record("Expected to throw TargetDependencyMappingError.targetNotFound") - } catch let error as TargetDependencyMappingError { - switch error { - case let .targetNotFound(targetName, path): - #expect(targetName == "NonExistentTarget") - #expect(path == sourceDirectory) - default: - Issue.record("Unexpected TargetDependencyMappingError: \(error)") - } - } catch { - Issue.record("Unexpected error: \(error)") - } - } }