Skip to content

Commit 279f18f

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 279f18f

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 {
@@ -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: 28 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])
@@ -165,25 +165,31 @@ private func _compressContentsOfDirectory(at directoryURL: URL) async throws ->
165165
// knows how to write PKZIP archives, while Windows inherited FreeBSD's tar
166166
// tool in Windows 10 Build 17063 (per https://techcommunity.microsoft.com/blog/containers/tar-and-curl-come-to-windows/382409).
167167
//
168-
// On Linux (which does not have FreeBSD's version of tar(1)), we can use
169-
// zip(1) instead.
168+
// On Linux and OpenBSD (which do not have FreeBSD's version of tar(1)), we
169+
// can use zip(1) instead. This tool compresses paths relative to the current
170+
// working directory, and posix_spawn_file_actions_addchdir_np() is not always
171+
// available for us to call, so we'll spawn a shell that calls cd before
172+
// calling zip(1) for us.
170173
//
171174
// OpenBSD's tar(1) does not support writing PKZIP archives, and /usr/bin/zip
172175
// tool is an optional install, so we check if it's present before trying to
173176
// execute it.
177+
#if os(Linux) || os(OpenBSD)
178+
let archiverPath = "/bin/sh"
174179
#if os(Linux)
175-
let archiverPath = "/usr/bin/zip"
176-
#elseif SWT_TARGET_OS_APPLE || os(FreeBSD)
177-
let archiverPath = "/usr/bin/tar"
178-
#elseif os(OpenBSD)
179-
let archiverPath = "/usr/local/bin/zip"
180+
let trueArchiverPath = "/usr/bin/zip"
181+
#else
182+
let trueArchiverPath = "/usr/local/bin/zip"
180183
var isDirectory = false
181-
if !FileManager.default.fileExists(atPath: archiverPath, isDirectory: &isDirectory) || isDirectory {
184+
if !FileManager.default.fileExists(atPath: trueArchiverPath, isDirectory: &isDirectory) || isDirectory {
182185
throw CocoaError(.fileNoSuchFile, userInfo: [
183186
NSLocalizedDescriptionKey: "The 'zip' package is not installed.",
184-
NSFilePathErrorKey: archiverPath
187+
NSFilePathErrorKey: trueArchiverPath
185188
])
186189
}
190+
#endif
191+
#elseif SWT_TARGET_OS_APPLE || os(FreeBSD)
192+
let archiverPath = "/usr/bin/tar"
187193
#elseif os(Windows)
188194
guard let archiverPath = _archiverPath else {
189195
throw CocoaError(.fileWriteUnknown, userInfo: [
@@ -196,20 +202,15 @@ private func _compressContentsOfDirectory(at directoryURL: URL) async throws ->
196202
throw CocoaError(.featureUnsupported, userInfo: [NSLocalizedDescriptionKey: "This platform does not support attaching directories to tests."])
197203
#endif
198204

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
205+
let sourcePath = directoryURL.fileSystemPath
206+
let destinationPath = temporaryURL.fileSystemPath
207+
let arguments = {
206208
#if os(Linux) || os(OpenBSD)
207209
// The zip command constructs relative paths from the current working
208210
// directory rather than from command-line arguments.
209-
process.arguments = [destinationPath, "--recurse-paths", "."]
210-
process.currentDirectoryURL = directoryURL
211+
["-c", #"cd "$0" && "$1" "$2" --recurse-paths ."#, sourcePath, trueArchiverPath, destinationPath]
211212
#elseif SWT_TARGET_OS_APPLE || os(FreeBSD)
212-
process.arguments = ["--create", "--auto-compress", "--directory", sourcePath, "--file", destinationPath, "."]
213+
["--create", "--auto-compress", "--directory", sourcePath, "--file", destinationPath, "."]
213214
#elseif os(Windows)
214215
// The Windows version of bsdtar can handle relative paths for other archive
215216
// formats, but produces empty archives when inferring the zip format with
@@ -218,30 +219,15 @@ private func _compressContentsOfDirectory(at directoryURL: URL) async throws ->
218219
// An alternative may be to use PowerShell's Compress-Archive command,
219220
// however that comes with a security risk as we'd be responsible for two
220221
// levels of command-line argument escaping.
221-
process.arguments = ["--create", "--auto-compress", "--file", destinationPath, sourcePath]
222+
["--create", "--auto-compress", "--file", destinationPath, sourcePath]
222223
#endif
224+
}()
223225

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-
}
226+
let exitStatus = try await spawnExecutableAtPathAndWait(archiverPath, arguments: arguments)
227+
guard case .exitCode(EXIT_SUCCESS) = exitStatus else {
228+
throw CocoaError(.fileWriteUnknown, userInfo: [
229+
NSLocalizedDescriptionKey: "The directory at '\(sourcePath)' could not be compressed (\(exitStatus)).",
230+
])
245231
}
246232

247233
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)
@@ -396,4 +397,29 @@ private func _escapeCommandLine(_ arguments: [String]) -> String {
396397
}.joined(separator: " ")
397398
}
398399
#endif
400+
401+
/// Spawn a child process and wait for it to terminate.
402+
///
403+
/// - Parameters:
404+
/// - executablePath: The path to the executable to spawn.
405+
/// - arguments: The arguments to pass to the executable, not including the
406+
/// executable path.
407+
/// - environment: The environment block to pass to the executable.
408+
///
409+
/// - Returns: The exit status of the spawned process.
410+
///
411+
/// - Throws: Any error that prevented the process from spawning or its exit
412+
/// condition from being read.
413+
///
414+
/// This function is a convenience that spawns the given process and waits for
415+
/// it to terminate. It is primarily for use by other targets in this package
416+
/// such as its cross-import overlays.
417+
package func spawnExecutableAtPathAndWait(
418+
_ executablePath: String,
419+
arguments: [String] = [],
420+
environment: [String: String] = [:]
421+
) async throws -> ExitStatus {
422+
let processID = try spawnExecutable(atPath: executablePath, arguments: arguments, environment: environment)
423+
return try await wait(for: processID)
424+
}
399425
#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)