refactor LLBuildManifest graphviz file visualization flow (#5576)

motivation: code cleanup

changes:
* move the code that prints the LLBuildManifest graphviz visualization to stdout from the build system to the build tool, which is where printg should be done
* remove printManifestGraphviz from BuildParameters as it was only necessary to drive outputing the visulaization from the build system
* move the LLBuild manifest dserializer to the Commands  module, with the rest of the user facing output serilizers
* create Utilities directory in the Commands module and move the various utilities there to better organize the code
* rename dependencies_serializer to DependenciesSerializer

Author: tomerd

Sources/Build/BuildOperation.swift

@@ -143,10 +143,14 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
                 }
             }
             // We need to perform actual planning if we reach here.
-            return try self.plan()
+            return try self.plan().description
         }
     }
 
+    public func getBuildManifest() throws -> LLBuildManifest.BuildManifest {
+        return try self.plan().manifest
+    }
+
     /// Cancel the active build operation.
     public func cancel(deadline: DispatchTime) throws {
         buildSystem?.cancel()
@@ -283,7 +287,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     }
 
     /// Create the build plan and return the build description.
-    private func plan() throws -> BuildDescription {
+    private func plan() throws -> (description: BuildDescription, manifest: LLBuildManifest.BuildManifest) {
         // Load the package graph.
         let graph = try getPackageGraph()
 
@@ -372,18 +376,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             observabilityScope: self.observabilityScope
         )
 
-        // 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 buildDescription
+        return (buildDescription, buildManifest)
     }
 
     /// Build the package structure target.
@@ -519,7 +513,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
 }
 
 extension BuildDescription {
-    static func create(with plan: BuildPlan, disableSandboxForPluginCommands: Bool, fileSystem: TSCBasic.FileSystem, observabilityScope: ObservabilityScope) throws -> (BuildDescription, BuildManifest) {
+    static func create(with plan: BuildPlan, disableSandboxForPluginCommands: Bool, fileSystem: TSCBasic.FileSystem, observabilityScope: ObservabilityScope) throws -> (BuildDescription, LLBuildManifest.BuildManifest) {
         // Generate the llbuild manifest.
         let llbuild = LLBuildManifestBuilder(plan, disableSandboxForPluginCommands: disableSandboxForPluginCommands, fileSystem: fileSystem, observabilityScope: observabilityScope)
         let buildManifest = try llbuild.generateManifest(at: plan.buildParameters.llbuildManifest)

Sources/Commands/CMakeLists.txt

@@ -7,12 +7,8 @@
 # See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 
 add_library(Commands
-  APIDigester.swift
-  Describe.swift
-  GenerateLinuxMain.swift
-  MultiRootSupport.swift
+
   Options.swift
-  show-dependencies.swift
   Snippets/CardEvent.swift
   Snippets/Cards/SnippetCard.swift
   Snippets/Cards/SnippetGroupCard.swift
@@ -27,9 +23,15 @@ add_library(Commands
   SwiftRunTool.swift
   SwiftTestTool.swift
   SwiftTool.swift
-  SymbolGraphExtract.swift
-  TestingSupport.swift
-  WatchmanHelper.swift)
+  Utilities/APIDigester.swift
+  Utilities/DependenciesSerializer.swift
+  Utilities/Describe.swift
+  Utilities/DOTManifestSerializer.swift
+  Utilities/GenerateLinuxMain.swift
+  Utilities/MultiRootSupport.swift
+  Utilities/SymbolGraphExtract.swift
+  Utilities/TestingSupport.swift
+  Utilities/WatchmanHelper.swift)
 target_link_libraries(Commands PUBLIC
   ArgumentParser
   Basics

Sources/Commands/Options.swift

@@ -339,12 +339,6 @@ struct BuildOptions: ParsableArguments {
     @Flag(name: .customLong("experimental-explicit-module-build"))
     var useExplicitModuleBuild: Bool = false
 
-    /// Whether to output a graphviz file visualization of the combined job graph for all targets
-    @Flag(
-        name: .customLong("print-manifest-job-graph"),
-        help: "Write the command graph for the build manifest as a graphviz file")
-    var printManifestGraphviz: Bool = false
-
     /// The build system to use.
     @Option(name: .customLong("build-system"))
     var _buildSystem: BuildSystemKind = .native

Sources/Commands/SwiftBuildTool.swift

@@ -69,6 +69,11 @@ struct BuildToolOptions: ParsableArguments {
     @Flag(name: .customLong("show-bin-path"), help: "Print the binary output path")
     var shouldPrintBinPath: Bool = false
 
+    /// Whether to output a graphviz file visualization of the combined job graph for all targets
+    @Flag(name: .customLong("print-manifest-job-graph"),
+          help: "Write the command graph for the build manifest as a graphviz file")
+    var printManifestGraphviz: Bool = false
+
     /// Specific target to build.
     @Option(help: "Build the specified target")
     var target: String?
@@ -96,7 +101,17 @@ public struct SwiftBuildTool: SwiftCommand {
 
     public func run(_ swiftTool: SwiftTool) throws {
         if options.shouldPrintBinPath {
-            try print(swiftTool.buildParameters().buildPath.description)
+            return try print(swiftTool.buildParameters().buildPath.description)
+        }
+
+        if options.printManifestGraphviz {
+            let buildOperation = try swiftTool.createBuildOperation()
+            let buildManifest = try buildOperation.getBuildManifest()
+            var serializer = DOTManifestSerializer(manifest: buildManifest)
+            // print to stdout
+            let outputStream = stdoutStream
+            serializer.writeDOT(to: outputStream)
+            outputStream.flush()
             return
         }
 

Sources/Commands/SwiftTool.swift

@@ -843,7 +843,6 @@ public class SwiftTool {
                 useIntegratedSwiftDriver: options.build.useIntegratedSwiftDriver,
                 useExplicitModuleBuild: options.build.useExplicitModuleBuild,
                 isXcodeBuildSystemEnabled: options.build.buildSystem == .xcode,
-                printManifestGraphviz: options.build.printManifestGraphviz,
                 forceTestDiscovery: options.build.enableTestDiscovery, // backwards compatibility, remove with --enable-test-discovery
                 linkerDeadStrip: options.linker.linkerDeadStrip,
                 verboseOutput: self.logLevel <= .info

Sources/Commands/APIDigester.swift renamed to Sources/Commands/Utilities/APIDigester.swift

@@ -10,16 +10,17 @@
 //
 //===----------------------------------------------------------------------===//
 
+import LLBuildManifest
 import TSCBasic
 
 /// Serializes an LLBuildManifest graph to a .dot file
 public struct DOTManifestSerializer {
     var kindCounter = [String: Int]()
     var hasEmittedStyling = Set<String>()
-    let manifest: BuildManifest
+    let manifest: LLBuildManifest.BuildManifest
 
     /// Creates a serializer that will serialize the given manifest.
-    public init(manifest: BuildManifest) {
+    public init(manifest: LLBuildManifest.BuildManifest) {
         self.manifest = manifest
     }
 

Sources/LLBuildManifest/DOTManifestSerializer.swift renamed to Sources/Commands/Utilities/DOTManifestSerializer.swift

@@ -9,8 +9,7 @@
 add_library(LLBuildManifest STATIC
   BuildManifest.swift
   Command.swift
-  ManifestWriter.swift
-  DOTManifestSerializer.swift
+  ManifestWriter.swift  
   Node.swift
   Target.swift
   Tools.swift)

Sources/Commands/show-dependencies.swift renamed to Sources/Commands/Utilities/DependenciesSerializer.swift

@@ -128,9 +128,6 @@ public struct BuildParameters: Encodable {
     /// Whether to use the explicit module build flow (with the integrated driver)
     public var useExplicitModuleBuild: Bool
 
-    /// Whether to output a graphviz file visualization of the combined job graph for all targets
-    public var printManifestGraphviz: Bool
-
     /// Whether to create dylibs for dynamic library products.
     public var shouldCreateDylibForDynamicProducts: Bool
 
@@ -200,7 +197,6 @@ public struct BuildParameters: Encodable {
         useIntegratedSwiftDriver: Bool = false,
         useExplicitModuleBuild: Bool = false,
         isXcodeBuildSystemEnabled: Bool = false,
-        printManifestGraphviz: Bool = false,
         enableTestability: Bool? = nil,
         forceTestDiscovery: Bool = false,
         linkerDeadStrip: Bool = true,
@@ -230,7 +226,6 @@ public struct BuildParameters: Encodable {
         self.useIntegratedSwiftDriver = useIntegratedSwiftDriver
         self.useExplicitModuleBuild = useExplicitModuleBuild
         self.isXcodeBuildSystemEnabled = isXcodeBuildSystemEnabled
-        self.printManifestGraphviz = printManifestGraphviz
         // decide on testability based on debug/release config
         // the goals of this being based on the build configuration is
         // that `swift build` followed by a `swift test` will need to do minimal rebuilding

Sources/Commands/Describe.swift renamed to Sources/Commands/Utilities/Describe.swift

@@ -334,4 +334,10 @@ final class BuildToolTests: CommandsTestCase {
         }
     }
 
+    func testPrintLLBuildManifestJobGraph() throws {
+        try fixture(name: "DependencyResolution/Internal/Simple") { fixturePath in
+            let output = try execute(["--print-manifest-job-graph"], packagePath: fixturePath).stdout
+            XCTAssertMatch(output, .prefix("digraph Jobs {"))
+        }
+    }
 }