diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift index 1bc01ec454c..3061a5bd101 100644 --- a/Sources/Commands/SwiftTool.swift +++ b/Sources/Commands/SwiftTool.swift @@ -640,6 +640,20 @@ 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) + } + return sharedSecurityDirectory + } catch { + self.observabilityScope.emit(warning: "Failed creating shared security directory: \(error)") + return .none + } + } /// Returns the currently active workspace. func getActiveWorkspace() throws -> Workspace { @@ -649,7 +663,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( @@ -658,7 +673,7 @@ public class SwiftTool { workingDirectory: buildPath, editsDirectory: self.editsDirectory(), resolvedVersionsFile: self.resolvedVersionsFile(), - sharedSecurityDirectory: localFileSystem.swiftPMSecurityDirectory, + sharedSecurityDirectory: sharedSecurityDirectory, sharedCacheDirectory: sharedCacheDirectory, sharedConfigurationDirectory: sharedConfigurationDirectory ), diff --git a/Sources/PackageFingerprint/Model.swift b/Sources/PackageFingerprint/Model.swift index 2d610784ff2..cadbb5723b1 100644 --- a/Sources/PackageFingerprint/Model.swift +++ b/Sources/PackageFingerprint/Model.swift @@ -65,5 +65,4 @@ public typealias PackageFingerprints = [Version: [Fingerprint.Kind: Fingerprint] public enum FingerprintCheckingMode: String { case strict case warn - case none } diff --git a/Sources/PackageFingerprint/PackageFingerprintStorage.swift b/Sources/PackageFingerprint/PackageFingerprintStorage.swift index d2223a8ad05..954ad0aba24 100644 --- a/Sources/PackageFingerprint/PackageFingerprintStorage.swift +++ b/Sources/PackageFingerprint/PackageFingerprintStorage.swift @@ -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" + } + } } diff --git a/Sources/PackageRegistry/RegistryClient.swift b/Sources/PackageRegistry/RegistryClient.swift index 68430e2117d..2a9390d4073 100644 --- a/Sources/PackageRegistry/RegistryClient.swift +++ b/Sources/PackageRegistry/RegistryClient.swift @@ -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, @@ -423,27 +423,23 @@ 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))") + 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 .none: + case .failure(let error): + // Don't throw write errors + observabilityScope.emit(warning: "Failed to save checksum '\(checksum) from \(registry.url) to fingerprints storage: \(error)") completion(.success(checksum)) } - case .failure(let error): - completion(.failure(error)) } + } else { + completion(.success(checksum)) } } catch { completion(.failure(RegistryError.failedRetrievingReleaseChecksum(error))) @@ -535,8 +531,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 } } @@ -567,25 +561,33 @@ public final class RegistryClient { // We either use a previously recorded checksum, or fetch it from the registry func withExpectedChecksum(body: @escaping (Result) -> 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) } } } diff --git a/Sources/Workspace/SourceControlPackageContainer.swift b/Sources/Workspace/SourceControlPackageContainer.swift index d7495055168..43749c13d4e 100644 --- a/Sources/Workspace/SourceControlPackageContainer.swift +++ b/Sources/Workspace/SourceControlPackageContainer.swift @@ -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 @@ -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 { @@ -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 } @@ -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, @@ -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, @@ -181,16 +185,8 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri callback: $0 ) } - } 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))" - switch self.fingerprintCheckingMode { - case .strict: - throw StringError(message) - case .warn: - observabilityScope.emit(warning: message) - case .none: - return - } + } catch { + observabilityScope.emit(warning: "Failed to save revision '\(revision.identifier) from \(sourceControlURL) to fingerprints storage: \(error)") } } catch { self.observabilityScope.emit(error: "Failed to get source control fingerprint for \(self.package) version \(version) from storage: \(error)") @@ -205,8 +201,6 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri throw StringError(message) case .warn: observabilityScope.emit(warning: message) - case .none: - return } } } diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 352e452371e..0366522777f 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -217,7 +217,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 @@ -259,6 +259,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. @@ -306,10 +307,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( diff --git a/Sources/Workspace/WorkspaceConfiguration.swift b/Sources/Workspace/WorkspaceConfiguration.swift index 6f575c7a3b0..a4bc3b9cce4 100644 --- a/Sources/Workspace/WorkspaceConfiguration.swift +++ b/Sources/Workspace/WorkspaceConfiguration.swift @@ -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? @@ -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. @@ -103,7 +103,7 @@ extension Workspace { workingDirectory: AbsolutePath, editsDirectory: AbsolutePath, resolvedVersionsFile: AbsolutePath, - sharedSecurityDirectory: AbsolutePath, + sharedSecurityDirectory: AbsolutePath?, sharedCacheDirectory: AbsolutePath?, sharedConfigurationDirectory: AbsolutePath? ) { diff --git a/Tests/PackageRegistryTests/RegistryClientTests.swift b/Tests/PackageRegistryTests/RegistryClientTests.swift index a0f7ebb658b..2053da97ac4 100644 --- a/Tests/PackageRegistryTests/RegistryClientTests.swift +++ b/Tests/PackageRegistryTests/RegistryClientTests.swift @@ -346,74 +346,6 @@ final class RegistryClientTests: XCTestCase { var configuration = RegistryConfiguration() configuration.defaultRegistry = Registry(url: URL(string: registryURL)!) - let fingerprintStorage = MockPackageFingerprintStorage([ - identity: [ - version: [.registry: Fingerprint(origin: .registry(URL(string: registryURL)!), value: "non-matching checksum")], - ], - ]) - let registryClient = makeRegistryClient(configuration: configuration, - httpClient: httpClient, - fingerprintStorage: fingerprintStorage, - fingerprintCheckingMode: .strict) - - XCTAssertThrowsError(try registryClient.fetchSourceArchiveChecksum(package: identity, version: version)) { error in - guard case RegistryError.checksumChanged = error else { - return XCTFail("Expected RegistryError.checksumChanged, got \(error)") - } - } - } - - func testFetchSourceArchiveChecksum_storageConflict_fingerprintChecking_warn() throws { - let registryURL = "https://packages.example.com" - let identity = PackageIdentity.plain("mona.LinkedList") - let (scope, name) = identity.scopeAndName! - let version = Version("1.1.1") - let metadataURL = URL(string: "\(registryURL)/\(scope)/\(name)/\(version)")! - let checksum = "a2ac54cf25fbc1ad0028f03f0aa4b96833b83bb05a14e510892bb27dea4dc812" - - let handler: HTTPClient.Handler = { request, _, completion in - switch (request.method, request.url) { - case (.get, metadataURL): - XCTAssertEqual(request.headers.get("Accept").first, "application/vnd.swift.registry.v1+json") - - let data = """ - { - "id": "mona.LinkedList", - "version": "1.1.1", - "resources": [ - { - "name": "source-archive", - "type": "application/zip", - "checksum": "\(checksum)" - } - ], - "metadata": { - "description": "One thing links to another." - } - } - """.data(using: .utf8)! - - completion(.success(.init( - statusCode: 200, - headers: .init([ - .init(name: "Content-Length", value: "\(data.count)"), - .init(name: "Content-Type", value: "application/json"), - .init(name: "Content-Version", value: "1"), - ]), - body: data - ))) - default: - completion(.failure(StringError("method and url should match"))) - } - } - - var httpClient = HTTPClient(handler: handler) - httpClient.configuration.circuitBreakerStrategy = .none - httpClient.configuration.retryStrategy = .none - - var configuration = RegistryConfiguration() - configuration.defaultRegistry = Registry(url: URL(string: registryURL)!) - let storedChecksum = "non-matching checksum" let fingerprintStorage = MockPackageFingerprintStorage([ identity: [ @@ -423,12 +355,11 @@ final class RegistryClientTests: XCTestCase { let registryClient = makeRegistryClient(configuration: configuration, httpClient: httpClient, fingerprintStorage: fingerprintStorage, - fingerprintCheckingMode: .warn) + fingerprintCheckingMode: .strict) let observability = ObservabilitySystem.makeForTesting() - // The checksum differs from that in storage, but error is not thrown - // because fingerprintCheckingMode=.warn + // The checksum differs from that in storage, but we don't throw write errors let checksumResponse = try registryClient.fetchSourceArchiveChecksum( package: identity, version: version, @@ -436,9 +367,9 @@ final class RegistryClientTests: XCTestCase { ) XCTAssertEqual(checksum, checksumResponse) - // But there should be a warning + // There should be a warning though testDiagnostics(observability.diagnostics) { result in - result.check(diagnostic: .contains("does not match previously recorded value"), severity: .warning) + result.check(diagnostic: .contains("different from previously recorded value"), severity: .warning) } // Storage should NOT be updated