diff --git a/README.md b/README.md index 0ea153de..65ab382a 100644 --- a/README.md +++ b/README.md @@ -122,9 +122,9 @@ let result = try await run( } ``` -The closure-based `run` returns an `ExecutionResult`. Access the closure's return value with `result.closureOutput`, and the termination status with `result.terminationStatus`. +The closure-based `run` returns an `ExecutionResult`. Access the closure's return value with `result.closureResult`, and the termination status with `result.terminationStatus`. -Because `input`, `output`, and `error` are separate parameters, you can mix streaming and capturing in the same call. For example, stream standard output from the closure while collecting standard error as a string, and return the closure's own value through `closureOutput`: +Because `input`, `output`, and `error` are separate parameters, you can mix streaming and capturing in the same call. For example, stream standard output from the closure while collecting standard error as a string, and return the closure's own value through `closureResult`: ```swift let result = try await run( @@ -140,7 +140,7 @@ let result = try await run( return lineCount } -print(result.closureOutput) // The line count returned from the closure. +print(result.closureResult) // The line count returned from the closure. print(result.standardError ?? "") // The captured standard error. ``` diff --git a/Sources/Subprocess/API.swift b/Sources/Subprocess/API.swift index 8906345b..51e20af5 100644 --- a/Sources/Subprocess/API.swift +++ b/Sources/Subprocess/API.swift @@ -127,7 +127,7 @@ public func run< /// - Returns: An ``ExecutionResult`` that contains the closure's return value and /// the termination status of the child process. public func run< - Result, + Result: ~Copyable, Input: InputProtocol, Output: OutputProtocol, Error: ErrorOutputProtocol @@ -238,7 +238,7 @@ public func run< /// - Returns: An ``ExecutionResult`` that contains the closure's return value and /// the termination status of the child process. public func run< - Result, + Result: ~Copyable, Input: InputProtocol, Output: OutputProtocol, Error: ErrorOutputProtocol @@ -251,16 +251,9 @@ public func run< Execution ) async throws -> Result ) async throws -> ExecutionResult { - typealias RunResult = ( - processIdentifier: ProcessIdentifier, - closureResult: Result, - output: Output.OutputType, - error: Error.OutputType - ) - let outputPipe = try output.createPipe() let errorPipe = try error.createPipe(from: outputPipe) - let result: ExecutionOutcome = try await configuration.run( + let outcome: ExecutionOutcome<_RunOutcome> = try await configuration.run( input: try input.createPipe(), as: Input.self, output: outputPipe, @@ -272,7 +265,14 @@ public func run< var outputIOBox = consume outputIO var errorIOBox = consume errorIO - return try await withThrowingTaskGroup(of: _RunGroupResult.self) { group in + // The body's (possibly noncopyable) result is moved out through this + // box. The task group returns only the captured output and error, + // because a noncopyable value can't be a `GroupResult`. + var resultBox: Result? = nil + let captured: (Output.OutputType, Error.OutputType) = try await withThrowingTaskGroup( + of: _RunGroupResult.self, + returning: (Output.OutputType, Error.OutputType).self + ) { group in var writer: StandardInputWriter? if inputIOBox != nil { let inputWriter = StandardInputWriter( @@ -340,9 +340,8 @@ public func run< outputStream: outputSequence, errorStream: errorSequence ) - let result: Result do { - result = try await body(execution) + resultBox = try await body(execution) } catch { if Input.self == CustomWriteInput.self { try await writer?.finish() @@ -371,16 +370,24 @@ public func run< if Error.OutputType.self == Void.self { capturedError = (() as Any) as? Error.OutputType } - return (processIdentifier, result, capturedOutput!, capturedError!) + return (capturedOutput!, capturedError!) } + return _RunOutcome( + processIdentifier: processIdentifier, + closureResult: resultBox.take()!, + output: captured.0, + error: captured.1 + ) } + let terminationStatus = outcome.terminationStatus + let capturedResult = outcome.value return ExecutionResult( - processIdentifier: result.value.processIdentifier, - terminationStatus: result.terminationStatus, - closureOutput: result.value.closureResult, - standardOutput: result.value.output, - standardError: result.value.error + processIdentifier: capturedResult.processIdentifier, + terminationStatus: terminationStatus, + closureResult: capturedResult.closureResult, + standardOutput: capturedResult.output, + standardError: capturedResult.error ) } @@ -389,3 +396,14 @@ private enum _RunGroupResult { case standardErrorCaptured(Error.OutputType) case inputWritten } + +private struct _RunOutcome< + ClosureResult: Sendable & ~Copyable, + Output: OutputProtocol, + Error: OutputProtocol +>: ~Copyable, Sendable { + let processIdentifier: ProcessIdentifier + let closureResult: ClosureResult + let output: Output.OutputType + let error: Error.OutputType +} diff --git a/Sources/Subprocess/Configuration.swift b/Sources/Subprocess/Configuration.swift index b6ab67d2..33ce6f63 100644 --- a/Sources/Subprocess/Configuration.swift +++ b/Sources/Subprocess/Configuration.swift @@ -66,7 +66,12 @@ public struct Configuration: Sendable { self.platformOptions = platformOptions } - internal func run( + internal func run< + Result: ~Copyable & Sendable, + Input: InputProtocol, + Output: OutputProtocol, + Error: OutputProtocol + >( input: consuming CreatedPipe, as inputType: Input.Type, output: consuming CreatedPipe, @@ -108,9 +113,13 @@ public struct Configuration: Sendable { // already finished cleanly and no cancellation is needed. let taskFinishFlag = AtomicCounter() - let (result, monitorError) = try await withThrowingTaskGroup( + // The body's result is moved out through this box rather than + // returned from the task group since a noncopyable `Result` can't be a + // `GroupResult`. The group returns only the (copyable) monitor error. + var resultBox: Swift.Result? = nil + let monitorError = try await withThrowingTaskGroup( of: SubprocessError?.self, - returning: (Swift.Result, SubprocessError?).self + returning: SubprocessError?.self ) { group in group.addTask { do throws(SubprocessError) { @@ -132,12 +141,11 @@ public struct Configuration: Sendable { let outputIO = spawnResults.outputReadEnd() let errorIO = spawnResults.errorReadEnd() - let result: Swift.Result do { // Body runs in the same isolation. let bodyResult = try await body(processIdentifier, inputIO, outputIO, errorIO) taskFinishFlag.addOne() - result = .success(bodyResult) + resultBox = .success(bodyResult) } catch { let execution = Execution( processIdentifier: processIdentifier, @@ -147,12 +155,11 @@ public struct Configuration: Sendable { ) // Attempt to terminate the child process when the body throws await execution.teardown(using: self.platformOptions.teardownSequence) - result = .failure(error) + resultBox = .failure(error) } // Wait for the monitor child task to finish. - let monitorError = try await group.next() ?? nil - return (result, monitorError) + return try await group.next() ?? nil } // Drop the cancellation marker before reaping the zombie. After @@ -170,7 +177,7 @@ public struct Configuration: Sendable { return try ExecutionOutcome( terminationStatus: terminationStatus, - value: result.get() + value: resultBox.take()!.get() ) } onCleanup: { let execution = Execution( @@ -1171,14 +1178,18 @@ extension Optional where Wrapped == String { /// In the latter case, `onCleanup` may be run concurrently with `body`. /// The `body` closure is guaranteed to run exactly once. /// The `onCleanup` closure is guaranteed to run only once, or not at all. -internal func withAsyncTaskCleanupHandler( +internal func withAsyncTaskCleanupHandler( _ body: () async throws -> Result, onCleanup handler: @Sendable @escaping () async -> Void, ) async throws -> Result { let (runCancellationHandlerStream, runCancellationHandlerContinuation) = AsyncThrowingStream.makeStream(of: Void.self) - return try await withThrowingTaskGroup( + // The body's result is moved out through this box rather than returned + // from the task group: a noncopyable `Result` can't be a `GroupResult`, + // which must be `Copyable`. The group itself returns `Void`. + var resultBox: Result? = nil + try await withThrowingTaskGroup( of: Void.self, - returning: Result.self + returning: Void.self ) { group in group.addTask { // Keep this task sleep indefinitely until the parent task is cancelled. @@ -1213,14 +1224,14 @@ internal func withAsyncTaskCleanupHandler( } do { - let result = try await body() + resultBox = try await body() runCancellationHandlerContinuation.finish() - return result } catch { runCancellationHandlerContinuation.finish(throwing: error) throw error } } + return resultBox.take()! } internal struct _OrderedSet: Hashable, Sendable { diff --git a/Sources/Subprocess/Result.swift b/Sources/Subprocess/Result.swift index 77be17a7..98cfdcdc 100644 --- a/Sources/Subprocess/Result.swift +++ b/Sources/Subprocess/Result.swift @@ -22,17 +22,17 @@ import SystemPackage /// /// The `ClosureResult` generic parameter is `Void` when you call a `run(...)` /// overload that doesn't take a `body` closure. It's the closure's return type -/// otherwise. You access the closure's return value with ``closureOutput``. +/// otherwise. You access the closure's return value with ``closureResult``. /// /// The ``standardOutput`` and ``standardError`` properties are available when /// the corresponding output type produces a non-`Void` value. They're /// unavailable for output types such as ``DiscardedOutput``, ``SequenceOutput``, /// and ``FileDescriptorOutput``. public struct ExecutionResult< - ClosureResult: Sendable, + ClosureResult: Sendable & ~Copyable, Output: OutputProtocol, Error: OutputProtocol ->: Sendable { +>: Sendable, ~Copyable { /// The process identifier of the subprocess. public let processIdentifier: ProcessIdentifier /// The termination status of the subprocess. @@ -44,25 +44,34 @@ public struct ExecutionResult< public let standardError: Error.OutputType /// The value returned by the body closure passed to `run`. - public let closureOutput: ClosureResult + public let closureResult: ClosureResult internal init( processIdentifier: ProcessIdentifier, terminationStatus: TerminationStatus, - closureOutput: ClosureResult, + closureResult: consuming ClosureResult, standardOutput: Output.OutputType, standardError: Error.OutputType ) { self.processIdentifier = processIdentifier self.terminationStatus = terminationStatus - self.closureOutput = closureOutput + self.closureResult = closureResult self.standardOutput = standardOutput self.standardError = standardError } } +extension ExecutionResult where ClosureResult: ~Copyable { + /// Consumes this result and returns the value produced by the `run` body closure. + public consuming func takeClosureResult() -> ClosureResult { + return self.closureResult + } +} + // MARK: - ExecutionResult Conformances +extension ExecutionResult: Copyable where ClosureResult: Copyable {} + extension ExecutionResult: Equatable where Output.OutputType: Equatable, Error.OutputType: Equatable, ClosureResult: Equatable {} extension ExecutionResult: Hashable where Output.OutputType: Hashable, Error.OutputType: Hashable, ClosureResult: Hashable {} @@ -74,7 +83,7 @@ extension ExecutionResult: CustomStringConvertible where Output.OutputType: Cust ExecutionResult( processIdentifier: \(self.processIdentifier), terminationStatus: \(self.terminationStatus.description), - closureOutput: \(String(describing: self.closureOutput)), + closureResult: \(String(describing: self.closureResult)), standardOutput: \(self.standardOutput.description) standardError: \(self.standardError.description) ) @@ -90,7 +99,7 @@ where Output.OutputType: CustomDebugStringConvertible, Error.OutputType: CustomD ExecutionResult( processIdentifier: \(self.processIdentifier), terminationStatus: \(self.terminationStatus.debugDescription), - closureOutput: \(String(describing: self.closureOutput)), + closureResult: \(String(describing: self.closureResult)), standardOutput: \(self.standardOutput.debugDescription) standardError: \(self.standardError.debugDescription) ) @@ -102,18 +111,20 @@ where Output.OutputType: CustomDebugStringConvertible, Error.OutputType: CustomD /// The outcome of a subprocess execution, containing the closure's return /// value and the termination status of the child process. -internal struct ExecutionOutcome: Sendable { +internal struct ExecutionOutcome: Sendable, ~Copyable { /// The termination status of the child process. internal let terminationStatus: TerminationStatus /// The value returned by the closure passed to the `run` method. internal let value: Result - internal init(terminationStatus: TerminationStatus, value: Result) { + internal init(terminationStatus: TerminationStatus, value: consuming Result) { self.terminationStatus = terminationStatus self.value = value } } +extension ExecutionOutcome: Copyable where Result: Copyable {} + extension ExecutionOutcome: Equatable where Result: Equatable {} extension ExecutionOutcome: Hashable where Result: Hashable {} diff --git a/Tests/SubprocessTests/IntegrationTests.swift b/Tests/SubprocessTests/IntegrationTests.swift index 0f74d72a..b8aaf358 100644 --- a/Tests/SubprocessTests/IntegrationTests.swift +++ b/Tests/SubprocessTests/IntegrationTests.swift @@ -1018,7 +1018,7 @@ extension SubprocessIntegrationTests { return try await buffer } #expect(result.terminationStatus.isSuccess) - #expect(result.closureOutput == expected) + #expect(result.closureResult == expected) } @Test func testNoInputTriggersEOF() async throws { @@ -1716,7 +1716,7 @@ extension SubprocessIntegrationTests { return try await buffer } #expect(result.terminationStatus.isSuccess) - #expect(result.closureOutput == expected) + #expect(result.closureResult == expected) } @Test func stressTestWithLittleOutput() async throws { @@ -2146,7 +2146,41 @@ extension SubprocessIntegrationTests { return 42 } #expect(result.terminationStatus.isSuccess) - #expect(result.closureOutput == 42) + #expect(result.closureResult == 42) + } + + @Test func testBodyClosureReturnsNoncopyableValue() async throws { + // A move-only value returned from the body closure: this only compiles + // if `run`'s `Result` (and `ExecutionResult.closureResult`) is `~Copyable`. + struct MoveOnlyToken: ~Copyable, Sendable { + let value: Int + } + #if os(Windows) + let setup = TestSetup( + executable: .name("cmd.exe"), + arguments: ["/c", "echo hello"] + ) + #else + let setup = TestSetup( + executable: .path("/bin/echo"), + arguments: ["hello"] + ) + #endif + let result = try await _run( + setup, + input: .none, + output: .discarded, + error: .discarded + ) { _ in + return MoveOnlyToken(value: 42) + } + // `result` is itself noncopyable (its ClosureResult is). Read the + // copyable field first, then move the token out via the consuming + // accessor (a non-`@frozen` type can't be partially consumed across + // module boundaries). + #expect(result.terminationStatus.isSuccess) + let token = result.takeClosureResult() + #expect(token.value == 42) } @Test func testBodyClosureReturnValueWithCapturedOutput() async throws { @@ -2170,7 +2204,7 @@ extension SubprocessIntegrationTests { return "closure-value" } #expect(result.terminationStatus.isSuccess) - #expect(result.closureOutput == "closure-value") + #expect(result.closureResult == "closure-value") let output = try #require(result.standardOutput) #expect(output.contains("hello")) } @@ -2255,7 +2289,7 @@ extension SubprocessIntegrationTests { return collected } #expect(result.terminationStatus.isSuccess) - #expect(result.closureOutput.contains("Hello Stdout")) + #expect(result.closureResult.contains("Hello Stdout")) let stderr = try #require(result.standardError) #expect(stderr.contains("Hello Stderr")) } @@ -2285,7 +2319,7 @@ extension SubprocessIntegrationTests { return collected } #expect(result.terminationStatus.isSuccess) - #expect(result.closureOutput.contains("Hello Stderr")) + #expect(result.closureResult.contains("Hello Stderr")) let stdout = try #require(result.standardOutput) #expect(stdout.contains("Hello Stdout")) } @@ -2382,7 +2416,7 @@ extension SubprocessIntegrationTests { return pidLine.flatMap { pid_t($0) } ?? 0 } - sleepPidToKill = result.closureOutput + sleepPidToKill = result.closureResult #expect(sleepPidToKill != 0) #expect(result.terminationStatus == .exited(0)) } @@ -2430,7 +2464,7 @@ extension SubprocessIntegrationTests { return capturedPid } - sleepPidToKill = result.closureOutput + sleepPidToKill = result.closureResult #expect(sleepPidToKill != 0) #expect(result.terminationStatus == .exited(0)) #expect((result.standardOutput ?? "").contains("GO")) @@ -2473,7 +2507,7 @@ extension SubprocessIntegrationTests { return capturedPid } - sleepPidToKill = result.closureOutput + sleepPidToKill = result.closureResult #expect(sleepPidToKill != 0) #expect(result.terminationStatus == .exited(0)) #expect((result.standardError ?? "").contains("GO")) @@ -2544,7 +2578,7 @@ extension SubprocessIntegrationTests { return pidLine.flatMap { DWORD($0) } ?? 0 } - grandchildPidToKill = result.closureOutput + grandchildPidToKill = result.closureResult #expect(grandchildPidToKill != 0) #expect(result.terminationStatus == .exited(0)) } @@ -2590,7 +2624,7 @@ extension SubprocessIntegrationTests { return DWORD(0) } - grandchildPidToKill = result.closureOutput + grandchildPidToKill = result.closureResult #expect(grandchildPidToKill != 0) #expect(result.terminationStatus == .exited(0)) #expect((result.standardOutput ?? "").contains("GO")) @@ -2631,7 +2665,7 @@ extension SubprocessIntegrationTests { return DWORD(0) } - grandchildPidToKill = result.closureOutput + grandchildPidToKill = result.closureResult #expect(grandchildPidToKill != 0) #expect(result.terminationStatus == .exited(0)) #expect((result.standardError ?? "").contains("GO")) @@ -2996,11 +3030,11 @@ extension SubprocessIntegrationTests { #expect(result.terminationStatus.isSuccess) #if os(Windows) // cmd.exe interactive mode prints more info - #expect(result.closureOutput.output.contains("hello stdout")) + #expect(result.closureResult.output.contains("hello stdout")) #else - #expect(result.closureOutput.output == "hello stdout") + #expect(result.closureResult.output == "hello stdout") #endif - #expect(result.closureOutput.error == "hello stderr") + #expect(result.closureResult.error == "hello stderr") } @Test func testSubprocessPipeChain() async throws { @@ -3566,7 +3600,7 @@ func _run< ) } -func _run( +func _run( _ setup: TestSetup, input: Input, output: Output, diff --git a/Tests/SubprocessTests/UnixTests.swift b/Tests/SubprocessTests/UnixTests.swift index 21713932..73e76d13 100644 --- a/Tests/SubprocessTests/UnixTests.swift +++ b/Tests/SubprocessTests/UnixTests.swift @@ -525,7 +525,7 @@ extension SubprocessUnixTests { // Make sure the grand child `/usr/bin/yes` actually exited // This is unfortunately racy because the pid isn't immediately invalided // once `kill` returns. Allow a few failures and delay to counter this - let grandChildPid = try #require(result.closureOutput) + let grandChildPid = try #require(result.closureResult) for _ in 0..<10 { let rc = kill(grandChildPid, 0) if rc == 0 { @@ -607,7 +607,7 @@ extension SubprocessUnixTests { return grandChildPid } #expect(result.terminationStatus == .signaled(SIGTERM)) - let grandChildPid = try #require(result.closureOutput) + let grandChildPid = try #require(result.closureResult) // Grandchild should have been signalled via the process group. // Allow a few iterations for signal propagation and reaping. for _ in 0..<10 {