Skip to content

Commit 653a2de

Browse files
committed
Use our posix_spawn() in the Foundation CIO rather than Process.
We already have custom `async`-friendly code for spawning child processes, so let's use it instead of relying on `Foundation.Process` when we need to call the platform's `zip` or `tar` tool. Currently we do this in the Foundation cross-import overlay (hence why we were using `Foundation.Process` at all).
1 parent 72afbb4 commit 653a2de

File tree

5 files changed

+145
-75
lines changed

5 files changed

+145
-75
lines changed

Documentation/Porting.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ platform-specific attention.
6767
> conflicting requirements (for example, attempting to enable support for pipes
6868
> without also enabling support for file I/O.) You should be able to resolve
6969
> these issues by updating `Package.swift` and/or `CompilerSettings.cmake`.
70+
>
71+
> Don't forget to add your platform to the `BuildSettingCondition/whenApple(_:)`
72+
> function in `Package.swift`.
7073
7174
Most platform dependencies can be resolved through the use of platform-specific
7275
API. For example, Swift Testing uses the C11 standard [`timespec`](https://en.cppreference.com/w/c/chrono/timespec)

Package.swift

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ let buildingForDevelopment = (git?.currentTag == nil)
2727
/// to change in the future.
2828
///
2929
/// - Bug: There is currently no way for us to tell if we are being asked to
30-
/// build for an Embedded Swift target at the package manifest level.
30+
/// build for an Embedded Swift target at the package manifest level.
3131
/// ([swift-syntax-#8431](https://github.com/swiftlang/swift-package-manager/issues/8431))
3232
let buildingForEmbedded: Bool = {
3333
guard let envvar = Context.environment["SWT_EMBEDDED"] else {
@@ -193,7 +193,7 @@ let package = Package(
193193
// The Foundation module only has Library Evolution enabled on Apple
194194
// platforms, and since this target's module publicly imports Foundation,
195195
// it can only enable Library Evolution itself on those platforms.
196-
swiftSettings: .packageSettings + .enableLibraryEvolution(applePlatformsOnly: true)
196+
swiftSettings: .packageSettings + .enableLibraryEvolution(.whenApple())
197197
),
198198

199199
// Utility targets: These are utilities intended for use when developing
@@ -229,11 +229,11 @@ extension BuildSettingCondition {
229229
/// Swift.
230230
///
231231
/// - Parameters:
232-
/// - nonEmbeddedCondition: The value to return if the target is not
233-
/// Embedded Swift. If `nil`, the build condition evaluates to `false`.
232+
/// - nonEmbeddedCondition: The value to return if the target is not
233+
/// Embedded Swift. If `nil`, the build condition evaluates to `false`.
234234
///
235235
/// - Returns: A build setting condition that evaluates to `true` for Embedded
236-
/// Swift or is equal to `nonEmbeddedCondition` for non-Embedded Swift.
236+
/// Swift or is equal to `nonEmbeddedCondition` for non-Embedded Swift.
237237
static func whenEmbedded(or nonEmbeddedCondition: @autoclosure () -> Self? = nil) -> Self? {
238238
if !buildingForEmbedded {
239239
if let nonEmbeddedCondition = nonEmbeddedCondition() {
@@ -248,6 +248,21 @@ extension BuildSettingCondition {
248248
nil
249249
}
250250
}
251+
252+
/// A build setting condition representing all Apple or non-Apple platforms.
253+
///
254+
/// - Parameters:
255+
/// - isApple: Whether or not the result represents Apple platforms.
256+
///
257+
/// - Returns: A build setting condition that evaluates to `isApple` for Apple
258+
/// platforms.
259+
static func whenApple(_ isApple: Bool = true) -> Self {
260+
if isApple {
261+
.when(platforms: [.macOS, .iOS, .macCatalyst, .watchOS, .tvOS, .visionOS])
262+
} else {
263+
.when(platforms: [.linux, .custom("freebsd"), .openbsd, .windows, .wasi, .android])
264+
}
265+
}
251266
}
252267

253268
extension Array where Element == PackageDescription.SwiftSetting {
@@ -292,13 +307,14 @@ extension Array where Element == PackageDescription.SwiftSetting {
292307
// executable rather than a library.
293308
.define("SWT_NO_LIBRARY_MACRO_PLUGINS"),
294309

295-
.define("SWT_TARGET_OS_APPLE", .when(platforms: [.macOS, .iOS, .macCatalyst, .watchOS, .tvOS, .visionOS])),
310+
.define("SWT_TARGET_OS_APPLE", .whenApple()),
296311

297312
.define("SWT_NO_EXIT_TESTS", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
298313
.define("SWT_NO_PROCESS_SPAWNING", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
299-
.define("SWT_NO_SNAPSHOT_TYPES", .whenEmbedded(or: .when(platforms: [.linux, .custom("freebsd"), .openbsd, .windows, .wasi, .android]))),
314+
.define("SWT_NO_SNAPSHOT_TYPES", .whenEmbedded(or: .whenApple(false))),
300315
.define("SWT_NO_DYNAMIC_LINKING", .whenEmbedded(or: .when(platforms: [.wasi]))),
301316
.define("SWT_NO_PIPES", .whenEmbedded(or: .when(platforms: [.wasi]))),
317+
.define("SWT_NO_FOUNDATION_FILE_COORDINATION", .whenEmbedded(or: .whenApple(false))),
302318

303319
.define("SWT_NO_LEGACY_TEST_DISCOVERY", .whenEmbedded()),
304320
.define("SWT_NO_LIBDISPATCH", .whenEmbedded()),
@@ -334,20 +350,16 @@ extension Array where Element == PackageDescription.SwiftSetting {
334350
]
335351
}
336352

337-
/// Create a Swift setting which enables Library Evolution, optionally
338-
/// constraining it to only Apple platforms.
353+
/// Create a Swift setting which enables Library Evolution.
339354
///
340355
/// - Parameters:
341-
/// - applePlatformsOnly: Whether to constrain this setting to only Apple
342-
/// platforms.
343-
static func enableLibraryEvolution(applePlatformsOnly: Bool = false) -> Self {
356+
/// - condition: A build setting condition to apply to this setting.
357+
///
358+
/// - Returns: A Swift setting that enables Library Evolution.
359+
static func enableLibraryEvolution(_ condition: BuildSettingCondition? = nil) -> Self {
344360
var result = [PackageDescription.SwiftSetting]()
345361

346362
if buildingForDevelopment {
347-
var condition: BuildSettingCondition?
348-
if applePlatformsOnly {
349-
condition = .when(platforms: [.macOS, .iOS, .macCatalyst, .watchOS, .tvOS, .visionOS])
350-
}
351363
result.append(.unsafeFlags(["-enable-library-evolution"], condition))
352364
}
353365

@@ -364,9 +376,10 @@ extension Array where Element == PackageDescription.CXXSetting {
364376
result += [
365377
.define("SWT_NO_EXIT_TESTS", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
366378
.define("SWT_NO_PROCESS_SPAWNING", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
367-
.define("SWT_NO_SNAPSHOT_TYPES", .whenEmbedded(or: .when(platforms: [.linux, .custom("freebsd"), .openbsd, .windows, .wasi, .android]))),
379+
.define("SWT_NO_SNAPSHOT_TYPES", .whenEmbedded(or: .whenApple(false))),
368380
.define("SWT_NO_DYNAMIC_LINKING", .whenEmbedded(or: .when(platforms: [.wasi]))),
369381
.define("SWT_NO_PIPES", .whenEmbedded(or: .when(platforms: [.wasi]))),
382+
.define("SWT_NO_FOUNDATION_FILE_COORDINATION", .whenEmbedded(or: .whenApple(false))),
370383

371384
.define("SWT_NO_LEGACY_TEST_DISCOVERY", .whenEmbedded()),
372385
.define("SWT_NO_LIBDISPATCH", .whenEmbedded()),

Sources/Overlays/_Testing_Foundation/Attachments/Attachment+URL.swift

Lines changed: 27 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ extension Attachment where AttachableValue == _AttachableURLWrapper {
7070
let url = url.resolvingSymlinksInPath()
7171
let isDirectory = try url.resourceValues(forKeys: [.isDirectoryKey]).isDirectory!
7272

73-
#if SWT_TARGET_OS_APPLE
73+
#if SWT_TARGET_OS_APPLE && !SWT_NO_FOUNDATION_FILE_COORDINATION
7474
let data: Data = try await withCheckedThrowingContinuation { continuation in
7575
let fileCoordinator = NSFileCoordinator()
7676
let fileAccessIntent = NSFileAccessIntent.readingIntent(with: url, options: [.forUploading])
@@ -196,52 +196,37 @@ private func _compressContentsOfDirectory(at directoryURL: URL) async throws ->
196196
throw CocoaError(.featureUnsupported, userInfo: [NSLocalizedDescriptionKey: "This platform does not support attaching directories to tests."])
197197
#endif
198198

199-
try await withCheckedThrowingContinuation { continuation in
200-
let process = Process()
201-
202-
process.executableURL = URL(fileURLWithPath: archiverPath, isDirectory: false)
203-
204-
let sourcePath = directoryURL.fileSystemPath
205-
let destinationPath = temporaryURL.fileSystemPath
199+
let sourcePath = directoryURL.fileSystemPath
200+
let destinationPath = temporaryURL.fileSystemPath
201+
var currentDirectoryPath: String?
206202
#if os(Linux) || os(OpenBSD)
207-
// The zip command constructs relative paths from the current working
208-
// directory rather than from command-line arguments.
209-
process.arguments = [destinationPath, "--recurse-paths", "."]
210-
process.currentDirectoryURL = directoryURL
203+
// The zip command constructs relative paths from the current working
204+
// directory rather than from command-line arguments.
205+
let arguments = [destinationPath, "--recurse-paths", "."]
206+
currentDirectoryPath = sourcePath
211207
#elseif SWT_TARGET_OS_APPLE || os(FreeBSD)
212-
process.arguments = ["--create", "--auto-compress", "--directory", sourcePath, "--file", destinationPath, "."]
208+
let arguments = ["--create", "--auto-compress", "--directory", sourcePath, "--file", destinationPath, "."]
213209
#elseif os(Windows)
214-
// The Windows version of bsdtar can handle relative paths for other archive
215-
// formats, but produces empty archives when inferring the zip format with
216-
// --auto-compress, so archive with absolute paths here.
217-
//
218-
// An alternative may be to use PowerShell's Compress-Archive command,
219-
// however that comes with a security risk as we'd be responsible for two
220-
// levels of command-line argument escaping.
221-
process.arguments = ["--create", "--auto-compress", "--file", destinationPath, sourcePath]
210+
// The Windows version of bsdtar can handle relative paths for other archive
211+
// formats, but produces empty archives when inferring the zip format with
212+
// --auto-compress, so archive with absolute paths here.
213+
//
214+
// An alternative may be to use PowerShell's Compress-Archive command,
215+
// however that comes with a security risk as we'd be responsible for two
216+
// levels of command-line argument escaping.
217+
let arguments = ["--create", "--auto-compress", "--file", destinationPath, sourcePath]
222218
#endif
223219

224-
process.standardOutput = nil
225-
process.standardError = nil
226-
227-
process.terminationHandler = { process in
228-
let terminationReason = process.terminationReason
229-
let terminationStatus = process.terminationStatus
230-
if terminationReason == .exit && terminationStatus == EXIT_SUCCESS {
231-
continuation.resume()
232-
} else {
233-
let error = CocoaError(.fileWriteUnknown, userInfo: [
234-
NSLocalizedDescriptionKey: "The directory at '\(sourcePath)' could not be compressed (\(terminationStatus)).",
235-
])
236-
continuation.resume(throwing: error)
237-
}
238-
}
239-
240-
do {
241-
try process.run()
242-
} catch {
243-
continuation.resume(throwing: error)
244-
}
220+
let exitStatus = try await spawnExecutableAtPathAndWait(
221+
atPath: archiverPath,
222+
arguments: arguments,
223+
environment: [:],
224+
currentDirectoryPath: currentDirectoryPath
225+
)
226+
guard case .exitCode(EXIT_SUCCESS) = exitStatus else {
227+
throw CocoaError(.fileWriteUnknown, userInfo: [
228+
NSLocalizedDescriptionKey: "The directory at '\(sourcePath)' could not be compressed (\(exitStatus)).",
229+
])
245230
}
246231

247232
return try Data(contentsOf: temporaryURL, options: [.mappedIfSafe])

Sources/Testing/ExitTests/SpawnProcess.swift

Lines changed: 84 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,16 @@ typealias ProcessID = HANDLE
2626
typealias ProcessID = Never
2727
#endif
2828

29+
/// A platform-specific wrapper type for various types used by `posix_spawn()`.
30+
///
31+
/// Darwin and Linux differ in their optionality for the `posix_spawn()` types
32+
/// we use, so we use this typealias to paper over the differences.
33+
#if SWT_TARGET_OS_APPLE || os(FreeBSD) || os(OpenBSD)
34+
fileprivate typealias P<T> = T?
35+
#elseif os(Linux)
36+
fileprivate typealias P<T> = T
37+
#endif
38+
2939
#if os(Linux) && !SWT_NO_DYNAMIC_LINKING
3040
/// Close file descriptors above a given value when spawing a new process.
3141
///
@@ -38,13 +48,29 @@ private let _posix_spawn_file_actions_addclosefrom_np = symbol(named: "posix_spa
3848
}
3949
#endif
4050

41-
/// Spawn a process and wait for it to terminate.
51+
#if !SWT_NO_DYNAMIC_LINKING
52+
/// Change the current directory in the new process.
53+
///
54+
/// This symbol is provided because `posix_spawn_file_actions_addchdir()` and
55+
/// its non-portable equivalent `posix_spawn_file_actions_addchdir_np()` are not
56+
/// consistently available across all platforms that implement the
57+
/// `posix_spawn()` API.
58+
private let _posix_spawn_file_actions_addchdir = symbol(named: "posix_spawn_file_actions_addchdir").map {
59+
castCFunction(at: $0, to: (@convention(c) (UnsafeMutablePointer<P<posix_spawn_file_actions_t>>, UnsafePointer<CChar>) -> CInt).self)
60+
} ?? symbol(named: "posix_spawn_file_actions_addchdir_np").map {
61+
castCFunction(at: $0, to: (@convention(c) (UnsafeMutablePointer<P<posix_spawn_file_actions_t>>, UnsafePointer<CChar>) -> CInt).self)
62+
}
63+
#endif
64+
65+
/// Spawn a child process.
4266
///
4367
/// - Parameters:
4468
/// - executablePath: The path to the executable to spawn.
4569
/// - arguments: The arguments to pass to the executable, not including the
4670
/// executable path.
4771
/// - environment: The environment block to pass to the executable.
72+
/// - currentDirectoryPath: The path to use as the executable's initial
73+
/// working directory.
4874
/// - standardInput: If not `nil`, a file handle the child process should
4975
/// inherit as its standard input stream. This file handle must be backed
5076
/// by a file descriptor and be open for reading.
@@ -61,39 +87,33 @@ private let _posix_spawn_file_actions_addclosefrom_np = symbol(named: "posix_spa
6187
/// eventually pass this value to ``wait(for:)`` to avoid leaking system
6288
/// resources.
6389
///
64-
/// - Throws: Any error that prevented the process from spawning or its exit
65-
/// condition from being read.
90+
/// - Throws: Any error that prevented the process from spawning.
6691
func spawnExecutable(
6792
atPath executablePath: String,
6893
arguments: [String],
6994
environment: [String: String],
95+
currentDirectoryPath: String? = nil,
7096
standardInput: borrowing FileHandle? = nil,
7197
standardOutput: borrowing FileHandle? = nil,
7298
standardError: borrowing FileHandle? = nil,
7399
additionalFileHandles: [UnsafePointer<FileHandle>] = []
74100
) throws -> ProcessID {
75-
// Darwin and Linux differ in their optionality for the posix_spawn types we
76-
// use, so use this typealias to paper over the differences.
77-
#if SWT_TARGET_OS_APPLE || os(FreeBSD) || os(OpenBSD)
78-
typealias P<T> = T?
79-
#elseif os(Linux)
80-
typealias P<T> = T
81-
#endif
82-
83101
#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD)
84102
return try withUnsafeTemporaryAllocation(of: P<posix_spawn_file_actions_t>.self, capacity: 1) { fileActions in
85103
let fileActions = fileActions.baseAddress!
86-
guard 0 == posix_spawn_file_actions_init(fileActions) else {
87-
throw CError(rawValue: swt_errno())
104+
let fileActionsInitialized = posix_spawn_file_actions_init(fileActions)
105+
guard 0 == fileActionsInitialized else {
106+
throw CError(rawValue: fileActionsInitialized)
88107
}
89108
defer {
90109
_ = posix_spawn_file_actions_destroy(fileActions)
91110
}
92111

93112
return try withUnsafeTemporaryAllocation(of: P<posix_spawnattr_t>.self, capacity: 1) { attrs in
94113
let attrs = attrs.baseAddress!
95-
guard 0 == posix_spawnattr_init(attrs) else {
96-
throw CError(rawValue: swt_errno())
114+
let attrsInitialized = posix_spawnattr_init(attrs)
115+
guard 0 == attrsInitialized else {
116+
throw CError(rawValue: attrsInitialized)
97117
}
98118
defer {
99119
_ = posix_spawnattr_destroy(attrs)
@@ -116,6 +136,25 @@ func spawnExecutable(
116136
flags |= CShort(POSIX_SPAWN_SETSIGDEF)
117137
}
118138

139+
// Set the current working directory (do this as early as possible, so
140+
// before inheriting or opening any file descriptors.)
141+
if let currentDirectoryPath {
142+
if let _posix_spawn_file_actions_addchdir {
143+
let directoryChanged = _posix_spawn_file_actions_addchdir(fileActions, currentDirectoryPath)
144+
guard 0 == directoryChanged else {
145+
throw CError(rawValue: directoryChanged)
146+
}
147+
} else {
148+
// This platform does not support setting the current directory via
149+
// posix_spawn(), so set it here in the parent process and hope
150+
// another thread does not stomp on the change (as Foundation does.)
151+
// Platforms known to take this path: Amazon Linux 2, OpenBSD, QNX
152+
guard 0 == chdir(currentDirectoryPath) else {
153+
throw CError(rawValue: swt_errno())
154+
}
155+
}
156+
}
157+
119158
// Forward standard I/O streams and any explicitly added file handles.
120159
var highestFD = max(STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO)
121160
func inherit(_ fileHandle: borrowing FileHandle, as standardFD: CInt? = nil) throws {
@@ -262,6 +301,7 @@ func spawnExecutable(
262301

263302
let commandLine = _escapeCommandLine(CollectionOfOne(executablePath) + arguments)
264303
let environ = environment.map { "\($0.key)=\($0.value)" }.joined(separator: "\0") + "\0\0"
304+
let currentDirectoryPath = currentDirectoryPath.map { Array($0.utf16) }
265305

266306
var flags = DWORD(CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT)
267307
#if DEBUG
@@ -281,7 +321,7 @@ func spawnExecutable(
281321
true, // bInheritHandles
282322
flags,
283323
.init(mutating: environ),
284-
nil,
324+
currentDirectoryPath,
285325
startupInfo.pointer(to: \.StartupInfo)!,
286326
&processInfo
287327
) else {
@@ -396,4 +436,32 @@ private func _escapeCommandLine(_ arguments: [String]) -> String {
396436
}.joined(separator: " ")
397437
}
398438
#endif
439+
440+
/// Spawn a child process and wait for it to terminate.
441+
///
442+
/// - Parameters:
443+
/// - executablePath: The path to the executable to spawn.
444+
/// - arguments: The arguments to pass to the executable, not including the
445+
/// executable path.
446+
/// - environment: The environment block to pass to the executable.
447+
/// - currentDirectoryPath: The path to use as the executable's initial
448+
/// working directory.
449+
///
450+
/// - Returns: The exit status of the spawned process.
451+
///
452+
/// - Throws: Any error that prevented the process from spawning or its exit
453+
/// condition from being read.
454+
///
455+
/// This function is a convenience that spawns the given process and waits for
456+
/// it to terminate. It is primarily for use by other targets in this package
457+
/// such as its cross-import overlays.
458+
package func spawnExecutableAtPathAndWait(
459+
atPath executablePath: String,
460+
arguments: [String] = [],
461+
environment: [String: String] = [:],
462+
currentDirectoryPath: String? = nil
463+
) async throws -> ExitStatus {
464+
let processID = try spawnExecutable(atPath: executablePath, arguments: arguments, environment: environment)
465+
return try await wait(for: processID)
466+
}
399467
#endif

cmake/modules/shared/CompilerSettings.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ if(CMAKE_SYSTEM_NAME IN_LIST SWT_NO_PROCESS_SPAWNING_LIST)
3333
endif()
3434
if(NOT APPLE)
3535
add_compile_definitions("SWT_NO_SNAPSHOT_TYPES")
36+
add_compile_definitions("SWT_NO_FOUNDATION_FILE_COORDINATION")
3637
endif()
3738
if(CMAKE_SYSTEM_NAME STREQUAL "WASI")
3839
add_compile_definitions("SWT_NO_DYNAMIC_LINKING")

0 commit comments

Comments
 (0)