Skip to content

Commit c560fe1

Browse files
committed
SR-6277 "WIP" Mute non-json output from dump-package
Submitting PR for feedback on the solution, in advance of writing unit tests. This PR mutes tool progress output by default. It also introduces a new dump-package option: ` --include-tool-output` to override this default and show dependency resolution and fetch progress as before. Diagnostics that contain an error are still printed to the console. I am seeking feedback on this solution as I an new to this codebase and unfamiliar with it's broader structure, and expectations regarding tool putput handling.
1 parent 5fd01de commit c560fe1

File tree

3 files changed

+56
-16
lines changed

3 files changed

+56
-16
lines changed

Sources/Commands/Options.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public class ToolOptions {
3131
/// Enable prefetching in resolver which will kick off parallel git cloning.
3232
public var shouldEnableResolverPrefetching = true
3333

34+
/// Mute the output of tool operations.
35+
public var shouldMuteOutput = true
36+
3437
/// If print version option was passed.
3538
public var shouldPrintVersion: Bool = false
3639

Sources/Commands/SwiftPackageTool.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,8 @@ public class SwiftPackageTool: SwiftTool<PackageToolOptions> {
175175
describe(graph.rootPackages[0].underlyingPackage, in: options.describeMode, on: stdoutStream)
176176

177177
case .dumpPackage:
178+
options.shouldMuteOutput = !options.dumpPackageOptions.includeToolOutput
179+
178180
let graph = try loadPackageGraph()
179181
let manifest = graph.rootPackages[0].manifest
180182
print(try manifest.jsonString())
@@ -212,7 +214,10 @@ public class SwiftPackageTool: SwiftTool<PackageToolOptions> {
212214
option: describeParser.add(option: "--type", kind: DescribeMode.self, usage: "json|text"),
213215
to: { $0.describeMode = $1 })
214216

215-
_ = parser.add(subparser: PackageMode.dumpPackage.rawValue, overview: "Print parsed Package.swift as JSON")
217+
let dumpParser = parser.add(subparser: PackageMode.dumpPackage.rawValue, overview: "Print parsed Package.swift as JSON")
218+
binder.bind(
219+
option: dumpParser.add(option: "--include-tool-output", kind: Bool.self, usage: "Show package manager output"),
220+
to: { $0.dumpPackageOptions.includeToolOutput = $1 })
216221

217222
let editParser = parser.add(subparser: PackageMode.edit.rawValue, overview: "Put a package in editable mode")
218223
binder.bind(
@@ -371,6 +376,12 @@ public class PackageToolOptions: ToolOptions {
371376
var inputPath: AbsolutePath?
372377
var showDepsMode: ShowDependenciesMode = .text
373378

379+
struct DumpPackageOptions {
380+
var includeToolOutput = false
381+
}
382+
383+
var dumpPackageOptions = DumpPackageOptions()
384+
374385
struct EditOptions {
375386
var packageName: String?
376387
var revision: String?

Sources/Commands/SwiftTool.swift

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ struct TargetNotFoundDiagnostic: DiagnosticData {
6969

7070
private class ToolWorkspaceDelegate: WorkspaceDelegate {
7171

72+
private let options: ToolOptions
73+
74+
init(toolOptions: ToolOptions) {
75+
options = toolOptions
76+
}
77+
7278
func packageGraphWillLoad(
7379
currentGraph: PackageGraph,
7480
dependencies: AnySequence<ManagedDependency>,
@@ -77,39 +83,53 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
7783
}
7884

7985
func fetchingWillBegin(repository: String) {
80-
print("Fetching \(repository)")
86+
if !options.shouldMuteOutput {
87+
print("Fetching \(repository)")
88+
}
8189
}
8290

8391
func fetchingDidFinish(repository: String, diagnostic: Diagnostic?) {
8492
}
8593

8694
func repositoryWillUpdate(_ repository: String) {
87-
print("Updating \(repository)")
95+
if !options.shouldMuteOutput {
96+
print("Updating \(repository)")
97+
}
8898
}
8999

90100
func repositoryDidUpdate(_ repository: String) {
91101
}
92102

93103
func dependenciesUpToDate() {
94-
print("Everything is already up-to-date")
104+
if !options.shouldMuteOutput {
105+
print("Everything is already up-to-date")
106+
}
95107
}
96108

97109
func cloning(repository: String) {
98-
print("Cloning \(repository)")
110+
if !options.shouldMuteOutput {
111+
print("Cloning \(repository)")
112+
}
99113
}
100114

101115
func checkingOut(repository: String, atReference reference: String, to path: AbsolutePath) {
102116
// FIXME: This is temporary output similar to old one, we will need to figure
103117
// out better reporting text.
104-
print("Resolving \(repository) at \(reference)")
118+
if !options.shouldMuteOutput {
119+
print("Resolving \(repository) at \(reference)")
120+
}
105121
}
106122

107123
func removing(repository: String) {
108-
print("Removing \(repository)")
124+
if !options.shouldMuteOutput {
125+
print("Removing \(repository)")
126+
}
109127
}
110128

111129
func warning(message: String) {
112-
print("warning: " + message)
130+
if !options.shouldMuteOutput {
131+
print("warning: " + message)
132+
}
113133
}
114134

115135
func managedDependenciesDidUpdate(_ dependencies: AnySequence<ManagedDependency>) {
@@ -202,7 +222,7 @@ public class SwiftTool<Options: ToolOptions> {
202222
let interruptHandler: InterruptHandler
203223

204224
/// The diagnostics engine.
205-
let diagnostics = DiagnosticsEngine(handlers: [SwiftTool.diagnosticsHandler])
225+
let diagnostics: DiagnosticsEngine
206226

207227
/// The execution status of the tool.
208228
var executionStatus: ExecutionStatus = .success
@@ -325,8 +345,9 @@ public class SwiftTool<Options: ToolOptions> {
325345

326346
var options = Options()
327347
try binder.fill(parseResult: result, into: &options)
328-
348+
diagnostics = DiagnosticsEngine(handlers: [setupDiagnosticsHandler(with: options)])
329349
self.options = options
350+
330351
// Honor package-path option is provided.
331352
if let packagePath = options.packagePath ?? options.chdir {
332353
// FIXME: This should be an API which takes AbsolutePath and maybe
@@ -368,7 +389,7 @@ public class SwiftTool<Options: ToolOptions> {
368389
self.buildPath = getEnvBuildPath() ??
369390
customBuildPath ??
370391
(packageRoot ?? localFileSystem.currentWorkingDirectory!).appending(component: ".build")
371-
392+
372393
if options.chdir != nil {
373394
diagnostics.emit(data: ChdirDeprecatedDiagnostic())
374395
}
@@ -394,7 +415,8 @@ public class SwiftTool<Options: ToolOptions> {
394415
if let workspace = _workspace {
395416
return workspace
396417
}
397-
let delegate = ToolWorkspaceDelegate()
418+
let delegate = ToolWorkspaceDelegate(toolOptions: options)
419+
398420
let rootPackage = try getPackageRoot()
399421
let provider = GitRepositoryProvider(processSet: processSet)
400422
let workspace = Workspace(
@@ -431,10 +453,6 @@ public class SwiftTool<Options: ToolOptions> {
431453
SwiftTool.exit(with: executionStatus)
432454
}
433455

434-
static func diagnosticsHandler(_ diagnostic: Diagnostic) {
435-
print(diagnostic: diagnostic)
436-
}
437-
438456
/// Exit the tool with the given execution status.
439457
private static func exit(with status: ExecutionStatus) -> Never {
440458
switch status {
@@ -680,6 +698,14 @@ public class SwiftTool<Options: ToolOptions> {
680698
}
681699
}
682700

701+
func setupDiagnosticsHandler(with options: ToolOptions) -> (Diagnostic) -> Void {
702+
return { diagnostic in
703+
if !options.shouldMuteOutput || diagnostic.behavior == .error {
704+
print(diagnostic: diagnostic)
705+
}
706+
}
707+
}
708+
683709
/// An enum representing what subset of the package to build.
684710
enum BuildSubset {
685711
/// Represents the subset of all products and non-test targets.

0 commit comments

Comments
 (0)