Skip to content
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
7 changes: 6 additions & 1 deletion Documentation/SwiftlyDocs.docc/swiftly-cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ swiftly [--version] [--help]
Install a new toolchain.

```
swiftly install [<version>] [--use] [--verify|no-verify] [--post-install-file=<post-install-file>] [--progress-file=<progress-file>] [--assume-yes] [--verbose] [--version] [--help]
swiftly install [<version>] [--use] [--verify|no-verify] [--post-install-file=<post-install-file>] [--progress-file=<progress-file>] [--format=<format>] [--assume-yes] [--verbose] [--version] [--help]
```

**version:**
Expand Down Expand Up @@ -89,6 +89,11 @@ Each progress entry contains timestamp, progress percentage, and a descriptive m
The file must be writable, else an error will be thrown.


**--format=\<format\>:**

*Output format (text, json)*


**--assume-yes:**

*Disable confirmation prompts by assuming 'yes'*
Expand Down
30 changes: 21 additions & 9 deletions Sources/Swiftly/Install.swift
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?

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.

} else {
ConsoleProgressReporter(stream: stdoutStream, header: "Downloading \(version)")
}

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
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
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)
}
}