BuildSystem delegate API (#3209)

* Rename public BuildDelegate to internal BuildOperationDelegate
* Make BuildOperationDelegate internal class
* Setup delegate links
* Outline delegate protocol
* Call BuildSystemDelegate methods
* Add buildSystemDidCancel call
* Report progress updates
* Set delegate at any point
* Public default delegate implementation
* Explicit module name
* Remove module name from function parameters
* Track progress update with raw messages
* Call callback on delegate queue
* Implement BuildSystemDelegate for XCBuildSystem
* Clarify in the comment what build system is referred
* Update CMake scripts with new files
* Disambiguate type

Author: krzyzanowskim

Sources/Build/BuildOperation.swift

@@ -19,6 +19,9 @@ import TSCUtility
 
 public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildSystem, BuildErrorAdviceProvider {
 
+    /// The delegate used by the build system.
+    public weak var delegate: SPMBuildCore.BuildSystemDelegate?
+
     /// The build parameters.
     public let buildParameters: BuildParameters
 
@@ -28,10 +31,10 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     /// The closure for loading the package graph.
     let packageGraphLoader: () throws -> PackageGraph
 
-    /// The build delegate reference.
-    private var buildDelegate: BuildDelegate?
+    /// The llbuild build delegate reference.
+    private var buildSystemDelegate: BuildOperationBuildSystemDelegateHandler?
 
-    /// The build system reference.
+    /// The llbuild build system reference.
     private var buildSystem: SPMLLBuild.BuildSystem?
 
     /// If build manifest caching should be enabled.
@@ -116,7 +119,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         let llbuildTarget = try computeLLBuildTargetName(for: subset)
         let success = buildSystem.build(target: llbuildTarget)
 
-        buildDelegate?.progressAnimation.complete(success: success)
+        buildSystemDelegate?.progressAnimation.complete(success: success)
+        delegate?.buildSystem(self, didFinishWithResult: success)
         guard success else { throw Diagnostics.fatalError }
 
         // Create backwards-compatibilty symlink to old build path.
@@ -195,24 +199,27 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         )
 
         // Create the build delegate.
-        let buildDelegate = BuildDelegate(
+        let buildSystemDelegate = BuildOperationBuildSystemDelegateHandler(
+            buildSystem: self,
             bctx: bctx,
             diagnostics: diagnostics,
             outputStream: self.stdoutStream,
-            progressAnimation: progressAnimation
+            progressAnimation: progressAnimation,
+            delegate: self.delegate
         )
-        self.buildDelegate = buildDelegate
-        buildDelegate.isVerbose = isVerbose
+        self.buildSystemDelegate = buildSystemDelegate
+        buildSystemDelegate.isVerbose = isVerbose
 
         let databasePath = buildParameters.dataPath.appending(component: "build.db").pathString
-        let buildSystem = BuildSystem(
+        let buildSystem = SPMLLBuild.BuildSystem(
             buildFile: buildParameters.llbuildManifest.pathString,
             databaseFile: databasePath,
-            delegate: buildDelegate,
+            delegate: buildSystemDelegate,
             schedulerLanes: buildParameters.jobs
         )
-        buildDelegate.onCommmandFailure = {
+        buildSystemDelegate.onCommmandFailure = {
             buildSystem.cancel()
+            self.delegate?.buildSystemDidCancel(self)
         }
 
         return buildSystem

Sources/Build/BuildDelegate.swift renamed to Sources/Build/BuildOperationBuildSystemDelegateHandler.swift

@@ -355,12 +355,15 @@ final class CopyCommand: CustomLLBuildCommand {
     }
 }
 
-public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParserDelegate {
+/// Convenient llbuild build system delegate implementation
+final class BuildOperationBuildSystemDelegateHandler: llbuildSwift.BuildSystemDelegate, SwiftCompilerOutputParserDelegate {
     private let diagnostics: DiagnosticsEngine
-    public var outputStream: ThreadSafeOutputByteStream
-    public var progressAnimation: ProgressAnimationProtocol
-    public var onCommmandFailure: (() -> Void)?
-    public var isVerbose: Bool = false
+    var outputStream: ThreadSafeOutputByteStream
+    var progressAnimation: ProgressAnimationProtocol
+    var onCommmandFailure: (() -> Void)?
+    var isVerbose: Bool = false
+    weak var delegate: SPMBuildCore.BuildSystemDelegate?
+    private let buildSystem: SPMBuildCore.BuildSystem
     private let queue = DispatchQueue(label: "org.swift.swiftpm.build-delegate")
     private var taskTracker = CommandTaskTracker()
     private var errorMessagesByTarget: [String: [String]] = [:]
@@ -371,30 +374,42 @@ public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParser
     /// The build execution context.
     private let buildExecutionContext: BuildExecutionContext
 
-    public init(
+    init(
+        buildSystem: SPMBuildCore.BuildSystem,
         bctx: BuildExecutionContext,
         diagnostics: DiagnosticsEngine,
         outputStream: OutputByteStream,
-        progressAnimation: ProgressAnimationProtocol
+        progressAnimation: ProgressAnimationProtocol,
+        delegate: SPMBuildCore.BuildSystemDelegate?
     ) {
         self.diagnostics = diagnostics
         // FIXME: Implement a class convenience initializer that does this once they are supported
         // https://forums.swift.org/t/allow-self-x-in-class-convenience-initializers/15924
         self.outputStream = outputStream as? ThreadSafeOutputByteStream ?? ThreadSafeOutputByteStream(outputStream)
         self.progressAnimation = progressAnimation
         self.buildExecutionContext = bctx
+        self.delegate = delegate
+        self.buildSystem = buildSystem
 
         let swiftParsers = bctx.buildDescription?.swiftCommands.mapValues { tool in
             SwiftCompilerOutputParser(targetName: tool.moduleName, delegate: self)
         } ?? [:]
         self.swiftParsers = swiftParsers
+
+        self.taskTracker.onTaskProgressUpdateText = { progressText, _ in
+            self.queue.async {
+                self.delegate?.buildSystem(self.buildSystem, didUpdateTaskProgress: progressText)
+            }
+        }
     }
 
-    public var fs: SPMLLBuild.FileSystem? {
+    // MARK: llbuildSwift.BuildSystemDelegate
+
+    var fs: SPMLLBuild.FileSystem? {
         return nil
     }
 
-    public func lookupTool(_ name: String) -> Tool? {
+    func lookupTool(_ name: String) -> Tool? {
         switch name {
         case TestDiscoveryTool.name:
             return InProcessTool(buildExecutionContext, type: TestDiscoveryCommand.self)
@@ -407,11 +422,11 @@ public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParser
         }
     }
 
-    public func hadCommandFailure() {
+    func hadCommandFailure() {
         onCommmandFailure?()
     }
 
-    public func handleDiagnostic(_ diagnostic: SPMLLBuild.Diagnostic) {
+    func handleDiagnostic(_ diagnostic: SPMLLBuild.Diagnostic) {
         switch diagnostic.kind {
         case .note:
             diagnostics.emit(note: diagnostic.message)
@@ -424,7 +439,7 @@ public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParser
         }
     }
 
-    public func commandStatusChanged(_ command: SPMLLBuild.Command, kind: CommandStatusKind) {
+    func commandStatusChanged(_ command: SPMLLBuild.Command, kind: CommandStatusKind) {
         guard !isVerbose else { return }
         guard command.shouldShowStatus else { return }
         guard !swiftParsers.keys.contains(command.name) else { return }
@@ -435,69 +450,75 @@ public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParser
         }
     }
 
-    public func commandPreparing(_ command: SPMLLBuild.Command) {
+    func commandPreparing(_ command: SPMLLBuild.Command) {
+        queue.async {
+            self.delegate?.buildSystem(self.buildSystem, willStartCommand: BuildSystemCommand(command))
+        }
     }
 
-    public func commandStarted(_ command: SPMLLBuild.Command) {
+    func commandStarted(_ command: SPMLLBuild.Command) {
         guard command.shouldShowStatus else { return }
 
         queue.async {
+            self.delegate?.buildSystem(self.buildSystem, didStartCommand: BuildSystemCommand(command))
             if self.isVerbose {
                 self.outputStream <<< command.verboseDescription <<< "\n"
                 self.outputStream.flush()
             }
         }
     }
 
-    public func shouldCommandStart(_ command: SPMLLBuild.Command) -> Bool {
+    func shouldCommandStart(_ command: SPMLLBuild.Command) -> Bool {
         return true
     }
 
-    public func commandFinished(_ command: SPMLLBuild.Command, result: CommandResult) {
-        guard !isVerbose else { return }
+    func commandFinished(_ command: SPMLLBuild.Command, result: CommandResult) {
         guard command.shouldShowStatus else { return }
         guard !swiftParsers.keys.contains(command.name) else { return }
 
         queue.async {
-            let targetName = self.swiftParsers[command.name]?.targetName
-            self.taskTracker.commandFinished(command, result: result, targetName: targetName)
-            self.updateProgress()
+            self.delegate?.buildSystem(self.buildSystem, didFinishCommand: BuildSystemCommand(command))
+            
+            if !self.isVerbose {
+                let targetName = self.swiftParsers[command.name]?.targetName
+                self.taskTracker.commandFinished(command, result: result, targetName: targetName)
+                self.updateProgress()
+            }
         }
     }
 
-    public func commandHadError(_ command: SPMLLBuild.Command, message: String) {
+    func commandHadError(_ command: SPMLLBuild.Command, message: String) {
         diagnostics.emit(error: message)
     }
 
-    public func commandHadNote(_ command: SPMLLBuild.Command, message: String) {
-        // FIXME: This is wrong.
-        diagnostics.emit(warning: message)
+    func commandHadNote(_ command: SPMLLBuild.Command, message: String) {
+        diagnostics.emit(note: message)
     }
 
-    public func commandHadWarning(_ command: SPMLLBuild.Command, message: String) {
+    func commandHadWarning(_ command: SPMLLBuild.Command, message: String) {
         diagnostics.emit(warning: message)
     }
 
-    public func commandCannotBuildOutputDueToMissingInputs(
+    func commandCannotBuildOutputDueToMissingInputs(
         _ command: SPMLLBuild.Command,
         output: BuildKey,
         inputs: [BuildKey]
     ) {
         diagnostics.emit(.missingInputs(output: output, inputs: inputs))
     }
 
-    public func cannotBuildNodeDueToMultipleProducers(output: BuildKey, commands: [SPMLLBuild.Command]) {
+    func cannotBuildNodeDueToMultipleProducers(output: BuildKey, commands: [SPMLLBuild.Command]) {
         diagnostics.emit(.multipleProducers(output: output, commands: commands))
     }
 
-    public func commandProcessStarted(_ command: SPMLLBuild.Command, process: ProcessHandle) {
+    func commandProcessStarted(_ command: SPMLLBuild.Command, process: ProcessHandle) {
     }
 
-    public func commandProcessHadError(_ command: SPMLLBuild.Command, process: ProcessHandle, message: String) {
+    func commandProcessHadError(_ command: SPMLLBuild.Command, process: ProcessHandle, message: String) {
         diagnostics.emit(.commandError(command: command, message: message))
     }
 
-    public func commandProcessHadOutput(_ command: SPMLLBuild.Command, process: ProcessHandle, data: [UInt8]) {
+    func commandProcessHadOutput(_ command: SPMLLBuild.Command, process: ProcessHandle, data: [UInt8]) {
         guard command.shouldShowStatus else { return }
 
         if let swiftParser = swiftParsers[command.name] {
@@ -511,7 +532,7 @@ public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParser
         }
     }
 
-    public func commandProcessFinished(
+    func commandProcessFinished(
         _ command: SPMLLBuild.Command,
         process: ProcessHandle,
         result: CommandExtendedResult
@@ -532,15 +553,21 @@ public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParser
         }
     }
 
-    public func cycleDetected(rules: [BuildKey]) {
+    func cycleDetected(rules: [BuildKey]) {
         diagnostics.emit(.cycleError(rules: rules))
+
+        queue.async {
+            self.delegate?.buildSystemDidDetectCycleInRules(self.buildSystem)
+        }
     }
 
-    public func shouldResolveCycle(rules: [BuildKey], candidate: BuildKey, action: CycleAction) -> Bool {
+    func shouldResolveCycle(rules: [BuildKey], candidate: BuildKey, action: CycleAction) -> Bool {
         return false
     }
 
-    public func swiftCompilerOutputParser(_ parser: SwiftCompilerOutputParser, didParse message: SwiftCompilerMessage) {
+    // MARK: SwiftCompilerOutputParserDelegate
+
+    func swiftCompilerOutputParser(_ parser: SwiftCompilerOutputParser, didParse message: SwiftCompilerMessage) {
         queue.async {
             if self.isVerbose {
                 if let text = message.verboseProgressText {
@@ -569,12 +596,14 @@ public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParser
         }
     }
 
-    public func swiftCompilerOutputParser(_ parser: SwiftCompilerOutputParser, didFailWith error: Error) {
+    func swiftCompilerOutputParser(_ parser: SwiftCompilerOutputParser, didFailWith error: Error) {
         let message = (error as? LocalizedError)?.errorDescription ?? error.localizedDescription
         diagnostics.emit(.swiftCompilerOutputParsingError(message))
         onCommmandFailure?()
     }
 
+    // MARK: Private
+
     private func updateProgress() {
         if let progressText = taskTracker.latestFinishedText {
             progressAnimation.update(
@@ -594,14 +623,14 @@ fileprivate struct CommandTaskTracker {
     /// The last task text before the task list was emptied.
     private(set) var latestFinishedText: String?
 
+    var onTaskProgressUpdateText: ((_ text: String, _ targetName: String?) -> Void)?
+
     mutating func commandStatusChanged(_ command: SPMLLBuild.Command, kind: CommandStatusKind) {
         switch kind {
         case .isScanning:
             totalCount += 1
-            break
         case .isUpToDate:
             totalCount -= 1
-            break
         case .isComplete:
             if (totalCount == finishedCount) {
                 let latestOutput: String? = latestFinishedText
@@ -610,15 +639,17 @@ fileprivate struct CommandTaskTracker {
                 * Build Completed!
                 """
             }
-            break
         @unknown default:
             assertionFailure("unhandled command status kind \(kind) for command \(command)")
             break
         }
     }
 
     mutating func commandFinished(_ command: SPMLLBuild.Command, result: CommandResult, targetName: String?) {
-        latestFinishedText = progressText(of: command, targetName: targetName)
+        let progressTextValue = progressText(of: command, targetName: targetName)
+        onTaskProgressUpdateText?(progressTextValue, targetName)
+
+        latestFinishedText = progressTextValue
 
         switch result {
         case .succeeded, .skipped:
@@ -635,6 +666,7 @@ fileprivate struct CommandTaskTracker {
         case .began(let info):
             if let text = progressText(of: message, targetName: targetName) {
                 swiftTaskProgressTexts[info.pid] = text
+                onTaskProgressUpdateText?(text, targetName)
             }
 
             totalCount += 1
@@ -739,3 +771,13 @@ private extension Diagnostic.Message {
         .error("failed parsing the Swift compiler output: \(error)")
     }
 }
+
+private extension BuildSystemCommand {
+    init(_ command: SPMLLBuild.Command) {
+        self.init(
+            name: command.name,
+            description: command.description,
+            verboseDescription: command.verboseDescription
+        )
+    }
+}

Sources/Build/CMakeLists.txt

@@ -7,7 +7,7 @@
 # See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 
 add_library(Build
-  BuildDelegate.swift
+  BuildOperationBuildSystemDelegateHandler.swift
   BuildOperation.swift
   BuildPlan.swift
   ManifestBuilder.swift

Sources/SPMBuildCore/BuildSystem.swift

@@ -29,6 +29,9 @@ public enum BuildSubset {
 /// implementation details between SwiftPM's `BuildOperation` and the XCBuild backed `XCBuildSystem`.
 public protocol BuildSystem {
 
+    /// The delegate used by the build system.
+    var delegate: BuildSystemDelegate? { get }
+
     /// The test products that this build system will build.
     var builtTestProducts: [BuiltTestProduct] { get }