Conditionally adopt swift-subprocess by jakepetroules · Pull Request #638 · swiftlang/swift-build

jakepetroules · 2025-07-03T01:44:17Z

This fully resolves working directory thread safety issues with subprocess spawning across all platforms. For now, subprocess is adopted conditionally in order to continue building in certain environments where the Subprocess module may not be available, in which case we fall back to Foundation Process.

Closes #441

(migrated from #584 in accordance with our new policy to require PRs from forks)

jakepetroules · 2025-07-03T01:44:39Z

@swift-ci test

jakepetroules · 2025-07-05T00:01:08Z

@swift-ci test

jakepetroules · 2025-07-05T00:11:54Z

+                    platformOptions.teardownSequence = [.gracefulShutDown(allowedDurationToNextStep: .seconds(5))]
+                    #if os(macOS)
+                    if highPriority {
+                        platformOptions.qualityOfService = .userInitiated


@compnerd Would it seem reasonable to call SetPriorityClass with HIGH_PRIORITY_CLASS for Windows (REALTIME_PRIORITY_CLASS definitely sounded excessive)? Currently this is used to elevate the build service process so that it can serve high-priority requests more efficiently (e.g. with Swift Concurrency task priority set higher than normal).

Hmm, we should try it - it can saturate the system to the point of being unusable though, so I am a little concerned about that.

jakepetroules · 2025-08-25T23:11:29Z

@swift-ci test

weissi · 2025-09-01T20:15:03Z

+            platformOptions.teardownSequence = [.gracefulShutDown(allowedDurationToNextStep: .seconds(5))]
+        }
+        let result = try await Subprocess.run(.path(FilePath(url.filePath.str)), arguments: .init(arguments), environment: environment.map { .custom(.init($0)) } ?? .inherit, workingDirectory: (currentDirectoryURL?.filePath.str).map { FilePath($0) } ?? nil, platformOptions: platformOptions, body: { execution, inputWriter, outputReader, errorReader in
+            try await inputWriter.finish()


potential deadlock here. You can't assume that you can fully unload the stdin w/o reading stdout/err

There's no deadlock here, all this is doing is closing a file descriptor. We have no input. That said, I'll move it below the output reading just to be correct.

weissi · 2025-09-01T20:15:34Z

+        }
+        let result = try await Subprocess.run(.path(FilePath(url.filePath.str)), arguments: .init(arguments), environment: environment.map { .custom(.init($0)) } ?? .inherit, workingDirectory: (currentDirectoryURL?.filePath.str).map { FilePath($0) } ?? nil, platformOptions: platformOptions, body: { execution, inputWriter, outputReader, errorReader in
+            try await inputWriter.finish()
+            async let stdoutBytes = outputReader.collect().flatMap { $0.withUnsafeBytes(Array.init) }


two *Unsafe*s here

so? we can't use Span yet, this still has to build with 6.1

weissi · 2025-09-01T20:16:37Z

+        let result = try await Subprocess.run(.path(FilePath(url.filePath.str)), arguments: .init(arguments), environment: environment.map { .custom(.init($0)) } ?? .inherit, workingDirectory: (currentDirectoryURL?.filePath.str).map { FilePath($0) } ?? nil, platformOptions: platformOptions, body: { execution, inputWriter, outputReader, errorReader in
+            try await inputWriter.finish()
+            async let stdoutBytes = outputReader.collect().flatMap { $0.withUnsafeBytes(Array.init) }
+            async let stderrBytes = errorReader.collect().flatMap { $0.withUnsafeBytes(Array.init) }


collect will load all of this potentially infinite stream into memory and I don't even see a limit, seems dangerous. If anything, limit it to something high like 32 MB and throw if we're blowing through that.

This is existing behavior; we can address it separately.

jakepetroules · 2025-10-18T07:50:04Z

@swift-ci test

jakepetroules · 2026-02-11T21:00:43Z

@swift-ci test

jakepetroules · 2026-03-14T08:28:38Z

Moving to draft -- SwiftSubprocess is on its way to 1.0 with some API changes, so we might as well hold off until then.

jakepetroules · 2026-04-03T08:53:17Z

@swift-ci test

jakepetroules · 2026-04-03T18:27:45Z

@swift-ci test

jakepetroules · 2026-10-05T22:41:39Z

Please test with following PRs:
swiftlang/swift-subprocess#376

@swift-ci please build toolchain Linux platform

jakepetroules · 2026-10-05T22:41:41Z

Please test with following PRs:
swiftlang/swift-subprocess#376

@swift-ci please build toolchain macOS platform

jakepetroules · 2026-10-05T22:41:42Z

Please test with following PRs:
swiftlang/swift-subprocess#376

@swift-ci please build toolchain Windows platform

jakepetroules · 2026-10-08T22:38:54Z

Now waiting on a new tag of swift-subprocess with the recent CMake changes, and for swiftlang/swift to adopt that in update-checkout.

This fully resolves working directory thread safety issues with subprocess spawning across all platforms. For now, subprocess is adopted conditionally in order to continue building in certain environments where the Subprocess module may not be available, in which case we fall back to Foundation Process. Closes swiftlang#441

@test

- Require swift-subprocess 1.0.1 and adapt to its API (Configuration(executable:), the Execution-based run overload, closureResult). - Use .combinedWithOutput for merged output instead of a manually shared pipe. - Collect output without relying on the SubprocessFoundation trait, which isn't enabled in CMake builds of swift-subprocess. - Map swift-subprocess launch failures to RunProcessLaunchError, matching the Foundation.Process code path. - Restore the @test attribute on ProcessTests.workingDirectory. - Build swift-system before swift-subprocess in the CMake smoke test so that swift-subprocess doesn't fall back to fetching swift-system from the network.

Optionally find SwiftSubprocess and link it into SWBUtil, so CMake builds (including the toolchain) use swift-subprocess rather than Foundation.Process. SwiftBuildConfig.cmake loads SwiftSubprocess for clients when it was used, so that the exported SWBUtil target's link interface resolves.

…ake smoke test

compnerd · 2026-10-15T01:00:44Z

+if("@SwiftSubprocess_FOUND@" AND NOT TARGET SwiftSubprocess::Subprocess)
+  find_package(SwiftSubprocess CONFIG REQUIRED
+    NO_DEFAULT_PATH PATHS "@SwiftSubprocess_DIR@")
+endif()


Can we avoid this and use CMakeFindDependencyMacro instead? That is meant to propagate the dependency across the boundary to the consumers without them having to do find_package and pass along the variables. It isn't perfect because it might lead to under-specifying the dependencies, but I think that it is still a good option.

jakepetroules requested review from aciidgh, mhrawdon, mirza-garibovic, neonichu and owenv as code owners July 3, 2025 01:44

jakepetroules force-pushed the eng/PR-swift-subprocess branch from 7e290ca to 5d1747c Compare July 5, 2025 00:01

jakepetroules requested a review from compnerd July 5, 2025 00:10

jakepetroules commented Jul 5, 2025

View reviewed changes

jakepetroules mentioned this pull request Jul 24, 2025

Introduce AsyncIO for Windows and Linux swiftlang/swift-subprocess#117

Merged

jakepetroules force-pushed the eng/PR-swift-subprocess branch from 5d1747c to 1d4c11c Compare August 25, 2025 23:10

neonichu approved these changes Aug 28, 2025

View reviewed changes

jakepetroules force-pushed the eng/PR-swift-subprocess branch 2 times, most recently from 922c246 to bd4c32d Compare September 1, 2025 08:13

weissi reviewed Sep 1, 2025

View reviewed changes

jakepetroules mentioned this pull request Sep 2, 2025

Introduce FileDescriptorInput.standardInput, FileDescriptorOutput.standardOutput, and FileDescriptorOutput.standardError to easily bind current process' descriptors to child processes swiftlang/swift-subprocess#167

Merged

jakepetroules force-pushed the eng/PR-swift-subprocess branch 3 times, most recently from 89d45c3 to f84bfcf Compare September 4, 2025 00:26

jakepetroules force-pushed the eng/PR-swift-subprocess branch 2 times, most recently from 5cd9b6f to ec02157 Compare September 15, 2025 18:22

jakepetroules force-pushed the eng/PR-swift-subprocess branch 3 times, most recently from e6eed8f to 553f25f Compare October 18, 2025 07:39

jakepetroules mentioned this pull request Oct 21, 2025

Retaining the StandardInputWriter prevents write from happening / being flushed swiftlang/swift-subprocess#186

Closed

jakepetroules marked this pull request as draft March 14, 2026 08:28

jakepetroules mentioned this pull request Mar 25, 2026

Build on OpenBSD #114

Open

jakepetroules force-pushed the eng/PR-swift-subprocess branch from ceaf055 to bed300e Compare April 3, 2026 08:53

jakepetroules force-pushed the eng/PR-swift-subprocess branch from bed300e to 4f3de6e Compare April 3, 2026 18:25

jakepetroules mentioned this pull request Apr 7, 2026

Correctly detect llbuild-cancelled tasks on Windows via STATUS_CONTROL_C_EXIT #1274

Merged

jakepetroules force-pushed the eng/PR-swift-subprocess branch from 4f3de6e to de0160b Compare October 3, 2026 01:01

jakepetroules mentioned this pull request Oct 3, 2026

Make the CMake build suitable for toolchain builds swiftlang/swift-subprocess#376

Merged

jakepetroules force-pushed the eng/PR-swift-subprocess branch from 0922413 to bf161b7 Compare October 5, 2026 22:40

jakepetroules force-pushed the eng/PR-swift-subprocess branch from bf161b7 to 9be9ce5 Compare October 6, 2026 20:48

jakepetroules mentioned this pull request Oct 6, 2026

Emit the swiftmodule at the nested path via CMake's own module properties swiftlang/swift-subprocess#379

Merged

jakepetroules force-pushed the eng/PR-swift-subprocess branch from 9be9ce5 to 814774e Compare October 6, 2026 22:20

jakepetroules mentioned this pull request Oct 6, 2026

[DNM] Toolchain build for swift-build testing swiftlang/swift#93014

Draft

jakepetroules force-pushed the eng/PR-swift-subprocess branch 2 times, most recently from 666f639 to b7a5d07 Compare October 7, 2026 06:30

This was referenced Oct 7, 2026

Retain sub-second modification time precision on non-Darwin platforms swiftlang/swift-driver#2228

Merged

Tolerate spurious emit-module in singleFileCompile on all non-Darwin hosts #1849

Merged

jakepetroules added 4 commits October 14, 2026 17:02

Disable dependency vendoring when building swift-subprocess in the CM…

a92d36c

…ake smoke test

jakepetroules force-pushed the eng/PR-swift-subprocess branch from b7a5d07 to a92d36c Compare October 15, 2026 00:05

compnerd reviewed Oct 15, 2026

View reviewed changes

Conversation

jakepetroules commented Jul 3, 2025

Uh oh!

jakepetroules commented Jul 3, 2025

Uh oh!

jakepetroules commented Jul 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakepetroules commented Aug 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakepetroules commented Oct 18, 2025

Uh oh!

jakepetroules commented Feb 11, 2026

Uh oh!

jakepetroules commented Mar 14, 2026

Uh oh!

jakepetroules commented Apr 3, 2026

Uh oh!

jakepetroules commented Apr 3, 2026

Uh oh!

jakepetroules commented Oct 5, 2026

Uh oh!

jakepetroules commented Oct 5, 2026

Uh oh!

jakepetroules commented Oct 5, 2026

Uh oh!

jakepetroules commented Oct 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants