Skip to content

Commit 132fe00

Browse files
tomerdtomerd
authored
cleanup use of pins (#6694)
motivaiton: nicer code changes: * remove unnecessary PinsStore::pins API * rename PinsStore::pinsMap tp PinsStore::pins * adjust call sites
1 parent ada0e31 commit 132fe00

File tree

11 files changed

+89
-94
lines changed

11 files changed

+89
-94
lines changed

‎Sources/Commands/PackageTools/Update.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ extension SwiftPackageTool {
6767

6868
var report = "\(changes.count) dependenc\(changes.count == 1 ? "y has" : "ies have") changed\(changes.count > 0 ? ":" : ".")"
6969
for (package, change) in changes {
70-
let currentVersion = pins.pinsMap[package.identity]?.state.description ?? ""
70+
let currentVersion = pins.pins[package.identity]?.state.description ?? ""
7171
switch change {
7272
case let .added(state):
7373
report += "\n"

‎Sources/PackageGraph/PinsStore.swift

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import enum TSCBasic.JSON
1919
import struct TSCUtility.Version
2020

2121
public final class PinsStore {
22-
public typealias PinsMap = [PackageIdentity: PinsStore.Pin]
22+
public typealias Pins = [PackageIdentity: PinsStore.Pin]
2323

2424
public struct Pin: Equatable {
2525
/// The package reference of the pinned dependency.
@@ -58,15 +58,10 @@ public final class PinsStore {
5858
private let _pins: ThreadSafeKeyValueStore<PackageIdentity, PinsStore.Pin>
5959

6060
/// The current pins.
61-
62-
public var pinsMap: PinsMap {
61+
public var pins: Pins {
6362
self._pins.get()
6463
}
6564

66-
public var pins: AnySequence<Pin> {
67-
AnySequence<Pin>(self.pinsMap.values)
68-
}
69-
7065
/// Create a new pins store.
7166
///
7267
/// - Parameters:
@@ -157,7 +152,7 @@ private struct PinsStorage {
157152
self.fileSystem = fileSystem
158153
}
159154

160-
func load(mirrors: DependencyMirrors) throws -> PinsStore.PinsMap {
155+
func load(mirrors: DependencyMirrors) throws -> PinsStore.Pins {
161156
if !self.fileSystem.exists(self.path) {
162157
return [:]
163158
}
@@ -190,7 +185,7 @@ private struct PinsStorage {
190185
}
191186

192187
func save(
193-
pins: PinsStore.PinsMap,
188+
pins: PinsStore.Pins,
194189
mirrors: DependencyMirrors,
195190
removeIfEmpty: Bool,
196191
toolsVersion: ToolsVersion
@@ -254,7 +249,7 @@ private struct PinsStorage {
254249
let version: Int
255250
let object: Container
256251

257-
init(pins: PinsStore.PinsMap, mirrors: DependencyMirrors) throws {
252+
init(pins: PinsStore.Pins, mirrors: DependencyMirrors) throws {
258253
self.version = Self.version
259254
self.object = try .init(
260255
pins: pins.values
@@ -356,7 +351,7 @@ private struct PinsStorage {
356351
let version: Int
357352
let pins: [Pin]
358353

359-
init(pins: PinsStore.PinsMap, mirrors: DependencyMirrors) throws {
354+
init(pins: PinsStore.Pins, mirrors: DependencyMirrors) throws {
360355
self.version = Self.version
361356
self.pins = try pins.values
362357
.sorted(by: { $0.packageRef.identity < $1.packageRef.identity })

‎Sources/PackageGraph/PubGrub/ContainerProvider.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ final class ContainerProvider {
2525
private let skipUpdate: Bool
2626

2727
/// Reference to the pins store.
28-
private let pinsMap: PinsStore.PinsMap
28+
private let pins: PinsStore.Pins
2929

3030
/// Observability scope to emit diagnostics with
3131
private let observabilityScope: ObservabilityScope
@@ -39,12 +39,12 @@ final class ContainerProvider {
3939
init(
4040
provider underlying: PackageContainerProvider,
4141
skipUpdate: Bool,
42-
pinsMap: PinsStore.PinsMap,
42+
pins: PinsStore.Pins,
4343
observabilityScope: ObservabilityScope
4444
) {
4545
self.underlying = underlying
4646
self.skipUpdate = skipUpdate
47-
self.pinsMap = pinsMap
47+
self.pins = pins
4848
self.observabilityScope = observabilityScope
4949
}
5050

@@ -84,7 +84,7 @@ final class ContainerProvider {
8484
on: .sharedConcurrent
8585
) { result in
8686
let result = result.tryMap { container -> PubGrubPackageContainer in
87-
let pubGrubContainer = PubGrubPackageContainer(underlying: container, pinsMap: self.pinsMap)
87+
let pubGrubContainer = PubGrubPackageContainer(underlying: container, pins: self.pins)
8888
// only cache positive results
8989
self.containersCache[package] = pubGrubContainer
9090
return pubGrubContainer
@@ -115,7 +115,7 @@ final class ContainerProvider {
115115
defer { self.prefetches[identifier]?.leave() }
116116
// only cache positive results
117117
if case .success(let container) = result {
118-
self.containersCache[identifier] = PubGrubPackageContainer(underlying: container, pinsMap: self.pinsMap)
118+
self.containersCache[identifier] = PubGrubPackageContainer(underlying: container, pins: self.pins)
119119
}
120120
}
121121
}

‎Sources/PackageGraph/PubGrub/PubGrubDependencyResolver.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public struct PubGrubDependencyResolver {
101101
}
102102

103103
/// Reference to the pins store, if provided.
104-
private let pinsMap: PinsStore.PinsMap
104+
private let pins: PinsStore.Pins
105105

106106
/// The container provider used to load package containers.
107107
private let provider: ContainerProvider
@@ -120,20 +120,20 @@ public struct PubGrubDependencyResolver {
120120

121121
public init(
122122
provider: PackageContainerProvider,
123-
pinsMap: PinsStore.PinsMap = [:],
123+
pins: PinsStore.Pins = [:],
124124
skipDependenciesUpdates: Bool = false,
125125
prefetchBasedOnResolvedFile: Bool = false,
126126
observabilityScope: ObservabilityScope,
127127
delegate: DependencyResolverDelegate? = nil
128128
) {
129129
self.packageContainerProvider = provider
130-
self.pinsMap = pinsMap
130+
self.pins = pins
131131
self.skipDependenciesUpdates = skipDependenciesUpdates
132132
self.prefetchBasedOnResolvedFile = prefetchBasedOnResolvedFile
133133
self.provider = ContainerProvider(
134134
provider: self.packageContainerProvider,
135135
skipUpdate: self.skipDependenciesUpdates,
136-
pinsMap: self.pinsMap,
136+
pins: self.pins,
137137
observabilityScope: observabilityScope
138138
)
139139
self.delegate = delegate
@@ -189,7 +189,7 @@ public struct PubGrubDependencyResolver {
189189
// We avoid prefetching packages that are overridden since
190190
// otherwise we'll end up creating a repository container
191191
// for them.
192-
let pins = self.pinsMap.values
192+
let pins = self.pins.values
193193
.map(\.packageRef)
194194
.filter { !inputs.overriddenPackages.keys.contains($0) }
195195
self.provider.prefetch(containers: pins)
@@ -362,7 +362,7 @@ public struct PubGrubDependencyResolver {
362362
// latest commit on that branch. Note that if this revision-based dependency is
363363
// already a commit, then its pin entry doesn't matter in practice.
364364
let revisionForDependencies: String
365-
if case .branch(revision, let pinRevision) = pinsMap[package.identity]?.state {
365+
if case .branch(revision, let pinRevision) = self.pins[package.identity]?.state {
366366
revisionForDependencies = pinRevision
367367

368368
// Mark the package as overridden with the pinned revision and record the branch as well.

‎Sources/PackageGraph/PubGrub/PubGrubPackageContainer.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ internal final class PubGrubPackageContainer {
2323
let underlying: PackageContainer
2424

2525
/// Reference to the pins map.
26-
private let pinsMap: PinsStore.PinsMap
26+
private let pins: PinsStore.Pins
2727

28-
init(underlying: PackageContainer, pinsMap: PinsStore.PinsMap) {
28+
init(underlying: PackageContainer, pins: PinsStore.Pins) {
2929
self.underlying = underlying
30-
self.pinsMap = pinsMap
30+
self.pins = pins
3131
}
3232

3333
var package: PackageReference {
@@ -36,7 +36,7 @@ internal final class PubGrubPackageContainer {
3636

3737
/// Returns the pinned version for this package, if any.
3838
var pinnedVersion: Version? {
39-
switch self.pinsMap[self.underlying.package.identity]?.state {
39+
switch self.pins[self.underlying.package.identity]?.state {
4040
case .version(let version, _):
4141
return version
4242
default:

‎Sources/SPMTestSupport/MockWorkspace.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -758,11 +758,11 @@ public final class MockWorkspace {
758758
}
759759

760760
public func check(notPresent name: String, file: StaticString = #file, line: UInt = #line) {
761-
XCTAssertFalse(self.store.pinsMap.keys.contains(where: { $0.description == name }), "Unexpectedly found \(name) in Package.resolved", file: file, line: line)
761+
XCTAssertFalse(self.store.pins.keys.contains(where: { $0.description == name }), "Unexpectedly found \(name) in Package.resolved", file: file, line: line)
762762
}
763763

764764
public func check(dependency package: String, at state: State, file: StaticString = #file, line: UInt = #line) {
765-
guard let pin = store.pinsMap.first(where: { $0.key.description == package })?.value else {
765+
guard let pin = store.pins.first(where: { $0.key.description == package })?.value else {
766766
XCTFail("Pin for \(package) not found", file: file, line: line)
767767
return
768768
}
@@ -789,7 +789,7 @@ public final class MockWorkspace {
789789
}
790790

791791
public func check(dependency package: String, url: String, file: StaticString = #file, line: UInt = #line) {
792-
guard let pin = store.pinsMap.first(where: { $0.key.description == package })?.value else {
792+
guard let pin = store.pins.first(where: { $0.key.description == package })?.value else {
793793
XCTFail("Pin for \(package) not found", file: file, line: line)
794794
return
795795
}

‎Sources/Workspace/Workspace.swift

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,18 +1011,18 @@ extension Workspace {
10111011
// Create constraints based on root manifest and pins for the update resolution.
10121012
updateConstraints += try graphRoot.constraints()
10131013

1014-
let pinsMap: PinsStore.PinsMap
1014+
let pins: PinsStore.Pins
10151015
if packages.isEmpty {
10161016
// No input packages so we have to do a full update. Set pins map to empty.
1017-
pinsMap = [:]
1017+
pins = [:]
10181018
} else {
10191019
// We have input packages so we have to partially update the package graph. Remove
10201020
// the pins for the input packages so only those packages are updated.
1021-
pinsMap = pinsStore.pinsMap.filter{ !packages.contains($0.value.packageRef.identity.description) && !packages.contains($0.value.packageRef.deprecatedName) }
1021+
pins = pinsStore.pins.filter{ !packages.contains($0.value.packageRef.identity.description) && !packages.contains($0.value.packageRef.deprecatedName) }
10221022
}
10231023

10241024
// Resolve the dependencies.
1025-
let resolver = try self.createResolver(pinsMap: pinsMap, observabilityScope: observabilityScope)
1025+
let resolver = try self.createResolver(pins: pins, observabilityScope: observabilityScope)
10261026
self.activeResolver = resolver
10271027

10281028
let updateResults = self.resolveDependencies(
@@ -1606,11 +1606,11 @@ extension Workspace {
16061606
// compare for any differences between the existing state and the stored one
16071607
// subtle changes between versions of SwiftPM could treat URLs differently
16081608
// in which case we don't want to cause unnecessary churn
1609-
if dependenciesToPin.count != storedPinStore.pinsMap.count {
1609+
if dependenciesToPin.count != storedPinStore.pins.count {
16101610
needsUpdate = true
16111611
} else {
16121612
for dependency in dependenciesToPin {
1613-
if let pin = storedPinStore.pinsMap.first(where: { $0.value.packageRef.equalsIncludingLocation(dependency.packageRef) }) {
1613+
if let pin = storedPinStore.pins.first(where: { $0.value.packageRef.equalsIncludingLocation(dependency.packageRef) }) {
16141614
if pin.value.state != PinsStore.Pin(dependency)?.state {
16151615
needsUpdate = true
16161616
break
@@ -2416,7 +2416,7 @@ extension Workspace {
24162416
// We just request the packages here, repository manager will
24172417
// automatically manage the parallelism.
24182418
let group = DispatchGroup()
2419-
for pin in pinsStore.pins {
2419+
for pin in pinsStore.pins.values {
24202420
group.enter()
24212421
let observabilityScope = observabilityScope.makeChildScope(description: "requesting package containers", metadata: pin.packageRef.diagnosticsMetadata)
24222422
packageContainerProvider.getContainer(for: pin.packageRef, skipUpdate: self.configuration.skipDependenciesUpdates, observabilityScope: observabilityScope, on: .sharedConcurrent, completion: { _ in
@@ -2429,7 +2429,7 @@ extension Workspace {
24292429
//
24302430
// We require cloning if there is no checkout or if the checkout doesn't
24312431
// match with the pin.
2432-
let requiredPins = pinsStore.pins.filter{ pin in
2432+
let requiredPins = pinsStore.pins.values.filter{ pin in
24332433
// also compare the location in case it has changed
24342434
guard let dependency = state.dependencies[comparingLocation: pin.packageRef] else {
24352435
return true
@@ -2574,7 +2574,7 @@ extension Workspace {
25742574
computedConstraints += try graphRoot.constraints() + constraints
25752575

25762576
// Perform dependency resolution.
2577-
let resolver = try self.createResolver(pinsMap: pinsStore.pinsMap, observabilityScope: observabilityScope)
2577+
let resolver = try self.createResolver(pins: pinsStore.pins, observabilityScope: observabilityScope)
25782578
self.activeResolver = resolver
25792579

25802580
let result = self.resolveDependencies(
@@ -2770,7 +2770,7 @@ extension Workspace {
27702770
constraints
27712771

27722772
let precomputationProvider = ResolverPrecomputationProvider(root: root, dependencyManifests: dependencyManifests)
2773-
let resolver = PubGrubDependencyResolver(provider: precomputationProvider, pinsMap: pinsStore.pinsMap, observabilityScope: observabilityScope)
2773+
let resolver = PubGrubDependencyResolver(provider: precomputationProvider, pins: pinsStore.pins, observabilityScope: observabilityScope)
27742774
let result = resolver.solve(constraints: computedConstraints)
27752775

27762776
guard !observabilityScope.errorsReported else {
@@ -2810,12 +2810,12 @@ extension Workspace {
28102810
// a required dependency that is already loaded (managed) should be represented in the pins store.
28112811
// also comparing location as it may have changed at this point
28122812
if requiredDependencies.contains(where: { $0.equalsIncludingLocation(dependency.packageRef) }) {
2813-
let pin = pinsStore.pinsMap[dependency.packageRef.identity]
2813+
let pin = pinsStore.pins[dependency.packageRef.identity]
28142814
// if pin not found, or location is different (it may have changed at this point) pin it
28152815
if !(pin?.packageRef.equalsIncludingLocation(dependency.packageRef) ?? false) {
28162816
pinsStore.pin(dependency)
28172817
}
2818-
} else if let pin = pinsStore.pinsMap[dependency.packageRef.identity] {
2818+
} else if let pin = pinsStore.pins[dependency.packageRef.identity] {
28192819
// otherwise, it should *not* be in the pins store.
28202820
pinsStore.remove(pin)
28212821
}
@@ -2969,7 +2969,7 @@ extension Workspace {
29692969
// If we have a branch and we shouldn't be updating the
29702970
// branches, use the revision from pin instead (if present).
29712971
if branch != nil, !updateBranches {
2972-
if case .branch(branch, let pinRevision) = pinsStore.pins.first(where: { $0.packageRef == packageRef })?.state {
2972+
if case .branch(branch, let pinRevision) = pinsStore.pins.values.first(where: { $0.packageRef == packageRef })?.state {
29732973
revision = Revision(identifier: pinRevision)
29742974
}
29752975
}
@@ -3018,7 +3018,7 @@ extension Workspace {
30183018
}
30193019

30203020
/// Creates resolver for the workspace.
3021-
fileprivate func createResolver(pinsMap: PinsStore.PinsMap, observabilityScope: ObservabilityScope) throws -> PubGrubDependencyResolver {
3021+
fileprivate func createResolver(pins: PinsStore.Pins, observabilityScope: ObservabilityScope) throws -> PubGrubDependencyResolver {
30223022
var delegate: DependencyResolverDelegate
30233023
let observabilityDelegate = ObservabilityDependencyResolverDelegate(observabilityScope: observabilityScope)
30243024
if let workspaceDelegate = self.delegate {
@@ -3032,7 +3032,7 @@ extension Workspace {
30323032

30333033
return PubGrubDependencyResolver(
30343034
provider: packageContainerProvider,
3035-
pinsMap: pinsMap,
3035+
pins: pins,
30363036
skipDependenciesUpdates: self.configuration.skipDependenciesUpdates,
30373037
prefetchBasedOnResolvedFile: self.configuration.prefetchBasedOnResolvedFile,
30383038
observabilityScope: observabilityScope,

‎Tests/CommandsTests/PackageToolTests.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -905,7 +905,7 @@ final class PackageToolTests: CommandsTestCase {
905905
let pinsStore = try PinsStore(pinsFile: pinsFile, workingDirectory: fixturePath, fileSystem: localFileSystem, mirrors: .init())
906906
let state = PinsStore.PinState.branch(name: "YOLO", revision: yoloRevision.identifier)
907907
let identity = PackageIdentity(path: barPath)
908-
XCTAssertEqual(pinsStore.pinsMap[identity]?.state, state)
908+
XCTAssertEqual(pinsStore.pins[identity]?.state, state)
909909
}
910910

911911
// Try to pin bar at a revision.
@@ -914,7 +914,7 @@ final class PackageToolTests: CommandsTestCase {
914914
let pinsStore = try PinsStore(pinsFile: pinsFile, workingDirectory: fixturePath, fileSystem: localFileSystem, mirrors: .init())
915915
let state = PinsStore.PinState.revision(yoloRevision.identifier)
916916
let identity = PackageIdentity(path: barPath)
917-
XCTAssertEqual(pinsStore.pinsMap[identity]?.state, state)
917+
XCTAssertEqual(pinsStore.pins[identity]?.state, state)
918918
}
919919

920920
// Try to pin bar at a bad revision.
@@ -956,7 +956,7 @@ final class PackageToolTests: CommandsTestCase {
956956
XCTAssertEqual(pinsStore.pins.map{$0}.count, 2)
957957
for pkg in ["bar", "baz"] {
958958
let path = try SwiftPM.packagePath(for: pkg, packageRoot: fooPath)
959-
let pin = pinsStore.pinsMap[PackageIdentity(path: path)]!
959+
let pin = pinsStore.pins[PackageIdentity(path: path)]!
960960
XCTAssertEqual(pin.packageRef.identity, PackageIdentity(path: path))
961961
guard case .localSourceControl(let path) = pin.packageRef.kind, path.pathString.hasSuffix(pkg) else {
962962
return XCTFail("invalid pin location \(path)")
@@ -980,7 +980,7 @@ final class PackageToolTests: CommandsTestCase {
980980
try execute("resolve", "bar")
981981
let pinsStore = try PinsStore(pinsFile: pinsFile, workingDirectory: fixturePath, fileSystem: localFileSystem, mirrors: .init())
982982
let identity = PackageIdentity(path: barPath)
983-
switch pinsStore.pinsMap[identity]?.state {
983+
switch pinsStore.pins[identity]?.state {
984984
case .version(let version, revision: _):
985985
XCTAssertEqual(version, "1.2.3")
986986
default:
@@ -1009,7 +1009,7 @@ final class PackageToolTests: CommandsTestCase {
10091009
try execute("resolve", "bar", "--version", "1.2.3")
10101010
let pinsStore = try PinsStore(pinsFile: pinsFile, workingDirectory: fixturePath, fileSystem: localFileSystem, mirrors: .init())
10111011
let identity = PackageIdentity(path: barPath)
1012-
switch pinsStore.pinsMap[identity]?.state {
1012+
switch pinsStore.pins[identity]?.state {
10131013
case .version(let version, revision: _):
10141014
XCTAssertEqual(version, "1.2.3")
10151015
default:

0 commit comments

Comments
 (0)