Merge pull request #4764 from apple/revert-4606-cb-no-chdir-on-process

Revert "Do not chdir before calling posix_spawn"

Author: shahmishal

CoreFoundation/Base.subproj/CFPlatform.c

@@ -2210,10 +2210,6 @@ CF_EXPORT int _CFPosixSpawnFileActionsAddClose(_CFPosixSpawnFileActionsRef file_
   return posix_spawn_file_actions_addclose((posix_spawn_file_actions_t *)file_actions, filedes);
 }
 
-CF_EXPORT int _CFPosixSpawnFileActionsAddChdirNP(_CFPosixSpawnFileActionsRef file_actions, const char *path) {
-  return posix_spawn_file_actions_addchdir_np((posix_spawn_file_actions_t *)file_actions, path);
-}
-
 CF_EXPORT int _CFPosixSpawn(pid_t *_CF_RESTRICT pid, const char *_CF_RESTRICT path, _CFPosixSpawnFileActionsRef file_actions, _CFPosixSpawnAttrRef _Nullable _CF_RESTRICT attrp, char *_Nullable const argv[_Nullable _CF_RESTRICT], char *_Nullable const envp[_Nullable _CF_RESTRICT]) {
   return posix_spawn(pid, path, (posix_spawn_file_actions_t *)file_actions, (posix_spawnattr_t *)attrp, argv, envp);
 }

CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h

@@ -709,10 +709,6 @@ CF_EXPORT int _CFPosixSpawn(pid_t *_CF_RESTRICT pid, const char *_CF_RESTRICT pa
 #else
 CF_EXPORT int _CFPosixSpawn(pid_t *_CF_RESTRICT pid, const char *_CF_RESTRICT path, _CFPosixSpawnFileActionsRef file_actions, _CFPosixSpawnAttrRef _Nullable _CF_RESTRICT attrp, char *_Nullable const argv[_Nullable _CF_RESTRICT], char *_Nullable const envp[_Nullable _CF_RESTRICT]);
 #endif // __cplusplus
-
-#if TARGET_OS_LINUX
-CF_EXPORT int _CFPosixSpawnFileActionsAddChdirNP(_CFPosixSpawnFileActionsRef file_actions, const char *path);
-#endif // TARGET_OS_LINUX
 #endif // !TARGET_OS_WIN32
 
 _CF_EXPORT_SCOPE_END

Sources/Foundation/Process.swift

@@ -944,17 +944,6 @@ open class Process: NSObject {
         }
 #endif
 
-        #if os(Linux)
-        if let dir = currentDirectoryURL?.path {
-            try _throwIfPosixError(_CFPosixSpawnFileActionsAddChdirNP(fileActions, dir))
-        }
-        #else
-        // This is an unfortunate workaround: posix_spawn has no POSIX-specified way to set the working directory
-        // of the child process. glibc has a non-POSIX API option, which we use above. Here we take a brute-force
-        // approach of just changing our current working directory. This is not a great implementation and it's likely
-        // to cause subtle issues in those environments. However, the Apple Foundation library doesn't have this problem,
-        // and this library does the right thing on Linux and Windows, so the overwhelming majority of users are
-        // well-served.
         let fileManager = FileManager()
         let previousDirectoryPath = fileManager.currentDirectoryPath
         if let dir = currentDirectoryURL?.path, !fileManager.changeCurrentDirectoryPath(dir) {
@@ -965,7 +954,6 @@ open class Process: NSObject {
             // Reset the previous working directory path.
             fileManager.changeCurrentDirectoryPath(previousDirectoryPath)
         }
-        #endif
 
         // Launch
         var pid = pid_t()

Tests/Foundation/Tests/TestProcess.swift

@@ -7,8 +7,6 @@
 // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 
-import Dispatch
-
 class TestProcess : XCTestCase {
 
     func test_exit0() throws {
@@ -701,55 +699,6 @@ class TestProcess : XCTestCase {
         }
     }
 
-    func test_currentDirectoryDoesNotChdirParentProcess() throws {
-        // This test only behaves correctly on Linux and Windows: other platforms don't have an
-        // appropriate API for this in posix_spawn or similar.
-        #if os(Linux) || os(Windows)
-        let backgroundQueue = DispatchQueue(label: "background-processor")
-        let group = DispatchGroup()
-        let startSemaphore = DispatchSemaphore(value: 0)
-        let currentWorkingDirectory = FileManager.default.currentDirectoryPath
-        var shouldRun = true
-        let shouldRunLock = NSLock()
-
-        XCTAssertNotEqual(currentWorkingDirectory, "/")
-
-        // Kick off the background task. This will spin on our current working directory and confirm
-        // it doesn't change.
-        backgroundQueue.async(group: group) {
-            startSemaphore.signal()
-
-            while true {
-                let newCWD = FileManager.default.currentDirectoryPath
-                XCTAssertEqual(newCWD, currentWorkingDirectory)
-
-                shouldRunLock.lock()
-                if shouldRun {
-                    shouldRunLock.unlock()
-                } else {
-                    shouldRunLock.unlock()
-                    break
-                }
-            }
-        }
-
-        startSemaphore.wait()
-
-        // We run the task 50 times just to try to encourage it to fail.
-        for _ in 0..<50 {
-            XCTAssertNoThrow(try runTask([xdgTestHelperURL().path, "--getcwd"], currentDirectoryPath: "/"))
-        }
-
-        shouldRunLock.lock()
-        shouldRun = false
-        shouldRunLock.unlock()
-
-        group.wait()
-        #else
-        throw XCTSkip()
-        #endif
-    }
-
     #if !os(Windows)
     func test_fileDescriptorsAreNotInherited() throws {
         let task = Process()
@@ -917,7 +866,6 @@ class TestProcess : XCTestCase {
             ("test_currentDirectory", test_currentDirectory),
             ("test_pipeCloseBeforeLaunch", test_pipeCloseBeforeLaunch),
             ("test_multiProcesses", test_multiProcesses),
-            ("test_currentDirectoryDoesNotChdirParentProcess", test_currentDirectoryDoesNotChdirParentProcess),
         ]
 
 #if !os(Windows)