Skip to content

Fix installations going out of sync #607

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
cmcgee1024 merged 3 commits into from
Oct 19, 2026
Merged

Fix installations going out of sync #607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e039a51
Fix installations going out of sync
MahdiBM Oct 2, 2026
aa91b0b
Improve the logged message, as suggested by code review
MahdiBM Oct 15, 2026
ed8d29b
Update error per code-review
MahdiBM Oct 17, 2026
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
34 changes: 24 additions & 10 deletions Sources/Swiftly/Install.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ struct Install: SwiftlyCommand {
let swiftlyBinDirContents =
(try? await fs.ls(atPath: swiftlyBinDir)) ?? [String]()
let toolchainBinDir = try await Swiftly.currentPlatform.findToolchainBinDir(ctx, version)
guard try await fs.exists(atPath: toolchainBinDir) else {
throw SwiftlyError(
message:
"Toolchain \(version) could not be located in \(toolchainBinDir). You can try `swiftly uninstall \(version)` to uninstall it and then `swiftly install \(version)` to install it again."
)
}
let toolchainBinDirContents = try await fs.ls(atPath: toolchainBinDir)

var existingProxies: [String] = []
Expand Down Expand Up @@ -250,18 +256,26 @@ struct Install: SwiftlyCommand {
assumeYes: Bool,
progressFile: FilePath? = nil
) async throws -> (postInstall: String?, pathChanged: Bool) {
guard !config.installedToolchains.contains(version) else {
let installInfo = InstallInfo(
version: version, alreadyInstalled: true
)
let pathChanged = if useInstalledToolchain {
try await Use.execute(ctx, version, globalDefault: false, verbose: verbose, &config)
} else {
false
if config.installedToolchains.contains(version) {

@etcwilde etcwilde Oct 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once we have the return value, I think that a guard here is easier to read.
e.g:

guard config.installedToolchains.contains(version) else {
  await ctx.message("Toolchain \(version) could not be located in \(toolchainBinDir)")
  <return (nil, false)>
}

// everything else ...

@MahdiBM MahdiBM Oct 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I mean the suggested code wouldn't really match the logic there 🤔.

@etcwilde etcwilde Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

doh, I misread the indentation level.

let toolchainBinDir = try await Swiftly.currentPlatform.findToolchainBinDir(ctx, version)

if try await fs.exists(atPath: toolchainBinDir) {
let installInfo = InstallInfo(
version: version, alreadyInstalled: true
)
let pathChanged = if useInstalledToolchain {
try await Use.execute(ctx, version, globalDefault: false, verbose: verbose, &config)
} else {
false
}

try await ctx.output(installInfo)
return (nil, pathChanged)
}

try await ctx.output(installInfo)
return (nil, pathChanged)
await ctx.message(
"Toolchain \(version) can no longer be located in \(toolchainBinDir). You can try `swiftly uninstall \(version.name); swiftly install \(version.name)` to reinstall it."
)
}

// Ensure the system is set up correctly before downloading it. Problems that prevent installation
Expand Down
20 changes: 20 additions & 0 deletions Tests/SwiftlyTests/InstallTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,26 @@ import Testing
#expect(before == after)
}

/// Tests that a toolchain that is installed according to the config, but missing from disk, is installed again.

@cmcgee1024 cmcgee1024 Oct 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

praise: It's always good to see another test for edge cases, especially ones that have a direct user impact.

@Test(.testHomeMockedToolchain()) func installToolchainMissingFromDisk() async throws {
try await SwiftlyTests.runCommand(Install.self, ["install", ToolchainVersion.newStable.name, "--post-install-file=\(fs.mktemp())"])

try await fs.remove(atPath: Swiftly.currentPlatform.findToolchainLocation(SwiftlyTests.ctx, .newStable))

let output = try await SwiftlyTests.runWithMockedIO(
Install.self, ["install", ToolchainVersion.newStable.name, "--post-install-file=\(fs.mktemp())", "--format", "json"], format: .json
)

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

#expect(installInfo.alreadyInstalled == false)

try await SwiftlyTests.validateInstalledToolchains([.newStable], description: "install a toolchain missing from disk")
}

/// Verify that the installed toolchain will be used if no toolchains currently are installed.
@Test(.testHomeMockedToolchain()) func installUsesFirstToolchain() async throws {
let config = try await Config.load()
Expand Down
10 changes: 10 additions & 0 deletions Tests/SwiftlyTests/UseTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,16 @@ import Testing
)
}

/// Tests that the `use` command reports a toolchain that is installed according to the config, but missing from disk.
@Test(.mockedSwiftlyVersion(), .mockHomeToolchains(toolchains: [.newStable])) func useToolchainMissingFromDisk() async throws {
try "swiftly binary".write(to: Swiftly.currentPlatform.swiftlyBinDir(SwiftlyTests.ctx) / "swiftly", atomically: true)
try await fs.remove(atPath: Swiftly.currentPlatform.findToolchainLocation(SwiftlyTests.ctx, .newStable))

await #expect(throws: SwiftlyError.self) {
try await SwiftlyTests.runCommand(Use.self, ["use", ToolchainVersion.newStable.name])
}
}

/// Tests that the `use` command works with all the installed toolchains in this test harness.
@Test(.mockedSwiftlyVersion(), .mockHomeToolchains()) func useAll() async throws {
let config = try await Config.load()
Expand Down
Loading