Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache binary artifact globally #7101

Merged
merged 4 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Sources/Commands/PackageTools/ComputeChecksum.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct ComputeChecksum: SwiftCommand {
authorizationProvider: swiftTool.getAuthorizationProvider(),
hostToolchain: swiftTool.getHostToolchain(),
checksumAlgorithm: SHA256(),
cachePath: .none,
customHTTPClient: .none,
customArchiver: .none,
delegate: .none
Expand Down
18 changes: 13 additions & 5 deletions Sources/Commands/ToolWorkspaceDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
tomerd marked this conversation as resolved.
Show resolved Hide resolved
guard case .success(let fetchDetails) = result, !self.observabilityScope.errorsReported else {
return
}

Expand All @@ -155,7 +159,11 @@ class ToolWorkspaceDelegate: WorkspaceDelegate {
}
}

self.outputHandler("Downloaded \(url) (\(duration.descriptionInSeconds))", false)
if fetchDetails.fromCache {
self.outputHandler("Fetched \(url) (\(duration.descriptionInSeconds))", false)
tomerd marked this conversation as resolved.
Show resolved Hide resolved
} else {
self.outputHandler("Downloaded \(url) (\(duration.descriptionInSeconds))", false)
}
}

func downloadingBinaryArtifact(from url: String, bytesDownloaded: Int64, totalBytesToDownload: Int64?) {
Expand Down
4 changes: 2 additions & 2 deletions Sources/SPMTestSupport/MockWorkspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceControl/RepositoryManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
161 changes: 129 additions & 32 deletions Sources/Workspace/Workspace+BinaryArtifacts.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand All @@ -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?
Expand All @@ -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
}

Expand Down Expand Up @@ -126,7 +135,7 @@ extension Workspace {
return (local: localArtifacts, remote: remoteArtifacts)
}

func download(
func fetch(
_ artifacts: [RemoteArtifact],
artifactsDirectory: AbsolutePath,
observabilityScope: ObservabilityScope
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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())
)
}

Expand All @@ -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())
)
}
})
Expand All @@ -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())
)
}
}
Expand Down Expand Up @@ -563,23 +559,124 @@ 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.
func downloadingBinaryArtifact(from url: String, bytesDownloaded: Int64, totalBytesToDownload: Int64?)
/// The workspace finished downloading all binary artifacts.
func didDownloadAllBinaryArtifacts()


tomerd marked this conversation as resolved.
Show resolved Hide resolved
}

extension Workspace.BinaryArtifactsManager {
Expand Down Expand Up @@ -833,7 +930,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
Expand Down
5 changes: 5 additions & 0 deletions Sources/Workspace/Workspace+Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions Sources/Workspace/Workspace+Delegation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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:))
Expand Down
Loading