Clean up process handling.

Author: dabrahams

Sources/gyb-swift/ExecutionContext.swift

@@ -141,7 +141,7 @@ struct CodeGenerator {
       throw Failure("Failed to write temporary Swift file to '\(temp.path)'", error)
     }
 
-    let result = try runProcess("swift", arguments: [temp.platformString])
+    let result = try resultsOfRunning(["swift", temp.platformString])
 
     guard result.exitStatus == 0 else {
       throw GYBError.executionFailed(filename: filename, errorOutput: result.stderr)
@@ -202,9 +202,8 @@ struct CodeGenerator {
 
   /// Compiles Swift source file to executable.
   private func compileSwiftCode(source: URL, output: URL, moduleCache: URL) throws {
-    let result = try runProcess(
-      "swiftc",
-      arguments: [
+    let result = try resultsOfRunning(
+      ["swiftc",
         source.platformString,
         "-o", output.platformString,
         "-module-cache-path", moduleCache.platformString,
@@ -217,7 +216,7 @@ struct CodeGenerator {
 
   /// Runs compiled executable and returns its output.
   private func runCompiledExecutable(_ executable: URL) throws -> String {
-    let result = try runProcess(executable.platformString, arguments: [])
+    let result = try resultsOfRunning([executable.platformString])
 
     guard result.exitStatus == 0 else {
       throw GYBError.executionFailed(filename: filename, errorOutput: result.stderr)

Sources/gyb-swift/ProcessUtilities.swift

@@ -25,14 +25,20 @@ private let environmentVariables =
     })
   : ProcessInfo.processInfo.environment
 
-/// Runs `executable` with `arguments`, returning stdout trimmed of whitespace.
-private func runProcessForOutput(
-  _ executable: String, arguments: [String]
+/// Returns the standard output from running `commandLine` passing
+/// `arguments`, trimmed of whitespace.
+///
+/// If `commandLine` contains no path separators, it is looked up in `PATH`.
+///
+/// - Parameter setSKDRoot: true iff xcrun should be used to set the SDKROOT
+///   environment variable for the process.
+private func standardOutputOf(
+  _ commandLine: [String], setSDKRoot: Bool = true
 ) throws -> String {
-  let result = try runProcess(executable, arguments: arguments)
+  let result = try resultsOfRunning(commandLine, setSDKRoot: setSDKRoot)
 
   guard result.exitStatus == 0 else {
-    throw Failure("\(executable) \(arguments) exited with \(result.exitStatus)")
+    throw Failure("\(commandLine) exited with \(result.exitStatus)")
   }
 
   return result.stdout.trimmingCharacters(in: .whitespacesAndNewlines)
@@ -70,7 +76,7 @@ private func findWindowsExecutableInPath(_ command: String) throws -> String {
     throw Failure("\(whereExe) doesn't exist")
   }
 
-  let output = try runProcessForOutput(whereExe, arguments: [command])
+  let output = try standardOutputOf([whereExe, command])
 
   // where.exe returns the first match on the first line
   guard let r = output.split(separator: "\n", maxSplits: 1).first else {
@@ -79,26 +85,57 @@ private func findWindowsExecutableInPath(_ command: String) throws -> String {
   return String(r)
 }
 
+/// The value to set for SDKROOT in subprocess environments on macOS.
+internal let swiftSDKRoot: String =
+  if isMacOS {
+    ProcessInfo.processInfo.environment["SDKROOT"]
+      ?? { () -> String in
+        do {
+          // Don't set SDKROOT in the environment to avoid infinite recursion.
+          return try standardOutputOf(
+            ["/usr/bin/xcrun", "--show-sdk-path"], setSDKRoot: false)
+        } catch let e {
+          fatalError("\(e)")
+        }
+      }()
+  } else {
+    "UNUSED"
+  }
+
 /// The SDK root path on macOS.
 ///
 /// - Precondition: running on macOS
 private func sdkRootPath() throws -> String {
   precondition(isMacOS)
-  let path = try runProcessForOutput("/usr/bin/xcrun", arguments: ["--show-sdk-path"])
+  let path = try standardOutputOf(["/usr/bin/xcrun", "--show-sdk-path"])
   if path.isEmpty { throw Failure("'xcrun --show-sdk-path' returned empty string") }
   return path
 }
 
-/// Returns a `Process` that runs `command` with the given `arguments`.
+/// Output from running a process.
+struct ProcessResults {
+  /// Standard output as UTF-8 string.
+  let stdout: String
+  /// Standard error as UTF-8 string.
+  let stderr: String
+  /// Process exit status.
+  let exitStatus: Int32
+}
+
+/// Returns the result of running `command` passing `arguments`.
+///
+/// If `commandLine[0]` contains no path separators, the executable is
+/// looked up in `PATH`.
 ///
-/// If `command` contains no path separators, it will be found in
-/// `PATH`.  On macOS, ensures SDKROOT is set in the environment in
-/// case the command needs it.
-func processForCommand(_ command: String, arguments: [String]) throws -> Process {
+/// - Parameter setSKDRoot: true iff xcrun should be used to set the SDKROOT
+///   environment variable for the process.
+func resultsOfRunning(_ commandLine: [String], setSDKRoot: Bool = true) throws -> ProcessResults {
   let p = Process()
 
-  p.arguments = arguments
-  // If command contains path separators, use it directly without PATH search
+  let command = commandLine.first!
+  var arguments = Array(commandLine.dropFirst())
+
+  // If executable contains path separators, use it directly without PATH search
   if command.contains(isWindows ? "\\" : "/") {
     p.executableURL = URL(fileURLWithPath: command)
   } else {
@@ -107,44 +144,24 @@ func processForCommand(_ command: String, arguments: [String]) throws -> Process
     } else {
       // Let env find and run the executable.
       p.executableURL = URL(fileURLWithPath: "/usr/bin/env")
-      p.arguments = [command] + arguments
+      arguments.insert(command, at: 0)
     }
   }
+  p.arguments = Array(arguments)
 
-  // Set SDKROOT on macOS if needed, but skip for xcrun itself to prevent infinite recursion
-  let isXcrun = command.hasSuffix("/xcrun") || command == "xcrun"
-  if isMacOS && !isXcrun {
-    var environment = ProcessInfo.processInfo.environment
-    if environment["SDKROOT"] == nil {
-      environment["SDKROOT"] = try sdkRootPath()
-      p.environment = environment
-    }
+  if setSDKRoot && isMacOS {
+    var e = ProcessInfo.processInfo.environment
+    e["SDKROOT"] = swiftSDKRoot
+    p.environment = e
   }
 
-  return p
-}
-
-/// Output from running a process.
-struct ProcessOutput {
-  /// Standard output as UTF-8 string.
-  let stdout: String
-  /// Standard error as UTF-8 string.
-  let stderr: String
-  /// Process exit status.
-  let exitStatus: Int32
-}
-
-/// Runs `command` with `arguments`, returning captured output and exit status.
-func runProcess(_ command: String, arguments: [String]) throws -> ProcessOutput {
-  let process = try processForCommand(command, arguments: arguments)
-
   let stdoutPipe = Pipe()
   let stderrPipe = Pipe()
-  process.standardOutput = stdoutPipe
-  process.standardError = stderrPipe
+  p.standardOutput = stdoutPipe
+  p.standardError = stderrPipe
 
   do {
-    try process.run()
+    try p.run()
   } catch {
     let commandLine =
       arguments.isEmpty
@@ -158,16 +175,16 @@ func runProcess(_ command: String, arguments: [String]) throws -> ProcessOutput
   let stdoutData = readPipeInBackground(stdoutPipe)
   let stderrData = readPipeInBackground(stderrPipe)
 
-  process.waitUntilExit()
+  p.waitUntilExit()
 
   // Retrieve the data (blocks until background reads complete)
   let stdout = try decodeUTF8(stdoutData(), as: "\(command) stdout")
   let stderr = try decodeUTF8(stderrData(), as: "\(command) stderr")
 
-  return ProcessOutput(
+  return ProcessResults(
     stdout: stdout,
     stderr: stderr,
-    exitStatus: process.terminationStatus
+    exitStatus: p.terminationStatus
   )
 }
 

Tests/gyb-swiftTests/SourceLocationTests.swift

@@ -128,7 +128,7 @@ private func runSwiftScript(
     // On Windows, atomically: true can cause file locking issues
     try swiftCode.write(toFile: tempFile, atomically: !isWindows, encoding: .utf8)
 
-    let result = try runProcess("swift", arguments: [tempFile])
+    let result = try resultsOfRunning(["swift", tempFile])
 
     if result.exitStatus != 0 {
       var diagnostics = "Exit code: \(result.exitStatus)"
@@ -222,7 +222,7 @@ func hasSyntaxErrors(_ code: String) -> Bool {
 @Test("Swift executable is accessible")
 func swiftExecutableAccessible() throws {
   do {
-    let result = try runProcess("swift", arguments: ["--version"])
+    let result = try resultsOfRunning(["swift", "--version"])
 
     print("Swift version check - Exit code: \(result.exitStatus)")
     if !result.stdout.isEmpty {