Skip to content

Commit e7851c5

Browse files
authored
Use our posix_spawn() wrapper in the Foundation CIO rather than Process. (#1114)
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). ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent eac4833 commit e7851c5

File tree

5 files changed

+95
-66
lines changed

5 files changed

+95
-66
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 {
@@ -208,7 +208,7 @@ let package = Package(
208208
// The Foundation module only has Library Evolution enabled on Apple
209209
// platforms, and since this target's module publicly imports Foundation,
210210
// it can only enable Library Evolution itself on those platforms.
211-
swiftSettings: .packageSettings + .enableLibraryEvolution(applePlatformsOnly: true)
211+
swiftSettings: .packageSettings + .enableLibraryEvolution(.whenApple())
212212
),
213213

214214
// Utility targets: These are utilities intended for use when developing
@@ -244,11 +244,11 @@ extension BuildSettingCondition {
244244
/// Swift.
245245
///
246246
/// - Parameters:
247-
/// - nonEmbeddedCondition: The value to return if the target is not
248-
/// Embedded Swift. If `nil`, the build condition evaluates to `false`.
247+
/// - nonEmbeddedCondition: The value to return if the target is not
248+
/// Embedded Swift. If `nil`, the build condition evaluates to `false`.
249249
///
250250
/// - Returns: A build setting condition that evaluates to `true` for Embedded
251-
/// Swift or is equal to `nonEmbeddedCondition` for non-Embedded Swift.
251+
/// Swift or is equal to `nonEmbeddedCondition` for non-Embedded Swift.
252252
static func whenEmbedded(or nonEmbeddedCondition: @autoclosure () -> Self? = nil) -> Self? {
253253
if !buildingForEmbedded {
254254
if let nonEmbeddedCondition = nonEmbeddedCondition() {
@@ -263,6 +263,21 @@ extension BuildSettingCondition {
263263
nil
264264
}
265265
}
266+
267+
/// A build setting condition representing all Apple or non-Apple platforms.
268+
///
269+
/// - Parameters:
270+
/// - isApple: Whether or not the result represents Apple platforms.
271+
///
272+
/// - Returns: A build setting condition that evaluates to `isApple` for Apple
273+
/// platforms.
274+
static func whenApple(_ isApple: Bool = true) -> Self {
275+
if isApple {
276+
.when(platforms: [.macOS, .iOS, .macCatalyst, .watchOS, .tvOS, .visionOS])
277+
} else {
278+
.when(platforms: [.linux, .custom("freebsd"), .openbsd, .windows, .wasi, .android])
279+
}
280+
}
266281
}
267282

268283
extension Array where Element == PackageDescription.SwiftSetting {
@@ -312,13 +327,14 @@ extension Array where Element == PackageDescription.SwiftSetting {
312327
// executable rather than a library.
313328
.define("SWT_NO_LIBRARY_MACRO_PLUGINS"),
314329

315-
.define("SWT_TARGET_OS_APPLE", .when(platforms: [.macOS, .iOS, .macCatalyst, .watchOS, .tvOS, .visionOS])),
330+
.define("SWT_TARGET_OS_APPLE", .whenApple()),
316331

317332
.define("SWT_NO_EXIT_TESTS", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
318333
.define("SWT_NO_PROCESS_SPAWNING", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
319-
.define("SWT_NO_SNAPSHOT_TYPES", .whenEmbedded(or: .when(platforms: [.linux, .custom("freebsd"), .openbsd, .windows, .wasi, .android]))),
334+
.define("SWT_NO_SNAPSHOT_TYPES", .whenEmbedded(or: .whenApple(false))),
320335
.define("SWT_NO_DYNAMIC_LINKING", .whenEmbedded(or: .when(platforms: [.wasi]))),
321336
.define("SWT_NO_PIPES", .whenEmbedded(or: .when(platforms: [.wasi]))),
337+
.define("SWT_NO_FOUNDATION_FILE_COORDINATION", .whenEmbedded(or: .whenApple(false))),
322338

323339
.define("SWT_NO_LEGACY_TEST_DISCOVERY", .whenEmbedded()),
324340
.define("SWT_NO_LIBDISPATCH", .whenEmbedded()),
@@ -354,20 +370,16 @@ extension Array where Element == PackageDescription.SwiftSetting {
354370
]
355371
}
356372

357-
/// Create a Swift setting which enables Library Evolution, optionally
358-
/// constraining it to only Apple platforms.
373+
/// Create a Swift setting which enables Library Evolution.
359374
///
360375
/// - Parameters:
361-
/// - applePlatformsOnly: Whether to constrain this setting to only Apple
362-
/// platforms.
363-
static func enableLibraryEvolution(applePlatformsOnly: Bool = false) -> Self {
376+
/// - condition: A build setting condition to apply to this setting.
377+
///
378+
/// - Returns: A Swift setting that enables Library Evolution.
379+
static func enableLibraryEvolution(_ condition: BuildSettingCondition? = nil) -> Self {
364380
var result = [PackageDescription.SwiftSetting]()
365381

366382
if buildingForDevelopment {
367-
var condition: BuildSettingCondition?
368-
if applePlatformsOnly {
369-
condition = .when(platforms: [.macOS, .iOS, .macCatalyst, .watchOS, .tvOS, .visionOS])
370-
}
371383
result.append(.unsafeFlags(["-enable-library-evolution"], condition))
372384
}
373385

@@ -410,9 +422,10 @@ extension Array where Element == PackageDescription.CXXSetting {
410422
result += [
411423
.define("SWT_NO_EXIT_TESTS", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
412424
.define("SWT_NO_PROCESS_SPAWNING", .whenEmbedded(or: .when(platforms: [.iOS, .watchOS, .tvOS, .visionOS, .wasi, .android]))),
413-
.define("SWT_NO_SNAPSHOT_TYPES", .whenEmbedded(or: .when(platforms: [.linux, .custom("freebsd"), .openbsd, .windows, .wasi, .android]))),
425+
.define("SWT_NO_SNAPSHOT_TYPES", .whenEmbedded(or: .whenApple(false))),
414426
.define("SWT_NO_DYNAMIC_LINKING", .whenEmbedded(or: .when(platforms: [.wasi]))),
415427
.define("SWT_NO_PIPES", .whenEmbedded(or: .when(platforms: [.wasi]))),
428+
.define("SWT_NO_FOUNDATION_FILE_COORDINATION", .whenEmbedded(or: .whenApple(false))),
416429

417430
.define("SWT_NO_LEGACY_TEST_DISCOVERY", .whenEmbedded()),
418431
.define("SWT_NO_LIBDISPATCH", .whenEmbedded()),

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

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

74-
#if SWT_TARGET_OS_APPLE
74+
#if SWT_TARGET_OS_APPLE && !SWT_NO_FOUNDATION_FILE_COORDINATION
7575
let data: Data = try await withCheckedThrowingContinuation { continuation in
7676
let fileCoordinator = NSFileCoordinator()
7777
let fileAccessIntent = NSFileAccessIntent.readingIntent(with: url, options: [.forUploading])
@@ -166,25 +166,31 @@ private func _compressContentsOfDirectory(at directoryURL: URL) async throws ->
166166
// knows how to write PKZIP archives, while Windows inherited FreeBSD's tar
167167
// tool in Windows 10 Build 17063 (per https://techcommunity.microsoft.com/blog/containers/tar-and-curl-come-to-windows/382409).
168168
//
169-
// On Linux (which does not have FreeBSD's version of tar(1)), we can use
170-
// zip(1) instead.
169+
// On Linux and OpenBSD (which do not have FreeBSD's version of tar(1)), we
170+
// can use zip(1) instead. This tool compresses paths relative to the current
171+
// working directory, and posix_spawn_file_actions_addchdir_np() is not always
172+
// available for us to call (not present on OpenBSD, requires glibc ≥ 2.28 on
173+
// Linux), so we'll spawn a shell that calls cd before calling zip(1).
171174
//
172175
// OpenBSD's tar(1) does not support writing PKZIP archives, and /usr/bin/zip
173176
// tool is an optional install, so we check if it's present before trying to
174177
// execute it.
178+
#if os(Linux) || os(OpenBSD)
179+
let archiverPath = "/bin/sh"
175180
#if os(Linux)
176-
let archiverPath = "/usr/bin/zip"
177-
#elseif SWT_TARGET_OS_APPLE || os(FreeBSD)
178-
let archiverPath = "/usr/bin/tar"
179-
#elseif os(OpenBSD)
180-
let archiverPath = "/usr/local/bin/zip"
181+
let trueArchiverPath = "/usr/bin/zip"
182+
#else
183+
let trueArchiverPath = "/usr/local/bin/zip"
181184
var isDirectory = false
182-
if !FileManager.default.fileExists(atPath: archiverPath, isDirectory: &isDirectory) || isDirectory {
185+
if !FileManager.default.fileExists(atPath: trueArchiverPath, isDirectory: &isDirectory) || isDirectory {
183186
throw CocoaError(.fileNoSuchFile, userInfo: [
184187
NSLocalizedDescriptionKey: "The 'zip' package is not installed.",
185-
NSFilePathErrorKey: archiverPath
188+
NSFilePathErrorKey: trueArchiverPath
186189
])
187190
}
191+
#endif
192+
#elseif SWT_TARGET_OS_APPLE || os(FreeBSD)
193+
let archiverPath = "/usr/bin/tar"
188194
#elseif os(Windows)
189195
guard let archiverPath = _archiverPath else {
190196
throw CocoaError(.fileWriteUnknown, userInfo: [
@@ -197,20 +203,15 @@ private func _compressContentsOfDirectory(at directoryURL: URL) async throws ->
197203
throw CocoaError(.featureUnsupported, userInfo: [NSLocalizedDescriptionKey: "This platform does not support attaching directories to tests."])
198204
#endif
199205

200-
try await withCheckedThrowingContinuation { continuation in
201-
let process = Process()
202-
203-
process.executableURL = URL(fileURLWithPath: archiverPath, isDirectory: false)
204-
205-
let sourcePath = directoryURL.fileSystemPath
206-
let destinationPath = temporaryURL.fileSystemPath
206+
let sourcePath = directoryURL.fileSystemPath
207+
let destinationPath = temporaryURL.fileSystemPath
208+
let arguments = {
207209
#if os(Linux) || os(OpenBSD)
208210
// The zip command constructs relative paths from the current working
209211
// directory rather than from command-line arguments.
210-
process.arguments = [destinationPath, "--recurse-paths", "."]
211-
process.currentDirectoryURL = directoryURL
212+
["-c", #"cd "$0" && "$1" "$2" --recurse-paths ."#, sourcePath, trueArchiverPath, destinationPath]
212213
#elseif SWT_TARGET_OS_APPLE || os(FreeBSD)
213-
process.arguments = ["--create", "--auto-compress", "--directory", sourcePath, "--file", destinationPath, "."]
214+
["--create", "--auto-compress", "--directory", sourcePath, "--file", destinationPath, "."]
214215
#elseif os(Windows)
215216
// The Windows version of bsdtar can handle relative paths for other archive
216217
// formats, but produces empty archives when inferring the zip format with
@@ -219,30 +220,15 @@ private func _compressContentsOfDirectory(at directoryURL: URL) async throws ->
219220
// An alternative may be to use PowerShell's Compress-Archive command,
220221
// however that comes with a security risk as we'd be responsible for two
221222
// levels of command-line argument escaping.
222-
process.arguments = ["--create", "--auto-compress", "--file", destinationPath, sourcePath]
223+
["--create", "--auto-compress", "--file", destinationPath, sourcePath]
223224
#endif
225+
}()
224226

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

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

Sources/Testing/ExitTests/SpawnProcess.swift

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ private let _posix_spawn_file_actions_addclosefrom_np = symbol(named: "posix_spa
3838
}
3939
#endif
4040

41-
/// Spawn a process and wait for it to terminate.
41+
/// Spawn a child process.
4242
///
4343
/// - Parameters:
4444
/// - executablePath: The path to the executable to spawn.
@@ -61,8 +61,7 @@ private let _posix_spawn_file_actions_addclosefrom_np = symbol(named: "posix_spa
6161
/// eventually pass this value to ``wait(for:)`` to avoid leaking system
6262
/// resources.
6363
///
64-
/// - Throws: Any error that prevented the process from spawning or its exit
65-
/// condition from being read.
64+
/// - Throws: Any error that prevented the process from spawning.
6665
func spawnExecutable(
6766
atPath executablePath: String,
6867
arguments: [String],
@@ -83,17 +82,19 @@ func spawnExecutable(
8382
#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD)
8483
return try withUnsafeTemporaryAllocation(of: P<posix_spawn_file_actions_t>.self, capacity: 1) { fileActions in
8584
let fileActions = fileActions.baseAddress!
86-
guard 0 == posix_spawn_file_actions_init(fileActions) else {
87-
throw CError(rawValue: swt_errno())
85+
let fileActionsInitialized = posix_spawn_file_actions_init(fileActions)
86+
guard 0 == fileActionsInitialized else {
87+
throw CError(rawValue: fileActionsInitialized)
8888
}
8989
defer {
9090
_ = posix_spawn_file_actions_destroy(fileActions)
9191
}
9292

9393
return try withUnsafeTemporaryAllocation(of: P<posix_spawnattr_t>.self, capacity: 1) { attrs in
9494
let attrs = attrs.baseAddress!
95-
guard 0 == posix_spawnattr_init(attrs) else {
96-
throw CError(rawValue: swt_errno())
95+
let attrsInitialized = posix_spawnattr_init(attrs)
96+
guard 0 == attrsInitialized else {
97+
throw CError(rawValue: attrsInitialized)
9798
}
9899
defer {
99100
_ = posix_spawnattr_destroy(attrs)
@@ -416,4 +417,29 @@ private func _escapeCommandLine(_ arguments: [String]) -> String {
416417
}.joined(separator: " ")
417418
}
418419
#endif
420+
421+
/// Spawn a child process and wait for it to terminate.
422+
///
423+
/// - Parameters:
424+
/// - executablePath: The path to the executable to spawn.
425+
/// - arguments: The arguments to pass to the executable, not including the
426+
/// executable path.
427+
/// - environment: The environment block to pass to the executable.
428+
///
429+
/// - Returns: The exit status of the spawned process.
430+
///
431+
/// - Throws: Any error that prevented the process from spawning or its exit
432+
/// condition from being read.
433+
///
434+
/// This function is a convenience that spawns the given process and waits for
435+
/// it to terminate. It is primarily for use by other targets in this package
436+
/// such as its cross-import overlays.
437+
package func spawnExecutableAtPathAndWait(
438+
_ executablePath: String,
439+
arguments: [String] = [],
440+
environment: [String: String] = [:]
441+
) async throws -> ExitStatus {
442+
let processID = try spawnExecutable(atPath: executablePath, arguments: arguments, environment: environment)
443+
return try await wait(for: processID)
444+
}
419445
#endif

cmake/modules/shared/CompilerSettings.cmake

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

0 commit comments

Comments
 (0)