diff --git a/Sources/Basics/FileSystem+Extensions.swift b/Sources/Basics/FileSystem+Extensions.swift index 4a18ebf6a7e..7d09a84c484 100644 --- a/Sources/Basics/FileSystem+Extensions.swift +++ b/Sources/Basics/FileSystem+Extensions.swift @@ -89,7 +89,7 @@ extension FileSystem { } extension FileSystem { - public func getOrCreateSwiftPMConfigurationDirectory() throws -> AbsolutePath { + public func getOrCreateSwiftPMConfigurationDirectory(warningHandler: (String) -> Void) throws -> AbsolutePath { let idiomaticConfigurationDirectory = self.swiftPMConfigurationDirectory // temporary 5.6, remove on next version: transition from previous configuration location @@ -111,9 +111,7 @@ extension FileSystem { if !self.exists(destination) { try self.copy(from: file, to: destination) } - // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.\n") - TSCBasic.stderrStream.flush() + warningHandler("Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.") } } // in the case where ~/.swiftpm/configuration is the idiomatic location (eg on Linux) @@ -130,9 +128,7 @@ extension FileSystem { if !self.exists(destination) { try self.copy(from: file, to: destination) } - // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.\n") - TSCBasic.stderrStream.flush() + warningHandler("Usage of \(file) has been deprecated. Please delete it and use the new \(destination) instead.") } } } diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift index 0b07733f005..57674ac4548 100644 --- a/Sources/Commands/SwiftTool.swift +++ b/Sources/Commands/SwiftTool.swift @@ -515,6 +515,7 @@ public class SwiftTool { sharedRepositoriesCacheEnabled: self.options.useRepositoriesCache, fingerprintCheckingMode: self.options.resolverFingerprintCheckingMode ), + initializationWarningHandler: { self.observabilityScope.emit(warning: $0) }, customManifestLoader: self.getManifestLoader(), // FIXME: ideally we would not customize the manifest loader customRepositoryProvider: repositoryProvider, // FIXME: ideally we would not customize the repository provider. its currently done for shutdown handling which can be better abstracted delegate: delegate diff --git a/Sources/SourceControl/RepositoryManager.swift b/Sources/SourceControl/RepositoryManager.swift index 5766c592bdd..e89ae9c9f15 100644 --- a/Sources/SourceControl/RepositoryManager.swift +++ b/Sources/SourceControl/RepositoryManager.swift @@ -72,18 +72,22 @@ public class RepositoryManager { /// Create a new empty manager. /// /// - Parameters: + /// - fileSystem: The filesystem to operate on. /// - path: The path under which to store repositories. This should be a /// directory in which the content can be completely managed by this /// instance. /// - provider: The repository provider. + /// - cachePath: The repository cache location. + /// - cacheLocalPackages: Should cache local packages as well. For testing purposes. + /// - initializationWarningHandler: Initialization warnings handler. /// - delegate: The repository manager delegate. - /// - fileSystem: The filesystem to operate on. public init( fileSystem: FileSystem, path: AbsolutePath, provider: RepositoryProvider, cachePath: AbsolutePath? = .none, cacheLocalPackages: Bool = false, + initializationWarningHandler: (String) -> Void, delegate: RepositoryManagerDelegate? = .none ) { self.fileSystem = fileSystem @@ -107,9 +111,7 @@ public class RepositoryManager { } catch { self.repositories = [:] try? self.storage.reset() - // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: unable to restore checkouts state: \(error)\n") - TSCBasic.stderrStream.flush() + initializationWarningHandler("unable to restore checkouts state: \(error)") } } diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index edef84087c4..b2e25e7a8de 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -254,6 +254,7 @@ public class Workspace { /// - location: Workspace location configuration. /// - authorizationProvider: Provider of authentication information for outbound network requests. /// - configuration: Configuration to fine tune the dependency resolution behavior. + /// - initializationWarningHandler: Initialization warnings handler /// - customManifestLoader: Custom manifest loader. Used to customize how manifest are loaded. /// - customPackageContainerProvider: Custom package container provider. Used to provide specialized package providers. /// - customRepositoryProvider: Custom repository provider. Used to customize source control access. @@ -263,6 +264,7 @@ public class Workspace { location: Location, authorizationProvider: AuthorizationProvider? = .none, configuration: WorkspaceConfiguration? = .none, + initializationWarningHandler: ((String) -> Void)? = .none, // optional customization used for advanced integration situations customManifestLoader: ManifestLoaderProtocol? = .none, customPackageContainerProvider: PackageContainerProvider? = .none, @@ -275,6 +277,7 @@ public class Workspace { location: location, authorizationProvider: authorizationProvider, configuration: configuration, + initializationWarningHandler: initializationWarningHandler, customRegistriesConfiguration: .none, customFingerprints: .none, customMirrors: .none, @@ -303,6 +306,7 @@ public class Workspace { /// - forRootPackage: The path for the root package. /// - authorizationProvider: Provider of authentication information for outbound network requests. /// - configuration: Configuration to fine tune the dependency resolution behavior. + /// - initializationWarningHandler: Initialization warnings handler /// - customManifestLoader: Custom manifest loader. Used to customize how manifest are loaded. /// - customPackageContainerProvider: Custom package container provider. Used to provide specialized package providers. /// - customRepositoryProvider: Custom repository provider. Used to customize source control access. @@ -312,6 +316,7 @@ public class Workspace { forRootPackage packagePath: AbsolutePath, authorizationProvider: AuthorizationProvider? = .none, configuration: WorkspaceConfiguration? = .none, + initializationWarningHandler: ((String) -> Void)? = .none, // optional customization used for advanced integration situations customManifestLoader: ManifestLoaderProtocol? = .none, customPackageContainerProvider: PackageContainerProvider? = .none, @@ -324,6 +329,7 @@ public class Workspace { try self.init( fileSystem: fileSystem, location: location, + initializationWarningHandler: initializationWarningHandler, customManifestLoader: customManifestLoader, customPackageContainerProvider: customPackageContainerProvider, customRepositoryProvider: customRepositoryProvider, @@ -342,6 +348,7 @@ public class Workspace { /// - forRootPackage: The path for the root package. /// - authorizationProvider: Provider of authentication information for outbound network requests. /// - configuration: Configuration to fine tune the dependency resolution behavior. + /// - initializationWarningHandler: Initialization warnings handler /// - customToolchain: Custom toolchain. Used to create a customized ManifestLoader, customizing how manifest are loaded. /// - customPackageContainerProvider: Custom package container provider. Used to provide specialized package providers. /// - customRepositoryProvider: Custom repository provider. Used to customize source control access. @@ -351,6 +358,7 @@ public class Workspace { forRootPackage packagePath: AbsolutePath, authorizationProvider: AuthorizationProvider? = .none, configuration: WorkspaceConfiguration? = .none, + initializationWarningHandler: ((String) -> Void)? = .none, // optional customization used for advanced integration situations customToolchain: UserToolchain, customPackageContainerProvider: PackageContainerProvider? = .none, @@ -369,6 +377,7 @@ public class Workspace { forRootPackage: packagePath, authorizationProvider: authorizationProvider, configuration: configuration, + initializationWarningHandler: initializationWarningHandler, customManifestLoader: manifestLoader, customPackageContainerProvider: customPackageContainerProvider, customRepositoryProvider: customRepositoryProvider, @@ -376,8 +385,6 @@ public class Workspace { ) } - - // deprecate 12/21 @_disfavoredOverload @available(*, deprecated, message: "use alternative initializer") @@ -418,6 +425,7 @@ public class Workspace { location: location, authorizationProvider: authorizationProvider, configuration: configuration, + initializationWarningHandler: .none, customRegistriesConfiguration: registries, customFingerprints: customFingerprintStorage, customMirrors: mirrors, @@ -533,6 +541,7 @@ public class Workspace { location: Location, authorizationProvider: AuthorizationProvider? = .none, configuration: WorkspaceConfiguration? = .none, + initializationWarningHandler: ((String) -> Void)? = .none, // optional customization, primarily designed for testing but also used in some cases by libSwiftPM consumers customRegistriesConfiguration: RegistryConfiguration? = .none, customFingerprints: PackageFingerprintStorage? = .none, @@ -555,6 +564,7 @@ public class Workspace { location: location, authorizationProvider: authorizationProvider, configuration: configuration, + initializationWarningHandler: initializationWarningHandler, customRegistriesConfiguration: customRegistriesConfiguration, customFingerprints: customFingerprints, customMirrors: customMirrors, @@ -578,6 +588,7 @@ public class Workspace { location: Location, authorizationProvider: AuthorizationProvider?, configuration: WorkspaceConfiguration?, + initializationWarningHandler: ((String) -> Void)?, // optional customization, primarily designed for testing but also used in some cases by libSwiftPM consumers customRegistriesConfiguration: RegistryConfiguration?, customFingerprints: PackageFingerprintStorage?, @@ -595,11 +606,11 @@ public class Workspace { // delegate delegate: WorkspaceDelegate? ) throws { - // we do not store the observabilityScope in the workspace initializer as the workspace is designed to be long lived. + // we do not store an observabilityScope in the workspace initializer as the workspace is designed to be long lived. // instead, observabilityScope is passed into the individual workspace methods which are short lived. - + let initializationWarningHandler = initializationWarningHandler ?? warnToStderr // validate locations, returning a potentially modified one to deal with non-accessible or non-writable shared locations - let location = try location.validatingSharedLocations(fileSystem: fileSystem) + let location = try location.validatingSharedLocations(fileSystem: fileSystem, warningHandler: initializationWarningHandler) let currentToolsVersion = customToolsVersion ?? ToolsVersion.currentToolsVersion let toolsVersionLoader = ToolsVersionLoader(currentToolsVersion: currentToolsVersion) @@ -623,6 +634,7 @@ public class Workspace { path: location.repositoriesDirectory, provider: repositoryProvider, cachePath: configuration.sharedRepositoriesCacheEnabled ? location.sharedRepositoriesCacheDirectory : .none, + initializationWarningHandler: initializationWarningHandler, delegate: delegate.map(WorkspaceRepositoryManagerDelegate.init(workspaceDelegate:)) ) @@ -680,7 +692,11 @@ public class Workspace { self.configuration = configuration - self.state = WorkspaceState(fileSystem: fileSystem, storageDirectory: self.location.workingDirectory) + self.state = WorkspaceState( + fileSystem: fileSystem, + storageDirectory: self.location.workingDirectory, + initializationWarningHandler: initializationWarningHandler + ) } } @@ -4012,16 +4028,16 @@ extension FileSystem { } extension Workspace.Location { - func validatingSharedLocations(fileSystem: FileSystem) throws -> Self { + func validatingSharedLocations( + fileSystem: FileSystem, + warningHandler: (String) -> Void + ) throws -> Self { var location = self - // local configuration directory must be accessible, throw if we cannot access it - //try fileSystem.withLock(on: location.localConfigurationDirectory, type: .exclusive, {}) - // check that shared configuration directory is accessible, or warn + reset if not if let sharedConfigurationDirectory = self.sharedConfigurationDirectory { // it may not always be possible to create default location (for example de to restricted sandbox) - let defaultDirectory = try? fileSystem.getOrCreateSwiftPMConfigurationDirectory() + let defaultDirectory = try? fileSystem.getOrCreateSwiftPMConfigurationDirectory(warningHandler: warningHandler) if sharedConfigurationDirectory != defaultDirectory { // custom location must be writable, throw if we cannot access it try withTemporaryFile(dir: sharedConfigurationDirectory) { _ in } @@ -4031,9 +4047,7 @@ extension Workspace.Location { try withTemporaryFile(dir: sharedConfigurationDirectory) { _ in } } catch { location.sharedConfigurationDirectory = .none - // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: \(sharedConfigurationDirectory) is not accessible or not writable, disabling user-level configuration features. \(error)\n") - TSCBasic.stderrStream.flush() + warningHandler("\(sharedConfigurationDirectory) is not accessible or not writable, disabling user-level configuration features. \(error)") } } } @@ -4051,9 +4065,7 @@ extension Workspace.Location { try withTemporaryFile(dir: sharedSecurityDirectory) { _ in } } catch { location.sharedSecurityDirectory = .none - // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: \(sharedSecurityDirectory) is not accessible or not writable, disabling user-level security features. \(error)\n") - TSCBasic.stderrStream.flush() + warningHandler("\(sharedSecurityDirectory) is not accessible or not writable, disabling user-level security features. \(error)") } } } @@ -4071,12 +4083,15 @@ extension Workspace.Location { try withTemporaryFile(dir: sharedCacheDirectory) { _ in } } catch { location.sharedCacheDirectory = .none - // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: \(sharedCacheDirectory) is not accessible or not writable, disabling user-level cache features. \(error)\n") - TSCBasic.stderrStream.flush() + warningHandler("\(sharedCacheDirectory) is not accessible or not writable, disabling user-level cache features. \(error)") } } } return location } } + +fileprivate func warnToStderr(_ message: String) { + TSCBasic.stderrStream.write("warning: \(message)\n") + TSCBasic.stderrStream.flush() +} diff --git a/Sources/Workspace/WorkspaceState.swift b/Sources/Workspace/WorkspaceState.swift index cd554bd35a7..db82cd529eb 100644 --- a/Sources/Workspace/WorkspaceState.swift +++ b/Sources/Workspace/WorkspaceState.swift @@ -29,7 +29,11 @@ public final class WorkspaceState { /// storage private let storage: WorkspaceStateStorage - init(fileSystem: FileSystem, storageDirectory: AbsolutePath) { + init( + fileSystem: FileSystem, + storageDirectory: AbsolutePath, + initializationWarningHandler: (String) -> Void + ) { self.storagePath = storageDirectory.appending(component: "workspace-state.json") self.storage = WorkspaceStateStorage(path: self.storagePath, fileSystem: fileSystem) @@ -49,9 +53,7 @@ public final class WorkspaceState { self.dependencies = Workspace.ManagedDependencies() self.artifacts = Workspace.ManagedArtifacts() try? self.storage.reset() - // FIXME: We should emit a warning here using the diagnostic engine. - TSCBasic.stderrStream.write("warning: unable to restore workspace state: \(error)\n") - TSCBasic.stderrStream.flush() + initializationWarningHandler("unable to restore workspace state: \(error)") } } diff --git a/Tests/SourceControlTests/RepositoryManagerTests.swift b/Tests/SourceControlTests/RepositoryManagerTests.swift index 225c2bddcd0..32f5729e394 100644 --- a/Tests/SourceControlTests/RepositoryManagerTests.swift +++ b/Tests/SourceControlTests/RepositoryManagerTests.swift @@ -705,6 +705,25 @@ class RepositoryManagerTests: XCTestCase { } extension RepositoryManager { + public convenience init( + fileSystem: FileSystem, + path: AbsolutePath, + provider: RepositoryProvider, + cachePath: AbsolutePath? = .none, + cacheLocalPackages: Bool = false, + delegate: RepositoryManagerDelegate? = .none + ) { + self.init( + fileSystem: fileSystem, + path: path, + provider: provider, + cachePath: cachePath, + cacheLocalPackages: cacheLocalPackages, + initializationWarningHandler: { _ in }, + delegate: delegate + ) + } + fileprivate func lookup(repository: RepositorySpecifier, skipUpdate: Bool = false) throws -> RepositoryHandle { return try tsc_await { self.lookup(repository: repository, skipUpdate: skipUpdate, on: .global(), completion: $0) } } diff --git a/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift b/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift index 58c03945369..dd0bd95f08c 100644 --- a/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift +++ b/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift @@ -673,3 +673,24 @@ extension PackageContainerProvider { try tsc_await { self.getContainer(for: package, skipUpdate: skipUpdate, observabilityScope: ObservabilitySystem.NOOP, on: .global(), completion: $0) } } } + +extension RepositoryManager { + fileprivate convenience init( + fileSystem: FileSystem, + path: AbsolutePath, + provider: RepositoryProvider, + cachePath: AbsolutePath? = .none, + cacheLocalPackages: Bool = false, + delegate: RepositoryManagerDelegate? = .none + ) { + self.init( + fileSystem: fileSystem, + path: path, + provider: provider, + cachePath: cachePath, + cacheLocalPackages: cacheLocalPackages, + initializationWarningHandler: { _ in }, + delegate: delegate + ) + } +} diff --git a/Tests/WorkspaceTests/WorkspaceStateTests.swift b/Tests/WorkspaceTests/WorkspaceStateTests.swift index e47ae1e3724..b754cb1c152 100644 --- a/Tests/WorkspaceTests/WorkspaceStateTests.swift +++ b/Tests/WorkspaceTests/WorkspaceStateTests.swift @@ -490,3 +490,9 @@ final class WorkspaceStateTests: XCTestCase { XCTAssertTrue(state.artifacts.isEmpty) } } + +extension WorkspaceState { + fileprivate convenience init(fileSystem: FileSystem, storageDirectory: AbsolutePath) { + self.init(fileSystem: fileSystem, storageDirectory: storageDirectory, initializationWarningHandler: { _ in }) + } +}