Remove swiftly from the path during proxying (#321)

Remove swiftly from the path during proxying (#311)

Retaining swiftly in the path increases the probability that
a tool might run the proxy again, resulting in circularities.
Also, tools will locate programs in the path to try and find
adjacent toolchain files that cannot be discovered in the swiftly
bin directory.

Remove the swiftly bin directory from the path when proxying
to help improve the experience with other tools.

Author: cmcgee1024

Sources/SwiftlyCore/Platform.swift

@@ -139,19 +139,25 @@ extension Platform {
     }
 
 #if os(macOS) || os(Linux)
-    internal func proxyEnv(_ toolchain: ToolchainVersion) throws -> [String: String] {
+    internal func proxyEnv(env: [String: String], toolchain: ToolchainVersion) throws -> [String: String] {
+        var newEnv = env
+
         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
+
+        var pathComponents = (newEnv["PATH"] ?? "").split(separator: ":").map { String($0) }
 
         // The toolchain goes to the beginning of the PATH
-        var newPath = newEnv["PATH"] ?? ""
-        if !newPath.hasPrefix(tcPath.path + ":") {
-            newPath = "\(tcPath.path):\(newPath)"
-        }
-        newEnv["PATH"] = newPath
+        pathComponents.removeAll(where: { $0 == tcPath.path })
+        pathComponents = [tcPath.path] + pathComponents
+
+        // Remove swiftly bin directory from the PATH entirely
+        let swiftlyBinDir = self.swiftlyBinDir
+        pathComponents.removeAll(where: { $0 == swiftlyBinDir.path })
+
+        newEnv["PATH"] = String(pathComponents.joined(by: ":"))
 
         return newEnv
     }
@@ -162,11 +168,22 @@ extension Platform {
     /// the exit code and program information.
     ///
     public func proxy(_ toolchain: ToolchainVersion, _ command: String, _ arguments: [String], _ env: [String: String] = [:]) async throws {
-        var newEnv = try self.proxyEnv(toolchain)
+        var newEnv = try self.proxyEnv(env: ProcessInfo.processInfo.environment, toolchain: toolchain)
+
+        let tcPath = self.findToolchainLocation(toolchain).appendingPathComponent("usr/bin")
+
+        let commandTcPath = tcPath.appendingPathComponent(command)
+        let commandToRun = if FileManager.default.fileExists(atPath: commandTcPath.path) {
+            commandTcPath.path
+        } else {
+            command
+        }
+
         for (key, value) in env {
             newEnv[key] = value
         }
-        try self.runProgram([command] + arguments, env: newEnv)
+
+        try self.runProgram([commandToRun] + arguments, env: newEnv)
     }
 
     /// Proxy the invocation of the provided command to the chosen toolchain and capture the output.
@@ -175,7 +192,16 @@ extension Platform {
     /// the exit code and program information.
     ///
     public func proxyOutput(_ toolchain: ToolchainVersion, _ command: String, _ arguments: [String]) async throws -> String? {
-        try await self.runProgramOutput(command, arguments, env: self.proxyEnv(toolchain))
+        let tcPath = self.findToolchainLocation(toolchain).appendingPathComponent("usr/bin")
+
+        let commandTcPath = tcPath.appendingPathComponent(command)
+        let commandToRun = if FileManager.default.fileExists(atPath: commandTcPath.path) {
+            commandTcPath.path
+        } else {
+            command
+        }
+
+        return try await self.runProgramOutput(commandToRun, arguments, env: self.proxyEnv(env: ProcessInfo.processInfo.environment, toolchain: toolchain))
     }
 
     /// Run a program.

Tests/SwiftlyTests/PlatformTests.swift

@@ -73,4 +73,34 @@ final class PlatformTests: SwiftlyTests {
             XCTAssertEqual(0, toolchains.count)
         }
     }
+
+#if os(macOS) || os(Linux)
+    func testProxyEnv() async throws {
+        try await self.rollbackLocalChanges {
+            var (mockedToolchainFile, version) = try await self.mockToolchainDownload(version: SwiftlyTests.newStable.name)
+            try Swiftly.currentPlatform.install(from: mockedToolchainFile, version: version, verbose: true)
+
+            for path in [
+                "/a/b/c:SWIFTLY_BIN_DIR:/d/e/f",
+                "SWIFTLY_BIN_DIR:/abcde",
+                "/defgh:SWIFTLY_BIN_DIR",
+                "/xyzabc:/1/3/4",
+                "",
+            ] {
+                // GIVEN: a PATH that may contain the swiftly bin directory
+                let env = ["PATH": path.replacing("SWIFTLY_BIN_DIR", with: Swiftly.currentPlatform.swiftlyBinDir.path)]
+
+                // WHEN: proxying to an installed toolchain
+                let newEnv = try Swiftly.currentPlatform.proxyEnv(env: env, toolchain: SwiftlyTests.newStable)
+
+                // THEN: the toolchain's bin directory is added to the beginning of the PATH
+                XCTAssert(newEnv["PATH"]!.hasPrefix(Swiftly.currentPlatform.findToolchainLocation(SwiftlyTests.newStable).appendingPathComponent("usr/bin").path))
+
+                // AND: the swiftly bin directory is removed from the PATH
+                XCTAssert(!newEnv["PATH"]!.contains(Swiftly.currentPlatform.swiftlyBinDir.path))
+                XCTAssert(!newEnv["PATH"]!.contains(Swiftly.currentPlatform.swiftlyBinDir.path))
+            }
+        }
+    }
+#endif
 }