Skip to content

Commit b6971ac

Browse files
tomerdtomerd
authored
refactor workspace delegates (#4071)
motivaton: SwiftPM clients need to reverse engineer the package identity from the url changes: * add PackageIdentity to WorkspaceDelegate::willFetchPackage * add PackageIdentity to WorkspaceDelegate::fetchingPackage * add PackageIdentity to WorkspaceDelegate::didFetchPackage * rename WorkspaceDelegate::repositoryWillUpdate to WorkspaceDelegate::willUpdateRepository and add PackageIdentity * rename WorkspaceDelegate::repositoryDidUpdate to WorkspaceDelegate::didUpdateRepository and add PackageIdentity * add PackageIdentity to WorkspaceDelegate::willCreateWorkingCopy * add PackageIdentity to WorkspaceDelegate::didCreateWorkingCopy * add PackageIdentity to WorkspaceDelegate::willCheckOut * add PackageIdentity to WorkspaceDelegate::didCheckOut * add PackageIdentity to WorkspaceDelegate::removing * remove redundant (not in use) Diagnostics argument from WorkspaceDelegate::didCreateWorkingCopy * remove redundant (not in use) Diagnostics argument from WorkspaceDelegate::didCheckOut * update call sites and test
1 parent 92f9d94 commit b6971ac

File tree

8 files changed

+129
-148
lines changed

8 files changed

+129
-148
lines changed

‎Package.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,10 @@ let package = Package(
167167
.target(
168168
/** Source control operations */
169169
name: "SourceControl",
170-
dependencies: ["Basics"],
170+
dependencies: [
171+
"Basics",
172+
"PackageModel"
173+
],
171174
exclude: ["CMakeLists.txt"]
172175
),
173176

‎Sources/Commands/SwiftTool.swift

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import WinSDK
3030

3131
typealias Diagnostic = Basics.Diagnostic
3232

33-
3433
private class ToolWorkspaceDelegate: WorkspaceDelegate {
3534
/// The stream to use for reporting progress.
3635
private let outputStream: ThreadSafeOutputByteStream
@@ -55,10 +54,10 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
5554
}
5655

5756
/// The progress of each individual downloads.
58-
private var downloadProgress: [String: DownloadProgress] = [:]
57+
private var binaryDownloadProgress: [String: DownloadProgress] = [:]
5958

6059
/// The progress of each individual fetch operation
61-
private var fetchProgress: [String: FetchProgress] = [:]
60+
private var fetchProgress: [PackageIdentity: FetchProgress] = [:]
6261

6362
private let queue = DispatchQueue(label: "org.swift.swiftpm.commands.tool-workspace-delegate")
6463

@@ -74,9 +73,9 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
7473
self.observabilityScope = observabilityScope
7574
}
7675

77-
func willFetchPackage(package: String, fetchDetails: PackageFetchDetails) {
76+
func willFetchPackage(package: PackageIdentity, packageLocation: String?, fetchDetails: PackageFetchDetails) {
7877
queue.async {
79-
self.outputStream <<< "Fetching \(package)"
78+
self.outputStream <<< "Fetching \(packageLocation ?? package.description)"
8079
if fetchDetails.fromCache {
8180
self.outputStream <<< " from cache"
8281
}
@@ -85,7 +84,7 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
8584
}
8685
}
8786

88-
func didFetchPackage(package: String, result: Result<PackageFetchDetails, Error>, duration: DispatchTimeInterval) {
87+
func didFetchPackage(package: PackageIdentity, packageLocation: String?, result: Result<PackageFetchDetails, Error>, duration: DispatchTimeInterval) {
8988
queue.async {
9089
if self.observabilityScope.errorsReported {
9190
self.fetchAnimation.clear()
@@ -99,13 +98,13 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
9998
self.fetchProgress.removeAll()
10099
}
101100

102-
self.outputStream <<< "Fetched \(package) (\(duration.descriptionInSeconds))"
101+
self.outputStream <<< "Fetched \(packageLocation ?? package.description) (\(duration.descriptionInSeconds))"
103102
self.outputStream <<< "\n"
104103
self.outputStream.flush()
105104
}
106105
}
107106

108-
func fetchingPackage(package: String, progress: Int64, total: Int64?) {
107+
func fetchingPackage(package: PackageIdentity, packageLocation: String?, progress: Int64, total: Int64?) {
109108
queue.async {
110109
self.fetchProgress[package] = FetchProgress(
111110
progress: progress,
@@ -123,17 +122,17 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
123122
}
124123
}
125124

126-
func repositoryWillUpdate(_ repository: String) {
125+
func willUpdateRepository(package: PackageIdentity, repository url: String) {
127126
queue.async {
128-
self.outputStream <<< "Updating \(repository)"
127+
self.outputStream <<< "Updating \(url)"
129128
self.outputStream <<< "\n"
130129
self.outputStream.flush()
131130
}
132131
}
133132

134-
func repositoryDidUpdate(_ repository: String, duration: DispatchTimeInterval) {
133+
func didUpdateRepository(package: PackageIdentity, repository url: String, duration: DispatchTimeInterval) {
135134
queue.async {
136-
self.outputStream <<< "Updated \(repository) (\(duration.descriptionInSeconds))"
135+
self.outputStream <<< "Updated \(url) (\(duration.descriptionInSeconds))"
137136
self.outputStream <<< "\n"
138137
self.outputStream.flush()
139138
}
@@ -147,32 +146,25 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
147146
}
148147
}
149148

150-
func willCreateWorkingCopy(repository: String, at path: AbsolutePath) {
149+
func willCreateWorkingCopy(package: PackageIdentity, repository url: String, at path: AbsolutePath) {
151150
queue.async {
152-
self.outputStream <<< "Creating working copy for \(repository)"
151+
self.outputStream <<< "Creating working copy for \(url)"
153152
self.outputStream <<< "\n"
154153
self.outputStream.flush()
155154
}
156155
}
157156

158-
func willCheckOut(repository: String, revision: String, at path: AbsolutePath) {
159-
// noop
160-
}
161-
162-
func didCheckOut(repository: String, revision: String, at path: AbsolutePath, error: Basics.Diagnostic?) {
163-
guard case .none = error else {
164-
return // error will be printed before hand
165-
}
157+
func didCheckOut(package: PackageIdentity, repository url: String, revision: String, at path: AbsolutePath) {
166158
queue.async {
167-
self.outputStream <<< "Working copy of \(repository) resolved at \(revision)"
159+
self.outputStream <<< "Working copy of \(url) resolved at \(revision)"
168160
self.outputStream <<< "\n"
169161
self.outputStream.flush()
170162
}
171163
}
172164

173-
func removing(repository: String) {
165+
func removing(package: PackageIdentity, packageLocation: String?) {
174166
queue.async {
175-
self.outputStream <<< "Removing \(repository)"
167+
self.outputStream <<< "Removing \(packageLocation ?? package.description)"
176168
self.outputStream <<< "\n"
177169
self.outputStream.flush()
178170
}
@@ -209,13 +201,13 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
209201
func downloadingBinaryArtifact(from url: String, bytesDownloaded: Int64, totalBytesToDownload: Int64?) {
210202
queue.async {
211203
if let totalBytesToDownload = totalBytesToDownload {
212-
self.downloadProgress[url] = DownloadProgress(
204+
self.binaryDownloadProgress[url] = DownloadProgress(
213205
bytesDownloaded: bytesDownloaded,
214206
totalBytesToDownload: totalBytesToDownload)
215207
}
216208

217-
let step = self.downloadProgress.values.reduce(0, { $0 + $1.bytesDownloaded }) / 1024
218-
let total = self.downloadProgress.values.reduce(0, { $0 + $1.totalBytesToDownload }) / 1024
209+
let step = self.binaryDownloadProgress.values.reduce(0, { $0 + $1.bytesDownloaded }) / 1024
210+
let total = self.binaryDownloadProgress.values.reduce(0, { $0 + $1.totalBytesToDownload }) / 1024
219211
self.downloadAnimation.update(step: Int(step), total: Int(total), text: "Downloading binary artifacts")
220212
}
221213
}
@@ -227,15 +219,16 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
227219
}
228220

229221
self.downloadAnimation.complete(success: true)
230-
self.downloadProgress.removeAll()
222+
self.binaryDownloadProgress.removeAll()
231223
}
232224
}
233225

234226
// noop
235227

236228
func willLoadManifest(packagePath: AbsolutePath, url: String, version: Version?, packageKind: PackageReference.Kind) {}
237229
func didLoadManifest(packagePath: AbsolutePath, url: String, version: Version?, packageKind: PackageReference.Kind, manifest: Manifest?, diagnostics: [Basics.Diagnostic]) {}
238-
func didCreateWorkingCopy(repository url: String, at path: AbsolutePath, error: Basics.Diagnostic?) {}
230+
func willCheckOut(package: PackageIdentity, repository url: String, revision: String, at path: AbsolutePath) {}
231+
func didCreateWorkingCopy(package: PackageIdentity, repository url: String, at path: AbsolutePath) {}
239232
func resolvedFileChanged() {}
240233
}
241234

‎Sources/SPMTestSupport/MockWorkspace.swift

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -755,47 +755,47 @@ public final class MockWorkspaceDelegate: WorkspaceDelegate {
755755

756756
public init() {}
757757

758-
public func repositoryWillUpdate(_ repository: String) {
759-
self.append("updating repo: \(repository)")
758+
public func willUpdateRepository(package: PackageIdentity, repository url: String) {
759+
self.append("updating repo: \(url)")
760760
}
761761

762-
public func repositoryDidUpdate(_ repository: String, duration: DispatchTimeInterval) {
763-
self.append("finished updating repo: \(repository)")
762+
public func didUpdateRepository(package: PackageIdentity, repository url: String, duration: DispatchTimeInterval) {
763+
self.append("finished updating repo: \(url)")
764764
}
765765

766766
public func dependenciesUpToDate() {
767767
self.append("Everything is already up-to-date")
768768
}
769769

770-
public func willFetchPackage(package: String, fetchDetails: PackageFetchDetails) {
771-
self.append("fetching package: \(package)")
770+
public func willFetchPackage(package: PackageIdentity, packageLocation: String?, fetchDetails: PackageFetchDetails) {
771+
self.append("fetching package: \(packageLocation ?? package.description)")
772772
}
773773

774-
public func fetchingPackage(package: String, progress: Int64, total: Int64?) {
774+
public func fetchingPackage(package: PackageIdentity, packageLocation: String?, progress: Int64, total: Int64?) {
775775
}
776776

777-
public func didFetchPackage(package: String, result: Result<PackageFetchDetails, Error>, duration: DispatchTimeInterval) {
778-
self.append("finished fetching package: \(package)")
777+
public func didFetchPackage(package: PackageIdentity, packageLocation: String?, result: Result<PackageFetchDetails, Error>, duration: DispatchTimeInterval) {
778+
self.append("finished fetching package: \(packageLocation ?? package.description)")
779779
}
780780

781-
public func willCreateWorkingCopy(repository url: String, at path: AbsolutePath) {
781+
public func willCreateWorkingCopy(package: PackageIdentity, repository url: String, at path: AbsolutePath) {
782782
self.append("creating working copy for: \(url)")
783783
}
784784

785-
public func didCreateWorkingCopy(repository url: String, at path: AbsolutePath, error: Basics.Diagnostic?) {
785+
public func didCreateWorkingCopy(package: PackageIdentity, repository url: String, at path: AbsolutePath) {
786786
self.append("finished creating working copy for: \(url)")
787787
}
788788

789-
public func willCheckOut(repository url: String, revision: String, at path: AbsolutePath) {
789+
public func willCheckOut(package: PackageIdentity, repository url: String, revision: String, at path: AbsolutePath) {
790790
self.append("checking out repo: \(url)")
791791
}
792792

793-
public func didCheckOut(repository url: String, revision: String, at path: AbsolutePath, error: Basics.Diagnostic?) {
793+
public func didCheckOut(package: PackageIdentity, repository url: String, revision: String, at path: AbsolutePath) {
794794
self.append("finished checking out repo: \(url)")
795795
}
796796

797-
public func removing(repository: String) {
798-
self.append("removing repo: \(repository)")
797+
public func removing(package: PackageIdentity, packageLocation: String?) {
798+
self.append("removing repo: \(packageLocation ?? package.description)")
799799
}
800800

801801
public func willResolveDependencies(reason: WorkspaceResolveReason) {

‎Sources/SourceControl/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ add_library(SourceControl
1313

1414
target_link_libraries(SourceControl PUBLIC
1515
Basics
16+
PackageModel
1617
TSCBasic
1718
TSCUtility
1819
Basics)

‎Sources/SourceControl/RepositoryManager.swift

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import Basics
1212
import Dispatch
1313
import Foundation
1414
import TSCBasic
15+
import PackageModel
1516

1617
/// Manages a collection of bare repositories.
1718
public class RepositoryManager {
@@ -86,13 +87,15 @@ public class RepositoryManager {
8687
/// completion block of another lookup will block.
8788
///
8889
/// - Parameters:
90+
/// - package: The package identity of the repository to fetch,
8991
/// - repository: The repository to look up.
9092
/// - skipUpdate: If a repository is available, skip updating it.
9193
/// - observabilityScope: The observability scope
9294
/// - delegateQueue: Dispatch queue for delegate events
9395
/// - callbackQueue: Dispatch queue for callbacks
9496
/// - completion: The completion block that should be called after lookup finishes.
9597
public func lookup(
98+
package: PackageIdentity,
9699
repository: RepositorySpecifier,
97100
skipUpdate: Bool,
98101
observabilityScope: ObservabilityScope,
@@ -119,13 +122,13 @@ public class RepositoryManager {
119122
// Update the repository when it is being looked up.
120123
let start = DispatchTime.now()
121124
delegateQueue.async {
122-
self.delegate?.willUpdate(repository: handle.repository)
125+
self.delegate?.willUpdate(package: package, repository: handle.repository)
123126
}
124127
let repository = try handle.open()
125128
try repository.fetch()
126129
let duration = start.distance(to: .now())
127130
delegateQueue.async {
128-
self.delegate?.didUpdate(repository: handle.repository, duration: duration)
131+
self.delegate?.didUpdate(package: package, repository: handle.repository, duration: duration)
129132
}
130133
return handle
131134
}))
@@ -139,6 +142,7 @@ public class RepositoryManager {
139142
pendingLookup.notify(queue: callbackQueue) {
140143
// at this point the previous lookup should be complete and we can re-lookup
141144
self.lookup(
145+
package: package,
142146
repository: repository,
143147
skipUpdate: skipUpdate,
144148
observabilityScope: observabilityScope,
@@ -160,7 +164,7 @@ public class RepositoryManager {
160164
let isCached = self.cachePath.map{ self.fileSystem.exists($0.appending(handle.subpath)) } ?? false
161165
delegateQueue.async {
162166
let details = FetchDetails(fromCache: isCached, updatedCache: false)
163-
self.delegate?.willFetch(repository: handle.repository, details: details)
167+
self.delegate?.willFetch(package: package, repository: handle.repository, details: details)
164168
}
165169

166170
let start = DispatchTime.now()
@@ -172,6 +176,7 @@ public class RepositoryManager {
172176
try? self.fileSystem.removeFileTree(repositoryPath)
173177
// Fetch the repo.
174178
let details = try self.fetchAndPopulateCache(
179+
package: package,
175180
handle: handle,
176181
repositoryPath: repositoryPath,
177182
observabilityScope: observabilityScope,
@@ -187,7 +192,7 @@ public class RepositoryManager {
187192
// Inform delegate.
188193
let duration = start.distance(to: .now())
189194
delegateQueue.async {
190-
self.delegate?.didFetch(repository: handle.repository, result: delegateResult, duration: duration)
195+
self.delegate?.didFetch(package: package, repository: handle.repository, result: delegateResult, duration: duration)
191196
}
192197

193198
// remove the pending lookup
@@ -203,12 +208,14 @@ public class RepositoryManager {
203208

204209
/// Fetches the repository into the cache. If no `cachePath` is set or an error occurred fall back to fetching the repository without populating the cache.
205210
/// - Parameters:
211+
/// - package: The package identity of the repository to fetch.
206212
/// - handle: The specifier of the repository to fetch.
207213
/// - repositoryPath: The path where the repository should be fetched to.
208214
/// - observabilityScope: The observability scope
209215
/// - delegateQueue: Dispatch queue for delegate events
210216
@discardableResult
211-
func fetchAndPopulateCache(
217+
private func fetchAndPopulateCache(
218+
package: PackageIdentity,
212219
handle: RepositoryHandle,
213220
repositoryPath: AbsolutePath,
214221
observabilityScope: ObservabilityScope,
@@ -223,6 +230,7 @@ public class RepositoryManager {
223230
if let total = progress.totalSteps {
224231
delegateQueue.async {
225232
self.delegate?.fetching(
233+
package: package,
226234
repository: handle.repository,
227235
objectsFetched: progress.step,
228236
totalObjectsToFetch: total
@@ -273,8 +281,6 @@ public class RepositoryManager {
273281
try self.provider.fetch(repository: handle.repository, to: repositoryPath, progressHandler: updateFetchProgress(progress:))
274282
}
275283
return FetchDetails(fromCache: cacheUsed, updatedCache: cacheUpdated)
276-
277-
278284
}
279285

280286
public func openWorkingCopy(at path: AbsolutePath) throws -> WorkingCheckout {
@@ -397,21 +403,21 @@ extension RepositoryManager {
397403
}
398404

399405
/// Delegate to notify clients about actions being performed by RepositoryManager.
400-
public protocol RepositoryManagerDelegate: AnyObject {
406+
public protocol RepositoryManagerDelegate {
401407
/// Called when a repository is about to be fetched.
402-
func willFetch(repository: RepositorySpecifier, details: RepositoryManager.FetchDetails)
408+
func willFetch(package: PackageIdentity, repository: RepositorySpecifier, details: RepositoryManager.FetchDetails)
403409

404410
/// Called every time the progress of a repository fetch operation updates.
405-
func fetching(repository: RepositorySpecifier, objectsFetched: Int, totalObjectsToFetch: Int)
411+
func fetching(package: PackageIdentity, repository: RepositorySpecifier, objectsFetched: Int, totalObjectsToFetch: Int)
406412

407413
/// Called when a repository has finished fetching.
408-
func didFetch(repository: RepositorySpecifier, result: Result<RepositoryManager.FetchDetails, Error>, duration: DispatchTimeInterval)
414+
func didFetch(package: PackageIdentity, repository: RepositorySpecifier, result: Result<RepositoryManager.FetchDetails, Error>, duration: DispatchTimeInterval)
409415

410416
/// Called when a repository has started updating from its remote.
411-
func willUpdate(repository: RepositorySpecifier)
417+
func willUpdate(package: PackageIdentity, repository: RepositorySpecifier)
412418

413419
/// Called when a repository has finished updating from its remote.
414-
func didUpdate(repository: RepositorySpecifier, duration: DispatchTimeInterval)
420+
func didUpdate(package: PackageIdentity, repository: RepositorySpecifier, duration: DispatchTimeInterval)
415421
}
416422

417423

0 commit comments

Comments
 (0)