Skip to content

json output install #392

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.

Sign up for GitHub

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 6 commits into from
Jul 8, 2025
Merged

json output install #392

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
79f5c8b
Json Progress File
roulpriya Jun 21, 2025
3e9e091
Improve JSON progress file handling and error reporting
roulpriya Jun 23, 2025
36f02ca
Refactor progress file parsing and improve test validation
roulpriya Jun 24, 2025
6d83aef
Refactored JSON Output for Install Command
roulpriya Jun 30, 2025
176e123
Added Test
roulpriya Jul 7, 2025
5d0f49f
updated doc
roulpriya Jul 8, 2025
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
32 changes: 22 additions & 10 deletions Sources/Swiftly/Install.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,17 @@ struct Install: SwiftlyCommand {
))
var progressFile: FilePath?

@Option(name: .long, help: "Output format (text, json)")
var format: SwiftlyCore.OutputFormat = .text

@OptionGroup var root: GlobalOptions

private enum CodingKeys: String, CodingKey {
case version, use, verify, postInstallFile, root, progressFile
case version, use, verify, postInstallFile, root, progressFile, format
}

mutating func run() async throws {
try await self.run(Swiftly.createDefaultContext())
try await self.run(Swiftly.createDefaultContext(format: self.format))
}

private func swiftlyHomeDir(_ ctx: SwiftlyCoreContext) -> FilePath {
Expand Down Expand Up @@ -266,7 +269,10 @@ struct Install: SwiftlyCommand {
progressFile: FilePath? = nil
) async throws -> (postInstall: String?, pathChanged: Bool) {
guard !config.installedToolchains.contains(version) else {
await ctx.message("\(version) is already installed.")
let installInfo = InstallInfo(
version: version, alreadyInstalled: true
)
try await ctx.output(installInfo)
return (nil, false)
}

Expand Down Expand Up @@ -312,16 +318,18 @@ struct Install: SwiftlyCommand {
}
}

let animation: ProgressReporterProtocol =
let animation: ProgressReporterProtocol? =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Now that the animation will always get a non-nil value in each case, this variable can become non-optional type. That should make the code below a bit simpler since it doesn't have to handle the optionality in every case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a condition that requires animation to be optional for the JSON format.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now. Can the ctx.format == .json case create a ConsoleProgressReporter with stderr as the stream, and then the else case uses stdout as the stream?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that is doable.

if let progressFile
{
try JsonFileProgressReporter(ctx, filePath: progressFile)
} else if ctx.format == .json {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Since we've been generally putting messages on stderr when json formatted output is requested, could the ConsoleProgressReporter do the same thing?

nil
} else {
ConsoleProgressReporter(stream: stdoutStream, header: "Downloading \(version)")
ConsoleProgressReporter(stream: stderrStream, header: "Downloading \(version)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (blocking): The default case should continue to report progress to stdout. The json output case would be the one to output to stderr, so that the progress wouldn't corrupt the json.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, yes I missed this while earlier review.

}

defer {
try? animation.close()
try? animation?.close()
}

var lastUpdate = Date()
Expand Down Expand Up @@ -351,7 +359,7 @@ struct Install: SwiftlyCommand {
lastUpdate = Date()

do {
try await animation.update(
try await animation?.update(
step: progress.receivedBytes,
total: progress.totalBytes!,
text:
Expand All @@ -368,10 +376,10 @@ struct Install: SwiftlyCommand {
throw SwiftlyError(
message: "\(version) does not exist at URL \(notFound.url), exiting")
} catch {
try? await animation.complete(success: false)
try? await animation?.complete(success: false)
throw error
}
try await animation.complete(success: true)
try await animation?.complete(success: true)

if verifySignature {
try await Swiftly.currentPlatform.verifyToolchainSignature(
Expand Down Expand Up @@ -427,7 +435,11 @@ struct Install: SwiftlyCommand {
return (pathChanged, config)
}
config = newConfig
await ctx.message("\(version) installed successfully!")
let installInfo = InstallInfo(
version: version,
alreadyInstalled: false
)
try await ctx.output(installInfo)
return (postInstallScript, pathChanged)
}
}
Expand Down
25 changes: 25 additions & 0 deletions Sources/Swiftly/OutputSchema.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,28 @@ struct InstalledToolchainsListInfo: OutputData {
return lines.joined(separator: "\n")
}
}

struct InstallInfo: OutputData {
let version: ToolchainVersion
let alreadyInstalled: Bool

init(version: ToolchainVersion, alreadyInstalled: Bool) {
self.version = version
self.alreadyInstalled = alreadyInstalled
}

var description: String {
"\(self.version) is \(self.alreadyInstalled ? "already installed" : "installed successfully!")"
}

private enum CodingKeys: String, CodingKey {
case version
case alreadyInstalled
}

public func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(self.version.name, forKey: .version)
try container.encode(self.alreadyInstalled, forKey: .alreadyInstalled)
}
}
34 changes: 34 additions & 0 deletions Tests/SwiftlyTests/InstallTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,4 +389,38 @@ import Testing
}
}
#endif

/// Tests that `install` command with JSON format outputs correctly structured JSON.
@Test(.testHomeMockedToolchain()) func installJsonFormat() async throws {
let output = try await SwiftlyTests.runWithMockedIO(
Install.self, ["install", "5.7.0", "--post-install-file=\(fs.mktemp())", "--format", "json"], format: .json
)

let installInfo = try JSONDecoder().decode(
InstallInfo.self,
from: output[0].data(using: .utf8)!
)

#expect(installInfo.version.name == "5.7.0")
#expect(installInfo.alreadyInstalled == false)
}

/// Tests that `install` command with JSON format correctly indicates when toolchain is already installed.
@Test(.testHomeMockedToolchain()) func installJsonFormatAlreadyInstalled() async throws {
// First install the toolchain
try await SwiftlyTests.runCommand(Install.self, ["install", "5.7.0", "--post-install-file=\(fs.mktemp())"])

// Then try to install it again with JSON format
let output = try await SwiftlyTests.runWithMockedIO(
Install.self, ["install", "5.7.0", "--post-install-file=\(fs.mktemp())", "--format", "json"], format: .json
)

let installInfo = try JSONDecoder().decode(
InstallInfo.self,
from: output[0].data(using: .utf8)!
)

#expect(installInfo.version.name == "5.7.0")
#expect(installInfo.alreadyInstalled == true)
}
}
Loading