Skip to content

Added modern URL API to Foundation.Process #1461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Added modern URL API to Foundation.Process #1461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions Foundation/Process.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,40 @@ open class Process: NSObject {

}

@available(*, deprecated)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are actually not marked as deprecated yet in the SDK.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new implementation looks reasonable but we can leave the deprecation annotation off for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is marked as deprecated in the documentation: https://developer.apple.com/documentation/foundation/process/1413110-currentdirectorypath

Maybe it is just an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the path API is not deprecated I would prefer to do the inverted implementation (just add the new URL API as an overlay of the path counterparts).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation actually got ahead of us here. It is "soft deprecated", indicating that will eventually be deprecated.

It may seem like splitting hairs but I think we should remain consistent with the annotations in the SDK. Once the SDK marks it deprecated (with a API_DEPRECATED macro, e.g.) then we can add the attribute here.

If you want to invert the implementation, that's fine with me. That's actually how we do it in the ObjC Foundation too.

open var launchPath: String? {
set {
guard let newValue = newValue else {
self.executableURL = nil
return
}

self.executableURL = URL(fileURLWithPath: newValue)
}

get {
return self.executableURL?.path
}
}

@available(*, deprecated)
open var currentDirectoryPath: String {
set {
self.currentDirectoryURL = URL(fileURLWithPath: newValue)
}

get {
return self.currentDirectoryURL.path
}
}

// these methods can only be set before a launch
open var launchPath: String?
open var arguments: [String]?
open var executableURL: URL?
open var currentDirectoryURL: URL = URL(fileURLWithPath: FileManager.default.currentDirectoryPath)

open var environment: [String : String]? // if not set, use current

open var currentDirectoryPath: String = FileManager.default.currentDirectoryPath

// standard I/O channels; could be either a FileHandle or a Pipe
open var standardInput: Any? {
willSet {
Expand Down Expand Up @@ -209,7 +236,7 @@ open class Process: NSObject {

// Ensure that the launch path is set

guard let launchPath = self.launchPath else {
guard let launchPath = self.executableURL?.path else {
fatalError()
}

Expand Down Expand Up @@ -389,7 +416,7 @@ open class Process: NSObject {

let fileManager = FileManager()
let previousDirectoryPath = fileManager.currentDirectoryPath
if !fileManager.changeCurrentDirectoryPath(currentDirectoryPath) {
if !fileManager.changeCurrentDirectoryPath(currentDirectoryURL.path) {
// Foundation throws an NSException when changing the working directory fails,
// and unfortunately launch() is not marked `throws`, so we get away with a
// fatalError.
Expand Down Expand Up @@ -485,7 +512,7 @@ extension Process {
// convenience; create and launch
open class func launchedProcess(launchPath path: String, arguments: [String]) -> Process {
let process = Process()
process.launchPath = path
process.executableURL = URL(fileURLWithPath: path)
process.arguments = arguments
process.launch()

Expand Down
27 changes: 14 additions & 13 deletions TestFoundation/TestProcess.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class TestProcess : XCTestCase {

let process = Process()

process.launchPath = "/bin/bash"
process.executableURL = URL(fileURLWithPath: "/bin/bash")
process.arguments = ["-c", "exit 0"]

process.launch()
Expand All @@ -58,7 +58,7 @@ class TestProcess : XCTestCase {

let process = Process()

process.launchPath = "/bin/bash"
process.executableURL = URL(fileURLWithPath: "/bin/bash")
process.arguments = ["-c", "exit 1"]

process.launch()
Expand All @@ -71,7 +71,7 @@ class TestProcess : XCTestCase {

let process = Process()

process.launchPath = "/bin/bash"
process.executableURL = URL(fileURLWithPath: "/bin/bash")
process.arguments = ["-c", "exit 100"]

process.launch()
Expand All @@ -84,7 +84,7 @@ class TestProcess : XCTestCase {

let process = Process()

process.launchPath = "/bin/bash"
process.executableURL = URL(fileURLWithPath: "/bin/bash")
process.arguments = ["-c", "sleep 2"]

process.launch()
Expand All @@ -97,7 +97,7 @@ class TestProcess : XCTestCase {

let process = Process()

process.launchPath = "/bin/bash"
process.executableURL = URL(fileURLWithPath: "/bin/bash")
process.arguments = ["-c", "sleep 2; exit 1"]

process.launch()
Expand All @@ -109,7 +109,7 @@ class TestProcess : XCTestCase {
func test_terminationReason_uncaughtSignal() {
let process = Process()

process.launchPath = "/bin/bash"
process.executableURL = URL(fileURLWithPath: "/bin/bash")
process.arguments = ["-c", "kill -TERM $$"]

process.launch()
Expand All @@ -121,7 +121,7 @@ class TestProcess : XCTestCase {
func test_pipe_stdin() {
let process = Process()

process.launchPath = "/bin/cat"
process.executableURL = URL(fileURLWithPath: "/bin/cat")

let outputPipe = Pipe()
process.standardOutput = outputPipe
Expand Down Expand Up @@ -150,7 +150,7 @@ class TestProcess : XCTestCase {
func test_pipe_stdout() {
let process = Process()

process.launchPath = "/usr/bin/which"
process.executableURL = URL(fileURLWithPath: "/usr/bin/which")
process.arguments = ["which"]

let pipe = Pipe()
Expand All @@ -171,7 +171,7 @@ class TestProcess : XCTestCase {
func test_pipe_stderr() {
let process = Process()

process.launchPath = "/bin/cat"
process.executableURL = URL(fileURLWithPath: "/bin/cat")
process.arguments = ["invalid_file_name"]

let errorPipe = Pipe()
Expand All @@ -193,7 +193,7 @@ class TestProcess : XCTestCase {
func test_pipe_stdout_and_stderr_same_pipe() {
let process = Process()

process.launchPath = "/bin/cat"
process.executableURL = URL(fileURLWithPath: "/bin/cat")
process.arguments = ["invalid_file_name"]

let pipe = Pipe()
Expand Down Expand Up @@ -223,7 +223,7 @@ class TestProcess : XCTestCase {
func test_file_stdout() {
let process = Process()

process.launchPath = "/usr/bin/which"
process.executableURL = URL(fileURLWithPath: "/usr/bin/which")
process.arguments = ["which"]

mkstemp(template: "TestProcess.XXXXXX") { handle in
Expand Down Expand Up @@ -314,7 +314,8 @@ private func runTask(_ arguments: [String], environment: [String: String]? = nil
let process = Process()

var arguments = arguments
process.launchPath = arguments.removeFirst()

process.executableURL = URL(fileURLWithPath: arguments.removeFirst())
process.arguments = arguments
// Darwin Foundation doesnt allow .environment to be set to nil although the documentation
// says it is an optional. https://developer.apple.com/documentation/foundation/process/1409412-environment
Expand All @@ -323,7 +324,7 @@ private func runTask(_ arguments: [String], environment: [String: String]? = nil
}

if let directoryPath = currentDirectoryPath {
process.currentDirectoryPath = directoryPath
process.currentDirectoryURL = URL(fileURLWithPath: directoryPath)
}

let stdoutPipe = Pipe()
Expand Down