Output stderr for failed commands. Closes #629 #840 (#847)

peripheryapp · Dec 18, 2024 · e00a816 · e00a816
1 parent 3f0ac59
commit e00a816
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 31 deletions.
diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock
diff --git a/Sources/ProjectDrivers/BazelProjectDriver.swift b/Sources/ProjectDrivers/BazelProjectDriver.swift
@@ -123,7 +123,7 @@ public class BazelProjectDriver: ProjectDriver {
 
     private func queryTargets() throws -> [String] {
         try shell
-            .exec(["bazel", "query", query], stderr: false)
+            .exec(["bazel", "query", query])
             .split(separator: "\n")
             .map { "\"@@\($0)\"" }
     }

diff --git a/Sources/ProjectDrivers/SPM.swift b/Sources/ProjectDrivers/SPM.swift
@@ -46,7 +46,7 @@ public enum SPM {
             if let path = configuration.jsonPackageManifestPath {
                 jsonData = try Data(contentsOf: path.url)
             } else {
-                let jsonString = try shell.exec(["swift", "package", "describe", "--type", "json"], stderr: false)
+                let jsonString = try shell.exec(["swift", "package", "describe", "--type", "json"])
 
                 guard let data = jsonString.data(using: .utf8) else {
                     throw PeripheryError.packageError(message: "Failed to read swift package description.")

diff --git a/Sources/Shared/Shell.swift b/Sources/Shared/Shell.swift
@@ -39,40 +39,32 @@ open class Shell {
     }
 
     @discardableResult
-    open func exec(
-        _ args: [String],
-        stderr: Bool = true
-    ) throws -> String {
-        let (status, output) = try exec(args, environment: environment, stderr: stderr)
+    open func exec(_ args: [String]) throws -> String {
+        let (status, stdout, stderr) = try exec(args)
 
         if status == 0 {
-            return output
+            return stdout
         }
 
         throw PeripheryError.shellCommandFailed(
             cmd: args,
             status: status,
-            output: output
+            output: [stdout, stderr].filter { !$0.isEmpty }.joined(separator: "\n").trimmed
         )
     }
 
     @discardableResult
-    open func execStatus(
-        _ args: [String],
-        stderr: Bool = true
-    ) throws -> Int32 {
-        let (status, _) = try exec(args, environment: environment, stderr: stderr, captureOutput: false)
+    open func execStatus(_ args: [String]) throws -> Int32 {
+        let (status, _, _) = try exec(args, captureOutput: false)
         return status
     }
 
     // MARK: - Private
 
     private func exec(
         _ args: [String],
-        environment: [String: String],
-        stderr: Bool = false,
         captureOutput: Bool = true
-    ) throws -> (Int32, String) {
+    ) throws -> (Int32, String, String) {
         let launchPath: String
         let newArgs: [String]
 
@@ -92,34 +84,49 @@ open class Shell {
         logger.debug("\(launchPath) \(newArgs.joined(separator: " "))")
         ShellProcessStore.shared.add(process)
 
-        var outputPipe: Pipe?
+        var stdoutPipe: Pipe?
+        var stderrPipe: Pipe?
 
         if captureOutput {
-            outputPipe = Pipe()
-            process.standardOutput = outputPipe
-            process.standardError = stderr ? outputPipe : nil
+            stdoutPipe = Pipe()
+            stderrPipe = Pipe()
+            process.standardOutput = stdoutPipe
+            process.standardError = stderrPipe
         }
 
         process.launch()
 
-        var output = ""
+        var stdout = ""
+        var stderr = ""
+
+        if let stdoutData = try stdoutPipe?.fileHandleForReading.readToEnd() {
+            guard let stdoutStr = String(data: stdoutData, encoding: .utf8)
+            else {
+                ShellProcessStore.shared.remove(process)
+                throw PeripheryError.shellOutputEncodingFailed(
+                    cmd: launchPath,
+                    args: newArgs,
+                    encoding: .utf8
+                )
+            }
+            stdout = stdoutStr
+        }
 
-        if let outputPipe,
-           let outputData = try outputPipe.fileHandleForReading.readToEnd()
-        {
-            guard let str = String(data: outputData, encoding: .utf8) else {
+        if let stderrData = try stderrPipe?.fileHandleForReading.readToEnd() {
+            guard let stderrStr = String(data: stderrData, encoding: .utf8)
+            else {
                 ShellProcessStore.shared.remove(process)
                 throw PeripheryError.shellOutputEncodingFailed(
                     cmd: launchPath,
                     args: newArgs,
                     encoding: .utf8
                 )
             }
-            output = str
+            stderr = stderrStr
         }
 
         process.waitUntilExit()
         ShellProcessStore.shared.remove(process)
-        return (process.terminationStatus, output)
+        return (process.terminationStatus, stdout, stderr)
     }
 }
diff --git a/Sources/XcodeSupport/Xcodebuild.swift b/Sources/XcodeSupport/Xcodebuild.swift
@@ -86,7 +86,7 @@ public final class Xcodebuild {
 
         let quotedArguments = quote(arguments: additionalArguments)
         let xcodebuild = "xcodebuild \((args + quotedArguments).joined(separator: " "))"
-        let lines = try shell.exec(["/bin/sh", "-c", xcodebuild], stderr: false).split(separator: "\n").map { String($0).trimmed }
+        let lines = try shell.exec(["/bin/sh", "-c", xcodebuild]).split(separator: "\n").map { String($0).trimmed }
 
         // xcodebuild may output unrelated warnings, we need to strip them out otherwise
         // JSON parsing will fail.

diff --git a/Tests/XcodeTests/ShellMock.swift b/Tests/XcodeTests/ShellMock.swift
@@ -10,7 +10,7 @@ class ShellMock: Shell {
         self.init(environment: ProcessInfo.processInfo.environment, logger: logger)
     }
 
-    override func exec(_: [String], stderr _: Bool = true) throws -> String {
+    override func exec(_: [String]) throws -> String {
         output
     }
 }