From bc9c310e8b5ae30ab45e895c0ded710d6d852dbd Mon Sep 17 00:00:00 2001 From: MDO190000 <72238042+MDO190000@users.noreply.github.com> Date: Sun, 24 Dec 2023 16:30:50 -0600 Subject: [PATCH 1/3] "swiftly uninstall all" command to remove all installed toolchains Hello, I maintain the [swiftly-bin AUR package](https://aur.archlinux.org/packages/swiftly-bin), and I appreciate all of the work that has gone into this utility, as it has been very useful to use (and definitely more flexible than just using the swift-bin AUR package!) The [.install script](https://aur.archlinux.org/cgit/aur.git/tree/swiftly-bin.install?h=swiftly-bin) I wrote for said package runs `swiftly uninstall latest` in a loop until the `~/.local/share/swiftly/toolchains` folder is empty. This is prone to an infinite loop if a non-toolchain file or folder is placed in that directory. To alleviate this, I propose some small changes to Uninstall.swift that allow for a one-command uninstallation of all installed toolchains with `swiftly uninstall all.` I am new to contributing, so please let me know if there are any issues with this proposal. Thank you! --- Sources/Swiftly/Uninstall.swift | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Sources/Swiftly/Uninstall.swift b/Sources/Swiftly/Uninstall.swift index 8200192b..f1763f04 100644 --- a/Sources/Swiftly/Uninstall.swift +++ b/Sources/Swiftly/Uninstall.swift @@ -33,6 +33,10 @@ struct Uninstall: SwiftlyCommand { The latest installed stable release can be uninstalled by specifying 'latest': $ swiftly uninstall latest + + Finally, all installed toolchains can be uninstalled by specifying 'all': + + $ swiftly uninstall all """ )) var toolchain: String @@ -44,7 +48,11 @@ struct Uninstall: SwiftlyCommand { var assumeYes: Bool = false mutating func run() async throws { - let selector = try ToolchainSelector(parsing: self.toolchain) + let selector: ToolchainSelector? = if self.toolchain != "all" { + try ToolchainSelector(parsing: self.toolchain) + } else { + nil // a nil selector causes listInstalledToolchains to return all toolchains, which is what we want + } let startingConfig = try Config.load() let toolchains = startingConfig.listInstalledToolchains(selector: selector) From 87f8905645b7be5e197692b3e5c0b96d792fed41 Mon Sep 17 00:00:00 2001 From: Patrick Freed Date: Sun, 24 Dec 2023 20:29:39 -0500 Subject: [PATCH 2/3] ensure the in-use toolchain is uninstalled last --- Sources/Swiftly/Uninstall.swift | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Sources/Swiftly/Uninstall.swift b/Sources/Swiftly/Uninstall.swift index f1763f04..6c7e4f2c 100644 --- a/Sources/Swiftly/Uninstall.swift +++ b/Sources/Swiftly/Uninstall.swift @@ -48,13 +48,19 @@ struct Uninstall: SwiftlyCommand { var assumeYes: Bool = false mutating func run() async throws { - let selector: ToolchainSelector? = if self.toolchain != "all" { - try ToolchainSelector(parsing: self.toolchain) + let startingConfig = try Config.load() + + let toolchains: [ToolchainVersion] + if self.toolchain == "all" { + // Sort the uninstalled toolchains such that the in-use toolchain will be uninstalled last. + // This avoids printing any unnecessary output from using new toolchains while the uninstall is in progress. + toolchains = startingConfig.listInstalledToolchains(selector: nil).sorted { a, b in + a != startingConfig.inUse && (b == startingConfig.inUse || a < b) + } } else { - nil // a nil selector causes listInstalledToolchains to return all toolchains, which is what we want + let selector = try ToolchainSelector(parsing: self.toolchain) + toolchains = startingConfig.listInstalledToolchains(selector: selector) } - let startingConfig = try Config.load() - let toolchains = startingConfig.listInstalledToolchains(selector: selector) guard !toolchains.isEmpty else { SwiftlyCore.print("No toolchains matched \"\(self.toolchain)\"") From 3e6dbe0f94138ea0f092b18e491d174cb2558bd9 Mon Sep 17 00:00:00 2001 From: Patrick Freed Date: Sun, 24 Dec 2023 20:30:41 -0500 Subject: [PATCH 3/3] add test for uninstall all --- Tests/SwiftlyTests/UninstallTests.swift | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Tests/SwiftlyTests/UninstallTests.swift b/Tests/SwiftlyTests/UninstallTests.swift index b7433cd3..5e17c611 100644 --- a/Tests/SwiftlyTests/UninstallTests.swift +++ b/Tests/SwiftlyTests/UninstallTests.swift @@ -283,4 +283,17 @@ final class UninstallTests: SwiftlyTests { ) } } + + /// Tests that providing "all" as an argument to uninstall will uninstall all toolchains. + func testUninstallAll() async throws { + let toolchains = Set([Self.oldStable, Self.newStable, Self.newMainSnapshot, Self.oldReleaseSnapshot]) + try await self.withMockedHome(homeName: Self.homeName, toolchains: toolchains, inUse: Self.newMainSnapshot) { + var uninstall = try self.parseCommand(Uninstall.self, ["uninstall", "-y", "all"]) + _ = try await uninstall.run() + try await self.validateInstalledToolchains( + [], + description: "uninstall did not uninstall all toolchains" + ) + } + } }