Skip to content

Commit 13fc545

Browse files
tomerdtomerd
committed
refactor managed artifacts
motivation: ManagedArtifacts are based on location instead of on identity, refactor them to be based on identity as part of the transition to identity based logic changes: * transition ManagedArtifacts underlying hashmap to be based on identity * update call sites to use package identity instead of location when reading/writing managed artifacts * adjust tests where needed
1 parent 615bd43 commit 13fc545

File tree

5 files changed

+40
-35
lines changed

5 files changed

+40
-35
lines changed

‎Sources/SPMTestSupport/MockWorkspace.swift

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,30 +478,38 @@ public final class MockWorkspace {
478478
self.managedArtifacts = managedArtifacts
479479
}
480480

481+
public func checkNotPresent(packageName: String, targetName: String, file: StaticString = #file, line: UInt = #line) {
482+
self.checkNotPresent(packageIdentity: .plain(packageName), targetName: targetName, file : file, line: line)
483+
}
484+
481485
public func checkNotPresent(
482-
packageName: String,
486+
packageIdentity: PackageIdentity,
483487
targetName: String,
484488
file: StaticString = #file,
485489
line: UInt = #line
486490
) {
487-
let artifact = self.managedArtifacts[packageName: packageName, targetName: targetName]
488-
XCTAssert(artifact == nil, "Unexpectedly found \(packageName).\(targetName) in managed artifacts", file: file, line: line)
491+
let artifact = self.managedArtifacts[packageIdentity: packageIdentity, targetName: targetName]
492+
XCTAssert(artifact == nil, "Unexpectedly found \(packageIdentity).\(targetName) in managed artifacts", file: file, line: line)
489493
}
490494

491495
public func checkEmpty(file: StaticString = #file, line: UInt = #line) {
492496
XCTAssertEqual(self.managedArtifacts.count, 0, file: file, line: line)
493497
}
494498

499+
public func check(packageName: String, targetName: String, source: Workspace.ManagedArtifact.Source, path: AbsolutePath, file: StaticString = #file, line: UInt = #line) {
500+
self.check(packageIdentity: .plain(packageName), targetName: targetName, source: source, path: path, file: file, line: line)
501+
}
502+
495503
public func check(
496-
packageName: String,
504+
packageIdentity: PackageIdentity,
497505
targetName: String,
498506
source: Workspace.ManagedArtifact.Source,
499507
path: AbsolutePath,
500508
file: StaticString = #file,
501509
line: UInt = #line
502510
) {
503-
guard let artifact = managedArtifacts[packageName: packageName, targetName: targetName] else {
504-
XCTFail("\(packageName).\(targetName) does not exists", file: file, line: line)
511+
guard let artifact = managedArtifacts[packageIdentity: packageIdentity, targetName: targetName] else {
512+
XCTFail("\(packageIdentity).\(targetName) does not exists", file: file, line: line)
505513
return
506514
}
507515
XCTAssertEqual(artifact.path, path)

‎Sources/Workspace/ManagedArtifact.swift

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -108,30 +108,30 @@ extension Workspace {
108108
public final class ManagedArtifacts {
109109

110110
/// A mapping from package url, to target name, to ManagedArtifact.
111-
private var artifactMap: [String: [String: ManagedArtifact]]
111+
private var artifactMap: [PackageIdentity: [String: ManagedArtifact]]
112112

113113
internal var artifacts: AnyCollection<ManagedArtifact> {
114114
AnyCollection(artifactMap.values.lazy.flatMap({ $0.values }))
115115
}
116116

117-
init(artifactMap: [String: [String: ManagedArtifact]] = [:]) {
118-
self.artifactMap = artifactMap
117+
init(_ artifacts: [ManagedArtifact] = []) {
118+
let artifactsByPackagePath = Dictionary(grouping: artifacts, by: { $0.packageRef.identity })
119+
self.artifactMap = artifactsByPackagePath.mapValues{ artifacts in
120+
Dictionary(uniqueKeysWithValues: artifacts.map({ ($0.targetName, $0) }))
121+
}
119122
}
120123

121-
public subscript(packageURL packageURL: String, targetName targetName: String) -> ManagedArtifact? {
122-
artifactMap[packageURL]?[targetName]
124+
public subscript(packageIdentity packageIdentity: PackageIdentity, targetName targetName: String) -> ManagedArtifact? {
125+
artifactMap[packageIdentity]?[targetName]
123126
}
124127

125-
public subscript(packageName packageName: String, targetName targetName: String) -> ManagedArtifact? {
126-
artifacts.first(where: { $0.packageRef.name == packageName && $0.targetName == targetName })
127-
}
128128

129129
public func add(_ artifact: ManagedArtifact) {
130-
artifactMap[artifact.packageRef.location, default: [:]][artifact.targetName] = artifact
130+
artifactMap[artifact.packageRef.identity, default: [:]][artifact.targetName] = artifact
131131
}
132132

133-
public func remove(packageURL: String, targetName: String) {
134-
artifactMap[packageURL]?[targetName] = nil
133+
public func remove(packageIdentity: PackageIdentity, targetName: String) {
134+
artifactMap[packageIdentity]?[targetName] = nil
135135
}
136136
}
137137
}

‎Sources/Workspace/Workspace.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,7 +1672,7 @@ extension Workspace {
16721672

16731673
for artifact in manifestArtifacts.local {
16741674
let existingArtifact = self.state.artifacts[
1675-
packageURL: artifact.packageRef.location,
1675+
packageIdentity: artifact.packageRef.identity,
16761676
targetName: artifact.targetName
16771677
]
16781678

@@ -1686,7 +1686,7 @@ extension Workspace {
16861686

16871687
for artifact in manifestArtifacts.remote {
16881688
let existingArtifact = self.state.artifacts[
1689-
packageURL: artifact.packageRef.location,
1689+
packageIdentity: artifact.packageRef.identity,
16901690
targetName: artifact.targetName
16911691
]
16921692

@@ -1711,7 +1711,7 @@ extension Workspace {
17111711
// Remove the artifacts and directories which are not needed anymore.
17121712
diagnostics.wrap {
17131713
for artifact in artifactsToRemove {
1714-
state.artifacts.remove(packageURL: artifact.packageRef.location, targetName: artifact.targetName)
1714+
state.artifacts.remove(packageIdentity: artifact.packageRef.identity, targetName: artifact.targetName)
17151715

17161716
if case .remote = artifact.source {
17171717
try fileSystem.removeFileTree(artifact.path)

‎Sources/Workspace/WorkspaceState.swift

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,8 @@ fileprivate struct WorkspaceStateStorage {
9696
case 1,2,3,4:
9797
let v4 = try self.decoder.decode(path: self.path, fileSystem: self.fileSystem, as: V4.self)
9898
let dependencies = v4.object.dependencies.map{ Workspace.ManagedDependency($0) }
99-
let artifactsByPackagePath = Dictionary(grouping: v4.object.artifacts, by: { $0.packageRef.location })
100-
let artifactMap = artifactsByPackagePath.mapValues{ artifacts in
101-
Dictionary(uniqueKeysWithValues: artifacts.map({ ($0.targetName, Workspace.ManagedArtifact($0)) }))
102-
}
103-
return (dependencies: .init(dependencies), artifacts: .init(artifactMap: artifactMap))
99+
let artifacts = v4.object.artifacts.map{ Workspace.ManagedArtifact($0) }
100+
return (dependencies: .init(dependencies), artifacts: .init(artifacts))
104101
default:
105102
throw InternalError("unknown RepositoryManager version: \(version)")
106103
}

‎Tests/WorkspaceTests/WorkspaceTests.swift

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4370,23 +4370,23 @@ final class WorkspaceTests: XCTestCase {
43704370
}
43714371

43724372
workspace.checkManagedArtifacts { result in
4373-
result.check(packageName: "A",
4373+
result.check(packageIdentity: .plain("a"),
43744374
targetName: "A1",
43754375
source: .remote(
43764376
url: "https://a.com/a1.zip",
43774377
checksum: "a1"
43784378
),
43794379
path: workspace.artifactsDir.appending(components: "A", "A1.xcframework")
43804380
)
4381-
result.check(packageName: "A",
4381+
result.check(packageIdentity: .plain("a"),
43824382
targetName: "A2",
43834383
source: .remote(
43844384
url: "https://a.com/a2.zip",
43854385
checksum: "a2"
43864386
),
43874387
path: workspace.artifactsDir.appending(components: "A", "A2.xcframework")
43884388
)
4389-
result.check(packageName: "B",
4389+
result.check(packageIdentity: .plain("b"),
43904390
targetName: "B",
43914391
source: .remote(
43924392
url: "https://b.com/b.zip",
@@ -4627,15 +4627,15 @@ final class WorkspaceTests: XCTestCase {
46274627
}
46284628

46294629
workspace.checkManagedArtifacts { result in
4630-
result.check(packageName: "A",
4630+
result.check(packageIdentity: .plain("a"),
46314631
targetName: "A1",
46324632
source: .remote(
46334633
url: "https://a.com/a1.zip",
46344634
checksum: "a1"
46354635
),
46364636
path: workspace.artifactsDir.appending(components: "A", "A1.xcframework")
46374637
)
4638-
result.check(packageName: "A",
4638+
result.check(packageIdentity: .plain("a"),
46394639
targetName: "A2",
46404640
source: .remote(
46414641
url: "https://a.com/a2.zip",
@@ -4644,13 +4644,13 @@ final class WorkspaceTests: XCTestCase {
46444644
path: workspace.artifactsDir.appending(components: "A", "A2.xcframework")
46454645
)
46464646
result.checkNotPresent(packageName: "A", targetName: "A3")
4647-
result.check(packageName: "A",
4647+
result.check(packageIdentity: .plain("a"),
46484648
targetName: "A4",
46494649
source: .local,
46504650
path: a4FrameworkPath
46514651
)
46524652
result.checkNotPresent(packageName: "A", targetName: "A5")
4653-
result.check(packageName: "B",
4653+
result.check(packageIdentity: .plain("b"),
46544654
targetName: "B",
46554655
source: .remote(
46564656
url: "https://b.com/b.zip",
@@ -5166,23 +5166,23 @@ final class WorkspaceTests: XCTestCase {
51665166
}
51675167

51685168
workspace.checkManagedArtifacts { result in
5169-
result.check(packageName: "A",
5169+
result.check(packageIdentity: .plain("a"),
51705170
targetName: "A1",
51715171
source: .remote(
51725172
url: "https://a.com/a1.zip",
51735173
checksum: "a1"
51745174
),
51755175
path: workspace.artifactsDir.appending(components: "A", "A1.artifactbundle")
51765176
)
5177-
result.check(packageName: "A",
5177+
result.check(packageIdentity: .plain("a"),
51785178
targetName: "A2",
51795179
source: .remote(
51805180
url: "https://a.com/a2/a2.zip",
51815181
checksum: "a2"
51825182
),
51835183
path: workspace.artifactsDir.appending(components: "A", "A2.artifactbundle")
51845184
)
5185-
result.check(packageName: "B",
5185+
result.check(packageIdentity: .plain("b"),
51865186
targetName: "B",
51875187
source: .remote(
51885188
url: "https://b.com/b.zip",

0 commit comments

Comments
 (0)