From c14bb5b60b15c877218071ba3e9bf824151c7a95 Mon Sep 17 00:00:00 2001 From: Andrew Hoos Date: Thu, 26 Jun 2025 09:51:32 -0700 Subject: [PATCH] Reduce usage of callback and dispatch methods in favor of async/await Remove unused callback API Remove unneeded withChecked*Continuation wrappers Use async/await in more tests Remove a usage of DispatchGroup in tests Remove unused callback queues from async methods Removed unused callback methods Moved workspace and plugins to async/await --- Sources/Build/BuildOperation.swift | 1 - .../PackageCommands/PluginCommand.swift | 1 - Sources/PackageRegistry/ChecksumTOFU.swift | 23 -- Sources/PackageRegistry/RegistryClient.swift | 19 - .../RegistryDownloadsManager.swift | 17 - .../PackageRegistry/SignatureValidation.swift | 57 --- .../PackageRegistry/SigningEntityTOFU.swift | 20 - .../Plugins/DefaultPluginScriptRunner.swift | 346 ++++++++---------- .../Plugins/PluginInvocation.swift | 173 ++------- .../Plugins/PluginScriptRunner.swift | 37 +- .../Workspace/Workspace+Dependencies.swift | 1 - Sources/Workspace/Workspace+Manifests.swift | 18 +- Sources/Workspace/Workspace.swift | 182 +++------ Tests/BasicsTests/AsyncProcessTests.swift | 24 +- Tests/BuildTests/PluginInvocationTests.swift | 37 +- Tests/FunctionalTests/PluginTests.swift | 13 +- .../RegistryClientTests.swift | 142 +++---- .../RepositoryManagerTests.swift | 93 ++--- .../SourceControlPackageContainerTests.swift | 16 +- ...olsVersionSpecificationRewriterTests.swift | 2 +- 20 files changed, 379 insertions(+), 843 deletions(-) diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index 37761bd2e38..ba57a625734 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -566,7 +566,6 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS pluginName: plugin.moduleName, toolsVersion: plugin.toolsVersion, observabilityScope: self.observabilityScope, - callbackQueue: DispatchQueue.sharedConcurrent, delegate: delegate ) diff --git a/Sources/Commands/PackageCommands/PluginCommand.swift b/Sources/Commands/PackageCommands/PluginCommand.swift index ec2243a6a71..ae47694096b 100644 --- a/Sources/Commands/PackageCommands/PluginCommand.swift +++ b/Sources/Commands/PackageCommands/PluginCommand.swift @@ -394,7 +394,6 @@ struct PluginCommand: AsyncSwiftCommand { fileSystem: swiftCommandState.fileSystem, modulesGraph: packageGraph, observabilityScope: swiftCommandState.observabilityScope, - callbackQueue: delegateQueue, delegate: pluginDelegate ) diff --git a/Sources/PackageRegistry/ChecksumTOFU.swift b/Sources/PackageRegistry/ChecksumTOFU.swift index c819cc54d44..3d98912f77a 100644 --- a/Sources/PackageRegistry/ChecksumTOFU.swift +++ b/Sources/PackageRegistry/ChecksumTOFU.swift @@ -67,29 +67,6 @@ struct PackageVersionChecksumTOFU { } } - @available(*, noasync, message: "Use the async alternative") - func validateSourceArchive( - registry: Registry, - package: PackageIdentity.RegistryIdentity, - version: Version, - checksum: String, - timeout: DispatchTimeInterval?, - observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - completion: @escaping @Sendable (Result) -> Void - ) { - callbackQueue.asyncResult(completion) { - try await self.validateSourceArchive( - registry: registry, - package: package, - version: version, - checksum: checksum, - timeout: timeout, - observabilityScope: observabilityScope - ) - } - } - private func getExpectedChecksum( registry: Registry, package: PackageIdentity.RegistryIdentity, diff --git a/Sources/PackageRegistry/RegistryClient.swift b/Sources/PackageRegistry/RegistryClient.swift index b19f7225547..0ed6a0a1856 100644 --- a/Sources/PackageRegistry/RegistryClient.swift +++ b/Sources/PackageRegistry/RegistryClient.swift @@ -613,25 +613,6 @@ public final class RegistryClient: AsyncCancellable { } } - @available(*, deprecated, message: "Use the async alternative") - public func getAvailableManifests( - package: PackageIdentity, - version: Version, - timeout: DispatchTimeInterval? = .none, - observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - completion: @escaping @Sendable (Result<[String: (toolsVersion: ToolsVersion, content: String?)], Error>) -> Void - ) { - callbackQueue.asyncResult(completion) { - try await self.getAvailableManifests( - package: package, - version: version, - timeout: timeout, - observabilityScope: observabilityScope - ) - } - } - public func getManifestContent( package: PackageIdentity, version: Version, diff --git a/Sources/PackageRegistry/RegistryDownloadsManager.swift b/Sources/PackageRegistry/RegistryDownloadsManager.swift index db92f4543af..ab272d7fea6 100644 --- a/Sources/PackageRegistry/RegistryDownloadsManager.swift +++ b/Sources/PackageRegistry/RegistryDownloadsManager.swift @@ -119,23 +119,6 @@ public class RegistryDownloadsManager: AsyncCancellable { return try await task.value } - @available(*, noasync, message: "Use the async alternative") - public func lookup( - package: PackageIdentity, - version: Version, - observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - completion: @escaping @Sendable (Result) -> Void - ) { - callbackQueue.asyncResult(completion) { - try await self.lookup( - package: package, - version: version, - observabilityScope: observabilityScope - ) - } - } - /// Cancel any outstanding requests public func cancel(deadline: DispatchTime) async throws { try await self.registryClient.cancel(deadline: deadline) diff --git a/Sources/PackageRegistry/SignatureValidation.swift b/Sources/PackageRegistry/SignatureValidation.swift index bf5169ff138..6f1f489057b 100644 --- a/Sources/PackageRegistry/SignatureValidation.swift +++ b/Sources/PackageRegistry/SignatureValidation.swift @@ -94,33 +94,6 @@ struct SignatureValidation { return signingEntity; } - @available(*, noasync, message: "Use the async alternative") - func validate( - registry: Registry, - package: PackageIdentity.RegistryIdentity, - version: Version, - content: Data, - configuration: RegistryConfiguration.Security.Signing, - timeout: DispatchTimeInterval?, - fileSystem: FileSystem, - observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - completion: @escaping @Sendable (Result) -> Void - ) { - callbackQueue.asyncResult(completion) { - try await self.validate( - registry: registry, - package: package, - version: version, - content: content, - configuration: configuration, - timeout: timeout, - fileSystem: fileSystem, - observabilityScope: observabilityScope - ) - } - } - private func getAndValidateSourceArchiveSignature( registry: Registry, package: PackageIdentity.RegistryIdentity, @@ -350,36 +323,6 @@ struct SignatureValidation { return signingEntity; } - - @available(*, noasync, message: "Use the async alternative") - func validate( - registry: Registry, - package: PackageIdentity.RegistryIdentity, - version: Version, - toolsVersion: ToolsVersion?, - manifestContent: String, - configuration: RegistryConfiguration.Security.Signing, - timeout: DispatchTimeInterval?, - fileSystem: FileSystem, - observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - completion: @escaping @Sendable (Result) -> Void - ) { - callbackQueue.asyncResult(completion) { - try await self.validate( - registry: registry, - package: package, - version: version, - toolsVersion: toolsVersion, - manifestContent: manifestContent, - configuration: configuration, - timeout: timeout, - fileSystem: fileSystem, - observabilityScope: observabilityScope - ) - } - } - private func getAndValidateManifestSignature( registry: Registry, package: PackageIdentity.RegistryIdentity, diff --git a/Sources/PackageRegistry/SigningEntityTOFU.swift b/Sources/PackageRegistry/SigningEntityTOFU.swift index 5ce3299d91c..3ee020f254f 100644 --- a/Sources/PackageRegistry/SigningEntityTOFU.swift +++ b/Sources/PackageRegistry/SigningEntityTOFU.swift @@ -76,26 +76,6 @@ struct PackageSigningEntityTOFU { ) } - @available(*, noasync, message: "Use the async alternative") - func validate( - registry: Registry, - package: PackageIdentity.RegistryIdentity, - version: Version, - signingEntity: SigningEntity?, - observabilityScope: ObservabilityScope, - completion: @escaping @Sendable (Result) -> Void - ) { - DispatchQueue.sharedConcurrent.asyncResult(completion) { - try await self.validate( - registry: registry, - package: package, - version: version, - signingEntity: signingEntity, - observabilityScope: observabilityScope - ) - } - } - private func validateSigningEntity( registry: Registry, package: PackageIdentity.RegistryIdentity, diff --git a/Sources/SPMBuildCore/Plugins/DefaultPluginScriptRunner.swift b/Sources/SPMBuildCore/Plugins/DefaultPluginScriptRunner.swift index f8a98a664e5..5beb8f4e5be 100644 --- a/Sources/SPMBuildCore/Plugins/DefaultPluginScriptRunner.swift +++ b/Sources/SPMBuildCore/Plugins/DefaultPluginScriptRunner.swift @@ -64,46 +64,30 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { allowNetworkConnections: [SandboxNetworkPermission], fileSystem: FileSystem, observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginScriptCompilerDelegate & PluginScriptRunnerDelegate, - completion: @escaping (Result) -> Void - ) { + delegate: PluginScriptCompilerDelegate & PluginScriptRunnerDelegate + ) async throws -> Int32 { // If needed, compile the plugin script to an executable (asynchronously). Compilation is skipped if the plugin hasn't changed since it was last compiled. - self.compilePluginScript( + let result = try await self.compilePluginScript( sourceFiles: sourceFiles, pluginName: pluginName, toolsVersion: toolsVersion, observabilityScope: observabilityScope, - callbackQueue: DispatchQueue.sharedConcurrent, - delegate: delegate, - completion: { - dispatchPrecondition(condition: .onQueue(DispatchQueue.sharedConcurrent)) - switch $0 { - case .success(let result): - if result.succeeded { - // Compilation succeeded, so run the executable. We are already running on an asynchronous queue. - self.invoke( - compiledExec: result.executableFile, - workingDirectory: workingDirectory, - writableDirectories: writableDirectories, - readOnlyDirectories: readOnlyDirectories, - allowNetworkConnections: allowNetworkConnections, - initialMessage: initialMessage, - observabilityScope: observabilityScope, - callbackQueue: callbackQueue, - delegate: delegate, - completion: completion) - } - else { - // Compilation failed, so throw an error. - callbackQueue.async { completion(.failure(DefaultPluginScriptRunnerError.compilationFailed(result))) } - } - case .failure(let error): - // Compilation failed, so just call the callback block on the appropriate queue. - callbackQueue.async { completion(.failure(error)) } - } - } - ) + delegate: delegate) + + guard result.succeeded else { + throw DefaultPluginScriptRunnerError.compilationFailed(result) + } + // Compilation succeeded, so run the executable. We are already running on an asynchronous queue. + return try await self.invoke( + compiledExec: result.executableFile, + workingDirectory: workingDirectory, + writableDirectories: writableDirectories, + readOnlyDirectories: readOnlyDirectories, + allowNetworkConnections: allowNetworkConnections, + initialMessage: initialMessage, + observabilityScope: observabilityScope, + delegate: delegate) + } public var hostTriple: Triple { @@ -116,10 +100,8 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { pluginName: String, toolsVersion: ToolsVersion, observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginScriptCompilerDelegate, - completion: @escaping (Result) -> Void - ) { + delegate: PluginScriptCompilerDelegate + ) async throws -> PluginCompilationResult { // Determine the path of the executable and other produced files. let execName = pluginName.spm_mangledToC99ExtendedIdentifier() #if os(Windows) @@ -242,9 +224,7 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { } catch { // Bail out right away if we didn't even get this far. - return callbackQueue.async { - completion(.failure(DefaultPluginScriptRunnerError.compilationPreparationFailed(error: error))) - } + throw DefaultPluginScriptRunnerError.compilationPreparationFailed(error: error) } // Hash the compiler inputs to decide whether we really need to recompile. @@ -333,9 +313,7 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { compilerOutput: compilationState.output, cached: true) delegate.skippedCompilingPlugin(cachedResult: result) - return callbackQueue.async { - completion(.success(result)) - } + return result } // Otherwise we need to recompile. We start by telling the delegate. @@ -352,47 +330,46 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { } // Now invoke the compiler asynchronously. - AsyncProcess.popen(arguments: commandLine, environment: environment, queue: callbackQueue) { - // We are now on our caller's requested callback queue, so we just call the completion handler directly. - dispatchPrecondition(condition: .onQueue(callbackQueue)) - completion($0.tryMap { process in - // Emit the compiler output as observable info. - let compilerOutput = ((try? process.utf8Output()) ?? "") + ((try? process.utf8stderrOutput()) ?? "") - if !compilerOutput.isEmpty { - observabilityScope.emit(info: compilerOutput) - } + let process = try await withCheckedThrowingContinuation { continuation in AsyncProcess.popen(arguments: commandLine, environment: environment, queue: .sharedConcurrent) { result in + continuation.resume(with: result) + } + } - // Save the persisted compilation state for possible reuse next time. - let compilationState = PersistedCompilationState( - commandLine: commandLine, - environment: toolchain.swiftCompilerEnvironment.cachable, - inputHash: compilerInputHash, - output: compilerOutput, - result: .init(process.exitStatus)) - do { - try JSONEncoder.makeWithDefaults().encode(path: stateFilePath, fileSystem: self.fileSystem, compilationState) - } - catch { - // We couldn't write out the `.state` file. We warn about it but proceed. - observabilityScope.emit(debug: "Couldn't save plugin compilation state", underlyingError: error) - } + // Emit the compiler output as observable info. + let compilerOutput = ((try? process.utf8Output()) ?? "") + ((try? process.utf8stderrOutput()) ?? "") + if !compilerOutput.isEmpty { + observabilityScope.emit(info: compilerOutput) + } - // Construct a PluginCompilationResult for both the successful and unsuccessful cases (to convey diagnostics, etc). - let result = PluginCompilationResult( - succeeded: compilationState.succeeded, - commandLine: commandLine, - executableFile: execFilePath, - diagnosticsFile: diagFilePath, - compilerOutput: compilerOutput, - cached: false) - - // Tell the delegate that we're done compiling the plugin, passing it the result. - delegate.didCompilePlugin(result: result) - - // Also return the result to the caller. - return result - }) + // Save the persisted compilation state for possible reuse next time. + let newCompilationState = PersistedCompilationState( + commandLine: commandLine, + environment: toolchain.swiftCompilerEnvironment.cachable, + inputHash: compilerInputHash, + output: compilerOutput, + result: .init(process.exitStatus)) + do { + try JSONEncoder.makeWithDefaults().encode(path: stateFilePath, fileSystem: self.fileSystem, newCompilationState) } + catch { + // We couldn't write out the `.state` file. We warn about it but proceed. + observabilityScope.emit(debug: "Couldn't save plugin compilation state", underlyingError: error) + } + + // Construct a PluginCompilationResult for both the successful and unsuccessful cases (to convey diagnostics, etc). + let result = PluginCompilationResult( + succeeded: newCompilationState.succeeded, + commandLine: commandLine, + executableFile: execFilePath, + diagnosticsFile: diagFilePath, + compilerOutput: compilerOutput, + cached: false) + + // Tell the delegate that we're done compiling the plugin, passing it the result. + delegate.didCompilePlugin(result: result) + + // Also return the result to the caller. + return result } /// Returns path to the sdk, if possible. @@ -429,34 +406,24 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { allowNetworkConnections: [SandboxNetworkPermission], initialMessage: Data, observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginScriptRunnerDelegate, - completion: @escaping (Result) -> Void - ) { + delegate: PluginScriptRunnerDelegate + ) async throws -> Int32 { #if canImport(Darwin) && !os(macOS) - callbackQueue.async { - completion(.failure(DefaultPluginScriptRunnerError.pluginUnavailable(reason: "subprocess invocations are unavailable on this platform"))) - } + throw DefaultPluginScriptRunnerError.pluginUnavailable(reason: "subprocess invocations are unavailable on this platform") #else - // Construct the command line. Currently we just invoke the executable built from the plugin without any parameters. - var command = [compiledExec.pathString] - // Optionally wrap the command in a sandbox, which places some limits on what it can do. In particular, it blocks network access and restricts the paths to which the plugin can make file system changes. It does allow writing to temporary directories. - if self.enableSandbox { - do { - command = try Sandbox.apply( - command: command, - fileSystem: self.fileSystem, - strictness: .writableTemporaryDirectory, - writableDirectories: writableDirectories + [self.cacheDir], - readOnlyDirectories: readOnlyDirectories, - allowNetworkConnections: allowNetworkConnections - ) - } catch { - return callbackQueue.async { - completion(.failure(error)) - } - } + + let command = if !self.enableSandbox { + [compiledExec.pathString] + } else { + try Sandbox.apply( + command: [compiledExec.pathString], + fileSystem: self.fileSystem, + strictness: .writableTemporaryDirectory, + writableDirectories: writableDirectories + [self.cacheDir], + readOnlyDirectories: readOnlyDirectories, + allowNetworkConnections: allowNetworkConnections + ) } // Create and configure a Process. We set the working directory to the cache directory, so that relative paths end up there. @@ -480,23 +447,23 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { process.environment = .init(env) process.currentDirectoryURL = workingDirectory.asURL - - // Set up a pipe for sending structured messages to the plugin on its stdin. - let stdinPipe = Pipe() - let outputHandle = stdinPipe.fileHandleForWriting - let outputQueue = DispatchQueue(label: "plugin-send-queue") - process.standardInput = stdinPipe - - // Set up a pipe for receiving messages from the plugin on its stdout. - let stdoutPipe = Pipe() - let stdoutLock = NSLock() - stdoutPipe.fileHandleForReading.readabilityHandler = { fileHandle in - // Receive the next message and pass it on to the delegate. - stdoutLock.withLock { - do { - while let message = try fileHandle.readPluginMessage() { - // FIXME: We should handle errors here. - callbackQueue.async { + + return try await withCheckedThrowingContinuation { continuation in + // Set up a pipe for sending structured messages to the plugin on its stdin. + let stdinPipe = Pipe() + let outputHandle = stdinPipe.fileHandleForWriting + let outputQueue = DispatchQueue(label: "plugin-send-queue") + process.standardInput = stdinPipe + + // Set up a pipe for receiving messages from the plugin on its stdout. + let stdoutPipe = Pipe() + let stdoutLock = NSLock() + stdoutPipe.fileHandleForReading.readabilityHandler = { fileHandle in + // Receive the next message and pass it on to the delegate. + stdoutLock.withLock { + do { + while let message = try fileHandle.readPluginMessage() { + // FIXME: We should handle errors here. do { try delegate.handleMessage(data: message, responder: { data in outputQueue.async { @@ -517,87 +484,80 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { } } } - } - catch { - print("error while trying to read message from plugin: \(error.interpolationDescription)") + catch { + print("error while trying to read message from plugin: \(error.interpolationDescription)") + } } } - } - process.standardOutput = stdoutPipe + process.standardOutput = stdoutPipe + + // Set up a pipe for receiving free-form text output from the plugin on its stderr. + let stderrPipe = Pipe() + let stderrLock = NSLock() + var stderrData = Data() + let stderrHandler = { (data: Data) in + // Pass on any available data to the delegate. + if data.isEmpty { return } + stderrData.append(contentsOf: data) + delegate.handleOutput(data: data) + } + stderrPipe.fileHandleForReading.readabilityHandler = { fileHandle in + // Read and pass on any available free-form text output from the plugin. + // We need the lock since we could run concurrently with the termination handler. + stderrLock.withLock { stderrHandler(fileHandle.availableData) } + } + process.standardError = stderrPipe - // Set up a pipe for receiving free-form text output from the plugin on its stderr. - let stderrPipe = Pipe() - let stderrLock = NSLock() - var stderrData = Data() - let stderrHandler = { (data: Data) in - // Pass on any available data to the delegate. - if data.isEmpty { return } - stderrData.append(contentsOf: data) - callbackQueue.async { delegate.handleOutput(data: data) } - } - stderrPipe.fileHandleForReading.readabilityHandler = { fileHandle in - // Read and pass on any available free-form text output from the plugin. - // We need the lock since we could run concurrently with the termination handler. - stderrLock.withLock { stderrHandler(fileHandle.availableData) } - } - process.standardError = stderrPipe - - // Add it to the list of currently running plugin processes, so it can be cancelled if the host is interrupted. - guard let cancellationKey = self.cancellator.register(process) else { - return callbackQueue.async { - completion(.failure(CancellationError())) + // Add it to the list of currently running plugin processes, so it can be cancelled if the host is interrupted. + guard let cancellationKey = self.cancellator.register(process) else { + return continuation.resume(throwing: CancellationError()) } - } - // Set up a handler to deal with the exit of the plugin process. - process.terminationHandler = { process in - // Remove the process from the list of currently running ones. - self.cancellator.deregister(cancellationKey) + // Set up a handler to deal with the exit of the plugin process. + process.terminationHandler = { process in + // Remove the process from the list of currently running ones. + self.cancellator.deregister(cancellationKey) - // Close the output handle through which we talked to the plugin. - try? outputHandle.close() + // Close the output handle through which we talked to the plugin. + try? outputHandle.close() + + // Read and pass on any remaining free-form text output from the plugin. + // We need the lock since we could run concurrently with the readability handler. + stderrLock.withLock { + try? stderrPipe.fileHandleForReading.readToEnd().map{ stderrHandler($0) } + } + + // Read and pass on any remaining messages from the plugin. + let handle = stdoutPipe.fileHandleForReading + if let handler = handle.readabilityHandler { + handler(handle) + } - // Read and pass on any remaining free-form text output from the plugin. - // We need the lock since we could run concurrently with the readability handler. - stderrLock.withLock { - try? stderrPipe.fileHandleForReading.readToEnd().map{ stderrHandler($0) } - } - // Read and pass on any remaining messages from the plugin. - let handle = stdoutPipe.fileHandleForReading - if let handler = handle.readabilityHandler { - handler(handle) + // We throw an error if the plugin ended with a signal. + if process.terminationReason == .uncaughtSignal { + continuation.resume(throwing: DefaultPluginScriptRunnerError.invocationEndedBySignal( + signal: process.terminationStatus, + command: command, + output: String(decoding: stderrData, as: UTF8.self))) + // Otherwise return the termination satatus. + } else { + continuation.resume(returning: process.terminationStatus) + } } - // Call the completion block with a result that depends on how the process ended. - callbackQueue.async { - completion(Result { - // We throw an error if the plugin ended with a signal. - if process.terminationReason == .uncaughtSignal { - throw DefaultPluginScriptRunnerError.invocationEndedBySignal( - signal: process.terminationStatus, - command: command, - output: String(decoding: stderrData, as: UTF8.self)) - } - // Otherwise return the termination satatus. - return process.terminationStatus - }) + // Start the plugin process. + do { + try process.run() } - } - - // Start the plugin process. - do { - try process.run() - } - catch { - callbackQueue.async { - completion(.failure(DefaultPluginScriptRunnerError.invocationFailed(error: error, command: command))) + catch { + continuation.resume(throwing: DefaultPluginScriptRunnerError.invocationFailed(error: error, command: command)) } - } - /// Send the initial message to the plugin. - outputQueue.async { - try? outputHandle.writePluginMessage(initialMessage) + /// Send the initial message to the plugin. + outputQueue.async { + try? outputHandle.writePluginMessage(initialMessage) + } } #endif } diff --git a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift index d86ef9ecad7..96cf829202e 100644 --- a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift +++ b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift @@ -60,50 +60,6 @@ public struct PluginTool { } extension PluginModule { - public func invoke( - action: PluginAction, - buildEnvironment: BuildEnvironment, - scriptRunner: PluginScriptRunner, - workingDirectory: AbsolutePath, - outputDirectory: AbsolutePath, - toolSearchDirectories: [AbsolutePath], - accessibleTools: [String: PluginTool], - writableDirectories: [AbsolutePath], - readOnlyDirectories: [AbsolutePath], - allowNetworkConnections: [SandboxNetworkPermission], - pkgConfigDirectories: [AbsolutePath], - sdkRootPath: AbsolutePath?, - fileSystem: FileSystem, - modulesGraph: ModulesGraph, - observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginInvocationDelegate - ) async throws -> Bool { - try await withCheckedThrowingContinuation { continuation in - self.invoke( - action: action, - buildEnvironment: buildEnvironment, - scriptRunner: scriptRunner, - workingDirectory: workingDirectory, - outputDirectory: outputDirectory, - toolSearchDirectories: toolSearchDirectories, - accessibleTools: accessibleTools, - writableDirectories: writableDirectories, - readOnlyDirectories: readOnlyDirectories, - allowNetworkConnections: allowNetworkConnections, - pkgConfigDirectories: pkgConfigDirectories, - sdkRootPath: sdkRootPath, - fileSystem: fileSystem, - modulesGraph: modulesGraph, - observabilityScope: observabilityScope, - callbackQueue: callbackQueue, - delegate: delegate, - completion: { - continuation.resume(with: $0) - } - ) - } - } /// Invokes the plugin by compiling its source code (if needed) and then running it as a subprocess. The specified /// plugin action determines which entry point is called in the subprocess, and the package and the tool mapping @@ -128,7 +84,6 @@ extension PluginModule { /// - fileSystem: The file system to which all of the paths refers. /// /// - Returns: A PluginInvocationResult that contains the results of invoking the plugin. - @available(*, noasync, message: "Use the async alternative") public func invoke( action: PluginAction, buildEnvironment: BuildEnvironment, @@ -145,16 +100,14 @@ extension PluginModule { fileSystem: FileSystem, modulesGraph: ModulesGraph, observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginInvocationDelegate, - completion: @escaping (Result) -> Void - ) { + delegate: PluginInvocationDelegate + ) async throws -> Bool { // Create the plugin's output directory if needed (but don't do anything with it if it already exists). do { try fileSystem.createDirectory(outputDirectory, recursive: true) } catch { - return callbackQueue.async { completion(.failure(PluginEvaluationError.couldNotCreateOuputDirectory(path: outputDirectory, underlyingError: error))) } + throw PluginEvaluationError.couldNotCreateOuputDirectory(path: outputDirectory, underlyingError: error) } // Serialize the plugin action to send as the initial message. @@ -263,7 +216,7 @@ extension PluginModule { initialMessage = try actionMessage.toData() } catch { - return callbackQueue.async { completion(.failure(PluginEvaluationError.couldNotSerializePluginInput(underlyingError: error))) } + throw PluginEvaluationError.couldNotSerializePluginInput(underlyingError: error) } // Handle messages and output from the plugin. @@ -410,7 +363,7 @@ extension PluginModule { let runnerDelegate = ScriptRunnerDelegate(invocationDelegate: delegate, observabilityScope: observabilityScope) // Call the plugin script runner to actually invoke the plugin. - scriptRunner.runPluginScript( + let exitCode = try await scriptRunner.runPluginScript( sourceFiles: sources.paths, pluginName: self.name, initialMessage: initialMessage, @@ -421,69 +374,21 @@ extension PluginModule { allowNetworkConnections: allowNetworkConnections, fileSystem: fileSystem, observabilityScope: observabilityScope, - callbackQueue: callbackQueue, - delegate: runnerDelegate) { result in - dispatchPrecondition(condition: .onQueue(callbackQueue)) - completion(result.map { exitCode in - // Return a result based on the exit code or the `exitEarly` parameter. If the plugin - // exits with an error but hasn't already emitted an error, we do so for it. - let exitedCleanly = (exitCode == 0) && !runnerDelegate.exitEarly - if !exitedCleanly && !runnerDelegate.hasReportedError { - delegate.pluginEmittedDiagnostic( - .error("Plugin ended with exit code \(exitCode)") - ) - } - return exitedCleanly - }) - } - } - - package func invoke( - module: ResolvedModule, - action: PluginAction, - buildEnvironment: BuildEnvironment, - scriptRunner: PluginScriptRunner, - workingDirectory: AbsolutePath, - outputDirectory: AbsolutePath, - toolSearchDirectories: [AbsolutePath], - accessibleTools: [String: PluginTool], - writableDirectories: [AbsolutePath], - readOnlyDirectories: [AbsolutePath], - allowNetworkConnections: [SandboxNetworkPermission], - pkgConfigDirectories: [AbsolutePath], - sdkRootPath: AbsolutePath?, - fileSystem: FileSystem, - modulesGraph: ModulesGraph, - observabilityScope: ObservabilityScope - ) async throws -> BuildToolPluginInvocationResult { - try await withCheckedThrowingContinuation { continuation in - self.invoke( - module: module, - action: action, - buildEnvironment: buildEnvironment, - scriptRunner: scriptRunner, - workingDirectory: workingDirectory, - outputDirectory: outputDirectory, - toolSearchDirectories: toolSearchDirectories, - accessibleTools: accessibleTools, - writableDirectories: writableDirectories, - readOnlyDirectories: readOnlyDirectories, - allowNetworkConnections: allowNetworkConnections, - pkgConfigDirectories: pkgConfigDirectories, - sdkRootPath: sdkRootPath, - fileSystem: fileSystem, - modulesGraph: modulesGraph, - observabilityScope: observabilityScope, - completion: { - continuation.resume(with: $0) - } + delegate: runnerDelegate) + + // Return a result based on the exit code or the `exitEarly` parameter. If the plugin + // exits with an error but hasn't already emitted an error, we do so for it. + let exitedCleanly = (exitCode == 0) && !runnerDelegate.exitEarly + if !exitedCleanly && !runnerDelegate.hasReportedError { + delegate.pluginEmittedDiagnostic( + .error("Plugin ended with exit code \(exitCode)") ) } + return exitedCleanly } /// This is a convenient way to get results of the plugin invocation without having /// to deal with delegates and other internal details. - @available(*, noasync, message: "Use the async alternative") package func invoke( module: ResolvedModule, action: PluginAction, @@ -500,13 +405,11 @@ extension PluginModule { sdkRootPath: AbsolutePath?, fileSystem: FileSystem, modulesGraph: ModulesGraph, - observabilityScope: ObservabilityScope, - completion: @escaping (Result) -> Void - ) { + observabilityScope: ObservabilityScope + ) async throws -> BuildToolPluginInvocationResult { /// Determine the package that contains the target. guard let package = modulesGraph.package(for: module) else { - completion(.failure(InternalError("Could not find package for \(self)"))) - return + throw InternalError("Could not find package for \(self)") } // Set up a delegate to handle callbacks from the build tool plugin. @@ -528,7 +431,7 @@ extension PluginModule { let startTime = DispatchTime.now() - self.invoke( + let success = (try? await self.invoke( action: action, buildEnvironment: buildEnvironment, scriptRunner: scriptRunner, @@ -544,33 +447,21 @@ extension PluginModule { fileSystem: fileSystem, modulesGraph: modulesGraph, observabilityScope: observabilityScope, - callbackQueue: delegateQueue, - delegate: delegate, - completion: { - let duration = startTime.distance(to: .now()) - - let success: Bool = switch $0 { - case .success(let result): - result - case .failure: - false - } + delegate: delegate + )) ?? false + let duration = startTime.distance(to: .now()) - let invocationResult = BuildToolPluginInvocationResult( - plugin: self, - pluginOutputDirectory: outputDirectory, - package: package, - target: module, - succeeded: success, - duration: duration, - diagnostics: delegate.diagnostics, - textOutput: String(decoding: delegate.outputData, as: UTF8.self), - buildCommands: delegate.buildCommands, - prebuildCommands: delegate.prebuildCommands - ) - - completion(.success(invocationResult)) - } + return BuildToolPluginInvocationResult( + plugin: self, + pluginOutputDirectory: outputDirectory, + package: package, + target: module, + succeeded: success, + duration: duration, + diagnostics: delegate.diagnostics, + textOutput: String(decoding: delegate.outputData, as: UTF8.self), + buildCommands: delegate.buildCommands, + prebuildCommands: delegate.prebuildCommands ) } } diff --git a/Sources/SPMBuildCore/Plugins/PluginScriptRunner.swift b/Sources/SPMBuildCore/Plugins/PluginScriptRunner.swift index fda28b919a0..def54115630 100644 --- a/Sources/SPMBuildCore/Plugins/PluginScriptRunner.swift +++ b/Sources/SPMBuildCore/Plugins/PluginScriptRunner.swift @@ -23,16 +23,13 @@ import TSCUtility public protocol PluginScriptRunner { /// Public protocol function that starts compiling the plugin script to an executable. The name is used as the basename for the executable and auxiliary files. The tools version controls the availability of APIs in PackagePlugin, and should be set to the tools version of the package that defines the plugin (not of the target to which it is being applied). This function returns immediately and then calls the completion handler on the callback queue when compilation ends. - @available(*, noasync, message: "Use the async alternative") func compilePluginScript( sourceFiles: [Basics.AbsolutePath], pluginName: String, toolsVersion: ToolsVersion, observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginScriptCompilerDelegate, - completion: @escaping (Result) -> Void - ) + delegate: PluginScriptCompilerDelegate + ) async throws -> PluginCompilationResult /// Implements the mechanics of running a plugin script implemented as a set of Swift source files, for use /// by the package graph when it is evaluating package plugins. @@ -55,40 +52,14 @@ public protocol PluginScriptRunner { allowNetworkConnections: [SandboxNetworkPermission], fileSystem: FileSystem, observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginScriptCompilerDelegate & PluginScriptRunnerDelegate, - completion: @escaping (Result) -> Void - ) + delegate: PluginScriptCompilerDelegate & PluginScriptRunnerDelegate + ) async throws -> Int32 /// Returns the Triple that represents the host for which plugin script tools should be built, or for which binary /// tools should be selected. var hostTriple: Basics.Triple { get throws } } -public extension PluginScriptRunner { - func compilePluginScript( - sourceFiles: [Basics.AbsolutePath], - pluginName: String, - toolsVersion: ToolsVersion, - observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginScriptCompilerDelegate - ) async throws -> PluginCompilationResult { - try await withCheckedThrowingContinuation { continuation in - self.compilePluginScript( - sourceFiles: sourceFiles, - pluginName: pluginName, - toolsVersion: toolsVersion, - observabilityScope: observabilityScope, - callbackQueue: callbackQueue, - delegate: delegate, - completion: { - continuation.resume(with: $0) - } - ) - } - } -} /// Protocol by which `PluginScriptRunner` communicates back to the caller as it compiles plugins. public protocol PluginScriptCompilerDelegate { diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift index 0dcacd84e2a..68b60e8d943 100644 --- a/Sources/Workspace/Workspace+Dependencies.swift +++ b/Sources/Workspace/Workspace+Dependencies.swift @@ -19,7 +19,6 @@ import func Basics.os_signpost import struct Basics.RelativePath import enum Basics.SignpostName import class Basics.ThreadSafeKeyValueStore -import class Dispatch.DispatchGroup import struct Dispatch.DispatchTime import enum Dispatch.DispatchTimeInterval import struct PackageGraph.Assignment diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift index 6a23983e8a5..45b10318b0a 100644 --- a/Sources/Workspace/Workspace+Manifests.swift +++ b/Sources/Workspace/Workspace+Manifests.swift @@ -19,7 +19,6 @@ import struct Basics.InternalError import class Basics.ObservabilityScope import struct Basics.SwiftVersion import class Basics.ThreadSafeKeyValueStore -import class Dispatch.DispatchGroup import struct Dispatch.DispatchTime import struct OrderedCollections.OrderedDictionary import struct OrderedCollections.OrderedSet @@ -1044,17 +1043,12 @@ extension Workspace { case .fileSystem: return nil case .custom: - let container = try await withCheckedThrowingContinuation { continuation in - self.packageContainerProvider.getContainer( - for: package, - updateStrategy: .never, - observabilityScope: observabilityScope, - on: .sharedConcurrent, - completion: { - continuation.resume(with: $0) - } - ) - } + let container = try await self.packageContainerProvider.getContainer( + for: package, + updateStrategy: .never, + observabilityScope: observabilityScope + ) + guard let customContainer = container as? CustomPackageContainer else { observabilityScope.emit(error: "invalid custom dependency container: \(container)") return nil diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index c8e6c315a33..6b5427634e9 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -1083,126 +1083,78 @@ extension Workspace { } } - /// Loads and returns manifests at the given paths. - @available(*, noasync, message: "Use the async alternative") - public func loadRootManifests( - packages: [AbsolutePath], - observabilityScope: ObservabilityScope, - completion: @escaping @Sendable (Result<[AbsolutePath: Manifest], Error>) -> Void - ) { - DispatchQueue.sharedConcurrent.asyncResult(completion) { - try await self.loadRootManifests( - packages: packages, - observabilityScope: observabilityScope - ) - } - } - /// Loads and returns manifest at the given path. public func loadRootManifest( at path: AbsolutePath, observabilityScope: ObservabilityScope ) async throws -> Manifest { - try await withCheckedThrowingContinuation { continuation in - self.loadRootManifest(at: path, observabilityScope: observabilityScope) { result in - continuation.resume(with: result) - } - } - } - - /// Loads and returns manifest at the given path. - public func loadRootManifest( - at path: AbsolutePath, - observabilityScope: ObservabilityScope, - completion: @escaping (Result) -> Void - ) { - self.loadRootManifests(packages: [path], observabilityScope: observabilityScope) { result in - completion(result.tryMap { - // normally, we call loadRootManifests which attempts to load any manifest it can and report errors via - // diagnostics - // in this case, we want to load a specific manifest, so if the diagnostics contains an error we want to - // throw - guard !observabilityScope.errorsReported else { - throw Diagnostics.fatalError - } - guard let manifest = $0[path] else { - throw InternalError("Unknown manifest for '\(path)'") - } - return manifest - }) + let result = try await self.loadRootManifests(packages: [path], observabilityScope: observabilityScope) + guard !observabilityScope.errorsReported else { + throw Diagnostics.fatalError } - } - - /// Loads root package - public func loadRootPackage(at path: AbsolutePath, observabilityScope: ObservabilityScope) async throws -> Package { - try await withCheckedThrowingContinuation { continuation in - self.loadRootPackage(at: path, observabilityScope: observabilityScope) { result in - continuation.resume(with: result) - } + guard let manifest = result[path] else { + throw InternalError("Unknown manifest for '\(path)'") } + return manifest } /// Loads root package public func loadRootPackage( at path: AbsolutePath, - observabilityScope: ObservabilityScope, - completion: @escaping (Result) -> Void - ) { - self.loadRootManifest(at: path, observabilityScope: observabilityScope) { result in - let result = result.tryMap { manifest -> Package in - let identity = try self.identityResolver.resolveIdentity(for: manifest.packageKind) - - // radar/82263304 - // compute binary artifacts for the sake of constructing a project model - // note this does not actually download remote artifacts and as such does not have the artifact's type - // or path - let binaryArtifacts = try manifest.targets.filter { $0.type == .binary } - .reduce(into: [String: BinaryArtifact]()) { partial, target in - if let path = target.path { - let artifactPath = try manifest.path.parentDirectory - .appending(RelativePath(validating: path)) - guard let (_, artifactKind) = try BinaryArtifactsManager.deriveBinaryArtifact( - fileSystem: self.fileSystem, - path: artifactPath, - observabilityScope: observabilityScope - ) else { - throw StringError("\(artifactPath) does not contain binary artifact") - } - partial[target.name] = BinaryArtifact( - kind: artifactKind, - originURL: .none, - path: artifactPath - ) - } else if let url = target.url.flatMap(URL.init(string:)) { - let fakePath = try manifest.path.parentDirectory.appending(components: "remote", "archive") - .appending(RelativePath(validating: url.lastPathComponent)) - partial[target.name] = BinaryArtifact( - kind: .unknown, - originURL: url.absoluteString, - path: fakePath - ) - } else { - throw InternalError("a binary target should have either a path or a URL and a checksum") - } - } + observabilityScope: ObservabilityScope + ) async throws -> Package { - let builder = PackageBuilder( - identity: identity, - manifest: manifest, - productFilter: .everything, - path: path, - additionalFileRules: [], - binaryArtifacts: binaryArtifacts, - prebuilts: [:], - fileSystem: self.fileSystem, - observabilityScope: observabilityScope, - // For now we enable all traits - enabledTraits: Set(manifest.traits.map(\.name)) - ) - return try builder.construct() + let manifest = try await self.loadRootManifest(at: path, observabilityScope: observabilityScope) + let identity = try self.identityResolver.resolveIdentity(for: manifest.packageKind) + + // radar/82263304 + // compute binary artifacts for the sake of constructing a project model + // note this does not actually download remote artifacts and as such does not have the artifact's type + // or path + let binaryArtifacts = try manifest.targets.filter { $0.type == .binary } + .reduce(into: [String: BinaryArtifact]()) { partial, target in + if let path = target.path { + let artifactPath = try manifest.path.parentDirectory + .appending(RelativePath(validating: path)) + guard let (_, artifactKind) = try BinaryArtifactsManager.deriveBinaryArtifact( + fileSystem: self.fileSystem, + path: artifactPath, + observabilityScope: observabilityScope + ) else { + throw StringError("\(artifactPath) does not contain binary artifact") + } + partial[target.name] = BinaryArtifact( + kind: artifactKind, + originURL: .none, + path: artifactPath + ) + } else if let url = target.url.flatMap(URL.init(string:)) { + let fakePath = try manifest.path.parentDirectory.appending(components: "remote", "archive") + .appending(RelativePath(validating: url.lastPathComponent)) + partial[target.name] = BinaryArtifact( + kind: .unknown, + originURL: url.absoluteString, + path: fakePath + ) + } else { + throw InternalError("a binary target should have either a path or a URL and a checksum") + } } - completion(result) - } + + let builder = PackageBuilder( + identity: identity, + manifest: manifest, + productFilter: .everything, + path: path, + additionalFileRules: [], + binaryArtifacts: binaryArtifacts, + prebuilts: [:], + fileSystem: self.fileSystem, + observabilityScope: observabilityScope, + // For now we enable all traits + enabledTraits: Set(manifest.traits.map(\.name)) + ) + return try builder.construct() } public func loadPluginImports( @@ -1270,24 +1222,6 @@ extension Workspace { return try builder.construct() } - /// Loads a single package in the context of a previously loaded graph. This can be useful for incremental loading - /// in a longer-lived program, like an IDE. - @available(*, noasync, message: "Use the async alternative") - public func loadPackage( - with identity: PackageIdentity, - packageGraph: ModulesGraph, - observabilityScope: ObservabilityScope, - completion: @escaping @Sendable (Result) -> Void - ) { - DispatchQueue.sharedConcurrent.asyncResult(completion) { - try await self.loadPackage( - with: identity, - packageGraph: packageGraph, - observabilityScope: observabilityScope - ) - } - } - public func changeSigningEntityFromVersion( package: PackageIdentity, version: Version, diff --git a/Tests/BasicsTests/AsyncProcessTests.swift b/Tests/BasicsTests/AsyncProcessTests.swift index 16da11dbcbb..265bfb105bf 100644 --- a/Tests/BasicsTests/AsyncProcessTests.swift +++ b/Tests/BasicsTests/AsyncProcessTests.swift @@ -66,7 +66,7 @@ final class AsyncProcessTests: XCTestCase { } } - func testPopenLegacyAsync() throws { + func testPopenLegacyAsync() async throws { #if os(Windows) let args = ["where.exe", "where"] let answer = "C:\\Windows\\System32\\where.exe" @@ -74,23 +74,13 @@ final class AsyncProcessTests: XCTestCase { let args = ["whoami"] let answer = NSUserName() #endif - var popenResult: Result? - let group = DispatchGroup() - group.enter() - AsyncProcess.popen(arguments: args) { result in - popenResult = result - group.leave() - } - group.wait() - switch popenResult { - case .success(let processResult): - let output = try processResult.utf8Output() - XCTAssertTrue(output.hasPrefix(answer)) - case .failure(let error): - XCTFail("error = \(error)") - case nil: - XCTFail() + let processResult = try await withCheckedThrowingContinuation { continuation in + AsyncProcess.popen(arguments: args) { result in + continuation.resume(with: result) + } } + let output = try processResult.utf8Output() + XCTAssert(output.hasPrefix(answer)) } @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) diff --git a/Tests/BuildTests/PluginInvocationTests.swift b/Tests/BuildTests/PluginInvocationTests.swift index 84c4ac524c1..26d5d2f7928 100644 --- a/Tests/BuildTests/PluginInvocationTests.swift +++ b/Tests/BuildTests/PluginInvocationTests.swift @@ -140,13 +140,9 @@ final class PluginInvocationTests: XCTestCase { pluginName: String, toolsVersion: ToolsVersion, observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginScriptCompilerDelegate, - completion: @escaping (Result) -> Void - ) { - callbackQueue.sync { - completion(.failure(StringError("unimplemented"))) - } + delegate: PluginScriptCompilerDelegate + ) async throws -> PluginCompilationResult{ + throw StringError("unimplemented") } func runPluginScript( @@ -160,21 +156,19 @@ final class PluginInvocationTests: XCTestCase { allowNetworkConnections: [SandboxNetworkPermission], fileSystem: FileSystem, observabilityScope: ObservabilityScope, - callbackQueue: DispatchQueue, - delegate: PluginScriptCompilerDelegate & PluginScriptRunnerDelegate, - completion: @escaping (Result) -> Void - ) { + delegate: PluginScriptCompilerDelegate & PluginScriptRunnerDelegate + ) async throws -> Int32 { // Check that we were given the right sources. XCTAssertEqual(sourceFiles, ["/Foo/Plugins/FooPlugin/source.swift"]) do { // Pretend the plugin emitted some output. - callbackQueue.sync { + do { delegate.handleOutput(data: Data("Hello Plugin!".utf8)) } // Pretend it emitted a warning. - try callbackQueue.sync { + do { let message = Data(""" { "emitDiagnostic": { "severity": "warning", @@ -188,7 +182,7 @@ final class PluginInvocationTests: XCTestCase { } // Pretend it defined a build command. - try callbackQueue.sync { + do { let message = Data(""" { "defineBuildCommand": { "configuration": { @@ -213,17 +207,9 @@ final class PluginInvocationTests: XCTestCase { try delegate.handleMessage(data: message, responder: { _ in }) } } - catch { - callbackQueue.sync { - completion(.failure(error)) - } - return - } // If we get this far we succeeded, so invoke the completion handler. - callbackQueue.sync { - completion(.success(0)) - } + return 0 } } @@ -384,7 +370,6 @@ final class PluginInvocationTests: XCTestCase { pluginName: buildToolPlugin.name, toolsVersion: buildToolPlugin.apiVersion, observabilityScope: observability.topScope, - callbackQueue: DispatchQueue.sharedConcurrent, delegate: delegate ) @@ -436,7 +421,6 @@ final class PluginInvocationTests: XCTestCase { pluginName: buildToolPlugin.name, toolsVersion: buildToolPlugin.apiVersion, observabilityScope: observability.topScope, - callbackQueue: DispatchQueue.sharedConcurrent, delegate: delegate ) @@ -486,7 +470,6 @@ final class PluginInvocationTests: XCTestCase { pluginName: buildToolPlugin.name, toolsVersion: buildToolPlugin.apiVersion, observabilityScope: observability.topScope, - callbackQueue: DispatchQueue.sharedConcurrent, delegate: delegate ) @@ -548,7 +531,6 @@ final class PluginInvocationTests: XCTestCase { pluginName: buildToolPlugin.name, toolsVersion: buildToolPlugin.apiVersion, observabilityScope: observability.topScope, - callbackQueue: DispatchQueue.sharedConcurrent, delegate: delegate ) @@ -601,7 +583,6 @@ final class PluginInvocationTests: XCTestCase { pluginName: buildToolPlugin.name, toolsVersion: buildToolPlugin.apiVersion, observabilityScope: observability.topScope, - callbackQueue: DispatchQueue.sharedConcurrent, delegate: delegate ) diff --git a/Tests/FunctionalTests/PluginTests.swift b/Tests/FunctionalTests/PluginTests.swift index 5f5443b73e1..de97be18e4e 100644 --- a/Tests/FunctionalTests/PluginTests.swift +++ b/Tests/FunctionalTests/PluginTests.swift @@ -689,8 +689,7 @@ final class PluginTests { ) let toolSearchDirectories = [try UserToolchain.default.swiftCompilerPath.parentDirectory] - let success = try await withCheckedThrowingContinuation { continuation in - plugin.invoke( + let success = try await plugin.invoke( action: .performCommand(package: package, arguments: arguments), buildEnvironment: BuildEnvironment(platform: .macOS, configuration: .debug), scriptRunner: scriptRunner, @@ -706,13 +705,8 @@ final class PluginTests { fileSystem: localFileSystem, modulesGraph: packageGraph, observabilityScope: observability.topScope, - callbackQueue: delegateQueue, - delegate: delegate, - completion: { - continuation.resume(with: $0) - } - ) - } + delegate: delegate + ) if expectFailure { #expect(!success, "expected command to fail, but it succeeded") } @@ -997,7 +991,6 @@ final class PluginTests { fileSystem: localFileSystem, modulesGraph: packageGraph, observabilityScope: observability.topScope, - callbackQueue: delegateQueue, delegate: delegate ) } onCancel: { diff --git a/Tests/PackageRegistryTests/RegistryClientTests.swift b/Tests/PackageRegistryTests/RegistryClientTests.swift index ab189d8aaae..28734ddbf29 100644 --- a/Tests/PackageRegistryTests/RegistryClientTests.swift +++ b/Tests/PackageRegistryTests/RegistryClientTests.swift @@ -105,15 +105,11 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") let metadata = try await registryClient.getPackageMetadata(package: identity) assert(metadata) - let metadataSync = try await withCheckedThrowingContinuation { continuation in - return registryClient.getPackageMetadata( - package: identity, - timeout: nil, - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent, - completion: { continuation.resume(with: $0) } - ) - } + let metadataSync = try await registryClient.getPackageMetadata( + package: identity, + timeout: nil, + observabilityScope: ObservabilitySystem.NOOP + ) assert(metadataSync) } @@ -427,16 +423,12 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") let metadata = try await registryClient.getPackageVersionMetadata(package: identity, version: version) assert(metadata) - let metadataSync = try await withCheckedThrowingContinuation { continuation in - return registryClient.getPackageVersionMetadata( - package: identity, - version: version, - fileSystem: InMemoryFileSystem(), - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent, - completion: { continuation.resume(with: $0) } - ) - } + let metadataSync = try await registryClient.getPackageVersionMetadata( + package: identity, + version: version, + fileSystem: InMemoryFileSystem(), + observabilityScope: ObservabilitySystem.NOOP + ) assert(metadataSync) } @@ -640,15 +632,11 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") ) assert(availableManifests) - let availableManifestsSync = try await withCheckedThrowingContinuation { continuation in - return registryClient.getAvailableManifests( - package: identity, - version: version, - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent, - completion: { continuation.resume(with: $0) } - ) - } + let availableManifestsSync = try await registryClient.getAvailableManifests( + package: identity, + version: version, + observabilityScope: ObservabilitySystem.NOOP + ) assert(availableManifestsSync) } @@ -1290,15 +1278,12 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") } do { - let manifestSync = try await withCheckedThrowingContinuation { continuation in - return registryClient.getManifestContent( - package: identity, - version: version, - customToolsVersion: toolsVersion, - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent - ) { continuation.resume(with: $0) } - } + let manifestSync = try await registryClient.getManifestContent( + package: identity, + version: version, + customToolsVersion: toolsVersion, + observabilityScope: ObservabilitySystem.NOOP + ) let parsedToolsVersion = try ToolsVersionParser.parse(utf8String: manifestSync) #expect(parsedToolsVersion == expectedToolsVersion) } @@ -2033,18 +2018,14 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") try assert(path) let syncPath = try! AbsolutePath(validating: "/\(identity)-\(version)-sync") - try await withCheckedThrowingContinuation { continuation in - registryClient.downloadSourceArchive( - package: identity, - version: version, - destinationPath: syncPath, - progressHandler: nil, - fileSystem: fileSystem, - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent, - completion: { continuation.resume(with: $0) } - ) - } + try await registryClient.downloadSourceArchive( + package: identity, + version: version, + destinationPath: syncPath, + progressHandler: nil, + fileSystem: fileSystem, + observabilityScope: ObservabilitySystem.NOOP + ) try assert(syncPath) } @@ -2876,14 +2857,10 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") let identities = try await registryClient.lookupIdentities(scmURL: packageURL) #expect([PackageIdentity.plain("mona.LinkedList")] == identities) - let syncIdentities = try await withCheckedThrowingContinuation { continuation in - registryClient.lookupIdentities( - scmURL: packageURL, - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent, - completion: { continuation.resume(with: $0) } - ) - } + let syncIdentities = try await registryClient.lookupIdentities( + scmURL: packageURL, + observabilityScope: ObservabilitySystem.NOOP + ) #expect([PackageIdentity.plain("mona.LinkedList")] == syncIdentities) } @@ -3066,14 +3043,10 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") ) try await registryClient.login(loginURL: loginURL) - try await withCheckedThrowingContinuation { continuation in - registryClient.login( - loginURL: loginURL, - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent, - completion: { continuation.resume(with: $0) } - ) - } + try await registryClient.login( + loginURL: loginURL, + observabilityScope: ObservabilitySystem.NOOP + ) } @Test func handlesMissingCredentials() async throws { @@ -3204,21 +3177,18 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") let registryClient = makeRegistryClient(configuration: configuration, httpClient: httpClient) - let result = try await withCheckedThrowingContinuation { continuation in - return registryClient.publish( - registryURL: registryURL, - packageIdentity: identity, - packageVersion: version, - packageArchive: archivePath, - packageMetadata: metadataPath, - signature: .none, - metadataSignature: .none, - signatureFormat: .none, - fileSystem: localFileSystem, - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent - ) { result in continuation.resume(with: result) } - } + let result = try await registryClient.publish( + registryURL: registryURL, + packageIdentity: identity, + packageVersion: version, + packageArchive: archivePath, + packageMetadata: metadataPath, + signature: .none, + metadataSignature: .none, + signatureFormat: .none, + fileSystem: localFileSystem, + observabilityScope: ObservabilitySystem.NOOP + ) #expect(result == .published(expectedLocation)) } @@ -3622,14 +3592,10 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") let status = try await registryClient.checkAvailability(registry: registry) #expect(status == .available) - let syncStatus = try await withCheckedThrowingContinuation { continuation in - registryClient.checkAvailability( - registry: registry, - observabilityScope: ObservabilitySystem.NOOP, - callbackQueue: .sharedConcurrent, - completion: { continuation.resume(with: $0) } - ) - } + let syncStatus = try await registryClient.checkAvailability( + registry: registry, + observabilityScope: ObservabilitySystem.NOOP + ) #expect(syncStatus == .available) } diff --git a/Tests/SourceControlTests/RepositoryManagerTests.swift b/Tests/SourceControlTests/RepositoryManagerTests.swift index e8a94bbdc29..b39fec9c0bf 100644 --- a/Tests/SourceControlTests/RepositoryManagerTests.swift +++ b/Tests/SourceControlTests/RepositoryManagerTests.swift @@ -431,7 +431,7 @@ final class RepositoryManagerTests: XCTestCase { } } - func testCancel() throws { + func testCancel() async throws { let observability = ObservabilitySystem.makeForTesting() let cancellator = Cancellator(observabilityScope: observability.topScope) @@ -446,51 +446,52 @@ final class RepositoryManagerTests: XCTestCase { cancellator.register(name: "repository manager", handler: manager) - let finishGroup = DispatchGroup() - let results = ThreadSafeKeyValueStore>() - for index in 0 ..< total { - let path = try AbsolutePath(validating: "/repo/\(index)") - let repository = RepositorySpecifier(path: path) - provider.startGroup.enter() - finishGroup.enter() - manager.lookup( - package: PackageIdentity(path: path), - repository: repository, - updateStrategy: .never, - observabilityScope: observability.topScope, - callbackQueue: .sharedConcurrent - ) { result in - defer { finishGroup.leave() } - results[repository] = result - } - } - - XCTAssertEqual(.success, provider.startGroup.wait(timeout: .now() + 5), "timeout starting tasks") - - let cancelled = cancellator._cancel(deadline: .now() + .seconds(1)) - XCTAssertEqual(cancelled, 1, "expected to be terminated") - XCTAssertNoDiagnostics(observability.diagnostics) - // this releases the fetch threads that are waiting to test if the call was cancelled - provider.terminatedGroup.leave() - - XCTAssertEqual(.success, finishGroup.wait(timeout: .now() + 5), "timeout finishing tasks") - - XCTAssertEqual(results.count, total, "expected \(total) results") - for (repository, result) in results.get() { - switch (Int(repository.basename)! < total / 2, result) { - case (true, .success): - break // as expected! - case (true, .failure(let error)): - XCTFail("expected success, but failed with \(type(of: error)) '\(error)'") - case (false, .success): - XCTFail("expected operation to be cancelled") - case (false, .failure(let error)): - XCTAssert(error is CancellationError, "expected error to be CancellationError, but was \(type(of: error)) '\(error)'") - } - } - - // wait for outstanding threads that would be cancelled and completion handlers thrown away - XCTAssertEqual(.success, provider.outstandingGroup.wait(timeout: .now() + .seconds(5)), "timeout waiting for outstanding tasks") + try await withThrowingTaskGroup(of: (specifier:RepositorySpecifier, result:Result).self) { group in + for index in 0 ..< total { + let path = try AbsolutePath(validating: "/repo/\(index)") + let repository = RepositorySpecifier(path: path) + provider.startGroup.enter() + group.addTask { + do { + let handle = try await manager.lookup( + package: PackageIdentity(path: path), + repository: repository, + updateStrategy: .never, + observabilityScope: observability.topScope) + return (repository, .success(handle)) + } catch { + return (repository, .failure(error)) + } + } + } + + XCTAssertEqual(.success, provider.startGroup.wait(timeout: .now() + 5), "timeout starting tasks") + + let cancelled = cancellator._cancel(deadline: .now() + .seconds(1)) + XCTAssertEqual(cancelled, 1, "expected to be terminated") + XCTAssertNoDiagnostics(observability.diagnostics) + // this releases the fetch threads that are waiting to test if the call was cancelled + provider.terminatedGroup.leave() + + var count = 0 + for try await (repository, result) in group { + count += 1 + switch (Int(repository.basename)! < total / 2, result) { + case (true, .success): + break // as expected! + case (true, .failure(let error)): + XCTFail("expected success, but failed with \(type(of: error)) '\(error)'") + case (false, .success): + XCTFail("expected operation to be cancelled") + case (false, .failure(let error)): + XCTAssert(error is CancellationError, "expected error to be CancellationError, but was \(type(of: error)) '\(error)'") + } + } + XCTAssertEqual(count, total, "expected \(total) results") + + // wait for outstanding threads that would be cancelled and completion handlers thrown away + XCTAssertEqual(.success, provider.outstandingGroup.wait(timeout: .now() + .seconds(5)), "timeout waiting for outstanding tasks") + } // the provider called in a thread managed by the RepositoryManager // the use of blocking semaphore is intentional diff --git a/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift b/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift index 45adb1a22f6..183d2bf1ef8 100644 --- a/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift +++ b/Tests/WorkspaceTests/SourceControlPackageContainerTests.swift @@ -744,17 +744,11 @@ extension PackageContainerProvider { for package: PackageReference, updateStrategy: ContainerUpdateStrategy = .always ) async throws -> PackageContainer { - try await withCheckedThrowingContinuation { continuation in - self.getContainer( - for: package, - updateStrategy: updateStrategy, - observabilityScope: ObservabilitySystem.NOOP, - on: .global(), - completion: { - continuation.resume(with: $0) - } - ) - } + try await self.getContainer( + for: package, + updateStrategy: updateStrategy, + observabilityScope: ObservabilitySystem.NOOP + ) } } diff --git a/Tests/WorkspaceTests/ToolsVersionSpecificationRewriterTests.swift b/Tests/WorkspaceTests/ToolsVersionSpecificationRewriterTests.swift index 9a67dd5ad5a..729ebcad731 100644 --- a/Tests/WorkspaceTests/ToolsVersionSpecificationRewriterTests.swift +++ b/Tests/WorkspaceTests/ToolsVersionSpecificationRewriterTests.swift @@ -120,7 +120,7 @@ fileprivate struct ToolsVersionSpecificationRewriterTests { ) // resultHandler(try inMemoryFileSystem.readFileContents(manifestFilePath)) - let actual = try #require(try inMemoryFileSystem.readFileContents(manifestFilePath)) + let actual = try inMemoryFileSystem.readFileContents(manifestFilePath) #expect(actual.validDescription == expected, "Actual is not expected") }