Remove preExecProcessAction (#149)

It's been deemed too tricky to make this API safe and there are relatively few use cases anyways, considering we want to use posix_spawn internally as much as possible.

Closes #148

Author: jakepetroules

Sources/Subprocess/Platforms/Subprocess+Darwin.swift

@@ -81,29 +81,6 @@ public struct PlatformOptions: Sendable {
                 inout posix_spawn_file_actions_t?
             ) throws -> Void
         )? = nil
-    /// A closure to configure platform-specific
-    /// spawning constructs. This closure enables direct
-    /// configuration or override of underlying platform-specific
-    /// spawn settings that `Subprocess` utilizes internally,
-    /// in cases where Subprocess does not provide higher-level
-    /// APIs for such modifications.
-    ///
-    /// On Darwin, Subprocess uses `posix_spawn()` as the
-    /// underlying spawning mechanism, but may require an initial `fork()`
-    /// depending on the configured `PlatformOptions`.
-    /// This closure is called after `fork()` but before `posix_spawn()`
-    /// (with the `POSIX_SPAWN_SETEXEC` flag set).
-    /// You may use it to call any necessary process setup functions.
-    ///
-    /// - note: You can set both `preExecProcessAction` and
-    /// `preSpawnProcessConfigurator` and both will be called.
-    /// Setting `preExecProcessAction` will always cause Subprocess
-    /// to pre-`fork()` before calling `posix_spawn()` (with the
-    /// `POSIX_SPAWN_SETEXEC` flag set) even if it would not have otherwise
-    /// done so based on the configured `PlatformOptions`.
-    ///
-    /// - warning: You may ONLY call [async-signal-safe functions](https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html) within this closure (note _"The following table defines a set of functions and function-like macros that shall be async-signal-safe."_).
-    public var preExecProcessAction: (@convention(c) @Sendable () -> Void)? = nil
 
     public init() {}
 }
@@ -161,7 +138,6 @@ extension PlatformOptions: CustomStringConvertible, CustomDebugStringConvertible
             \(indent)    processGroupID: \(String(describing: processGroupID)),
             \(indent)    createSession: \(createSession),
             \(indent)    preSpawnProcessConfigurator: \(self.preSpawnProcessConfigurator == nil ? "not set" : "set")
-            \(indent)    preExecProcessAction: \(self.preExecProcessAction == nil ? "not set" : "set")
             \(indent))
             """
     }
@@ -426,8 +402,7 @@ extension Configuration {
                             gidPtr,
                             Int32(supplementaryGroups?.count ?? 0),
                             sgroups?.baseAddress,
-                            self.platformOptions.createSession ? 1 : 0,
-                            self.platformOptions.preExecProcessAction,
+                            self.platformOptions.createSession ? 1 : 0
                         )
                     }
                 }

Sources/Subprocess/Platforms/Subprocess+Linux.swift

@@ -151,8 +151,7 @@ extension Configuration {
                                     processGroupIDPtr,
                                     CInt(supplementaryGroups?.count ?? 0),
                                     sgroups?.baseAddress,
-                                    self.platformOptions.createSession ? 1 : 0,
-                                    self.platformOptions.preExecProcessAction
+                                    self.platformOptions.createSession ? 1 : 0
                                 )
                             }
                         }
@@ -284,20 +283,6 @@ public struct PlatformOptions: Sendable {
     /// the child process terminates.
     /// Always ends in sending a `.kill` signal at the end.
     public var teardownSequence: [TeardownStep] = []
-    /// A closure to configure platform-specific
-    /// spawning constructs. This closure enables direct
-    /// configuration or override of underlying platform-specific
-    /// spawn settings that `Subprocess` utilizes internally,
-    /// in cases where Subprocess does not provide higher-level
-    /// APIs for such modifications.
-    ///
-    /// On Linux, Subprocess uses `fork/exec` as the
-    /// underlying spawning mechanism. This closure is called
-    /// after `fork()` but before `exec()`. You may use it to
-    /// call any necessary process setup functions.
-    ///
-    /// - warning: You may ONLY call [async-signal-safe functions](https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html) within this closure (note _"The following table defines a set of functions and function-like macros that shall be async-signal-safe."_).
-    public var preExecProcessAction: (@convention(c) @Sendable () -> Void)? = nil
 
     public init() {}
 }
@@ -311,8 +296,7 @@ extension PlatformOptions: CustomStringConvertible, CustomDebugStringConvertible
             \(indent)    groupID: \(String(describing: groupID)),
             \(indent)    supplementaryGroups: \(String(describing: supplementaryGroups)),
             \(indent)    processGroupID: \(String(describing: processGroupID)),
-            \(indent)    createSession: \(createSession),
-            \(indent)    preExecProcessAction: \(self.preExecProcessAction == nil ? "not set" : "set")
+            \(indent)    createSession: \(createSession)
             \(indent))
             """
     }

Sources/_SubprocessCShims/include/process_shims.h

@@ -43,8 +43,7 @@ int _subprocess_spawn(
     uid_t * _Nullable uid,
     gid_t * _Nullable gid,
     int number_of_sgroups, const gid_t * _Nullable sgroups,
-    int create_session,
-    void (* _Nullable configurator)(void)
+    int create_session
 );
 #endif // TARGET_OS_MAC
 
@@ -60,8 +59,7 @@ int _subprocess_fork_exec(
     gid_t * _Nullable gid,
     gid_t * _Nullable process_group_id,
     int number_of_sgroups, const gid_t * _Nullable sgroups,
-    int create_session,
-    void (* _Nullable configurator)(void)
+    int create_session
 );
 
 int _was_process_exited(int status);

Sources/_SubprocessCShims/process_shims.c

@@ -97,8 +97,7 @@ static int _subprocess_spawn_prefork(
     uid_t * _Nullable uid,
     gid_t * _Nullable gid,
     int number_of_sgroups, const gid_t * _Nullable sgroups,
-    int create_session,
-    void (* _Nullable configurator)(void)
+    int create_session
 ) {
 #define write_error_and_exit int error = errno; \
     write(pipefd[1], &error, sizeof(error));\
@@ -191,11 +190,6 @@ static int _subprocess_spawn_prefork(
             (void)setsid();
         }
 
-        // Run custom configuratior
-        if (configurator != NULL) {
-            configurator();
-        }
-
         // Use posix_spawnas exec
         int error = posix_spawn(pid, exec_path, file_actions, spawn_attrs, args, env);
         // If we reached this point, something went wrong
@@ -244,22 +238,20 @@ int _subprocess_spawn(
     uid_t * _Nullable uid,
     gid_t * _Nullable gid,
     int number_of_sgroups, const gid_t * _Nullable sgroups,
-    int create_session,
-    void (* _Nullable configurator)(void)
+    int create_session
 ) {
     int require_pre_fork = uid != NULL ||
         gid != NULL ||
         number_of_sgroups > 0 ||
-        create_session > 0 ||
-        configurator != NULL;
+        create_session > 0;
 
     if (require_pre_fork != 0) {
         int rc = _subprocess_spawn_prefork(
             pid,
             exec_path,
             file_actions, spawn_attrs,
             args, env,
-            uid, gid, number_of_sgroups, sgroups, create_session, configurator
+            uid, gid, number_of_sgroups, sgroups, create_session
         );
         return rc;
     }
@@ -486,8 +478,7 @@ int _subprocess_fork_exec(
     gid_t * _Nullable gid,
     gid_t * _Nullable process_group_id,
     int number_of_sgroups, const gid_t * _Nullable sgroups,
-    int create_session,
-    void (* _Nullable configurator)(void)
+    int create_session
 ) {
 #define write_error_and_exit int error = errno; \
     write(pipefd[1], &error, sizeof(error));\
@@ -681,10 +672,6 @@ int _subprocess_fork_exec(
             }
         }
 
-        // Run custom configuratior
-        if (configurator != NULL) {
-            configurator();
-        }
         // Finally, exec
         execve(exec_path, args, env);
         // If we reached this point, something went wrong

Tests/SubprocessTests/DarwinTests.swift

@@ -26,78 +26,6 @@ import Testing
 // MARK: PlatformOptions Tests
 @Suite(.serialized)
 struct SubprocessDarwinTests {
-    @Test func testSubprocessPlatformOptionsPreExecProcessAction() async throws {
-        var platformOptions = PlatformOptions()
-        platformOptions.preExecProcessAction = {
-            exit(1234567)
-        }
-        let idResult = try await Subprocess.run(
-            .path("/bin/pwd"),
-            platformOptions: platformOptions,
-            output: .discarded
-        )
-        #expect(idResult.terminationStatus == .exited(1234567))
-    }
-
-    @Test(
-        .disabled("Constantly fails on macOS 26 and Swift 6.2"),
-        .bug("https://github.com/swiftlang/swift-subprocess/issues/148")
-    ) func testSubprocessPlatformOptionsPreExecProcessActionAndProcessConfigurator() async throws {
-        let (readFD, writeFD) = try FileDescriptor.pipe()
-        try await readFD.closeAfter {
-            let childPID = try await writeFD.closeAfter {
-                // Allocate some constant high-numbered FD that's unlikely to be used.
-                let specialFD = try writeFD.duplicate(as: FileDescriptor(rawValue: 9000))
-                return try await specialFD.closeAfter {
-                    // Make the fd non-blocking just to avoid the test hanging if it fails
-                    let opts = fcntl(specialFD.rawValue, F_GETFD)
-                    #expect(opts >= 0)
-                    #expect(fcntl(specialFD.rawValue, F_SETFD, opts | O_NONBLOCK) >= 0)
-
-                    var platformOptions = PlatformOptions()
-                    platformOptions.preExecProcessAction = {
-                        var pid: Int32 = getpid()
-                        if write(9000, &pid, 4) != 4 {
-                            exit(EXIT_FAILURE)
-                        }
-                    }
-                    platformOptions.preSpawnProcessConfigurator = { spawnAttr, _ in
-                        // Set POSIX_SPAWN_SETSID flag, which implies calls
-                        // to setsid
-                        var flags: Int16 = 0
-                        posix_spawnattr_getflags(&spawnAttr, &flags)
-                        posix_spawnattr_setflags(&spawnAttr, flags | Int16(POSIX_SPAWN_SETSID))
-                    }
-                    // Check the process ID (pid), process group ID (pgid), and
-                    // controlling terminal's process group ID (tpgid)
-                    let psResult = try await Subprocess.run(
-                        .path("/bin/bash"),
-                        arguments: ["-c", "ps -o pid,pgid,tpgid -p $$"],
-                        platformOptions: platformOptions,
-                        input: .none,
-                        error: .discarded
-                    ) { execution, standardOutput in
-                        var buffer = Data()
-                        for try await chunk in standardOutput {
-                            let currentChunk = chunk.withUnsafeBytes { Data($0) }
-                            buffer += currentChunk
-                        }
-                        return (pid: execution.processIdentifier.value, standardOutput: buffer)
-                    }
-                    try assertNewSessionCreated(terminationStatus: psResult.terminationStatus, output: String(decoding: psResult.value.standardOutput, as: UTF8.self))
-                    return psResult.value.pid
-                }
-            }
-
-            let bytes = try await readFD.readUntilEOF(upToLength: 4)
-            var pid: Int32 = -1
-            _ = withUnsafeMutableBytes(of: &pid) { ptr in
-                bytes.copyBytes(to: ptr)
-            }
-            #expect(pid == childPID)
-        }
-    }
-
     @Test func testSubprocessPlatformOptionsProcessConfiguratorUpdateSpawnAttr() async throws {
         var platformOptions = PlatformOptions()
         platformOptions.preSpawnProcessConfigurator = { spawnAttr, _ in

Tests/SubprocessTests/LinuxTests.swift

@@ -29,40 +29,6 @@ import _SubprocessCShims
 // MARK: PlatformOption Tests
 @Suite(.serialized)
 struct SubprocessLinuxTests {
-    @Test(
-        .enabled(
-            if: getgid() == 0,
-            "This test requires root privileges"
-        )
-    )
-    func testSubprocessPlatformOptionsPreExecProcessAction() async throws {
-        var platformOptions = PlatformOptions()
-        platformOptions.preExecProcessAction = {
-            guard setgid(4321) == 0 else {
-                // Returns EPERM when:
-                //   The calling process is not privileged (does not have the
-                //   CAP_SETGID capability in its user namespace), and gid does
-                //   not match the real group ID or saved set-group-ID of the
-                //   calling process.
-                abort()
-            }
-        }
-        let idResult = try await Subprocess.run(
-            .path("/usr/bin/id"),
-            arguments: ["-g"],
-            platformOptions: platformOptions,
-            output: .string(limit: 32),
-            error: .string(limit: 32)
-        )
-        let error = try #require(idResult.standardError)
-        try #require(error == "")
-        #expect(idResult.terminationStatus.isSuccess)
-        let id = try #require(idResult.standardOutput)
-        #expect(
-            id.trimmingCharacters(in: .whitespacesAndNewlines) == "\(4321)"
-        )
-    }
-
     @Test func testSuspendResumeProcess() async throws {
         func blockAndWaitForStatus(
             pid: pid_t,