Make updates better and more resilient (#245)

cmcgee1024 · web-flow · commit 5a0b96b7491b · 2025-03-11T13:27:11.000-04:00
Put a check in-place to unset the global default toolchain if it is no longer installed

Set the global default to the installed toolchain if it is not set

Add full toolchain selection resolution to the update operation resolve update parameters

Fix the use command and toolchain selection routine to consider a global default set to a toolchain that is not installed as no selection at all

Add check for the physical presence of a toolchain to proxy so that it prevents circularity errors and provides an actionable message

Allow uninstalling of partially installed toolchains from config.json

Control verbosity of uninstall operation on macOS and other platforms
diff --git a/Sources/LinuxPlatform/Linux.swift b/Sources/LinuxPlatform/Linux.swift
@@ -376,7 +376,7 @@ public struct Linux: Platform {
         try self.runProgram(tmpDir.appendingPathComponent("swiftly").path, "init")
     }
 
-    public func uninstall(_ toolchain: ToolchainVersion) throws {
+    public func uninstall(_ toolchain: ToolchainVersion, verbose _: Bool) throws {
         let toolchainDir = self.swiftlyToolchainsDir.appendingPathComponent(toolchain.name)
         try FileManager.default.removeItem(at: toolchainDir)
     }
diff --git a/Sources/MacOSPlatform/MacOS.swift b/Sources/MacOSPlatform/MacOS.swift
@@ -120,7 +120,7 @@ public struct MacOS: Platform {
         try self.runProgram(homeDir.appendingPathComponent("usr/local/bin/swiftly").path, "init")
     }
 
-    public func uninstall(_ toolchain: ToolchainVersion) throws {
+    public func uninstall(_ toolchain: ToolchainVersion, verbose: Bool) throws {
         SwiftlyCore.print("Uninstalling package in user home directory...")
 
         let toolchainDir = self.swiftlyToolchainsDir.appendingPathComponent("\(toolchain.identifier).xctoolchain", isDirectory: true)
@@ -138,7 +138,7 @@ public struct MacOS: Platform {
         try FileManager.default.removeItem(at: toolchainDir)
 
         let homedir = ProcessInfo.processInfo.environment["HOME"]!
-        try? runProgram("pkgutil", "--volume", homedir, "--forget", pkgInfo.CFBundleIdentifier)
+        try? runProgram("pkgutil", "--volume", homedir, "--forget", pkgInfo.CFBundleIdentifier, quiet: !verbose)
     }
 
     public func getExecutableName() -> String {
diff --git a/Sources/Swiftly/Install.swift b/Sources/Swiftly/Install.swift
@@ -297,11 +297,18 @@ struct Install: SwiftlyCommand {
 
         // If this is the first installed toolchain, mark it as in-use regardless of whether the
         // --use argument was provided.
-        if useInstalledToolchain || config.inUse == nil {
-            // TODO: consider adding the global default option to this commands flags
+        if useInstalledToolchain {
             try await Use.execute(version, globalDefault: false, &config)
         }
 
+        // We always update the global default toolchain if there is none set. This could
+        //  be the only toolchain that is installed, which makes it the only choice.
+        if config.inUse == nil {
+            config.inUse = version
+            try config.save()
+            SwiftlyCore.print("The global default toolchain has been set to `\(version)`")
+        }
+
         SwiftlyCore.print("\(version) installed successfully!")
         return (postInstallScript, pathChanged)
     }
diff --git a/Sources/Swiftly/Proxy.swift b/Sources/Swiftly/Proxy.swift
@@ -53,7 +53,7 @@ public enum Proxy {
             }
 
             guard let toolchain = toolchain else {
-                throw SwiftlyError(message: "No swift toolchain could be selected from either from a .swift-version file, or the default. You can try using `swiftly install <toolchain version>` to install one.")
+                throw SwiftlyError(message: "No installed swift toolchain is selected from either from a .swift-version file, or the default. You can try using one that's already installed with `swiftly use <toolchain version>` or install a new toolchain to use with `swiftly install --use <toolchain version>`.")
             }
 
             // Prevent circularities with a memento environment variable
diff --git a/Sources/Swiftly/Run.swift b/Sources/Swiftly/Run.swift
@@ -86,7 +86,7 @@ internal struct Run: SwiftlyCommand {
         }
 
         guard let toolchain = toolchain else {
-            throw SwiftlyError(message: "No swift toolchain could be selected from either from a .swift-version file, or the default. You can try using `swiftly install <toolchain version>` to install one.")
+            throw SwiftlyError(message: "No installed swift toolchain is selected from either from a .swift-version file, or the default. You can try using one that's already installed with `swiftly use <toolchain version>` or install a new toolchain to use with `swiftly install --use <toolchain version>`.")
         }
 
         do {
diff --git a/Sources/Swiftly/Uninstall.swift b/Sources/Swiftly/Uninstall.swift
@@ -56,7 +56,12 @@ struct Uninstall: SwiftlyCommand {
             }
         } else {
             let selector = try ToolchainSelector(parsing: self.toolchain)
-            toolchains = startingConfig.listInstalledToolchains(selector: selector)
+            var installedToolchains = startingConfig.listInstalledToolchains(selector: selector)
+            // This is in the unusual case that the inUse toolchain is not listed in the installed toolchains
+            if let inUse = startingConfig.inUse, selector.matches(toolchain: inUse) && !startingConfig.installedToolchains.contains(inUse) {
+                installedToolchains.append(inUse)
+            }
+            toolchains = installedToolchains
         }
 
         guard !toolchains.isEmpty else {
@@ -108,18 +113,23 @@ struct Uninstall: SwiftlyCommand {
                 }
             }
 
-            try await Self.execute(toolchain, &config)
+            try await Self.execute(toolchain, &config, verbose: self.root.verbose)
         }
 
         SwiftlyCore.print()
         SwiftlyCore.print("\(toolchains.count) toolchain(s) successfully uninstalled")
     }
 
-    static func execute(_ toolchain: ToolchainVersion, _ config: inout Config) async throws {
+    static func execute(_ toolchain: ToolchainVersion, _ config: inout Config, verbose: Bool) async throws {
         SwiftlyCore.print("Uninstalling \(toolchain)...", terminator: "")
-        try Swiftly.currentPlatform.uninstall(toolchain)
         config.installedToolchains.remove(toolchain)
+        // This is here to prevent the inUse from referencing a toolchain that is not installed
+        if config.inUse == toolchain {
+            config.inUse = nil
+        }
         try config.save()
+
+        try Swiftly.currentPlatform.uninstall(toolchain, verbose: verbose)
         SwiftlyCore.print("done")
     }
 }
diff --git a/Sources/Swiftly/Update.swift b/Sources/Swiftly/Update.swift
@@ -81,7 +81,7 @@ struct Update: SwiftlyCommand {
         try validateSwiftly()
         var config = try Config.load()
 
-        guard let parameters = try self.resolveUpdateParameters(config) else {
+        guard let parameters = try await self.resolveUpdateParameters(&config) else {
             if let toolchain = self.toolchain {
                 SwiftlyCore.print("No installed toolchain matched \"\(toolchain)\"")
             } else {
@@ -101,7 +101,7 @@ struct Update: SwiftlyCommand {
         }
 
         if !self.root.assumeYes {
-            SwiftlyCore.print("Update \(parameters.oldToolchain) ⟶ \(newToolchain)?")
+            SwiftlyCore.print("Update \(parameters.oldToolchain) -> \(newToolchain)?")
             guard SwiftlyCore.promptForConfirmation(defaultBehavior: true) else {
                 SwiftlyCore.print("Aborting")
                 return
@@ -117,7 +117,7 @@ struct Update: SwiftlyCommand {
             assumeYes: self.root.assumeYes
         )
 
-        try await Uninstall.execute(parameters.oldToolchain, &config)
+        try await Uninstall.execute(parameters.oldToolchain, &config, verbose: self.root.verbose)
         SwiftlyCore.print("Successfully updated \(parameters.oldToolchain) ⟶ \(newToolchain)")
 
         if let postInstallScript = postInstallScript {
@@ -152,7 +152,7 @@ struct Update: SwiftlyCommand {
     /// If the selector does not match an installed toolchain, this returns nil.
     /// If no selector is provided, the currently in-use toolchain will be used as the basis for the returned
     /// parameters.
-    private func resolveUpdateParameters(_ config: Config) throws -> UpdateParameters? {
+    private func resolveUpdateParameters(_ config: inout Config) async throws -> UpdateParameters? {
         let selector = try self.toolchain.map { try ToolchainSelector(parsing: $0) }
 
         let oldToolchain: ToolchainVersion?
@@ -163,7 +163,7 @@ struct Update: SwiftlyCommand {
             // 5.5.1 and 5.5.2 are installed (5.5.2 will be updated).
             oldToolchain = toolchains.max()
         } else {
-            oldToolchain = config.inUse
+            (oldToolchain, _) = try await selectToolchain(config: &config)
         }
 
         guard let oldToolchain else {
diff --git a/Sources/Swiftly/Use.swift b/Sources/Swiftly/Use.swift
@@ -133,7 +133,7 @@ internal struct Use: SwiftlyCommand {
         } else {
             config.inUse = toolchain
             try config.save()
-            message = "The global default toolchain has set to `\(toolchain)`"
+            message = "The global default toolchain has been set to `\(toolchain)`"
         }
 
         if let selectedVersion = selectedVersion {
@@ -177,7 +177,8 @@ public enum ToolchainSelectionResult {
 /// Selection of a toolchain can be accomplished in a number of ways. There is the
 /// the configuration's global default 'inUse' setting. This is the fallback selector
 /// if there are no other selections. The returned tuple will contain the default toolchain
-/// version and the result will be .default.
+/// version and the result will be .globalDefault. This will always be the result if
+/// the globalDefault parameter is true.
 ///
 /// A toolchain can also be selected from a `.swift-version` file in the current
 /// working directory, or an ancestor directory. If it successfully selects a toolchain
@@ -233,5 +234,11 @@ public func selectToolchain(config: inout Config, globalDefault: Bool = false) a
         }
     }
 
-    return (config.inUse, .globalDefault)
+    // Check to ensure that the global default in use toolchain matches one of the installed toolchains, and return
+    // no selected toolchain if it doesn't.
+    guard let defaultInUse = config.inUse, config.installedToolchains.contains(defaultInUse) else {
+        return (nil, .globalDefault)
+    }
+
+    return (defaultInUse, .globalDefault)
 }
diff --git a/Sources/SwiftlyCore/Platform.swift b/Sources/SwiftlyCore/Platform.swift
@@ -72,7 +72,7 @@ public protocol Platform {
 
     /// Uninstalls a toolchain associated with the given version.
     /// If this version is in use, the next latest version will be used afterwards.
-    func uninstall(_ version: ToolchainVersion) throws
+    func uninstall(_ version: ToolchainVersion, verbose: Bool) throws
 
     /// Get the name of the swiftly release binary.
     func getExecutableName() -> String
@@ -141,6 +141,9 @@ extension Platform {
 #if os(macOS) || os(Linux)
     internal func proxyEnv(_ toolchain: ToolchainVersion) throws -> [String: String] {
         let tcPath = self.findToolchainLocation(toolchain).appendingPathComponent("usr/bin")
+        guard tcPath.fileExists() else {
+            throw SwiftlyError(message: "Toolchain \(toolchain) could not be located. You can try `swiftly uninstall \(toolchain)` to uninstall it and then `swiftly install \(toolchain)` to install it again.")
+        }
         var newEnv = ProcessInfo.processInfo.environment
 
         // The toolchain goes to the beginning of the PATH
diff --git a/Tests/SwiftlyTests/PlatformTests.swift b/Tests/SwiftlyTests/PlatformTests.swift
@@ -53,21 +53,21 @@ final class PlatformTests: SwiftlyTests {
             (mockedToolchainFile, version) = try await self.mockToolchainDownload(version: "5.6.3")
             try Swiftly.currentPlatform.install(from: mockedToolchainFile, version: version, verbose: true)
             // WHEN: one of the toolchains is uninstalled
-            try Swiftly.currentPlatform.uninstall(version)
+            try Swiftly.currentPlatform.uninstall(version, verbose: true)
             // THEN: there is only one remaining toolchain installed
             var toolchains = try FileManager.default.contentsOfDirectory(at: Swiftly.currentPlatform.swiftlyToolchainsDir, includingPropertiesForKeys: nil)
             XCTAssertEqual(1, toolchains.count)
 
             // GIVEN; there is only one toolchain installed
             // WHEN: a non-existent toolchain is uninstalled
-            try? Swiftly.currentPlatform.uninstall(ToolchainVersion(parsing: "5.9.1"))
+            try? Swiftly.currentPlatform.uninstall(ToolchainVersion(parsing: "5.9.1"), verbose: true)
             // THEN: there is the one remaining toolchain that is still installed
             toolchains = try FileManager.default.contentsOfDirectory(at: Swiftly.currentPlatform.swiftlyToolchainsDir, includingPropertiesForKeys: nil)
             XCTAssertEqual(1, toolchains.count)
 
             // GIVEN: there is only one toolchain installed
             // WHEN: the last toolchain is uninstalled
-            try Swiftly.currentPlatform.uninstall(ToolchainVersion(parsing: "5.8.0"))
+            try Swiftly.currentPlatform.uninstall(ToolchainVersion(parsing: "5.8.0"), verbose: true)
             // THEN: there are no toolchains installed
             toolchains = try FileManager.default.contentsOfDirectory(at: Swiftly.currentPlatform.swiftlyToolchainsDir, includingPropertiesForKeys: nil)
             XCTAssertEqual(0, toolchains.count)
diff --git a/Tests/SwiftlyTests/UninstallTests.swift b/Tests/SwiftlyTests/UninstallTests.swift
@@ -296,4 +296,22 @@ final class UninstallTests: SwiftlyTests {
             )
         }
     }
+
+    /// Tests that uninstalling a toolchain that is the global default, but is not in the list of installed toolchains.
+    func testUninstallNotInstalled() 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 config = try await Config.load()
+            config.inUse = Self.newMainSnapshot
+            config.installedToolchains.remove(Self.newMainSnapshot)
+            try await config.save()
+
+            var uninstall = try self.parseCommand(Uninstall.self, ["uninstall", "-y", Self.newMainSnapshot.name])
+            _ = try await uninstall.run()
+            try await self.validateInstalledToolchains(
+                [Self.oldStable, Self.newStable, Self.oldReleaseSnapshot],
+                description: "uninstall did not uninstall all toolchains"
+            )
+        }
+    }
 }
diff --git a/Tests/SwiftlyTests/UpdateTests.swift b/Tests/SwiftlyTests/UpdateTests.swift
@@ -109,9 +109,9 @@ final class UpdateTests: SwiftlyTests {
         }
     }
 
-    /// Verifies that updating the currently in-use toolchain can be updated, and that after update the new toolchain
-    /// will be in-use instead.
-    func testUpdateInUse() async throws {
+    /// Verifies that updating the currently global default toolchain can be updated, and that after update the new toolchain
+    /// will be the global default instead.
+    func testUpdateGlobalDefault() async throws {
         try await self.withTestHome {
             try await self.withMockedToolchain {
                 try await self.installMockedToolchain(selector: "6.0.0")
@@ -136,6 +136,34 @@ final class UpdateTests: SwiftlyTests {
         }
     }
 
+    /// Verifies that updating the currently in-use toolchain can be updated, and that after update the new toolchain
+    /// will be in-use with the swift version file updated.
+    func testUpdateInUse() async throws {
+        try await self.withTestHome {
+            try await self.withMockedToolchain {
+                try await self.installMockedToolchain(selector: "6.0.0")
+
+                let versionFile = URL(fileURLWithPath: FileManager.default.currentDirectoryPath).appendingPathComponent(".swift-version")
+                try "6.0.0".write(to: versionFile, atomically: true, encoding: .utf8)
+
+                var update = try self.parseCommand(Update.self, ["update", "-y", "--no-verify", "--post-install-file=\(Swiftly.currentPlatform.getTempFilePath().path)"])
+                try await update.run()
+
+                let versionFileContents = try String(contentsOf: versionFile, encoding: .utf8)
+                let inUse = try ToolchainVersion(parsing: versionFileContents)
+                XCTAssertGreaterThan(inUse, .init(major: 6, minor: 0, patch: 0))
+
+                // Since the global default was set to 6.0.0, and that toolchain is no longer installed
+                // the update should have unset it to prevent the config from going into a bad state.
+                let config = try Config.load()
+                XCTAssertTrue(config.inUse == nil)
+
+                // The new toolchain should be installed
+                XCTAssertTrue(config.installedToolchains.contains(inUse))
+            }
+        }
+    }
+
     /// Verifies that snapshots, both from the main branch and from development branches, can be updated.
     func testUpdateSnapshot() async throws {
         let snapshotsAvailable = try await self.snapshotsAvailable()

Original file line number	Diff line number	Diff line change
`@@ -376,7 +376,7 @@ public struct Linux: Platform {`
`376`	`376`	`try self.runProgram(tmpDir.appendingPathComponent("swiftly").path, "init")`
`377`	`377`	`}`
`378`	`378`
`379`		`- public func uninstall(_ toolchain: ToolchainVersion) throws {`
	`379`	`+ public func uninstall(_ toolchain: ToolchainVersion, verbose _: Bool) throws {`
`380`	`380`	`let toolchainDir = self.swiftlyToolchainsDir.appendingPathComponent(toolchain.name)`
`381`	`381`	`try FileManager.default.removeItem(at: toolchainDir)`
`382`	`382`	`}`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ public struct MacOS: Platform {`
`120`	`120`	`try self.runProgram(homeDir.appendingPathComponent("usr/local/bin/swiftly").path, "init")`
`121`	`121`	`}`
`122`	`122`
`123`		`- public func uninstall(_ toolchain: ToolchainVersion) throws {`
	`123`	`+ public func uninstall(_ toolchain: ToolchainVersion, verbose: Bool) throws {`
`124`	`124`	`SwiftlyCore.print("Uninstalling package in user home directory...")`
`125`	`125`
`126`	`126`	`let toolchainDir = self.swiftlyToolchainsDir.appendingPathComponent("\(toolchain.identifier).xctoolchain", isDirectory: true)`
`@@ -138,7 +138,7 @@ public struct MacOS: Platform {`
`138`	`138`	`try FileManager.default.removeItem(at: toolchainDir)`
`139`	`139`
`140`	`140`	`let homedir = ProcessInfo.processInfo.environment["HOME"]!`
`141`		`- try? runProgram("pkgutil", "--volume", homedir, "--forget", pkgInfo.CFBundleIdentifier)`
	`141`	`+ try? runProgram("pkgutil", "--volume", homedir, "--forget", pkgInfo.CFBundleIdentifier, quiet: !verbose)`
`142`	`142`	`}`
`143`	`143`
`144`	`144`	`public func getExecutableName() -> String {`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ public enum Proxy {`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`guard let toolchain = toolchain else {`
`56`		- throw SwiftlyError(message: "No swift toolchain could be selected from either from a .swift-version file, or the default. You can try using `swiftly install <toolchain version>` to install one.")
	`56`	+ throw SwiftlyError(message: "No installed swift toolchain is selected from either from a .swift-version file, or the default. You can try using one that's already installed with `swiftly use <toolchain version>` or install a new toolchain to use with `swiftly install --use <toolchain version>`.")
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`// Prevent circularities with a memento environment variable`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ internal struct Run: SwiftlyCommand {`
`86`	`86`	`}`
`87`	`87`
`88`	`88`	`guard let toolchain = toolchain else {`
`89`		- throw SwiftlyError(message: "No swift toolchain could be selected from either from a .swift-version file, or the default. You can try using `swiftly install <toolchain version>` to install one.")
	`89`	+ throw SwiftlyError(message: "No installed swift toolchain is selected from either from a .swift-version file, or the default. You can try using one that's already installed with `swiftly use <toolchain version>` or install a new toolchain to use with `swiftly install --use <toolchain version>`.")
`90`	`90`	`}`
`91`	`91`
`92`	`92`	`do {`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,12 @@ struct Uninstall: SwiftlyCommand {`
`56`	`56`	`}`
`57`	`57`	`} else {`
`58`	`58`	`let selector = try ToolchainSelector(parsing: self.toolchain)`
`59`		`- toolchains = startingConfig.listInstalledToolchains(selector: selector)`
	`59`	`+ var installedToolchains = startingConfig.listInstalledToolchains(selector: selector)`
	`60`	`+ // This is in the unusual case that the inUse toolchain is not listed in the installed toolchains`
	`61`	`+ if let inUse = startingConfig.inUse, selector.matches(toolchain: inUse) && !startingConfig.installedToolchains.contains(inUse) {`
	`62`	`+ installedToolchains.append(inUse)`
	`63`	`+ }`
	`64`	`+ toolchains = installedToolchains`
`60`	`65`	`}`
`61`	`66`
`62`	`67`	`guard !toolchains.isEmpty else {`
`@@ -108,18 +113,23 @@ struct Uninstall: SwiftlyCommand {`
`108`	`113`	`}`
`109`	`114`	`}`
`110`	`115`
`111`		`- try await Self.execute(toolchain, &config)`
	`116`	`+ try await Self.execute(toolchain, &config, verbose: self.root.verbose)`
`112`	`117`	`}`
`113`	`118`
`114`	`119`	`SwiftlyCore.print()`
`115`	`120`	`SwiftlyCore.print("\(toolchains.count) toolchain(s) successfully uninstalled")`
`116`	`121`	`}`
`117`	`122`
`118`		`- static func execute(_ toolchain: ToolchainVersion, _ config: inout Config) async throws {`
	`123`	`+ static func execute(_ toolchain: ToolchainVersion, _ config: inout Config, verbose: Bool) async throws {`
`119`	`124`	`SwiftlyCore.print("Uninstalling \(toolchain)...", terminator: "")`
`120`		`- try Swiftly.currentPlatform.uninstall(toolchain)`
`121`	`125`	`config.installedToolchains.remove(toolchain)`
	`126`	`+ // This is here to prevent the inUse from referencing a toolchain that is not installed`
	`127`	`+ if config.inUse == toolchain {`
	`128`	`+ config.inUse = nil`
	`129`	`+ }`
`122`	`130`	`try config.save()`
	`131`	`+`
	`132`	`+ try Swiftly.currentPlatform.uninstall(toolchain, verbose: verbose)`
`123`	`133`	`SwiftlyCore.print("done")`
`124`	`134`	`}`
`125`	`135`	`}`