Skip to content

Commit ad5792c

Browse files
MaxDesiatovMaxDesiatov
committed
Fix target dependencies equality
1 parent c6fcbea commit ad5792c

File tree

5 files changed

+111
-36
lines changed

5 files changed

+111
-36
lines changed

‎Sources/Build/BuildPlan/BuildPlan+Test.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ extension BuildPlan {
8585
testDiscoverySrc: Sources(paths: discoveryPaths, root: discoveryDerivedDir)
8686
)
8787
let discoveryResolvedTarget = ResolvedTarget(
88+
packageIdentity: testProduct.packageIdentity,
8889
underlying: discoveryTarget,
8990
dependencies: testProduct.targets.map { .target($0, conditions: []) },
9091
defaultLocalization: testProduct.defaultLocalization,
@@ -124,6 +125,7 @@ extension BuildPlan {
124125
testEntryPointSources: entryPointSources
125126
)
126127
let entryPointResolvedTarget = ResolvedTarget(
128+
packageIdentity: testProduct.packageIdentity,
127129
underlying: entryPointTarget,
128130
dependencies: testProduct.targets.map { .target($0, conditions: []) } + resolvedTargetDependencies,
129131
defaultLocalization: testProduct.defaultLocalization,
@@ -168,6 +170,7 @@ extension BuildPlan {
168170
testEntryPointSources: entryPointResolvedTarget.underlying.sources
169171
)
170172
let entryPointResolvedTarget = ResolvedTarget(
173+
packageIdentity: testProduct.packageIdentity,
171174
underlying: entryPointTarget,
172175
dependencies: entryPointResolvedTarget.dependencies + resolvedTargetDependencies,
173176
defaultLocalization: testProduct.defaultLocalization,

‎Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ private func createResolvedPackages(
373373
// Create target builders for each target in the package.
374374
let targetBuilders = package.targets.map {
375375
ResolvedTargetBuilder(
376+
packageIdentity: package.identity,
376377
target: $0,
377378
observabilityScope: packageObservabilityScope,
378379
platformVersionProvider: platformVersionProvider
@@ -861,6 +862,7 @@ private final class ResolvedProductBuilder: ResolvedBuilder<ResolvedProduct> {
861862

862863
override func constructImpl() throws -> ResolvedProduct {
863864
return ResolvedProduct(
865+
packageIdentity: packageBuilder.package.identity,
864866
product: product,
865867
targets: try targets.map{ try $0.construct() }
866868
)
@@ -879,6 +881,9 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
879881
case product(_ product: ResolvedProductBuilder, conditions: [PackageCondition])
880882
}
881883

884+
/// The reference to its package.
885+
let packageIdentity: PackageIdentity
886+
882887
/// The target reference.
883888
let target: Target
884889

@@ -895,10 +900,12 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
895900
let platformVersionProvider: PlatformVersionProvider
896901

897902
init(
903+
packageIdentity: PackageIdentity,
898904
target: Target,
899905
observabilityScope: ObservabilityScope,
900906
platformVersionProvider: PlatformVersionProvider
901907
) {
908+
self.packageIdentity = packageIdentity
902909
self.target = target
903910
self.observabilityScope = observabilityScope
904911
self.platformVersionProvider = platformVersionProvider
@@ -929,6 +936,7 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
929936
}
930937

931938
return ResolvedTarget(
939+
packageIdentity: self.packageIdentity,
932940
underlying: self.target,
933941
dependencies: dependencies,
934942
defaultLocalization: self.defaultLocalization,

‎Sources/PackageGraph/Resolution/ResolvedProduct.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ public struct ResolvedProduct: Hashable {
2424
self.underlying.type
2525
}
2626

27+
public let packageIdentity: PackageIdentity
28+
2729
/// The underlying product.
2830
public let underlying: Product
2931

@@ -61,8 +63,9 @@ public struct ResolvedProduct: Hashable {
6163
}
6264
}
6365

64-
public init(product: Product, targets: [ResolvedTarget]) {
66+
public init(packageIdentity: PackageIdentity, product: Product, targets: [ResolvedTarget]) {
6567
assert(product.targets.count == targets.count && product.targets.map(\.name) == targets.map(\.name))
68+
self.packageIdentity = packageIdentity
6669
self.underlying = product
6770
self.targets = targets
6871

@@ -85,6 +88,7 @@ public struct ResolvedProduct: Hashable {
8588
testEntryPointPath: testEntryPointPath
8689
)
8790
return ResolvedTarget(
91+
packageIdentity: packageIdentity,
8892
underlying: swiftTarget,
8993
dependencies: targets.map { .target($0, conditions: []) },
9094
defaultLocalization: defaultLocalization ?? .none, // safe since this is a derived product

‎Sources/PackageGraph/Resolution/ResolvedTarget.swift

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import func TSCBasic.topologicalSort
1717
/// Represents a fully resolved target. All the dependencies for this target are also stored as resolved.
1818
public struct ResolvedTarget: Hashable {
1919
/// Represents dependency of a resolved target.
20-
public enum Dependency: Hashable {
20+
public enum Dependency {
2121
/// Direct dependency of the target. This target is in the same package and should be statically linked.
2222
case target(_ target: ResolvedTarget, conditions: [PackageCondition])
2323

@@ -129,6 +129,8 @@ public struct ResolvedTarget: Hashable {
129129
return underlying.sources
130130
}
131131

132+
let packageIdentity: PackageIdentity
133+
132134
/// The underlying target represented in this resolved target.
133135
public let underlying: Target
134136

@@ -148,12 +150,14 @@ public struct ResolvedTarget: Hashable {
148150

149151
/// Create a resolved target instance.
150152
public init(
153+
packageIdentity: PackageIdentity,
151154
underlying: Target,
152155
dependencies: [ResolvedTarget.Dependency],
153156
defaultLocalization: String? = nil,
154157
supportedPlatforms: [SupportedPlatform],
155158
platformVersionProvider: PlatformVersionProvider
156159
) {
160+
self.packageIdentity = packageIdentity
157161
self.underlying = underlying
158162
self.dependencies = dependencies
159163
self.defaultLocalization = defaultLocalization
@@ -190,3 +194,49 @@ extension ResolvedTarget.Dependency: CustomStringConvertible {
190194
return str
191195
}
192196
}
197+
198+
extension ResolvedTarget.Dependency: Identifiable {
199+
public struct ID: Hashable {
200+
enum Kind: Hashable {
201+
case target
202+
case product
203+
}
204+
205+
let kind: Kind
206+
let packageIdentity: PackageIdentity
207+
let name: String
208+
}
209+
210+
public var id: ID {
211+
switch self {
212+
case .target(let target, _):
213+
return .init(kind: .target, packageIdentity: target.packageIdentity, name: target.name)
214+
case .product(let product, _):
215+
return .init(kind: .product, packageIdentity: product.packageIdentity, name: product.name)
216+
}
217+
}
218+
}
219+
220+
extension ResolvedTarget.Dependency: Equatable {
221+
public static func == (lhs: ResolvedTarget.Dependency, rhs: ResolvedTarget.Dependency) -> Bool {
222+
switch (lhs, rhs) {
223+
case (.target(let lhsTarget, _), .target(let rhsTarget, _)):
224+
return lhsTarget == rhsTarget
225+
case (.product(let lhsProduct, _), .product(let rhsProduct, _)):
226+
return lhsProduct == rhsProduct
227+
case (.product, .target), (.target, .product):
228+
return false
229+
}
230+
}
231+
}
232+
233+
extension ResolvedTarget.Dependency: Hashable {
234+
public func hash(into hasher: inout Hasher) {
235+
switch self {
236+
case .target(let target, _):
237+
hasher.combine(target)
238+
case .product(let product, _):
239+
hasher.combine(product)
240+
}
241+
}
242+
}

‎Tests/PackageGraphTests/TargetTests.swift

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
import XCTest
1414

1515
import PackageGraph
16-
import PackageModel
16+
@testable import PackageModel
1717

1818
private extension ResolvedTarget {
19-
init(name: String, deps: ResolvedTarget..., conditions: [PackageCondition] = []) {
19+
init(packageIdentity: String, name: String, deps: ResolvedTarget..., conditions: [PackageCondition] = []) {
2020
self.init(
21+
packageIdentity: .init(packageIdentity),
2122
underlying: SwiftTarget(
2223
name: name,
2324
type: .library,
@@ -28,7 +29,7 @@ private extension ResolvedTarget {
2829
swiftVersion: .v4,
2930
usesUnsafeFlags: false
3031
),
31-
dependencies: deps.map { .target($0, conditions: []) },
32+
dependencies: deps.map { .target($0, conditions: conditions) },
3233
defaultLocalization: nil,
3334
supportedPlatforms: [],
3435
platformVersionProvider: .init(implementation: .minimumDeploymentTargetDefault)
@@ -38,54 +39,54 @@ private extension ResolvedTarget {
3839

3940
class TargetDependencyTests: XCTestCase {
4041
func test1() throws {
41-
let t1 = ResolvedTarget(name: "t1")
42-
let t2 = ResolvedTarget(name: "t2", deps: t1)
43-
let t3 = ResolvedTarget(name: "t3", deps: t2)
42+
let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1")
43+
let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1)
44+
let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2)
4445

4546
XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1])
4647
XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1])
4748
}
4849

4950
func test2() throws {
50-
let t1 = ResolvedTarget(name: "t1")
51-
let t2 = ResolvedTarget(name: "t2", deps: t1)
52-
let t3 = ResolvedTarget(name: "t3", deps: t2, t1)
53-
let t4 = ResolvedTarget(name: "t4", deps: t2, t3, t1)
51+
let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1")
52+
let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1)
53+
let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2, t1)
54+
let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t2, t3, t1)
5455

5556
XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1])
5657
XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1])
5758
XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1])
5859
}
5960

6061
func test3() throws {
61-
let t1 = ResolvedTarget(name: "t1")
62-
let t2 = ResolvedTarget(name: "t2", deps: t1)
63-
let t3 = ResolvedTarget(name: "t3", deps: t2, t1)
64-
let t4 = ResolvedTarget(name: "t4", deps: t1, t2, t3)
62+
let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1")
63+
let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1)
64+
let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2, t1)
65+
let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t1, t2, t3)
6566

6667
XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1])
6768
XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1])
6869
XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1])
6970
}
7071

7172
func test4() throws {
72-
let t1 = ResolvedTarget(name: "t1")
73-
let t2 = ResolvedTarget(name: "t2", deps: t1)
74-
let t3 = ResolvedTarget(name: "t3", deps: t2)
75-
let t4 = ResolvedTarget(name: "t4", deps: t3)
73+
let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1")
74+
let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1)
75+
let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2)
76+
let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3)
7677

7778
XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1])
7879
XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1])
7980
XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1])
8081
}
8182

8283
func test5() throws {
83-
let t1 = ResolvedTarget(name: "t1")
84-
let t2 = ResolvedTarget(name: "t2", deps: t1)
85-
let t3 = ResolvedTarget(name: "t3", deps: t2)
86-
let t4 = ResolvedTarget(name: "t4", deps: t3)
87-
let t5 = ResolvedTarget(name: "t5", deps: t2)
88-
let t6 = ResolvedTarget(name: "t6", deps: t5, t4)
84+
let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1")
85+
let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1)
86+
let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2)
87+
let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3)
88+
let t5 = ResolvedTarget(packageIdentity: "pkg", name: "t5", deps: t2)
89+
let t6 = ResolvedTarget(packageIdentity: "pkg", name: "t6", deps: t5, t4)
8990

9091
// precise order is not important, but it is important that the following are true
9192
let t6rd = try t6.recursiveTargetDependencies()
@@ -102,12 +103,12 @@ class TargetDependencyTests: XCTestCase {
102103
}
103104

104105
func test6() throws {
105-
let t1 = ResolvedTarget(name: "t1")
106-
let t2 = ResolvedTarget(name: "t2", deps: t1)
107-
let t3 = ResolvedTarget(name: "t3", deps: t2)
108-
let t4 = ResolvedTarget(name: "t4", deps: t3)
109-
let t5 = ResolvedTarget(name: "t5", deps: t2)
110-
let t6 = ResolvedTarget(name: "t6", deps: t4, t5) // same as above, but these two swapped
106+
let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1")
107+
let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1)
108+
let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2)
109+
let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3)
110+
let t5 = ResolvedTarget(packageIdentity: "pkg", name: "t5", deps: t2)
111+
let t6 = ResolvedTarget(packageIdentity: "pkg", name: "t6", deps: t4, t5) // same as above, but these two swapped
111112

112113
// precise order is not important, but it is important that the following are true
113114
let t6rd = try t6.recursiveTargetDependencies()
@@ -124,10 +125,19 @@ class TargetDependencyTests: XCTestCase {
124125
}
125126

126127
func testConditions() throws {
127-
let t1 = ResolvedTarget(name: "t1")
128-
let t2 = ResolvedTarget(name: "t2", deps: t1)
129-
let t2Conditions = ResolvedTarget(name: "t2", deps: t1, conditions: [.init(platforms: [.linux])])
128+
let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1")
129+
let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1)
130+
let t2NoConditions = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1)
131+
let t2WithConditions = ResolvedTarget(
132+
packageIdentity: "pkg",
133+
name: "t2",
134+
deps: t1,
135+
conditions: [.init(platforms: [.linux])]
136+
)
130137

131-
XCTAssertEqual(t2, t2Conditions)
138+
// FIXME: we should test for actual `t2` and `t2NoConditions` equality, but `SwiftTarget` is a reference type,
139+
// which currently breaks this test, and it shouldn't
140+
XCTAssertEqual(t2.dependencies, t2NoConditions.dependencies)
141+
XCTAssertEqual(t2.dependencies, t2WithConditions.dependencies)
132142
}
133143
}

0 commit comments

Comments
 (0)