refactor stream handling (#3729)

motivation: consolidate and clarify which output stream is used throught the code

changes:
* reanme all local variables called "stdoutStream" to "outputStream" to differenciate with the global stdoutStream from TSC
* audit code for use of stdoutStream and replace with a configurable outputStream so that we can centraliy control the output
* clean up diagnostics print code

Author: tomerd

Sources/Build/BuildOperation.swift

@@ -52,8 +52,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     /// The loaded package graph.
     private var packageGraph: PackageGraph?
 
-    /// The stdout stream for the build delegate.
-    let stdoutStream: OutputByteStream
+    /// The output stream for the build delegate.
+    let outputStream: OutputByteStream
 
     public var builtTestProducts: [BuiltTestProduct] {
         (try? getBuildDescription())?.builtTestProducts ?? []
@@ -65,14 +65,14 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         packageGraphLoader: @escaping () throws -> PackageGraph,
         pluginInvoker: @escaping (PackageGraph) throws -> [ResolvedTarget: [PluginInvocationResult]],
         diagnostics: DiagnosticsEngine,
-        stdoutStream: OutputByteStream
+        outputStream: OutputByteStream
     ) {
         self.buildParameters = buildParameters
         self.cacheBuildManifest = cacheBuildManifest
         self.packageGraphLoader = packageGraphLoader
         self.pluginInvoker = pluginInvoker
         self.diagnostics = diagnostics
-        self.stdoutStream = stdoutStream
+        self.outputStream = outputStream
     }
 
     public func getPackageGraph() throws -> PackageGraph {
@@ -121,7 +121,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     /// Perform a build using the given build description and subset.
     public func build(subset: BuildSubset) throws {
         // Create the build system.
-        let buildSystem = try createBuildSystem(with: getBuildDescription())
+        let buildDescription = try self.getBuildDescription()
+        let buildSystem = try createBuildSystem(with: buildDescription)
         self.buildSystem = buildSystem
 
         // Perform the build.
@@ -194,9 +195,21 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             diagnostics: diagnostics
         )
         self.buildPlan = plan
+
+        let (buildDescription, buildManifest) = try BuildDescription.create(with: plan)
+
+        // FIXME: ideally this would be done outside of the planning phase,
+        // but it would require deeper changes in how we serialize BuildDescription
+        // Output a dot graph
+        if buildParameters.printManifestGraphviz {
+            // FIXME: this seems like the wrong place to print
+            var serializer = DOTManifestSerializer(manifest: buildManifest)
+            serializer.writeDOT(to: self.outputStream)
+            self.outputStream.flush()
+        }
 
         // Finally create the llbuild manifest from the plan.
-        return try BuildDescription.create(with: plan)
+        return buildDescription
     }
 
     /// Build the package structure target.
@@ -220,8 +233,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         // Figure out which progress bar we have to use during the build.
         let isVerbose = verbosity != .concise
         let progressAnimation: ProgressAnimationProtocol = isVerbose
-            ? MultiLineNinjaProgressAnimation(stream: self.stdoutStream)
-            : NinjaProgressAnimation(stream: self.stdoutStream)
+            ? MultiLineNinjaProgressAnimation(stream: self.outputStream)
+            : NinjaProgressAnimation(stream: self.outputStream)
 
         let bctx = BuildExecutionContext(
             buildParameters,
@@ -235,7 +248,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             buildSystem: self,
             bctx: bctx,
             diagnostics: diagnostics,
-            outputStream: self.stdoutStream,
+            outputStream: self.outputStream,
             progressAnimation: progressAnimation,
             delegate: self.delegate
         )
@@ -294,10 +307,10 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
 }
 
 extension BuildDescription {
-    static func create(with plan: BuildPlan) throws -> BuildDescription {
+    static func create(with plan: BuildPlan) throws -> (BuildDescription, BuildManifest) {
         // Generate the llbuild manifest.
         let llbuild = LLBuildManifestBuilder(plan)
-        try llbuild.generateManifest(at: plan.buildParameters.llbuildManifest)
+        let buildManifest = try llbuild.generateManifest(at: plan.buildParameters.llbuildManifest)
 
         let swiftCommands = llbuild.manifest.getCmdToolMap(kind: SwiftCompilerTool.self)
         let swiftFrontendCommands = llbuild.manifest.getCmdToolMap(kind: SwiftFrontendTool.self)
@@ -317,7 +330,7 @@ extension BuildDescription {
             recursive: true
         )
         try buildDescription.write(to: plan.buildParameters.buildDescriptionPath)
-        return buildDescription
+        return (buildDescription, buildManifest)
     }
 }
 

Sources/Build/BuildPlan.swift

@@ -35,14 +35,6 @@ extension BuildParameters {
         return buildPath.appending(component: "ModuleCache")
     }
 
-    /// Checks if stdout stream is tty.
-    fileprivate static var isTTY: Bool = {
-        guard let stream = stdoutStream.stream as? LocalFileOutputByteStream else {
-            return false
-        }
-        return TerminalController.isTTY(stream)
-    }()
-
     /// Extra flags to pass to Swift compiler.
     public var swiftCompilerFlags: [String] {
         var flags = self.flags.cCompilerFlags.flatMap({ ["-Xcc", $0] })
@@ -779,7 +771,7 @@ public final class SwiftTargetBuildDescription {
         }
 
         // Add arguments to colorize output if stdout is tty
-        if BuildParameters.isTTY {
+        if buildParameters.isTTY {
             args += ["-color-diagnostics"]
         }
 

Sources/Build/ManifestBuilder.swift

@@ -49,7 +49,7 @@ public class LLBuildManifestBuilder {
 
     // MARK:- Generate Manifest
     /// Generate manifest at the given path.
-    public func generateManifest(at path: AbsolutePath) throws {
+    public func generateManifest(at path: AbsolutePath) throws -> BuildManifest {
         manifest.createTarget(TargetKind.main.targetName)
         manifest.createTarget(TargetKind.test.targetName)
         manifest.defaultTarget = TargetKind.main.targetName
@@ -82,14 +82,8 @@ public class LLBuildManifestBuilder {
             try self.createProductCommand(description)
         }
 
-        // Output a dot graph
-        if buildParameters.printManifestGraphviz {
-            var serializer = DOTManifestSerializer(manifest: manifest)
-            serializer.writeDOT(to: &stdoutStream)
-            stdoutStream.flush()
-        }
-
         try ManifestWriter().write(manifest, at: path)
+        return manifest
     }
 
     func addNode(_ node: Node, toTarget targetKind: TargetKind) {

Sources/Commands/APIDigester.swift

@@ -54,9 +54,12 @@ struct APIDigesterBaselineDumper {
     }
 
     /// Emit the API baseline files and return the path to their directory.
-    func emitAPIBaseline(for modulesToDiff: Set<String>,
-                         at baselineDir: AbsolutePath?,
-                         force: Bool) throws -> AbsolutePath {
+    func emitAPIBaseline(
+        for modulesToDiff: Set<String>,
+        at baselineDir: AbsolutePath?,
+        force: Bool,
+        outputStream: OutputByteStream
+    ) throws -> AbsolutePath {
         var modulesToDiff = modulesToDiff
         let apiDiffDir = inputBuildParameters.apiDiff
         let baselineDir = (baselineDir ?? apiDiffDir).appending(component: baselineRevision.identifier)
@@ -121,7 +124,7 @@ struct APIDigesterBaselineDumper {
             packageGraphLoader: { graph },
             pluginInvoker: { _ in [:] },
             diagnostics: diags,
-            stdoutStream: stdoutStream
+            outputStream: outputStream
         )
 
         try buildOp.build()

Sources/Commands/CMakeLists.txt

@@ -9,7 +9,6 @@
 add_library(Commands
   APIDigester.swift
   Describe.swift
-  Error.swift
   GenerateLinuxMain.swift
   MultiRootSupport.swift
   Options.swift

Sources/Commands/Error.swift

@@ -35,7 +35,7 @@ struct CardStack {
     private var needsToClearScreen = true
 
     init(package: ResolvedPackage, snippetGroups: [SnippetGroup], swiftTool: SwiftTool) {
-        self.terminal = TerminalController(stream: stdoutStream)!
+        self.terminal = TerminalController(stream: swiftTool.outputStream)!
         self.cards = [TopCard(package: package, snippetGroups: snippetGroups, swiftTool: swiftTool)]
         self.swiftTool = swiftTool
     }