Skip to content

use verbosity flags to drive diagnostics logging level #3809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion Sources/Basics/Observability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
28 changes: 19 additions & 9 deletions Sources/Build/BuildOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
) {
Expand All @@ -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")
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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)

Expand All @@ -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(
Expand All @@ -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)
}
Expand Down Expand Up @@ -418,3 +422,9 @@ extension BuildSubset {
}
}
}

extension Basics.Diagnostic.Severity {
var isVerbose: Bool {
return self <= .info
}
}
32 changes: 17 additions & 15 deletions Sources/Build/BuildOperationBuildSystemDelegateHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -434,7 +436,7 @@ final class BuildOperationBuildSystemDelegateHandler: LLBuildBuildSystemDelegate
}

func hadCommandFailure() {
onCommmandFailure?()
self.commandFailureHandler?()
}

func handleDiagnostic(_ diagnostic: SPMLLBuild.Diagnostic) {
Expand All @@ -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 }

Expand All @@ -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()
}
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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()
}

Expand All @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion Sources/Commands/APIDigester.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -126,6 +127,7 @@ struct APIDigesterBaselineDumper {
packageGraphLoader: { graph },
pluginInvoker: { _ in [:] },
outputStream: outputStream,
logLevel: logLevel,
fileSystem: localFileSystem,
observabilityScope: observabilityScope
)
Expand Down
41 changes: 21 additions & 20 deletions Sources/Commands/Options.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -56,7 +56,7 @@ struct BuildFlagsGroup: ParsableArguments {
xswiftc: swiftCompilerFlags,
xlinker: linkerFlags)
}

init() {}
}

Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -263,7 +264,7 @@ public struct SwiftToolOptions: ParsableArguments {
}
}
}

@Flag(help: "Enable or disable indexing-while-building feature")
var indexStoreMode: StoreMode = .autoIndexStore

Expand Down Expand Up @@ -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)
Expand All @@ -353,6 +354,6 @@ public struct SwiftToolOptions: ParsableArguments {

@Flag(name: .customLong("netrc-optional"), help: .hidden)
var _deprecated_netrcOptional: Bool = false

public init() {}
}
3 changes: 2 additions & 1 deletion Sources/Commands/SwiftPackageTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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>()
Expand Down
Loading