Skip to content

Commit 38dcc92

Browse files
jakepetroulesjakepetroules
committed
Make Configuration.workingDirectory optional
On all platforms, retrieving the current working directory is a failable operation. When a Configuration is given a null working directory, instead of resolving that to the current working directory at initialization time (which can't be done safely because Configuration.init is non-failable), make Configuration.workingDirectory optional and preserve any nil input through to process creation time. Both CreateProcess (Windows) and posix_spawn or fork/exec (POSIX) inherit the working directory of the calling process if it is not specified, so this doesn't change any behavior. It also better matches the design of other properties on the Configuration struct like the Executable, which defer resolution of their concrete value rather than doing so at Configuration.init time. Lastly, it removes incorrect code: the implementation of FilePath.currentWorkingDirectory could crash on POSIX platforms if the current directory was no longer accessible or getcwd failed for any other reason, and on Windows it would not properly support non-ASCII characters in path names nor path names longer than 260 characters.
1 parent 25f4791 commit 38dcc92

File tree

5 files changed

+44
-43
lines changed

5 files changed

+44
-43
lines changed

‎Sources/Subprocess/Configuration.swift

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public struct Configuration: Sendable {
3939
/// The environment to use when running the executable.
4040
public var environment: Environment
4141
/// The working directory to use when running the executable.
42-
public var workingDirectory: FilePath
42+
public var workingDirectory: FilePath?
4343
/// The platform specific options to use when
4444
/// running the subprocess.
4545
public var platformOptions: PlatformOptions
@@ -54,7 +54,7 @@ public struct Configuration: Sendable {
5454
self.executable = executable
5555
self.arguments = arguments
5656
self.environment = environment
57-
self.workingDirectory = workingDirectory ?? .currentWorkingDirectory
57+
self.workingDirectory = workingDirectory
5858
self.platformOptions = platformOptions
5959
}
6060

@@ -107,7 +107,7 @@ extension Configuration: CustomStringConvertible, CustomDebugStringConvertible {
107107
executable: \(self.executable.description),
108108
arguments: \(self.arguments.description),
109109
environment: \(self.environment.description),
110-
workingDirectory: \(self.workingDirectory),
110+
workingDirectory: \(self.workingDirectory?.string ?? ""),
111111
platformOptions: \(self.platformOptions.description(withIndent: 1))
112112
)
113113
"""
@@ -119,7 +119,7 @@ extension Configuration: CustomStringConvertible, CustomDebugStringConvertible {
119119
executable: \(self.executable.debugDescription),
120120
arguments: \(self.arguments.debugDescription),
121121
environment: \(self.environment.debugDescription),
122-
workingDirectory: \(self.workingDirectory),
122+
workingDirectory: \(self.workingDirectory?.string ?? ""),
123123
platformOptions: \(self.platformOptions.description(withIndent: 1))
124124
)
125125
"""
@@ -714,14 +714,6 @@ internal struct CreatedPipe: ~Copyable {
714714
}
715715
}
716716

717-
extension FilePath {
718-
static var currentWorkingDirectory: Self {
719-
let path = getcwd(nil, 0)!
720-
defer { free(path) }
721-
return .init(String(cString: path))
722-
}
723-
}
724-
725717
extension Optional where Wrapped: Collection {
726718
func withOptionalUnsafeBufferPointer<Result>(
727719
_ body: ((UnsafeBufferPointer<Wrapped.Element>)?) throws -> Result

‎Sources/Subprocess/Platforms/Subprocess+Darwin.swift

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,13 @@ extension Configuration {
349349
}
350350

351351
// Setup cwd
352-
let intendedWorkingDir = self.workingDirectory.string
353-
let chdirError: Int32 = intendedWorkingDir.withPlatformString { workDir in
354-
return posix_spawn_file_actions_addchdir_np(&fileActions, workDir)
352+
let chdirError: Int32
353+
if let intendedWorkingDir = self.workingDirectory?.string {
354+
chdirError = intendedWorkingDir.withPlatformString { workDir in
355+
return posix_spawn_file_actions_addchdir_np(&fileActions, workDir)
356+
}
357+
} else {
358+
chdirError = 0
355359
}
356360

357361
// Error handling
@@ -457,12 +461,13 @@ extension Configuration {
457461
errorRead: errorReadFileDescriptor,
458462
errorWrite: errorWriteFileDescriptor
459463
)
460-
let workingDirectory = self.workingDirectory.string
461-
guard Configuration.pathAccessible(workingDirectory, mode: F_OK) else {
462-
throw SubprocessError(
463-
code: .init(.failedToChangeWorkingDirectory(workingDirectory)),
464-
underlyingError: .init(rawValue: ENOENT)
465-
)
464+
if let workingDirectory = self.workingDirectory?.string {
465+
guard Configuration.pathAccessible(workingDirectory, mode: F_OK) else {
466+
throw SubprocessError(
467+
code: .init(.failedToChangeWorkingDirectory(workingDirectory)),
468+
underlyingError: .init(rawValue: ENOENT)
469+
)
470+
}
466471
}
467472
throw SubprocessError(
468473
code: .init(.executableNotFound(self.executable.description)),

‎Sources/Subprocess/Platforms/Subprocess+Linux.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,10 @@ extension Configuration {
8585
errorReadFileDescriptor?.platformDescriptor() ?? -1,
8686
]
8787

88-
let workingDirectory: String = self.workingDirectory.string
8988
// Spawn
9089
var pid: pid_t = 0
9190
let spawnError: CInt = possibleExecutablePath.withCString { exePath in
92-
return workingDirectory.withCString { workingDir in
91+
return self.workingDirectory?.string?.withOptionalCString { workingDir in
9392
return supplementaryGroups.withOptionalUnsafeBufferPointer { sgroups in
9493
return fileDescriptors.withUnsafeBufferPointer { fds in
9594
return _subprocess_fork_exec(
@@ -173,12 +172,13 @@ extension Configuration {
173172
errorRead: errorReadFileDescriptor,
174173
errorWrite: errorWriteFileDescriptor
175174
)
176-
let workingDirectory = self.workingDirectory.string
177-
guard Configuration.pathAccessible(workingDirectory, mode: F_OK) else {
178-
throw SubprocessError(
179-
code: .init(.failedToChangeWorkingDirectory(workingDirectory)),
180-
underlyingError: .init(rawValue: ENOENT)
181-
)
175+
if let workingDirectory = self.workingDirectory?.string {
176+
guard Configuration.pathAccessible(workingDirectory, mode: F_OK) else {
177+
throw SubprocessError(
178+
code: .init(.failedToChangeWorkingDirectory(workingDirectory)),
179+
underlyingError: .init(rawValue: ENOENT)
180+
)
181+
}
182182
}
183183
throw SubprocessError(
184184
code: .init(.executableNotFound(self.executable.description)),

‎Sources/Subprocess/Platforms/Subprocess+Windows.swift

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ extension Configuration {
8484
try environment.withCString(
8585
encodedAs: UTF16.self
8686
) { environmentW in
87-
try intendedWorkingDir.withNTPathRepresentation { intendedWorkingDirW in
87+
try intendedWorkingDir.withOptionalNTPathRepresentation { intendedWorkingDirW in
8888
let created = CreateProcessW(
8989
applicationNameW,
9090
UnsafeMutablePointer<WCHAR>(mutating: commandAndArgsW),
@@ -223,7 +223,7 @@ extension Configuration {
223223
try environment.withCString(
224224
encodedAs: UTF16.self
225225
) { environmentW in
226-
try intendedWorkingDir.withNTPathRepresentation { intendedWorkingDirW in
226+
try intendedWorkingDir.withOptionalNTPathRepresentation { intendedWorkingDirW in
227227
let created = CreateProcessWithLogonW(
228228
usernameW,
229229
domainW,
@@ -769,7 +769,7 @@ extension Configuration {
769769
applicationName: String?,
770770
commandAndArgs: String,
771771
environment: String,
772-
intendedWorkingDir: String
772+
intendedWorkingDir: String?
773773
) {
774774
// Prepare environment
775775
var env: [String: String] = [:]
@@ -806,19 +806,21 @@ extension Configuration {
806806
commandAndArgs
807807
) = try self.generateWindowsCommandAndAgruments()
808808
// Validate workingDir
809-
guard Self.pathAccessible(self.workingDirectory.string) else {
810-
throw SubprocessError(
811-
code: .init(
812-
.failedToChangeWorkingDirectory(self.workingDirectory.string)
813-
),
814-
underlyingError: nil
815-
)
809+
if let workingDirectory = self.workingDirectory?.string {
810+
guard Self.pathAccessible(workingDirectory) else {
811+
throw SubprocessError(
812+
code: .init(
813+
.failedToChangeWorkingDirectory(workingDirectory)
814+
),
815+
underlyingError: nil
816+
)
817+
}
816818
}
817819
return (
818820
applicationName: applicationName,
819821
commandAndArgs: commandAndArgs,
820822
environment: environmentString,
821-
intendedWorkingDir: self.workingDirectory.string
823+
intendedWorkingDir: self.workingDirectory?.string
822824
)
823825
}
824826

‎Sources/_SubprocessCShims/process_shims.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,11 @@ static int _subprocess_posix_spawn_fallback(
397397
if (rc != 0) { return rc; }
398398
}
399399
// Setup working directory
400-
rc = _subprocess_addchdir_np(&file_actions, working_directory);
401-
if (rc != 0) {
402-
return rc;
400+
if (working_directory != NULL) {
401+
rc = _subprocess_addchdir_np(&file_actions, working_directory);
402+
if (rc != 0) {
403+
return rc;
404+
}
403405
}
404406

405407
// Close parent side

0 commit comments

Comments
 (0)