Skip to content

[5.6] Cherry-pick TOFU changes from main branch #3930

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

Merged
merged 4 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
21 changes: 19 additions & 2 deletions Sources/Commands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,22 @@ public class SwiftTool {
return .none
}
}

private func getSharedSecurityDirectory() throws -> AbsolutePath? {
do {
let fileSystem = localFileSystem
let sharedSecurityDirectory = fileSystem.swiftPMSecurityDirectory
if !fileSystem.exists(sharedSecurityDirectory) {
try fileSystem.createDirectory(sharedSecurityDirectory, recursive: true)
}
// And make sure we can write files (locking the directory writes a lock file)
try fileSystem.withLock(on: sharedSecurityDirectory, type: .exclusive) { }
return sharedSecurityDirectory
} catch {
self.observabilityScope.emit(warning: "Failed creating shared security directory: \(error)")
return .none
}
}

/// Returns the currently active workspace.
func getActiveWorkspace() throws -> Workspace {
Expand All @@ -649,7 +665,8 @@ public class SwiftTool {

let delegate = ToolWorkspaceDelegate(self.outputStream, logLevel: self.logLevel, observabilityScope: self.observabilityScope)
let provider = GitRepositoryProvider(processSet: processSet)
let sharedCacheDirectory = try self.getSharedCacheDirectory()
let sharedSecurityDirectory = try self.getSharedSecurityDirectory()
let sharedCacheDirectory = try self.getSharedCacheDirectory()
let sharedConfigurationDirectory = try self.getSharedConfigurationDirectory()
let isXcodeBuildSystemEnabled = self.options.buildSystem == .xcode
let workspace = try Workspace(
Expand All @@ -658,7 +675,7 @@ public class SwiftTool {
workingDirectory: buildPath,
editsDirectory: self.editsDirectory(),
resolvedVersionsFile: self.resolvedVersionsFile(),
sharedSecurityDirectory: localFileSystem.swiftPMSecurityDirectory,
sharedSecurityDirectory: sharedSecurityDirectory,
sharedCacheDirectory: sharedCacheDirectory,
sharedConfigurationDirectory: sharedConfigurationDirectory
),
Expand Down
1 change: 0 additions & 1 deletion Sources/PackageFingerprint/Model.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,4 @@ public typealias PackageFingerprints = [Version: [Fingerprint.Kind: Fingerprint]
public enum FingerprintCheckingMode: String {
case strict
case warn
case none
}
11 changes: 10 additions & 1 deletion Sources/PackageFingerprint/PackageFingerprintStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,16 @@ public extension PackageFingerprintStorage {
}
}

public enum PackageFingerprintStorageError: Error, Equatable {
public enum PackageFingerprintStorageError: Error, Equatable, CustomStringConvertible {
case conflict(given: Fingerprint, existing: Fingerprint)
case notFound

public var description: String {
switch self {
case .conflict(let given, let existing):
return "Fingerprint \(given) is different from previously recorded value \(existing)"
case .notFound:
return "Not found"
}
}
}
86 changes: 47 additions & 39 deletions Sources/PackageRegistry/RegistryClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ public final class RegistryClient {
private let archiverProvider: (FileSystem) -> Archiver
private let httpClient: HTTPClient
private let authorizationProvider: HTTPClientAuthorizationProvider?
private let fingerprintStorage: PackageFingerprintStorage
private let fingerprintStorage: PackageFingerprintStorage?
private let fingerprintCheckingMode: FingerprintCheckingMode
private let jsonDecoder: JSONDecoder

public init(
configuration: RegistryConfiguration,
identityResolver: IdentityResolver,
fingerprintStorage: PackageFingerprintStorage,
fingerprintStorage: PackageFingerprintStorage? = .none,
fingerprintCheckingMode: FingerprintCheckingMode,
authorizationProvider: HTTPClientAuthorizationProvider? = .none,
customHTTPClient: HTTPClient? = .none,
Expand Down Expand Up @@ -423,27 +423,29 @@ public final class RegistryClient {
throw RegistryError.invalidSourceArchive
}

self.fingerprintStorage.put(package: package,
version: version,
fingerprint: .init(origin: .registry(registry.url), value: checksum),
observabilityScope: observabilityScope,
callbackQueue: callbackQueue) { storageResult in
switch storageResult {
case .success:
completion(.success(checksum))
case .failure(PackageFingerprintStorageError.conflict(_, let existing)):
switch self.fingerprintCheckingMode {
case .strict:
completion(.failure(RegistryError.checksumChanged(latest: checksum, previous: existing.value)))
case .warn:
observabilityScope.emit(warning: "The checksum \(checksum) from \(registry.url.absoluteString) does not match previously recorded value \(existing.value) from \(String(describing: existing.origin.url?.absoluteString))")
completion(.success(checksum))
case .none:
if let fingerprintStorage = self.fingerprintStorage {
fingerprintStorage.put(package: package,
version: version,
fingerprint: .init(origin: .registry(registry.url), value: checksum),
observabilityScope: observabilityScope,
callbackQueue: callbackQueue) { storageResult in
switch storageResult {
case .success:
completion(.success(checksum))
case .failure(PackageFingerprintStorageError.conflict(_, let existing)):
switch self.fingerprintCheckingMode {
case .strict:
completion(.failure(RegistryError.checksumChanged(latest: checksum, previous: existing.value)))
case .warn:
observabilityScope.emit(warning: "The checksum \(checksum) from \(registry.url.absoluteString) does not match previously recorded value \(existing.value) from \(String(describing: existing.origin.url?.absoluteString))")
completion(.success(checksum))
}
case .failure(let error):
completion(.failure(error))
}
case .failure(let error):
completion(.failure(error))
}
} else {
completion(.success(checksum))
}
} catch {
completion(.failure(RegistryError.failedRetrievingReleaseChecksum(error)))
Expand Down Expand Up @@ -535,8 +537,6 @@ public final class RegistryClient {
return completion(.failure(RegistryError.invalidChecksum(expected: expectedChecksum, actual: actualChecksum)))
case .warn:
observabilityScope.emit(warning: "The checksum \(actualChecksum) does not match previously recorded value \(expectedChecksum)")
case .none:
break
}
}

Expand Down Expand Up @@ -567,25 +567,33 @@ public final class RegistryClient {

// We either use a previously recorded checksum, or fetch it from the registry
func withExpectedChecksum(body: @escaping (Result<String, Error>) -> Void) {
self.fingerprintStorage.get(package: package,
version: version,
kind: .registry,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue) { result in
switch result {
case .success(let fingerprint):
body(.success(fingerprint.value))
case .failure(let error):
if error as? PackageFingerprintStorageError != .notFound {
observabilityScope.emit(error: "Failed to get registry fingerprint for \(package) \(version) from storage: \(error)")
if let fingerprintStorage = self.fingerprintStorage {
fingerprintStorage.get(package: package,
version: version,
kind: .registry,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue) { result in
switch result {
case .success(let fingerprint):
body(.success(fingerprint.value))
case .failure(let error):
if error as? PackageFingerprintStorageError != .notFound {
observabilityScope.emit(error: "Failed to get registry fingerprint for \(package) \(version) from storage: \(error)")
}
// Try fetching checksum from registry again no matter which kind of error it is
self.fetchSourceArchiveChecksum(package: package,
version: version,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue,
completion: body)
}
// Try fetching checksum from registry again no matter which kind of error it is
self.fetchSourceArchiveChecksum(package: package,
version: version,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue,
completion: body)
}
} else {
self.fetchSourceArchiveChecksum(package: package,
version: version,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue,
completion: body)
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions Sources/Workspace/SourceControlPackageContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
private let manifestLoader: ManifestLoaderProtocol
private let toolsVersionLoader: ToolsVersionLoaderProtocol
private let currentToolsVersion: ToolsVersion
private let fingerprintStorage: PackageFingerprintStorage
private let fingerprintStorage: PackageFingerprintStorage?
private let fingerprintCheckingMode: FingerprintCheckingMode
private let observabilityScope: ObservabilityScope

Expand All @@ -79,7 +79,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
manifestLoader: ManifestLoaderProtocol,
toolsVersionLoader: ToolsVersionLoaderProtocol,
currentToolsVersion: ToolsVersion,
fingerprintStorage: PackageFingerprintStorage,
fingerprintStorage: PackageFingerprintStorage?,
fingerprintCheckingMode: FingerprintCheckingMode,
observabilityScope: ObservabilityScope
) throws {
Expand Down Expand Up @@ -150,6 +150,10 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
}

func checkIntegrity(version: Version, revision: Revision) throws {
guard let fingerprintStorage = self.fingerprintStorage else {
return
}

guard case .remoteSourceControl(let sourceControlURL) = self.package.kind else {
return
}
Expand All @@ -158,7 +162,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
let fingerprint: Fingerprint
do {
fingerprint = try temp_await {
self.fingerprintStorage.get(
fingerprintStorage.get(
package: packageIdentity,
version: version,
kind: .sourceControl,
Expand All @@ -172,7 +176,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
// Write to storage if fingerprint not yet recorded
do {
try temp_await {
self.fingerprintStorage.put(
fingerprintStorage.put(
package: packageIdentity,
version: version,
fingerprint: fingerprint,
Expand All @@ -182,14 +186,12 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
)
}
} catch PackageFingerprintStorageError.conflict(_, let existing) {
let message = "Revision \(revision.identifier) for \(self.package) version \(version) does not match previously recorded value \(existing.value) from \(String(describing: existing.origin.url?.absoluteString))"
let message = "Revision \(revision.identifier) for \(self.package) version \(version) does not match previously recorded value \(existing.value) from \(String(describing: existing.origin.url?.absoluteString))"
switch self.fingerprintCheckingMode {
case .strict:
throw StringError(message)
case .warn:
observabilityScope.emit(warning: message)
case .none:
return
}
}
} catch {
Expand All @@ -205,8 +207,6 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
throw StringError(message)
case .warn:
observabilityScope.emit(warning: message)
case .none:
return
}
}
}
Expand Down
13 changes: 8 additions & 5 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public class Workspace {
fileprivate let checksumAlgorithm: HashAlgorithm

/// The package fingerprint storage
fileprivate let fingerprintStorage: PackageFingerprintStorage
fileprivate let fingerprintStorage: PackageFingerprintStorage?

/// Enable prefetching containers in resolver.
fileprivate let resolverPrefetchingEnabled: Bool
Expand Down Expand Up @@ -265,6 +265,7 @@ public class Workspace {
/// - customHTTPClient: A custom http client.
/// - customArchiver: A custom archiver.
/// - customChecksumAlgorithm: A custom checksum algorithm.
/// - customFingerprintStorage: A custom fingerprint storage.
/// - additionalFileRules: File rules to determine resource handling behavior.
/// - resolverUpdateEnabled: Enables the dependencies resolver automatic version update check. Enabled by default. When disabled the resolver relies only on the resolved version file
/// - resolverPrefetchingEnabled: Enables the dependencies resolver prefetching based on the resolved version file. Enabled by default.
Expand Down Expand Up @@ -314,10 +315,12 @@ public class Workspace {
delegate: delegate.map(WorkspaceRepositoryManagerDelegate.init(workspaceDelegate:)),
cachePath: sharedRepositoriesCacheEnabled ? location.sharedRepositoriesCacheDirectory : .none
)
let fingerprintStorage = customFingerprintStorage ?? FilePackageFingerprintStorage(
fileSystem: fileSystem,
directoryPath: location.sharedFingerprintsDirectory
)
let fingerprintStorage = customFingerprintStorage ?? location.sharedFingerprintsDirectory.map {
FilePackageFingerprintStorage(
fileSystem: fileSystem,
directoryPath: $0
)
}

let registryClient = customRegistryClient ?? registries.map { configuration in
RegistryClient(
Expand Down
8 changes: 4 additions & 4 deletions Sources/Workspace/WorkspaceConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ extension Workspace {
public var resolvedVersionsFile: AbsolutePath

/// Path to the shared security directory
public var sharedSecurityDirectory: AbsolutePath
public var sharedSecurityDirectory: AbsolutePath?

/// Path to the shared cache directory
public var sharedCacheDirectory: AbsolutePath?
Expand Down Expand Up @@ -61,8 +61,8 @@ extension Workspace {
}

/// Path to the shared fingerprints directory.
public var sharedFingerprintsDirectory: AbsolutePath {
self.sharedSecurityDirectory.appending(component: "fingerprints")
public var sharedFingerprintsDirectory: AbsolutePath? {
self.sharedSecurityDirectory.map { $0.appending(component: "fingerprints") }
}

/// Path to the shared repositories cache.
Expand Down Expand Up @@ -103,7 +103,7 @@ extension Workspace {
workingDirectory: AbsolutePath,
editsDirectory: AbsolutePath,
resolvedVersionsFile: AbsolutePath,
sharedSecurityDirectory: AbsolutePath,
sharedSecurityDirectory: AbsolutePath?,
sharedCacheDirectory: AbsolutePath?,
sharedConfigurationDirectory: AbsolutePath?
) {
Expand Down
2 changes: 1 addition & 1 deletion Tests/WorkspaceTests/RegistryPackageContainerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ class RegistryPackageContainerTests: XCTestCase {
configuration: configuration!,
identityResolver: identityResolver,
fingerprintStorage: fingerprintStorage,
fingerprintCheckingMode: .none,
fingerprintCheckingMode: .strict,
authorizationProvider: .none,
customHTTPClient: HTTPClient(configuration: .init(), handler: { request, progress , completion in
var pathComponents = request.url.pathComponents
Expand Down