cache binary artifact globally (#7101)

motivation: like other dependencies, binary artifacts are good
candidates for user level caching such that they do not need to be
re-downloaded

changes:
* update BinaryArtifactsManager to take cache path and use the cache to
store binary artifacts when downloading them
* update test infra to enable/disable artifacts caching
* update workspaace call sites
* update workspace delegate to indicate when using cached binary
artifact
* update and add tests

rdar://111774147

Author: tomerd

Sources/Commands/PackageTools/ComputeChecksum.swift

@@ -33,6 +33,7 @@ struct ComputeChecksum: SwiftCommand {
             authorizationProvider: swiftTool.getAuthorizationProvider(),
             hostToolchain: swiftTool.getHostToolchain(),
             checksumAlgorithm: SHA256(),
+            cachePath: .none,
             customHTTPClient: .none,
             customArchiver: .none,
             delegate: .none

Sources/Commands/ToolWorkspaceDelegate.swift

@@ -81,7 +81,7 @@ class ToolWorkspaceDelegate: WorkspaceDelegate {
             }
         }
 
-        self.outputHandler("Fetched \(packageLocation ?? package.description) (\(duration.descriptionInSeconds))", false)
+        self.outputHandler("Fetched \(packageLocation ?? package.description) from cache (\(duration.descriptionInSeconds))", false)
     }
 
     func fetchingPackage(package: PackageIdentity, packageLocation: String?, progress: Int64, total: Int64?) {
@@ -135,12 +135,16 @@ class ToolWorkspaceDelegate: WorkspaceDelegate {
         self.outputHandler("Computed \(location) at \(version) (\(duration.descriptionInSeconds))", false)
     }
 
-    func willDownloadBinaryArtifact(from url: String) {
-        self.outputHandler("Downloading binary artifact \(url)", false)
+    func willDownloadBinaryArtifact(from url: String, fromCache: Bool) {
+        if fromCache {
+            self.outputHandler("Fetching binary artifact \(url) from cache", false)
+        } else {
+            self.outputHandler("Downloading binary artifact \(url)", false)
+        }
     }
 
-    func didDownloadBinaryArtifact(from url: String, result: Result<AbsolutePath, Error>, duration: DispatchTimeInterval) {
-        guard case .success = result, !self.observabilityScope.errorsReported else {
+    func didDownloadBinaryArtifact(from url: String, result: Result<(path: AbsolutePath, fromCache: Bool), Error>, duration: DispatchTimeInterval) {
+        guard case .success(let fetchDetails) = result, !self.observabilityScope.errorsReported else {
             return
         }
 
@@ -155,7 +159,11 @@ class ToolWorkspaceDelegate: WorkspaceDelegate {
             }
         }
 
-        self.outputHandler("Downloaded \(url) (\(duration.descriptionInSeconds))", false)
+        if fetchDetails.fromCache {
+            self.outputHandler("Fetched \(url) from cache (\(duration.descriptionInSeconds))", false)
+        } else {
+            self.outputHandler("Downloaded \(url) (\(duration.descriptionInSeconds))", false)
+        }
     }
 
     func downloadingBinaryArtifact(from url: String, bytesDownloaded: Int64, totalBytesToDownload: Int64?) {

Sources/SPMTestSupport/MockWorkspace.swift

@@ -893,11 +893,11 @@ public final class MockWorkspaceDelegate: WorkspaceDelegate {
         // noop
     }
 
-    public func willDownloadBinaryArtifact(from url: String) {
+    public func willDownloadBinaryArtifact(from url: String, fromCache: Bool) {
         self.append("downloading binary artifact package: \(url)")
     }
 
-    public func didDownloadBinaryArtifact(from url: String, result: Result<AbsolutePath, Error>, duration: DispatchTimeInterval) {
+    public func didDownloadBinaryArtifact(from url: String, result: Result<(path: AbsolutePath, fromCache: Bool), Error>, duration: DispatchTimeInterval) {
         self.append("finished downloading binary artifact package: \(url)")
     }
 

Sources/SourceControl/RepositoryManager.swift

@@ -441,7 +441,7 @@ public class RepositoryManager: Cancellable {
     }
 
     /// Sets up the cache directories if they don't already exist.
-    public func initializeCacheIfNeeded(cachePath: AbsolutePath) throws {
+    private func initializeCacheIfNeeded(cachePath: AbsolutePath) throws {
         // Create the supplied cache directory.
         if !self.fileSystem.exists(cachePath) {
             try self.fileSystem.createDirectory(cachePath, recursive: true)

Sources/Workspace/Workspace+BinaryArtifacts.swift

@@ -26,10 +26,16 @@ extension Workspace {
     public struct CustomBinaryArtifactsManager {
         let httpClient: LegacyHTTPClient?
         let archiver: Archiver?
+        let useCache: Bool?
 
-        public init(httpClient: LegacyHTTPClient? = .none, archiver: Archiver? = .none) {
+        public init(
+            httpClient: LegacyHTTPClient? = .none,
+            archiver: Archiver? = .none,
+            useCache: Bool? = .none
+        ) {
             self.httpClient = httpClient
             self.archiver = archiver
+            self.useCache = useCache
         }
     }
 
@@ -43,13 +49,15 @@ extension Workspace {
         private let httpClient: LegacyHTTPClient
         private let archiver: Archiver
         private let checksumAlgorithm: HashAlgorithm
+        private let cachePath: AbsolutePath?
         private let delegate: Delegate?
 
         public init(
             fileSystem: FileSystem,
             authorizationProvider: AuthorizationProvider?,
             hostToolchain: UserToolchain,
             checksumAlgorithm: HashAlgorithm,
+            cachePath: AbsolutePath?,
             customHTTPClient: LegacyHTTPClient?,
             customArchiver: Archiver?,
             delegate: Delegate?
@@ -60,6 +68,7 @@ extension Workspace {
             self.checksumAlgorithm = checksumAlgorithm
             self.httpClient = customHTTPClient ?? LegacyHTTPClient()
             self.archiver = customArchiver ?? ZipArchiver(fileSystem: fileSystem)
+            self.cachePath = cachePath
             self.delegate = delegate
         }
 
@@ -126,7 +135,7 @@ extension Workspace {
             return (local: localArtifacts, remote: remoteArtifacts)
         }
 
-        func download(
+        func fetch(
             _ artifacts: [RemoteArtifact],
             artifactsDirectory: AbsolutePath,
             observabilityScope: ObservabilityScope
@@ -229,37 +238,24 @@ extension Workspace {
                 }
 
                 group.enter()
-                var headers = HTTPClientHeaders()
-                headers.add(name: "Accept", value: "application/octet-stream")
-                var request = LegacyHTTPClient.Request.download(
-                    url: artifact.url,
-                    headers: headers,
-                    fileSystem: self.fileSystem,
-                    destination: archivePath
-                )
-                request.options.authorizationProvider = self.authorizationProvider?.httpAuthorizationHeader(for:)
-                request.options.retryStrategy = .exponentialBackoff(maxAttempts: 3, baseDelay: .milliseconds(50))
-                request.options.validResponseCodes = [200]
-
-                let downloadStart: DispatchTime = .now()
-                self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString)
-                observabilityScope.emit(debug: "downloading \(artifact.url) to \(archivePath)")
-                self.httpClient.execute(
-                    request,
+                let fetchStart: DispatchTime = .now()
+                self.fetch(
+                    artifact: artifact,
+                    destination: archivePath,
+                    observabilityScope: observabilityScope,
                     progress: { bytesDownloaded, totalBytesToDownload in
                         self.delegate?.downloadingBinaryArtifact(
                             from: artifact.url.absoluteString,
                             bytesDownloaded: bytesDownloaded,
                             totalBytesToDownload: totalBytesToDownload
                         )
                     },
-                    completion: { downloadResult in
+                    completion: { fetchResult in
                         defer { group.leave() }
 
-                        // TODO: Use the same extraction logic for both remote and local archived artifacts.
-                        switch downloadResult {
-                        case .success:
-
+                        switch fetchResult {
+                        case .success(let cached):
+                            // TODO: Use the same extraction logic for both remote and local archived artifacts.
                             group.enter()
                             observabilityScope.emit(debug: "validating \(archivePath)")
                             self.archiver.validate(path: archivePath, completion: { validationResult in
@@ -381,8 +377,8 @@ extension Workspace {
                                                 )
                                                 self.delegate?.didDownloadBinaryArtifact(
                                                     from: artifact.url.absoluteString,
-                                                    result: .success(artifactPath),
-                                                    duration: downloadStart.distance(to: .now())
+                                                    result: .success((path: artifactPath, fromCache: cached)),
+                                                    duration: fetchStart.distance(to: .now())
                                                 )
                                             case .failure(let error):
                                                 observabilityScope.emit(.remoteArtifactFailedExtraction(
@@ -393,7 +389,7 @@ extension Workspace {
                                                 self.delegate?.didDownloadBinaryArtifact(
                                                     from: artifact.url.absoluteString,
                                                     result: .failure(error),
-                                                    duration: downloadStart.distance(to: .now())
+                                                    duration: fetchStart.distance(to: .now())
                                                 )
                                             }
 
@@ -409,7 +405,7 @@ extension Workspace {
                                     self.delegate?.didDownloadBinaryArtifact(
                                         from: artifact.url.absoluteString,
                                         result: .failure(error),
-                                        duration: downloadStart.distance(to: .now())
+                                        duration: fetchStart.distance(to: .now())
                                     )
                                 }
                             })
@@ -423,7 +419,7 @@ extension Workspace {
                             self.delegate?.didDownloadBinaryArtifact(
                                 from: artifact.url.absoluteString,
                                 result: .failure(error),
-                                duration: downloadStart.distance(to: .now())
+                                duration: fetchStart.distance(to: .now())
                             )
                         }
                     }
@@ -563,17 +559,116 @@ extension Workspace {
                 try cancellableArchiver.cancel(deadline: deadline)
             }
         }
+
+        private func fetch(
+            artifact: RemoteArtifact,
+            destination: AbsolutePath,
+            observabilityScope: ObservabilityScope,
+            progress: @escaping (Int64, Optional<Int64>) -> Void,
+            completion: @escaping (Result<Bool, Error>) -> Void
+        ) {
+            // not using cache, download directly
+            guard let cachePath = self.cachePath else {
+                self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString, fromCache: false)
+                return self.download(
+                    artifact: artifact,
+                    destination: destination,
+                    observabilityScope: observabilityScope,
+                    progress: progress,
+                    completion: { result in
+                        // not fetched from cache
+                        completion(result.map{ _ in false })
+                    }
+                )
+            }
+
+            // initialize cache if necessary
+            do {
+                if !self.fileSystem.exists(cachePath) {
+                    try self.fileSystem.createDirectory(cachePath, recursive: true)
+                }
+            } catch {
+                return completion(.failure(error))
+            }
+
+
+            // try to fetch from cache, or download and cache
+            // / FIXME: use better escaping of URL
+            let cacheKey = artifact.url.absoluteString.spm_mangledToC99ExtendedIdentifier()
+            let cachedArtifactPath = cachePath.appending(cacheKey)
+
+            if self.fileSystem.exists(cachedArtifactPath) {
+                observabilityScope.emit(debug: "copying cached binary artifact for \(artifact.url) from \(cachedArtifactPath)")
+                self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString, fromCache: true)
+                return completion(
+                    Result.init(catching: {
+                        // copy from cache to destination
+                        try self.fileSystem.copy(from: cachedArtifactPath, to: destination)
+                        return true // fetched from cache
+                    })
+                )
+            }
+
+            // download to the cache
+            observabilityScope.emit(debug: "downloading binary artifact for \(artifact.url) to cached at \(cachedArtifactPath)")
+            self.download(
+                artifact: artifact,
+                destination: cachedArtifactPath,
+                observabilityScope: observabilityScope,
+                progress: progress,
+                completion: { result in
+                    self.delegate?.willDownloadBinaryArtifact(from: artifact.url.absoluteString, fromCache: false)
+                    completion(result.flatMap {
+                        Result.init(catching: {
+                            // copy from cache to destination
+                            try self.fileSystem.copy(from: cachedArtifactPath, to: destination)
+                            return false // not fetched from cache
+                        })
+                    })
+                }
+            )
+        }
+
+        private func download(
+            artifact: RemoteArtifact,
+            destination: AbsolutePath,
+            observabilityScope: ObservabilityScope,
+            progress: @escaping (Int64, Optional<Int64>) -> Void,
+            completion: @escaping (Result<Void, Error>) -> Void
+        ) {
+            observabilityScope.emit(debug: "downloading \(artifact.url) to \(destination)")
+
+            var headers = HTTPClientHeaders()
+            headers.add(name: "Accept", value: "application/octet-stream")
+            var request = LegacyHTTPClient.Request.download(
+                url: artifact.url,
+                headers: headers,
+                fileSystem: self.fileSystem,
+                destination: destination
+            )
+            request.options.authorizationProvider = self.authorizationProvider?.httpAuthorizationHeader(for:)
+            request.options.retryStrategy = .exponentialBackoff(maxAttempts: 3, baseDelay: .milliseconds(50))
+            request.options.validResponseCodes = [200]
+
+            self.httpClient.execute(
+                request,
+                progress: progress,
+                completion: { result in
+                    completion(result.map{ _ in Void() })
+                }
+            )
+        }
     }
 }
 
 /// Delegate to notify clients about actions being performed by BinaryArtifactsDownloadsManage.
 public protocol BinaryArtifactsManagerDelegate {
     /// The workspace has started downloading a binary artifact.
-    func willDownloadBinaryArtifact(from url: String)
+    func willDownloadBinaryArtifact(from url: String, fromCache: Bool)
     /// The workspace has finished downloading a binary artifact.
     func didDownloadBinaryArtifact(
         from url: String,
-        result: Result<AbsolutePath, Error>,
+        result: Result<(path: AbsolutePath, fromCache: Bool), Error>,
         duration: DispatchTimeInterval
     )
     /// The workspace is downloading a binary artifact.
@@ -833,7 +928,7 @@ extension Workspace {
         }
 
         // Download the artifacts
-        let downloadedArtifacts = try self.binaryArtifactsManager.download(
+        let downloadedArtifacts = try self.binaryArtifactsManager.fetch(
             artifactsToDownload,
             artifactsDirectory: self.location.artifactsDirectory,
             observabilityScope: observabilityScope

Sources/Workspace/Workspace+Configuration.swift

@@ -145,6 +145,11 @@ extension Workspace {
             self.sharedCacheDirectory.map { $0.appending(components: "registry", "downloads") }
         }
 
+        /// Path to the shared repositories cache.
+        public var sharedBinaryArtifactsCacheDirectory: AbsolutePath? {
+            self.sharedCacheDirectory.map { $0.appending("artifacts") }
+        }
+
         /// Create a new workspace location.
         ///
         /// - Parameters:

Sources/Workspace/Workspace+Delegation.swift

@@ -126,11 +126,11 @@ public protocol WorkspaceDelegate: AnyObject {
     func resolvedFileChanged()
 
     /// The workspace has started downloading a binary artifact.
-    func willDownloadBinaryArtifact(from url: String)
+    func willDownloadBinaryArtifact(from url: String, fromCache: Bool)
     /// The workspace has finished downloading a binary artifact.
     func didDownloadBinaryArtifact(
         from url: String,
-        result: Result<AbsolutePath, Error>,
+        result: Result<(path: AbsolutePath, fromCache: Bool), Error>,
         duration: DispatchTimeInterval
     )
     /// The workspace is downloading a binary artifact.
@@ -399,13 +399,13 @@ struct WorkspaceBinaryArtifactsManagerDelegate: Workspace.BinaryArtifactsManager
         self.workspaceDelegate = workspaceDelegate
     }
 
-    func willDownloadBinaryArtifact(from url: String) {
-        self.workspaceDelegate?.willDownloadBinaryArtifact(from: url)
+    func willDownloadBinaryArtifact(from url: String, fromCache: Bool) {
+        self.workspaceDelegate?.willDownloadBinaryArtifact(from: url, fromCache: fromCache)
     }
 
     func didDownloadBinaryArtifact(
         from url: String,
-        result: Result<AbsolutePath, Error>,
+        result: Result<(path: AbsolutePath, fromCache: Bool), Error>,
         duration: DispatchTimeInterval
     ) {
         self.workspaceDelegate?.didDownloadBinaryArtifact(from: url, result: result, duration: duration)

Sources/Workspace/Workspace.swift

@@ -528,6 +528,7 @@ public class Workspace {
             authorizationProvider: authorizationProvider,
             hostToolchain: hostToolchain,
             checksumAlgorithm: checksumAlgorithm,
+            cachePath:  customBinaryArtifactsManager?.useCache == false || !configuration.sharedDependenciesCacheEnabled ? .none : location.sharedBinaryArtifactsCacheDirectory,
             customHTTPClient: customBinaryArtifactsManager?.httpClient,
             customArchiver: customBinaryArtifactsManager?.archiver,
             delegate: delegate.map(WorkspaceBinaryArtifactsManagerDelegate.init(workspaceDelegate:))