Skip to content

fix: do not map project target dependency to an underlying type #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fortmarek merged 1 commit into from
Feb 10, 2025
Merged

fix: do not map project target dependency to an underlying type #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Package.resolved
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 2 additions & 8 deletions Sources/XcodeGraphMapper/Mappers/Graph/XcodeGraphMapper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,11 @@ public struct XcodeGraphMapper: XcodeGraphMapping {
private func resolveDependencies(
for projects: [AbsolutePath: Project]
) async throws -> ([GraphDependency: Set<GraphDependency>], [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<GraphDependency>], [GraphEdge: PlatformCondition]) {
var dependencies = [GraphDependency: Set<GraphDependency>]()
var dependencyConditions = [GraphEdge: PlatformCondition]()
Expand All @@ -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)
Expand Down
71 changes: 2 additions & 69 deletions Sources/XcodeGraphMapper/Mappers/Targets/TargetDependency+GraphMapping.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(),
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
81 changes: 3 additions & 78 deletions Tests/XcodeGraphMapperTests/MapperTests/Target/TargetDependencyExtensionsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,15 +18,14 @@ struct TargetDependencyExtensionsTests {
// When
let graphDep = try await dependency.graphDependency(
sourceDirectory: sourceDirectory,
allTargetsMap: allTargetsMap,
target: target
)

// Then
#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(
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -131,7 +88,6 @@ struct TargetDependencyExtensionsTests {
// When
let graphDep = try await dependency.graphDependency(
sourceDirectory: sourceDirectory,
allTargetsMap: allTargetsMap,
target: target
)

Expand All @@ -156,7 +112,6 @@ struct TargetDependencyExtensionsTests {
// When
let graphDep = try await dependency.graphDependency(
sourceDirectory: sourceDirectory,
allTargetsMap: allTargetsMap,
target: target
)
let expected = GraphDependency.library(
Expand All @@ -179,7 +134,6 @@ struct TargetDependencyExtensionsTests {
// When
let graphDep = try await dependency.graphDependency(
sourceDirectory: sourceDirectory,
allTargetsMap: allTargetsMap,
target: target
)
// Then
Expand All @@ -194,7 +148,6 @@ struct TargetDependencyExtensionsTests {
// When
let graphDep = try await dependency.graphDependency(
sourceDirectory: sourceDirectory,
allTargetsMap: allTargetsMap,
target: target
)
// Then
Expand All @@ -221,7 +174,6 @@ struct TargetDependencyExtensionsTests {
// When
let graphDep = try await dependency.graphDependency(
sourceDirectory: sourceDirectory,
allTargetsMap: allTargetsMap,
target: target
)
let expected = GraphDependency.framework(
Expand All @@ -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)")
}
}
}