use verbosity flags to drive diagnostics logging level (#3809)

motivation: allow users to specify verbosity which would impact how much diagnostic information is outputed

changes:
* add "very-verbose" CLI flag
* drive diagnostics logging level based on verbosity flags: verbose == .info, very-verbose == .debug
* filter diagnostic printing based on log level
* pass logLevel where appropriate instead of isVerbose
* add test

Author: tomerd

Sources/Basics/Observability.swift

@@ -292,11 +292,28 @@ public struct Diagnostic: CustomStringConvertible, Equatable {
         Self(severity: .debug, message: message.description, metadata: metadata)
     }
 
-    public enum Severity: Equatable {
+    public enum Severity: Comparable {
         case error
         case warning
         case info
         case debug
+
+        internal var naturalIntegralValue: Int {
+            switch self {
+            case .debug:
+                return 0
+            case .info:
+                return 1
+            case .warning:
+                return 2
+            case .error:
+                return 3
+            }
+        }
+
+        public static func < (lhs: Self, rhs: Self) -> Bool {
+            return lhs.naturalIntegralValue < rhs.naturalIntegralValue
+        }
     }
 }
 

Sources/Build/BuildOperation.swift

@@ -52,6 +52,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     /// The output stream for the build delegate.
     private let outputStream: OutputByteStream
 
+    /// The verbosity level to print out at
+    private let logLevel: Basics.Diagnostic.Severity
+
     /// File system to operate on
     private let fileSystem: TSCBasic.FileSystem
 
@@ -68,6 +71,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         packageGraphLoader: @escaping () throws -> PackageGraph,
         pluginInvoker: @escaping (PackageGraph) throws -> [ResolvedTarget: [PluginInvocationResult]],
         outputStream: OutputByteStream,
+        logLevel: Basics.Diagnostic.Severity,
         fileSystem: TSCBasic.FileSystem,
         observabilityScope: ObservabilityScope
     ) {
@@ -76,6 +80,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         self.packageGraphLoader = packageGraphLoader
         self.pluginInvoker = pluginInvoker
         self.outputStream = outputStream
+        self.logLevel = logLevel
         self.fileSystem = fileSystem
         self.observabilityScope = observabilityScope.makeChildScope(description: "Build Operation")
     }
@@ -127,7 +132,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     public func build(subset: BuildSubset) throws {
         // Create the build system.
         let buildDescription = try self.getBuildDescription()
-        let buildSystem = try createBuildSystem(with: buildDescription)
+        let buildSystem = try self.createBuildSystem(buildDescription: buildDescription)
         self.buildSystem = buildSystem
 
         // Perform the build.
@@ -224,7 +229,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
 
     /// Build the package structure target.
     private func buildPackageStructure() throws {
-        let buildSystem = try self.createBuildSystem(with: nil)
+        let buildSystem = try self.createBuildSystem(buildDescription: .none)
         self.buildSystem = buildSystem
 
         // Build the package structure target which will re-generate the llbuild manifest, if necessary.
@@ -237,12 +242,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     ///
     /// The build description should only be omitted when creating the build system for
     /// building the package structure target.
-    private func createBuildSystem(
-        with buildDescription: BuildDescription?
-    ) throws -> SPMLLBuild.BuildSystem {
+    private func createBuildSystem(buildDescription: BuildDescription?) throws -> SPMLLBuild.BuildSystem {
         // Figure out which progress bar we have to use during the build.
-        let isVerbose = verbosity != .concise
-        let progressAnimation: ProgressAnimationProtocol = isVerbose
+        let progressAnimation: ProgressAnimationProtocol = self.logLevel.isVerbose
             ? MultiLineNinjaProgressAnimation(stream: self.outputStream)
             : NinjaProgressAnimation(stream: self.outputStream)
 
@@ -260,11 +262,11 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             buildExecutionContext: buildExecutionContext,
             outputStream: self.outputStream,
             progressAnimation: progressAnimation,
+            logLevel: self.logLevel,
             observabilityScope: self.observabilityScope,
             delegate: self.delegate
         )
         self.buildSystemDelegate = buildSystemDelegate
-        buildSystemDelegate.isVerbose = isVerbose
 
         let databasePath = buildParameters.dataPath.appending(component: "build.db").pathString
         let buildSystem = SPMLLBuild.BuildSystem(
@@ -273,7 +275,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             delegate: buildSystemDelegate,
             schedulerLanes: buildParameters.jobs
         )
-        buildSystemDelegate.onCommmandFailure = {
+
+        // TODO: this seems fragile, perhaps we replace commandFailureHandler by adding relevant calls in the delegates chain 
+        buildSystemDelegate.commandFailureHandler = {
             buildSystem.cancel()
             self.delegate?.buildSystemDidCancel(self)
         }
@@ -418,3 +422,9 @@ extension BuildSubset {
         }
     }
 }
+
+extension Basics.Diagnostic.Severity {
+    var isVerbose: Bool {
+        return self <= .info
+    }
+}

Sources/Build/BuildOperationBuildSystemDelegateHandler.swift

@@ -368,11 +368,11 @@ final class CopyCommand: CustomLLBuildCommand {
 
 /// Convenient llbuild build system delegate implementation
 final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate, SwiftCompilerOutputParserDelegate {
-    var outputStream: ThreadSafeOutputByteStream
-    var progressAnimation: ProgressAnimationProtocol
-    var onCommmandFailure: (() -> Void)?
-    var isVerbose: Bool = false
-    weak var delegate: SPMBuildCore.BuildSystemDelegate?
+    private let outputStream: ThreadSafeOutputByteStream
+    private let progressAnimation: ProgressAnimationProtocol
+    var commandFailureHandler: (() -> Void)?
+    private let logLevel: Basics.Diagnostic.Severity
+    private weak var delegate: SPMBuildCore.BuildSystemDelegate?
     private let buildSystem: SPMBuildCore.BuildSystem
     private let queue = DispatchQueue(label: "org.swift.swiftpm.build-delegate")
     private var taskTracker = CommandTaskTracker()
@@ -390,17 +390,19 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
         buildExecutionContext: BuildExecutionContext,
         outputStream: OutputByteStream,
         progressAnimation: ProgressAnimationProtocol,
+        logLevel: Basics.Diagnostic.Severity,
         observabilityScope: ObservabilityScope,
         delegate: SPMBuildCore.BuildSystemDelegate?
     ) {
+        self.buildSystem = buildSystem
+        self.buildExecutionContext = buildExecutionContext
         // 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 = buildExecutionContext
-        self.delegate = delegate
-        self.buildSystem = buildSystem
+        self.logLevel = logLevel
         self.observabilityScope = observabilityScope
+        self.delegate = delegate
 
         let swiftParsers = buildExecutionContext.buildDescription?.swiftCommands.mapValues { tool in
             SwiftCompilerOutputParser(targetName: tool.moduleName, delegate: self)
@@ -434,7 +436,7 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
     }
 
     func hadCommandFailure() {
-        onCommmandFailure?()
+        self.commandFailureHandler?()
     }
 
     func handleDiagnostic(_ diagnostic: SPMLLBuild.Diagnostic) {
@@ -451,7 +453,7 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
     }
 
     func commandStatusChanged(_ command: SPMLLBuild.Command, kind: CommandStatusKind) {
-        guard !isVerbose else { return }
+        guard !self.logLevel.isVerbose else { return }
         guard command.shouldShowStatus else { return }
         guard !swiftParsers.keys.contains(command.name) else { return }
 
@@ -472,7 +474,7 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
 
         queue.async {
             self.delegate?.buildSystem(self.buildSystem, didStartCommand: BuildSystemCommand(command))
-            if self.isVerbose {
+            if self.logLevel.isVerbose {
                 self.outputStream <<< command.verboseDescription <<< "\n"
                 self.outputStream.flush()
             }
@@ -490,7 +492,7 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
         queue.async {
             self.delegate?.buildSystem(self.buildSystem, didFinishCommand: BuildSystemCommand(command))
 
-            if !self.isVerbose {
+            if !self.logLevel.isVerbose {
                 let targetName = self.swiftParsers[command.name]?.targetName
                 self.taskTracker.commandFinished(command, result: result, targetName: targetName)
                 self.updateProgress()
@@ -580,7 +582,7 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
 
     func swiftCompilerOutputParser(_ parser: SwiftCompilerOutputParser, didParse message: SwiftCompilerMessage) {
         queue.async {
-            if self.isVerbose {
+            if self.logLevel.isVerbose {
                 if let text = message.verboseProgressText {
                     self.outputStream <<< text <<< "\n"
                     self.outputStream.flush()
@@ -592,7 +594,7 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
 
             if let output = message.standardOutput {
                 // first we want to print the output so users have it handy
-                if !self.isVerbose {
+                if !self.logLevel.isVerbose {
                     self.progressAnimation.clear()
                 }
 
@@ -614,7 +616,7 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
     func swiftCompilerOutputParser(_ parser: SwiftCompilerOutputParser, didFailWith error: Error) {
         let message = (error as? LocalizedError)?.errorDescription ?? error.localizedDescription
         self.observabilityScope.emit(.swiftCompilerOutputParsingError(message))
-        onCommmandFailure?()
+        self.commandFailureHandler?()
     }
 
     func buildComplete(success: Bool) {

Sources/Commands/APIDigester.swift

@@ -58,7 +58,8 @@ struct APIDigesterBaselineDumper {
         for modulesToDiff: Set<String>,
         at baselineDir: AbsolutePath?,
         force: Bool,
-        outputStream: OutputByteStream
+        outputStream: OutputByteStream,
+        logLevel: Diagnostic.Severity
     ) throws -> AbsolutePath {
         var modulesToDiff = modulesToDiff
         let apiDiffDir = inputBuildParameters.apiDiff
@@ -126,6 +127,7 @@ struct APIDigesterBaselineDumper {
             packageGraphLoader: { graph },
             pluginInvoker: { _ in [:] },
             outputStream: outputStream,
+            logLevel: logLevel,
             fileSystem: localFileSystem,
             observabilityScope: observabilityScope
         )

Sources/Commands/Options.swift

@@ -20,22 +20,22 @@ struct BuildFlagsGroup: ParsableArguments {
             parsing: .unconditionalSingleValue,
             help: "Pass flag through to all C compiler invocations")
     var cCompilerFlags: [String] = []
-    
+
     @Option(name: .customLong("Xswiftc", withSingleDash: true),
             parsing: .unconditionalSingleValue,
             help: "Pass flag through to all Swift compiler invocations")
     var swiftCompilerFlags: [String] = []
-    
+
     @Option(name: .customLong("Xlinker", withSingleDash: true),
             parsing: .unconditionalSingleValue,
             help: "Pass flag through to all linker invocations")
     var linkerFlags: [String] = []
-    
+
     @Option(name: .customLong("Xcxx", withSingleDash: true),
             parsing: .unconditionalSingleValue,
             help: "Pass flag through to all C++ compiler invocations")
     var cxxCompilerFlags: [String] = []
-    
+
     @Option(name: .customLong("Xxcbuild", withSingleDash: true),
             parsing: .unconditionalSingleValue,
             help: ArgumentHelp(
@@ -56,7 +56,7 @@ struct BuildFlagsGroup: ParsableArguments {
             xswiftc: swiftCompilerFlags,
             xlinker: linkerFlags)
     }
-    
+
     init() {}
 }
 
@@ -77,7 +77,7 @@ extension AbsolutePath: ExpressibleByArgument {
             self = path
         }
     }
-  
+
     public static var defaultCompletionKind: CompletionKind {
         // This type is most commonly used to select a directory, not a file.
         // Specify '.file()' in an argument declaration when necessary.
@@ -114,7 +114,7 @@ public extension Sanitizer {
 public struct SwiftToolOptions: ParsableArguments {
     @OptionGroup()
     var buildFlagsGroup: BuildFlagsGroup
-    
+
     /// Custom arguments to pass to C compiler, swift compiler and the linker.
     var buildFlags: BuildFlags {
         buildFlagsGroup.buildFlags
@@ -162,12 +162,13 @@ public struct SwiftToolOptions: ParsableArguments {
     @Flag(name: .customLong("prefetching"), inversion: .prefixedEnableDisable)
     var shouldEnableResolverPrefetching: Bool = true
 
-    // FIXME: We need to allow -vv type options for this.
     /// The verbosity of informational output.
-    @Flag(name: .shortAndLong, help: "Increase verbosity of informational output")
+    @Flag(name: .shortAndLong, help: "Increase verbosity to include informational output")
     var verbose: Bool = false
-    
-    var verbosity: Int { verbose ? 1 : 0 }
+
+    /// The verbosity of informational output.
+    @Flag(name: [.long, .customLong("vv")], help: "Increase verbosity to include debug output")
+    var veryVerbose: Bool = false
 
     /// Disables sandboxing when executing subprocesses.
     @Flag(name: .customLong("disable-sandbox"), help: "Disable using the sandbox when executing subprocesses")
@@ -198,11 +199,11 @@ public struct SwiftToolOptions: ParsableArguments {
     /// The compilation destination’s target triple.
     @Option(name: .customLong("triple"), transform: Triple.init)
     var customCompileTriple: Triple?
-    
+
     /// Path to the compilation destination’s SDK.
     @Option(name: .customLong("sdk"))
     var customCompileSDK: AbsolutePath?
-    
+
     /// Path to the compilation destination’s toolchain.
     @Option(name: .customLong("toolchain"))
     var customCompileToolchain: AbsolutePath?
@@ -232,13 +233,13 @@ public struct SwiftToolOptions: ParsableArguments {
     var enabledSanitizers: EnabledSanitizers {
         EnabledSanitizers(Set(sanitizers))
     }
-    
+
     /// Whether to enable code coverage.
     @Flag(name: .customLong("code-coverage"),
           inversion: .prefixedEnableDisable,
           help: "Enable code coverage")
     var shouldEnableCodeCoverage: Bool = false
-    
+
     /// Use Package.resolved file for resolving dependencies.
     @Flag(name: [.long, .customLong("disable-automatic-resolution"), .customLong("only-use-versions-from-resolved-file")], help: "Only use versions from the Package.resolved file and fail resolution if it is out-of-date")
     var forceResolvedVersions: Bool = false
@@ -250,7 +251,7 @@ public struct SwiftToolOptions: ParsableArguments {
         case autoIndexStore
         case enableIndexStore
         case disableIndexStore
-        
+
         /// The mode to use for indexing-while-building feature.
         var indexStoreMode: BuildParameters.IndexStoreMode {
             switch self {
@@ -263,7 +264,7 @@ public struct SwiftToolOptions: ParsableArguments {
             }
         }
     }
-    
+
     @Flag(help: "Enable or disable indexing-while-building feature")
     var indexStoreMode: StoreMode = .autoIndexStore
 
@@ -326,14 +327,14 @@ public struct SwiftToolOptions: ParsableArguments {
           exclusivity: .exclusive,
           help: "Load credentials from a .netrc file")
     var netrc: Bool = true
-    
+
     /// The path to the .netrc file used when `netrc` is `true`.
     @Option(
         name: .customLong("netrc-file"),
         help: "Specify the .netrc file path.",
         completion: .file())
     var netrcFilePath: AbsolutePath?
-    
+
     /// Whether to use keychain for authenticating with remote servers
     /// when downloading binary artifacts or communicating with a registry.
 #if canImport(Security)
@@ -353,6 +354,6 @@ public struct SwiftToolOptions: ParsableArguments {
 
     @Flag(name: .customLong("netrc-optional"), help: .hidden)
     var _deprecated_netrcOptional: Bool = false
-    
+
     public init() {}
 }

Sources/Commands/SwiftPackageTool.swift

@@ -408,7 +408,8 @@ extension SwiftPackageTool {
                 for: modulesToDiff,
                 at: overrideBaselineDir,
                 force: regenerateBaseline,
-                outputStream: swiftTool.outputStream
+                outputStream: swiftTool.outputStream,
+                logLevel: swiftTool.logLevel
             )
 
             let results = ThreadSafeArrayStore<SwiftAPIDigester.ComparisonResult>()