From 6c5164ca742509f8164ebc839a367537699d6110 Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Tue, 8 Apr 2025 15:37:03 -0400 Subject: [PATCH 1/4] Remove swiftly from the path during proxying 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. --- Sources/SwiftlyCore/Platform.swift | 23 +++++++++++++++-------- Tests/SwiftlyTests/PlatformTests.swift | 26 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/Sources/SwiftlyCore/Platform.swift b/Sources/SwiftlyCore/Platform.swift index f28171e9..0d015789 100644 --- a/Sources/SwiftlyCore/Platform.swift +++ b/Sources/SwiftlyCore/Platform.swift @@ -135,19 +135,26 @@ extension Platform { } #if os(macOS) || os(Linux) - func proxyEnv(_ ctx: SwiftlyCoreContext, _ toolchain: ToolchainVersion) throws -> [String: String] { + func proxyEnv(_ ctx: SwiftlyCoreContext, env: [String: String], toolchain: ToolchainVersion) throws -> [String: String] { + var newEnv = env + let tcPath = self.findToolchainLocation(ctx, 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)" + if pathComponents.first ?? "" != tcPath.path { + pathComponents = [tcPath.path] + pathComponents } - newEnv["PATH"] = newPath + + // Remove swiftly bin directory from the PATH + let swiftlyBinDir = self.swiftlyBinDir(ctx).path + pathComponents.removeAll(where: { $0 == swiftlyBinDir }) + + newEnv["PATH"] = String(pathComponents.joined(by: ":")) return newEnv } @@ -158,7 +165,7 @@ extension Platform { /// the exit code and program information. /// public func proxy(_ ctx: SwiftlyCoreContext, _ toolchain: ToolchainVersion, _ command: String, _ arguments: [String], _ env: [String: String] = [:]) async throws { - var newEnv = try self.proxyEnv(ctx, toolchain) + var newEnv = try self.proxyEnv(ctx, env: ProcessInfo.processInfo.environment, toolchain: toolchain) for (key, value) in env { newEnv[key] = value } @@ -171,7 +178,7 @@ extension Platform { /// the exit code and program information. /// public func proxyOutput(_ ctx: SwiftlyCoreContext, _ toolchain: ToolchainVersion, _ command: String, _ arguments: [String]) async throws -> String? { - try await self.runProgramOutput(command, arguments, env: self.proxyEnv(ctx, toolchain)) + try await self.runProgramOutput(command, arguments, env: self.proxyEnv(ctx, env: ProcessInfo.processInfo.environment, toolchain: toolchain)) } /// Run a program. diff --git a/Tests/SwiftlyTests/PlatformTests.swift b/Tests/SwiftlyTests/PlatformTests.swift index 6a548010..c402f84d 100644 --- a/Tests/SwiftlyTests/PlatformTests.swift +++ b/Tests/SwiftlyTests/PlatformTests.swift @@ -85,4 +85,30 @@ import Testing toolchains = try FileManager.default.contentsOfDirectory(at: Swiftly.currentPlatform.swiftlyToolchainsDir(SwiftlyTests.ctx), includingPropertiesForKeys: nil) #expect(0 == toolchains.count) } + +#if os(macOS) || os(Linux) + @Test( + .mockHomeToolchains(), + arguments: [ + "/a/b/c:SWIFTLY_BIN_DIR:/d/e/f", + "SWIFTLY_BIN_DIR:/abcde", + "/defgh:SWIFTLY_BIN_DIR", + "/xyzabc:/1/3/4", + "", + ] + ) func proxyEnv(_ path: String) async throws { + // GIVEN: a PATH that may contain the swiftly bin directory + let env = ["PATH": path.replacing("SWIFTLY_BIN_DIR", with: Swiftly.currentPlatform.swiftlyBinDir(SwiftlyTests.ctx).path)] + + // WHEN: proxying to an installed toolchain + let newEnv = try Swiftly.currentPlatform.proxyEnv(SwiftlyTests.ctx, env: env, toolchain: .newStable) + + // THEN: the toolchain's bin directory is added to the beginning of the PATH + #expect(newEnv["PATH"]!.hasPrefix(Swiftly.currentPlatform.findToolchainLocation(SwiftlyTests.ctx, .newStable).appendingPathComponent("usr/bin").path)) + + // AND: the swiftly bin directory is removed from the PATH + #expect(!newEnv["PATH"]!.contains(Swiftly.currentPlatform.swiftlyBinDir(SwiftlyTests.ctx).path)) + #expect(!newEnv["PATH"]!.contains(Swiftly.currentPlatform.swiftlyBinDir(SwiftlyTests.ctx).path)) + } +#endif } From edb9439edb294fe9181d46590a3ccf80047aa3b7 Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Tue, 8 Apr 2025 15:53:39 -0400 Subject: [PATCH 2/4] Use fully qualified command paths to give drivers the information that they need from argv[0] --- Sources/SwiftlyCore/Platform.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftlyCore/Platform.swift b/Sources/SwiftlyCore/Platform.swift index 0d015789..5e6a4c88 100644 --- a/Sources/SwiftlyCore/Platform.swift +++ b/Sources/SwiftlyCore/Platform.swift @@ -165,11 +165,13 @@ extension Platform { /// the exit code and program information. /// public func proxy(_ ctx: SwiftlyCoreContext, _ toolchain: ToolchainVersion, _ command: String, _ arguments: [String], _ env: [String: String] = [:]) async throws { + let tcPath = self.findToolchainLocation(ctx, toolchain).appendingPathComponent("usr/bin") + var newEnv = try self.proxyEnv(ctx, env: ProcessInfo.processInfo.environment, toolchain: toolchain) for (key, value) in env { newEnv[key] = value } - try self.runProgram([command] + arguments, env: newEnv) + try self.runProgram([tcPath.appendingPathComponent(command).path] + arguments, env: newEnv) } /// Proxy the invocation of the provided command to the chosen toolchain and capture the output. @@ -178,7 +180,8 @@ extension Platform { /// the exit code and program information. /// public func proxyOutput(_ ctx: SwiftlyCoreContext, _ toolchain: ToolchainVersion, _ command: String, _ arguments: [String]) async throws -> String? { - try await self.runProgramOutput(command, arguments, env: self.proxyEnv(ctx, env: ProcessInfo.processInfo.environment, toolchain: toolchain)) + let tcPath = self.findToolchainLocation(ctx, toolchain).appendingPathComponent("usr/bin") + return try await self.runProgramOutput(tcPath.appendingPathComponent(command).path, arguments, env: self.proxyEnv(ctx, env: ProcessInfo.processInfo.environment, toolchain: toolchain)) } /// Run a program. From a11b7e959393a380d5de5116e90d25def3c96cbd Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Tue, 8 Apr 2025 16:38:31 -0400 Subject: [PATCH 3/4] Use fully qualified command paths only if the binary can be found in the selected toolchain --- Sources/Swiftly/Run.swift | 3 +++ Sources/SwiftlyCore/Platform.swift | 20 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Sources/Swiftly/Run.swift b/Sources/Swiftly/Run.swift index 056617d9..91a5f1c5 100644 --- a/Sources/Swiftly/Run.swift +++ b/Sources/Swiftly/Run.swift @@ -105,6 +105,9 @@ struct Run: SwiftlyCommand { try await Swiftly.currentPlatform.proxy(ctx, toolchain, command[0], [String](command[1...])) } catch let terminated as RunProgramError { + if ctx.mockedHomeDir != nil { + throw terminated + } Foundation.exit(terminated.exitCode) } catch { throw error diff --git a/Sources/SwiftlyCore/Platform.swift b/Sources/SwiftlyCore/Platform.swift index 5e6a4c88..4300096f 100644 --- a/Sources/SwiftlyCore/Platform.swift +++ b/Sources/SwiftlyCore/Platform.swift @@ -167,11 +167,19 @@ extension Platform { public func proxy(_ ctx: SwiftlyCoreContext, _ toolchain: ToolchainVersion, _ command: String, _ arguments: [String], _ env: [String: String] = [:]) async throws { let tcPath = self.findToolchainLocation(ctx, toolchain).appendingPathComponent("usr/bin") + let commandTcPath = tcPath.appendingPathComponent(command) + let commandToRun = if FileManager.default.fileExists(atPath: commandTcPath.path) { + commandTcPath.path + } else { + command + } + var newEnv = try self.proxyEnv(ctx, env: ProcessInfo.processInfo.environment, toolchain: toolchain) for (key, value) in env { newEnv[key] = value } - try self.runProgram([tcPath.appendingPathComponent(command).path] + 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. @@ -181,7 +189,15 @@ extension Platform { /// public func proxyOutput(_ ctx: SwiftlyCoreContext, _ toolchain: ToolchainVersion, _ command: String, _ arguments: [String]) async throws -> String? { let tcPath = self.findToolchainLocation(ctx, toolchain).appendingPathComponent("usr/bin") - return try await self.runProgramOutput(tcPath.appendingPathComponent(command).path, arguments, env: self.proxyEnv(ctx, env: ProcessInfo.processInfo.environment, toolchain: toolchain)) + + 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(ctx, env: ProcessInfo.processInfo.environment, toolchain: toolchain)) } /// Run a program. From 02a49b48d3820ecea85e3a601193a4623f9e50ae Mon Sep 17 00:00:00 2001 From: Chris McGee Date: Fri, 11 Apr 2025 08:01:48 -0400 Subject: [PATCH 4/4] Remove existing entries on the PATH for the selected toolchain, ensuring it is only in the front --- Sources/SwiftlyCore/Platform.swift | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Sources/SwiftlyCore/Platform.swift b/Sources/SwiftlyCore/Platform.swift index 04de3e73..885b828c 100644 --- a/Sources/SwiftlyCore/Platform.swift +++ b/Sources/SwiftlyCore/Platform.swift @@ -178,13 +178,12 @@ extension Platform { var pathComponents = (newEnv["PATH"] ?? "").split(separator: ":").map { String($0) } // The toolchain goes to the beginning of the PATH - if pathComponents.first ?? "" != tcPath.path { - pathComponents = [tcPath.path] + pathComponents - } + pathComponents.removeAll(where: { $0 == tcPath.path }) + pathComponents = [tcPath.path] + pathComponents - // Remove swiftly bin directory from the PATH - let swiftlyBinDir = self.swiftlyBinDir(ctx).path - pathComponents.removeAll(where: { $0 == swiftlyBinDir }) + // Remove swiftly bin directory from the PATH entirely + let swiftlyBinDir = self.swiftlyBinDir(ctx) + pathComponents.removeAll(where: { $0 == swiftlyBinDir.path }) newEnv["PATH"] = String(pathComponents.joined(by: ":"))