[Commands] Redirect output to stderr when using swift-run

aciidgh · aciidgh · commit b76046734963 · 2018-07-01T10:45:34.000-07:00
rdar://problem/34059859 SR-7823
diff --git a/Sources/Commands/Error.swift b/Sources/Commands/Error.swift
@@ -75,9 +75,15 @@ func print(error: Any) {
     writer.write("\n")
 }
 
-func print(diagnostic: Diagnostic) {
+func print(diagnostic: Diagnostic, stdoutStream: OutputByteStream) {
 
-    let writer = diagnostic.behavior == .note ? InteractiveWriter.stdout : InteractiveWriter.stderr
+    let writer: InteractiveWriter
+
+    if diagnostic.behavior == .note {
+        writer = InteractiveWriter(stream: stdoutStream)
+    } else {
+        writer = InteractiveWriter.stderr
+    }
 
     if !(diagnostic.location is UnknownLocation) {
         writer.write(diagnostic.location.localizedDescription)
diff --git a/Sources/Commands/SwiftRunTool.swift b/Sources/Commands/SwiftRunTool.swift
@@ -102,16 +102,18 @@ public class SwiftRunTool: SwiftTool<RunToolOptions> {
         case .run:
             // Detect deprecated uses of swift run to interpret scripts.
             if let executable = options.executable, isValidSwiftFilePath(executable) {
-                print(diagnostic: Diagnostic(
-                    location: UnknownLocation.location,
-                    data: RunFileDeprecatedDiagnostic()))
+                diagnostics.emit(data: RunFileDeprecatedDiagnostic())
                 // Redirect execution to the toolchain's swift executable.
                 let swiftInterpreterPath = try getToolchain().swiftInterpreter
                 // Prepend the script to interpret to the arguments.
                 let arguments = [executable] + options.arguments
                 try run(swiftInterpreterPath, arguments: arguments)
                 return
             }
+
+            // Redirect stdout to stderr because swift-run clients usually want
+            // to ignore swiftpm's output and only care about the tool's output.
+            self.redirectStdoutToStderr()
                     
             let plan = try BuildPlan(buildParameters: self.buildParameters(), graph: loadPackageGraph(), diagnostics: diagnostics)
             let product = try findProduct(in: plan.graph)
diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift
@@ -69,6 +69,13 @@ struct TargetNotFoundDiagnostic: DiagnosticData {
 
 private class ToolWorkspaceDelegate: WorkspaceDelegate {
 
+    /// The stream to use for reporting progress.
+    private let stdoutStream: OutputByteStream
+
+    init(_ stdoutStream: OutputByteStream) {
+        self.stdoutStream = stdoutStream
+    }
+
     func packageGraphWillLoad(
         currentGraph: PackageGraph,
         dependencies: AnySequence<ManagedDependency>,
@@ -77,39 +84,52 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
     }
 
     func fetchingWillBegin(repository: String) {
-        print("Fetching \(repository)")
+        stdoutStream <<< "Fetching \(repository)"
+        stdoutStream <<< "\n"
+        stdoutStream.flush()
     }
 
     func fetchingDidFinish(repository: String, diagnostic: Diagnostic?) {
     }
 
     func repositoryWillUpdate(_ repository: String) {
-        print("Updating \(repository)")
+        stdoutStream <<< "Updating \(repository)"
+        stdoutStream <<< "\n"
+        stdoutStream.flush()
     }
 
     func repositoryDidUpdate(_ repository: String) {
     }
     
     func dependenciesUpToDate() {
-        print("Everything is already up-to-date")
+        stdoutStream <<< "Everything is already up-to-date"
+        stdoutStream <<< "\n"
+        stdoutStream.flush()
     }
 
     func cloning(repository: String) {
-        print("Cloning \(repository)")
+        stdoutStream <<< "Cloning \(repository)"
+        stdoutStream <<< "\n"
+        stdoutStream.flush()
     }
 
     func checkingOut(repository: String, atReference reference: String, to path: AbsolutePath) {
-        // FIXME: This is temporary output similar to old one, we will need to figure
-        // out better reporting text.
-        print("Resolving \(repository) at \(reference)")
+        stdoutStream <<< "Resolving \(repository) at \(reference)"
+        stdoutStream <<< "\n"
+        stdoutStream.flush()
     }
 
     func removing(repository: String) {
-        print("Removing \(repository)")
+        stdoutStream <<< "Removing \(repository)"
+        stdoutStream <<< "\n"
+        stdoutStream.flush()
     }
 
     func warning(message: String) {
-        print("warning: " + message)
+        // FIXME: We should emit warnings through the diagnostic engine.
+        stdoutStream <<< "warning: " <<< message
+        stdoutStream <<< "\n"
+        stdoutStream.flush()
     }
 
     func managedDependenciesDidUpdate(_ dependencies: AnySequence<ManagedDependency>) {
@@ -165,6 +185,22 @@ final class BuildManifestRegenerationToken {
     }
 }
 
+/// Handler for the main DiagnosticsEngine used by the SwiftTool class.
+private final class DiagnosticsEngineHandler {
+
+    /// The standard output stream.
+    var stdoutStream = Basic.stdoutStream
+
+    /// The default instance.
+    static let `default` = DiagnosticsEngineHandler()
+
+    private init() {}
+
+    func diagnosticsHandler(_ diagnostic: Diagnostic) {
+        print(diagnostic: diagnostic, stdoutStream: stderrStream)
+    }
+}
+
 public class SwiftTool<Options: ToolOptions> {
     /// The original working directory.
     let originalWorkingDirectory: AbsolutePath
@@ -202,11 +238,19 @@ public class SwiftTool<Options: ToolOptions> {
     let interruptHandler: InterruptHandler
 
     /// The diagnostics engine.
-    let diagnostics = DiagnosticsEngine(handlers: [SwiftTool.diagnosticsHandler])
+    let diagnostics: DiagnosticsEngine = DiagnosticsEngine(
+        handlers: [DiagnosticsEngineHandler.default.diagnosticsHandler])
 
     /// The execution status of the tool.
     var executionStatus: ExecutionStatus = .success
 
+    /// The stream to print standard output on.
+    fileprivate var stdoutStream: OutputByteStream = Basic.stdoutStream
+
+    /// If true, Redirects the stdout stream to stderr when invoking
+    /// `swift-build-tool`.
+    private var shouldRedirectStdoutToStderr = false
+
     /// Create an instance of this tool.
     ///
     /// - parameter args: The command line arguments to be passed to this tool.
@@ -394,7 +438,7 @@ public class SwiftTool<Options: ToolOptions> {
         if let workspace = _workspace {
             return workspace
         }
-        let delegate = ToolWorkspaceDelegate()
+        let delegate = ToolWorkspaceDelegate(self.stdoutStream)
         let rootPackage = try getPackageRoot()
         let provider = GitRepositoryProvider(processSet: processSet)
         let workspace = Workspace(
@@ -431,10 +475,6 @@ public class SwiftTool<Options: ToolOptions> {
         SwiftTool.exit(with: executionStatus)
     }
 
-    static func diagnosticsHandler(_ diagnostic: Diagnostic) {
-        print(diagnostic: diagnostic)
-    }
-
     /// Exit the tool with the given execution status.
     private static func exit(with status: ExecutionStatus) -> Never {
         switch status {
@@ -448,6 +488,13 @@ public class SwiftTool<Options: ToolOptions> {
         fatalError("Must be implemented by subclasses")
     }
 
+    /// Start redirecting the standard output stream to the standard error stream.
+    func redirectStdoutToStderr() {
+        self.shouldRedirectStdoutToStderr = true
+        self.stdoutStream = Basic.stderrStream
+        DiagnosticsEngineHandler.default.stdoutStream = Basic.stdoutStream
+    }
+
     /// Resolve the dependencies.
     func resolve() throws {
         let workspace = try getActiveWorkspace()
@@ -601,11 +648,19 @@ public class SwiftTool<Options: ToolOptions> {
         env["TMPDIR"] = tempDir.asString
 
         // Run llbuild and print output on standard streams.
-        let process = Process(arguments: args, environment: env, redirectOutput: false)
+        let process = Process(arguments: args, environment: env, redirectOutput: shouldRedirectStdoutToStderr)
         try process.launch()
         try processSet.add(process)
         let result = try process.waitUntilExit()
 
+        // Emit the output to the selected stream if we need to redirect the
+        // stream.
+        if shouldRedirectStdoutToStderr {
+            self.stdoutStream <<< (try result.utf8stderrOutput())
+            self.stdoutStream <<< (try result.utf8Output())
+            self.stdoutStream.flush()
+        }
+
         guard result.exitStatus == .terminated(code: 0) else {
             throw ProcessResult.Error.nonZeroExit(result)
         }
diff --git a/Sources/TestSupport/SwiftPMProduct.swift b/Sources/TestSupport/SwiftPMProduct.swift
@@ -82,6 +82,31 @@ public enum SwiftPMProduct {
         env: [String: String]? = nil,
         printIfError: Bool = false
     ) throws -> String {
+
+        let result = try executeProcess(
+            args, packagePath: packagePath,
+            env: env)
+
+        let output = try result.utf8Output()
+        let stderr = try result.utf8stderrOutput()
+
+        if result.exitStatus == .terminated(code: 0) {
+            // FIXME: We should return stderr separately.
+            return output + stderr
+        }
+        throw SwiftPMProductError.executionFailure(
+            error: ProcessResult.Error.nonZeroExit(result),
+            output: output,
+            stderr: stderr
+        )
+    }
+
+    public func executeProcess(
+        _ args: [String],
+        packagePath: AbsolutePath? = nil,
+        env: [String: String]? = nil
+    ) throws -> ProcessResult {
+
         var environment = ProcessInfo.processInfo.environment
         for (key, value) in (env ?? [:]) {
             environment[key] = value
@@ -102,25 +127,7 @@ public enum SwiftPMProduct {
         }
         completeArgs += args
 
-        let result = try Process.popen(arguments: completeArgs, environment: environment)
-        let output = try result.utf8Output()
-        let stderr = try result.utf8stderrOutput()
-
-        if result.exitStatus == .terminated(code: 0) {
-            // FIXME: We should return stderr separately.
-            return output + stderr
-        }
-        if printIfError {
-            print("**** FAILURE EXECUTING SUBPROCESS ****")
-            print("command: " + completeArgs.map({ $0.shellEscaped() }).joined(separator: " "))
-            print("SWIFT_EXEC:", environment["SWIFT_EXEC"] ?? "nil")
-            print("output:", output)
-        }
-        throw SwiftPMProductError.executionFailure(
-            error: ProcessResult.Error.nonZeroExit(result),
-            output: output,
-            stderr: stderr
-        )
+        return try Process.popen(arguments: completeArgs, environment: environment)
     }
 
     public static func packagePath(for packageName: String, packageRoot: AbsolutePath) throws -> AbsolutePath {
diff --git a/Tests/CommandsTests/RunToolTests.swift b/Tests/CommandsTests/RunToolTests.swift
@@ -34,18 +34,25 @@ final class RunToolTests: XCTestCase {
 
     func testUnkownProductAndArgumentPassing() throws {
         fixture(name: "Miscellaneous/EchoExecutable") { path in
+
+            let result = try SwiftPMProduct.SwiftRun.executeProcess(
+                ["secho", "1", "--hello", "world"], packagePath: path)
+
+            // We only expect tool's output on the stdout stream.
+            XCTAssertMatch(try result.utf8Output(), .contains("""
+                "1" "--hello" "world"
+                """))
+
+            // swift-build-tool output should go to stderr.
+            XCTAssertMatch(try result.utf8stderrOutput(), .contains("Compile Swift Module"))
+            XCTAssertMatch(try result.utf8stderrOutput(), .contains("Linking"))
+
             do {
                 _ = try execute(["unknown"], packagePath: path)
                 XCTFail("Unexpected success")
             } catch SwiftPMProductError.executionFailure(_, _, let stderr) {
                 XCTAssertEqual(stderr, "error: no executable product named 'unknown'\n")
             }
-
-            let runOutput = try execute(["secho", "1", "--hello", "world"], packagePath: path)
-            let outputLines = runOutput.split(separator: "\n")
-            XCTAssertEqual(String(outputLines.last!), """
-                "\(getcwd())" "1" "--hello" "world"
-                """)
         }
     }
 
@@ -59,19 +66,18 @@ final class RunToolTests: XCTestCase {
             }
             
             var runOutput = try execute(["exec1"], packagePath: path)
-            var outputLines = runOutput.split(separator: "\n")
-            XCTAssertEqual(outputLines.last!, "1")
+            XCTAssertMatch(runOutput, .contains("1"))
+
             runOutput = try execute(["exec2"], packagePath: path)
-            outputLines = runOutput.split(separator: "\n")
-            XCTAssertEqual(outputLines.last!, "2")
+            XCTAssertMatch(runOutput, .contains("2"))
         }
     }
 
     func testUnreachableExecutable() throws {
         fixture(name: "Miscellaneous/UnreachableTargets") { path in
             let output = try execute(["bexec"], packagePath: path.appending(component: "A"))
             let outputLines = output.split(separator: "\n")
-            XCTAssertEqual(outputLines.last!, "BTarget2")
+            XCTAssertEqual(outputLines.first, "BTarget2")
         }
     }
 
