Skip to content

Commit 0a3e43b

Browse files
authored
improve handling of redundant output on stdout for commands that emit consumable information (#4118)
motvation: some CLI command emit consumable informaiton such as json. in those case we dont want to include any other information on stdout, so that it can be consumed by pipes changes: * for the following commands: * swift package dump-package * swift package dump-pif * swift package describe * swift package experimental-api-diff * swift package show-dependencies * swift test --list * redirect stdout to stderr prior to calling other actions * adjust result printing to always use stdout * adjust tests rdar://78612028
1 parent 71ffe05 commit 0a3e43b

File tree

3 files changed

+33
-11
lines changed

3 files changed

+33
-11
lines changed

Sources/Commands/SwiftPackageTool.swift

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ extension SwiftPackageTool {
173173
var type: DescribeMode = .text
174174

175175
func run(_ swiftTool: SwiftTool) throws {
176+
// redirect all other output to stderr so that the output is the only thing that is printed on stdout
177+
swiftTool.redirectStdoutToStderr()
178+
176179
let workspace = try swiftTool.getActiveWorkspace()
177180

178181
guard let packagePath = try swiftTool.getWorkspaceRoot().packages.first else {
@@ -187,11 +190,11 @@ extension SwiftPackageTool {
187190
)
188191
}
189192

190-
try self.describe(package, in: type, on: swiftTool.outputStream)
193+
try self.describe(package, in: type)
191194
}
192195

193196
/// Emits a textual description of `package` to `stream`, in the format indicated by `mode`.
194-
func describe(_ package: Package, in mode: DescribeMode, on stream: OutputByteStream) throws {
197+
func describe(_ package: Package, in mode: DescribeMode) throws {
195198
let desc = DescribedPackage(from: package)
196199
let data: Data
197200
switch mode {
@@ -204,8 +207,7 @@ extension SwiftPackageTool {
204207
encoder.formattingOptions = [.prettyPrinted]
205208
data = try encoder.encode(desc)
206209
}
207-
stream <<< String(decoding: data, as: UTF8.self) <<< "\n"
208-
stream.flush()
210+
print(String(decoding: data, as: UTF8.self))
209211
}
210212

211213
enum DescribeMode: String, ExpressibleByArgument {
@@ -366,6 +368,9 @@ extension SwiftPackageTool {
366368
var regenerateBaseline: Bool = false
367369

368370
func run(_ swiftTool: SwiftTool) throws {
371+
// redirect all other output to stderr so that the output is the only thing that is printed on stdout
372+
swiftTool.redirectStdoutToStderr()
373+
369374
let apiDigesterPath = try swiftTool.getToolchain().getSwiftAPIDigester()
370375
let apiDigesterTool = SwiftAPIDigester(tool: apiDigesterPath)
371376

@@ -595,6 +600,9 @@ extension SwiftPackageTool {
595600
var swiftOptions: SwiftToolOptions
596601

597602
func run(_ swiftTool: SwiftTool) throws {
603+
// redirect all other output to stderr so that the output is the only thing that is printed on stdout
604+
swiftTool.redirectStdoutToStderr()
605+
598606
let workspace = try swiftTool.getActiveWorkspace()
599607
let root = try swiftTool.getWorkspaceRoot()
600608

@@ -626,6 +634,9 @@ extension SwiftPackageTool {
626634
var preserveStructure: Bool = false
627635

628636
func run(_ swiftTool: SwiftTool) throws {
637+
// redirect all other output to stderr so that the output is the only thing that is printed on stdout
638+
swiftTool.redirectStdoutToStderr()
639+
629640
let graph = try swiftTool.loadPackageGraph(createMultipleTestProducts: true)
630641
let parameters = try PIFBuilderParameters(swiftTool.buildParameters())
631642
let builder = PIFBuilder(
@@ -715,8 +726,13 @@ extension SwiftPackageTool {
715726
var outputPath: AbsolutePath?
716727

717728
func run(_ swiftTool: SwiftTool) throws {
729+
if outputPath == nil {
730+
// redirect all other output to stderr so that the output is the only thing that is printed on stdout
731+
swiftTool.redirectStdoutToStderr()
732+
}
733+
718734
let graph = try swiftTool.loadPackageGraph()
719-
let stream = try outputPath.map { try LocalFileOutputByteStream($0) } ?? swiftTool.outputStream
735+
let stream: OutputByteStream = try outputPath.map { try LocalFileOutputByteStream($0) } ?? TSCBasic.stdoutStream
720736
Self.dumpDependenciesOf(rootPackage: graph.rootPackages[0], mode: format, on: stream)
721737
}
722738

Sources/Commands/SwiftTestTool.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@ public struct SwiftTestTool: SwiftCommand {
211211

212212
switch options.mode {
213213
case .listTests:
214+
// redirect all other output to stderr so that the list is the only thing that is printed on stdout
215+
swiftTool.redirectStdoutToStderr()
216+
214217
let testProducts = try buildTestsIfNeeded(swiftTool: swiftTool)
215218
let testSuites = try TestingSupport.getTestSuites(in: testProducts, swiftTool: swiftTool, swiftOptions: swiftOptions)
216219
let tests = try testSuites

Tests/FunctionalTests/DependencyResolutionTests.swift

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,10 @@ class DependencyResolutionTests: XCTestCase {
9191
// run with no mirror
9292
do {
9393
let output = try executeSwiftPackage(appPath, extraArgs: ["show-dependencies"])
94-
XCTAssertMatch(output.stdout, .contains("Fetching \(prefix.pathString)/Foo\n"))
95-
XCTAssertMatch(output.stdout, .contains("Fetching \(prefix.pathString)/Bar\n"))
94+
// logs are in stderr
95+
XCTAssertMatch(output.stderr, .contains("Fetching \(prefix.pathString)/Foo\n"))
96+
XCTAssertMatch(output.stderr, .contains("Fetching \(prefix.pathString)/Bar\n"))
97+
// results are in stdout
9698
XCTAssertMatch(output.stdout, .contains("foo<\(prefix.pathString)/Foo@unspecified"))
9799
XCTAssertMatch(output.stdout, .contains("bar<\(prefix.pathString)/Bar@unspecified"))
98100

@@ -115,10 +117,11 @@ class DependencyResolutionTests: XCTestCase {
115117
// run with mirror
116118
do {
117119
let output = try executeSwiftPackage(appPath, extraArgs: ["show-dependencies"])
118-
XCTAssertMatch(output.stdout, .contains("Fetching \(prefix.pathString)/Foo\n"))
119-
XCTAssertMatch(output.stdout, .contains("Fetching \(prefix.pathString)/BarMirror\n"))
120-
XCTAssertNoMatch(output.stdout, .contains("Fetching \(prefix.pathString)/Bar\n"))
121-
120+
// logs are in stderr
121+
XCTAssertMatch(output.stderr, .contains("Fetching \(prefix.pathString)/Foo\n"))
122+
XCTAssertMatch(output.stderr, .contains("Fetching \(prefix.pathString)/BarMirror\n"))
123+
XCTAssertNoMatch(output.stderr, .contains("Fetching \(prefix.pathString)/Bar\n"))
124+
// result are in stdout
122125
XCTAssertMatch(output.stdout, .contains("foo<\(prefix.pathString)/Foo@unspecified"))
123126
XCTAssertMatch(output.stdout, .contains("barmirror<\(prefix.pathString)/BarMirror@unspecified"))
124127
XCTAssertNoMatch(output.stdout, .contains("bar<\(prefix.pathString)/Bar@unspecified"))

0 commit comments

Comments
 (0)