From 90fd1148b7487d8cf4d6692768124955d2d47ce8 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Mon, 18 Dec 2023 09:11:35 -0800 Subject: [PATCH] Basic: support case insensitivity for environment Windows is case insensitive for the environment control block, which we did not honour. This causes issues when Swift is used with programs which are incorrectly cased (e.g. emacs). Introduce an explicit wrapper type for Windows to make the lookup case insensitive, canonicalising the name to lowercase. This allows us to treat Path and PATH identically (along with any other environment variable and case matching) which respects the Windows behaviour. Additionally, migrate away from the POSIX variants which do not handle the case properly to the Windows version which does. The introduced type `ProcessEnvironmentBlock` is just a typealias, allowing access to the dictionary itself which is important to preserve the behaviour for the clients. The `CaseInsensitiveString` is a case insensitive, case preserving string representation that allows us to recreate the environment on Windows as the environment is case insensitive, so it should not be possible to have conflicts when reading the Process Environment Block from the Process Execution Block. This is a partial resolution to environment variable handling as the tools need subsequent changes to adopt the new API. Fixes: #446 Co-authored-by: Max Desiatov --- Sources/TSCBasic/FileSystem.swift | 2 +- Sources/TSCBasic/Process/Process.swift | 281 +++++++++++++++++++--- Sources/TSCBasic/Process/ProcessEnv.swift | 72 +++++- Tests/TSCBasicTests/ProcessEnvTests.swift | 9 + 4 files changed, 314 insertions(+), 50 deletions(-) diff --git a/Sources/TSCBasic/FileSystem.swift b/Sources/TSCBasic/FileSystem.swift index 8667130d..0fe6a36c 100644 --- a/Sources/TSCBasic/FileSystem.swift +++ b/Sources/TSCBasic/FileSystem.swift @@ -450,7 +450,7 @@ private struct LocalFileSystem: FileSystem { var tempDirectory: AbsolutePath { get throws { - let override = ProcessEnv.vars["TMPDIR"] ?? ProcessEnv.vars["TEMP"] ?? ProcessEnv.vars["TMP"] + let override = ProcessEnv.block["TMPDIR"] ?? ProcessEnv.block["TEMP"] ?? ProcessEnv.block["TMP"] if let path = override.flatMap({ try? AbsolutePath(validating: $0) }) { return path } diff --git a/Sources/TSCBasic/Process/Process.swift b/Sources/TSCBasic/Process/Process.swift index 971d7855..2cc5b12d 100644 --- a/Sources/TSCBasic/Process/Process.swift +++ b/Sources/TSCBasic/Process/Process.swift @@ -53,7 +53,12 @@ public struct ProcessResult: CustomStringConvertible, Sendable { public let arguments: [String] /// The environment with which the process was launched. - public let environment: [String: String] + public let environmentBlock: ProcessEnvironmentBlock + + @available(*, deprecated, renamed: "env") + public var environment: [String:String] { + Dictionary(uniqueKeysWithValues: self.environmentBlock.map { ($0.key.value, $0.value) }) + } /// The exit status of the process. public let exitStatus: ExitStatus @@ -71,7 +76,7 @@ public struct ProcessResult: CustomStringConvertible, Sendable { /// See `waitpid(2)` for information on the exit status code. public init( arguments: [String], - environment: [String: String], + environmentBlock: ProcessEnvironmentBlock, exitStatusCode: Int32, normal: Bool, output: Result<[UInt8], Swift.Error>, @@ -92,25 +97,60 @@ public struct ProcessResult: CustomStringConvertible, Sendable { exitStatus = .terminated(code: WEXITSTATUS(exitStatusCode)) } #endif - self.init(arguments: arguments, environment: environment, exitStatus: exitStatus, output: output, - stderrOutput: stderrOutput) + self.init(arguments: arguments, environmentBlock: environmentBlock, exitStatus: exitStatus, output: output, stderrOutput: stderrOutput) + } + + @available(*, deprecated, message: "use `init(arguments:environmentBlock:exitStatusCode:output:stderrOutput:)`") + public init( + arguments: [String], + environment: [String:String], + exitStatusCode: Int32, + normal: Bool, + output: Result<[UInt8], Swift.Error>, + stderrOutput: Result<[UInt8], Swift.Error> + ) { + self.init( + arguments: arguments, + environmentBlock: .init(environment), + exitStatusCode: exitStatusCode, + normal: normal, + output: output, + stderrOutput: stderrOutput + ) } /// Create an instance using an exit status and output result. public init( arguments: [String], - environment: [String: String], + environmentBlock: ProcessEnvironmentBlock, exitStatus: ExitStatus, output: Result<[UInt8], Swift.Error>, stderrOutput: Result<[UInt8], Swift.Error> ) { self.arguments = arguments - self.environment = environment + self.environmentBlock = environmentBlock self.output = output self.stderrOutput = stderrOutput self.exitStatus = exitStatus } + @available(*, deprecated, message: "use `init(arguments:environmentBlock:exitStatus:output:stderrOutput:)`") + public init( + arguments: [String], + environment: [String:String], + exitStatus: ExitStatus, + output: Result<[UInt8], Swift.Error>, + stderrOutput: Result<[UInt8], Swift.Error> + ) { + self.init( + arguments: arguments, + environmentBlock: .init(environment), + exitStatus: exitStatus, + output: output, + stderrOutput: stderrOutput + ) + } + /// Converts stdout output bytes to string, assuming they're UTF8. public func utf8Output() throws -> String { return String(decoding: try output.get(), as: Unicode.UTF8.self) @@ -245,14 +285,23 @@ public final class Process { /// The current environment. @available(*, deprecated, message: "use ProcessEnv.vars instead") static public var env: [String: String] { - return ProcessInfo.processInfo.environment + ProcessEnv.vars } /// The arguments to execute. public let arguments: [String] /// The environment with which the process was executed. - public let environment: [String: String] + @available(*, deprecated, message: "use `environmentBlock` instead") + public var environment: [String:String] { + #if os(Windows) + Dictionary(uniqueKeysWithValues: environmentBlock.map { ($0.key.value, $0.value) }) + #else + environmentBlock + #endif + } + + public let environmentBlock: ProcessEnvironmentBlock /// The path to the directory under which to run the process. public let workingDirectory: AbsolutePath? @@ -323,23 +372,43 @@ public final class Process { /// continue running even if the parent is killed or interrupted. Default value is true. /// - loggingHandler: Handler for logging messages /// - @available(macOS 10.15, *) public init( arguments: [String], - environment: [String: String] = ProcessEnv.vars, + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, workingDirectory: AbsolutePath, outputRedirection: OutputRedirection = .collect, startNewProcessGroup: Bool = true, loggingHandler: LoggingHandler? = .none ) { self.arguments = arguments - self.environment = environment + self.environmentBlock = environmentBlock self.workingDirectory = workingDirectory self.outputRedirection = outputRedirection self.startNewProcessGroup = startNewProcessGroup self.loggingHandler = loggingHandler ?? Process.loggingHandler } + @_disfavoredOverload + @available(macOS 10.15, *) + @available(*, deprecated, renamed: "init(arguments:environmentBlock:workingDirectory:outputRedirection:startNewProcessGroup:loggingHandler:)") + public convenience init( + arguments: [String], + environment: [String:String] = ProcessEnv.vars, + workingDirectory: AbsolutePath, + outputRedirection: OutputRedirection = .collect, + startNewProcessGroup: Bool = true, + loggingHandler: LoggingHandler? = .none + ) { + self.init( + arguments: arguments, + environmentBlock: .init(environment), + workingDirectory: workingDirectory, + outputRedirection: outputRedirection, + startNewProcessGroup: startNewProcessGroup, + loggingHandler: loggingHandler + ) + } + /// Create a new process instance. /// /// - Parameters: @@ -351,21 +420,49 @@ public final class Process { /// - startNewProcessGroup: If true, a new progress group is created for the child making it /// continue running even if the parent is killed or interrupted. Default value is true. /// - loggingHandler: Handler for logging messages - public init( - arguments: [String], - environment: [String: String] = ProcessEnv.vars, - outputRedirection: OutputRedirection = .collect, - startNewProcessGroup: Bool = true, - loggingHandler: LoggingHandler? = .none - ) { + public init(arguments: [String], environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, outputRedirection: OutputRedirection = .collect, startNewProcessGroup: Bool = true, loggingHandler: LoggingHandler? = .none) { self.arguments = arguments - self.environment = environment + self.environmentBlock = environmentBlock self.workingDirectory = nil self.outputRedirection = outputRedirection self.startNewProcessGroup = startNewProcessGroup self.loggingHandler = loggingHandler ?? Process.loggingHandler } + @_disfavoredOverload + @available(*, deprecated, renamed: "init(arguments:environmentBlock:outputRedirection:startNewProcessGroup:loggingHandler:)") + public convenience init( + arguments: [String], + environment: [String:String] = ProcessEnv.vars, + outputRedirection: OutputRedirection = .collect, + startNewProcessGroup: Bool = true, + loggingHandler: LoggingHandler? = .none + ) { + self.init( + arguments: arguments, + environmentBlock: .init(environment), + outputRedirection: outputRedirection, + startNewProcessGroup: startNewProcessGroup, + loggingHandler: loggingHandler + ) + } + + public convenience init( + args: String..., + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, + outputRedirection: OutputRedirection = .collect, + loggingHandler: LoggingHandler? = .none + ) { + self.init( + arguments: args, + environmentBlock: environmentBlock, + outputRedirection: outputRedirection, + loggingHandler: loggingHandler + ) + } + + @_disfavoredOverload + @available(*, deprecated, renamed: "init(args:environmentBlock:outputRedirection:loggingHandler:)") public convenience init( args: String..., environment: [String: String] = ProcessEnv.vars, @@ -374,7 +471,7 @@ public final class Process { ) { self.init( arguments: args, - environment: environment, + environmentBlock: .init(environment), outputRedirection: outputRedirection, loggingHandler: loggingHandler ) @@ -460,7 +557,7 @@ public final class Process { process.currentDirectoryURL = workingDirectory.asURL } process.executableURL = executablePath.asURL - process.environment = environment + process.environment = Dictionary(uniqueKeysWithValues: environmentBlock.map { ($0.key.value, $0.value) }) let stdinPipe = Pipe() process.standardInput = stdinPipe @@ -817,7 +914,7 @@ public final class Process { // Construct the result. let executionResult = ProcessResult( arguments: arguments, - environment: environment, + environmentBlock: environmentBlock, exitStatusCode: exitStatusCode, normal: normalExit, output: stdoutResult, @@ -900,15 +997,14 @@ extension Process { /// - environment: The environment to pass to subprocess. By default the current process environment /// will be inherited. /// - loggingHandler: Handler for logging messages - @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) static public func popen( arguments: [String], - environment: [String: String] = ProcessEnv.vars, + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, loggingHandler: LoggingHandler? = .none ) async throws -> ProcessResult { let process = Process( arguments: arguments, - environment: environment, + environmentBlock: environmentBlock, outputRedirection: .collect, loggingHandler: loggingHandler ) @@ -916,6 +1012,17 @@ extension Process { return try await process.waitUntilExit() } + @_disfavoredOverload + @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) + @available(*, deprecated, renamed: "popen(arguments:environmentBlock:loggingHandler:)") + static public func popen( + arguments: [String], + environment: [String:String] = ProcessEnv.vars, + loggingHandler: LoggingHandler? = .none + ) async throws -> ProcessResult { + try await popen(arguments: arguments, environmentBlock: .init(environment), loggingHandler: loggingHandler) + } + /// Execute a subprocess and returns the result when it finishes execution /// /// - Parameters: @@ -923,13 +1030,23 @@ extension Process { /// - environment: The environment to pass to subprocess. By default the current process environment /// will be inherited. /// - loggingHandler: Handler for logging messages + static public func popen( + args: String..., + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, + loggingHandler: LoggingHandler? = .none + ) async throws -> ProcessResult { + try await popen(arguments: args, environmentBlock: environmentBlock, loggingHandler: loggingHandler) + } + + @_disfavoredOverload @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) + @available(*, deprecated, renamed: "popen(args:environmentBlock:loggingHandler:)") static public func popen( args: String..., environment: [String: String] = ProcessEnv.vars, loggingHandler: LoggingHandler? = .none ) async throws -> ProcessResult { - try await popen(arguments: args, environment: environment, loggingHandler: loggingHandler) + try await popen(arguments: args, environmentBlock: .init(environment), loggingHandler: loggingHandler) } /// Execute a subprocess and get its (UTF-8) output if it has a non zero exit. @@ -940,14 +1057,13 @@ extension Process { /// will be inherited. /// - loggingHandler: Handler for logging messages /// - Returns: The process output (stdout + stderr). - @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) @discardableResult static public func checkNonZeroExit( arguments: [String], - environment: [String: String] = ProcessEnv.vars, + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, loggingHandler: LoggingHandler? = .none ) async throws -> String { - let result = try await popen(arguments: arguments, environment: environment, loggingHandler: loggingHandler) + let result = try await popen(arguments: arguments, environmentBlock: environmentBlock, loggingHandler: loggingHandler) // Throw if there was a non zero termination. guard result.exitStatus == .terminated(code: 0) else { throw ProcessResult.Error.nonZeroExit(result) @@ -955,6 +1071,18 @@ extension Process { return try result.utf8Output() } + @_disfavoredOverload + @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) + @available(*, deprecated, renamed: "checkNonZeroExit(arguments:environmentBlock:loggingHandler:)") + @discardableResult + static public func checkNonZeroExit( + arguments: [String], + environment: [String: String] = ProcessEnv.vars, + loggingHandler: LoggingHandler? = .none + ) async throws -> String { + try await checkNonZeroExit(arguments: arguments, environmentBlock: .init(environment), loggingHandler: loggingHandler) + } + /// Execute a subprocess and get its (UTF-8) output if it has a non zero exit. /// /// - Parameters: @@ -963,14 +1091,25 @@ extension Process { /// will be inherited. /// - loggingHandler: Handler for logging messages /// - Returns: The process output (stdout + stderr). + @discardableResult + static public func checkNonZeroExit( + args: String..., + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, + loggingHandler: LoggingHandler? = .none + ) async throws -> String { + try await checkNonZeroExit(arguments: args, environmentBlock: environmentBlock, loggingHandler: loggingHandler) + } + + @_disfavoredOverload @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) + @available(*, deprecated, renamed: "checkNonZeroExit(args:environmentBlock:loggingHandler:)") @discardableResult static public func checkNonZeroExit( args: String..., environment: [String: String] = ProcessEnv.vars, loggingHandler: LoggingHandler? = .none ) async throws -> String { - try await checkNonZeroExit(arguments: args, environment: environment, loggingHandler: loggingHandler) + try await checkNonZeroExit(arguments: args, environmentBlock: .init(environment), loggingHandler: loggingHandler) } } @@ -989,7 +1128,7 @@ extension Process { // #endif static public func popen( arguments: [String], - environment: [String: String] = ProcessEnv.vars, + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, loggingHandler: LoggingHandler? = .none, queue: DispatchQueue? = nil, completion: @escaping (Result) -> Void @@ -999,7 +1138,7 @@ extension Process { do { let process = Process( arguments: arguments, - environment: environment, + environmentBlock: environmentBlock, outputRedirection: .collect, loggingHandler: loggingHandler ) @@ -1013,6 +1152,24 @@ extension Process { } } + @_disfavoredOverload + @available(*, deprecated, renamed: "popen(arguments:environmentBlock:loggingHandler:queue:completion:)") + static public func popen( + arguments: [String], + environment: [String:String] = ProcessEnv.vars, + loggingHandler: LoggingHandler? = .none, + queue: DispatchQueue? = nil, + completion: @escaping (Result) -> Void + ) { + popen( + arguments: arguments, + environmentBlock: .init(environment), + loggingHandler: loggingHandler, + queue: queue, + completion: completion + ) + } + /// Execute a subprocess and block until it finishes execution /// /// - Parameters: @@ -1027,12 +1184,12 @@ extension Process { @discardableResult static public func popen( arguments: [String], - environment: [String: String] = ProcessEnv.vars, + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, loggingHandler: LoggingHandler? = .none ) throws -> ProcessResult { let process = Process( arguments: arguments, - environment: environment, + environmentBlock: environmentBlock, outputRedirection: .collect, loggingHandler: loggingHandler ) @@ -1040,6 +1197,17 @@ extension Process { return try process.waitUntilExit() } + @_disfavoredOverload + @available(*, deprecated, renamed: "popen(arguments:environmentBlock:loggingHandler:)") + @discardableResult + static public func popen( + arguments: [String], + environment: [String:String] = ProcessEnv.vars, + loggingHandler: LoggingHandler? = .none + ) throws -> ProcessResult { + try popen(arguments: arguments, environmentBlock: .init(environment), loggingHandler: loggingHandler) + } + /// Execute a subprocess and block until it finishes execution /// /// - Parameters: @@ -1054,10 +1222,21 @@ extension Process { @discardableResult static public func popen( args: String..., - environment: [String: String] = ProcessEnv.vars, + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, + loggingHandler: LoggingHandler? = .none + ) throws -> ProcessResult { + return try Process.popen(arguments: args, environmentBlock: environmentBlock, loggingHandler: loggingHandler) + } + + @_disfavoredOverload + @available(*, deprecated, renamed: "popen(args:environmentBlock:loggingHandler:)") + @discardableResult + static public func popen( + args: String..., + environment: [String:String] = ProcessEnv.vars, loggingHandler: LoggingHandler? = .none ) throws -> ProcessResult { - return try Process.popen(arguments: args, environment: environment, loggingHandler: loggingHandler) + return try Process.popen(arguments: args, environmentBlock: .init(environment), loggingHandler: loggingHandler) } /// Execute a subprocess and get its (UTF-8) output if it has a non zero exit. @@ -1074,12 +1253,12 @@ extension Process { @discardableResult static public func checkNonZeroExit( arguments: [String], - environment: [String: String] = ProcessEnv.vars, + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, loggingHandler: LoggingHandler? = .none ) throws -> String { let process = Process( arguments: arguments, - environment: environment, + environmentBlock: environmentBlock, outputRedirection: .collect, loggingHandler: loggingHandler ) @@ -1092,6 +1271,17 @@ extension Process { return try result.utf8Output() } + @_disfavoredOverload + @available(*, deprecated, renamed: "checkNonZeroExit(arguments:environmentBlock:loggingHandler:)") + @discardableResult + static public func checkNonZeroExit( + arguments: [String], + environment: [String:String] = ProcessEnv.vars, + loggingHandler: LoggingHandler? = .none + ) throws -> String { + try checkNonZeroExit(arguments: arguments, environmentBlock: .init(environment), loggingHandler: loggingHandler) + } + /// Execute a subprocess and get its (UTF-8) output if it has a non zero exit. /// /// - Parameters: @@ -1106,10 +1296,21 @@ extension Process { @discardableResult static public func checkNonZeroExit( args: String..., - environment: [String: String] = ProcessEnv.vars, + environmentBlock: ProcessEnvironmentBlock = ProcessEnv.block, + loggingHandler: LoggingHandler? = .none + ) throws -> String { + return try checkNonZeroExit(arguments: args, environmentBlock: environmentBlock, loggingHandler: loggingHandler) + } + + @_disfavoredOverload + @available(*, deprecated, renamed: "checkNonZeroExit(args:environmentBlock:loggingHandler:)") + @discardableResult + static public func checkNonZeroExit( + args: String..., + environment: [String:String] = ProcessEnv.vars, loggingHandler: LoggingHandler? = .none ) throws -> String { - return try checkNonZeroExit(arguments: args, environment: environment, loggingHandler: loggingHandler) + try checkNonZeroExit(arguments: args, environmentBlock: .init(environment), loggingHandler: loggingHandler) } } diff --git a/Sources/TSCBasic/Process/ProcessEnv.swift b/Sources/TSCBasic/Process/ProcessEnv.swift index 226406aa..239b4652 100644 --- a/Sources/TSCBasic/Process/ProcessEnv.swift +++ b/Sources/TSCBasic/Process/ProcessEnv.swift @@ -11,16 +11,75 @@ import Foundation import TSCLibc +public struct ProcessEnvironmentKey { + public let value: String + public init(_ value: String) { + self.value = value + } +} + +extension ProcessEnvironmentKey: Equatable { + public static func == (_ lhs: Self, _ rhs: Self) -> Bool { + #if os(Windows) + // TODO: is this any faster than just doing a lowercased conversion and compare? + return lhs.value.caseInsensitiveCompare(rhs.value) == .orderedSame + #else + return lhs.value == rhs.value + #endif + } +} + +extension ProcessEnvironmentKey: ExpressibleByStringLiteral { + public init(stringLiteral value: String) { + self.init(value) + } +} + +extension ProcessEnvironmentKey: Hashable { + public func hash(into hasher: inout Hasher) { + #if os(Windows) + self.value.lowercased().hash(into: &hasher) + #else + self.value.hash(into: &hasher) + #endif + } +} + +extension ProcessEnvironmentKey: Sendable {} + +public typealias ProcessEnvironmentBlock = [ProcessEnvironmentKey:String] +extension ProcessEnvironmentBlock { + public init(_ dictionary: [String:String]) { + self.init(uniqueKeysWithValues: dictionary.map { (ProcessEnvironmentKey($0.key), $0.value) }) + } +} + +extension ProcessEnvironmentBlock: Sendable {} + /// Provides functionality related a process's environment. public enum ProcessEnv { + @available(*, deprecated, message: "Use `block` instead") + public static var vars: [String:String] { + Dictionary(uniqueKeysWithValues: _vars.map { ($0.key.value, $0.value) }) + } + /// Returns a dictionary containing the current environment. - public static var vars: [String: String] { _vars } - private static var _vars = ProcessInfo.processInfo.environment + public static var block: ProcessEnvironmentBlock { _vars } + + private static var _vars = ProcessEnvironmentBlock( + uniqueKeysWithValues: ProcessInfo.processInfo.environment.map { + (ProcessEnvironmentBlock.Key($0.key), $0.value) + } + ) /// Invalidate the cached env. public static func invalidateEnv() { - _vars = ProcessInfo.processInfo.environment + _vars = ProcessEnvironmentBlock( + uniqueKeysWithValues: ProcessInfo.processInfo.environment.map { + (ProcessEnvironmentKey($0.key), $0.value) + } + ) } /// Set the given key and value in the process's environment. @@ -53,12 +112,7 @@ public enum ProcessEnv { /// `PATH` variable in the process's environment (`Path` under Windows). public static var path: String? { -#if os(Windows) - let pathArg = "Path" -#else - let pathArg = "PATH" -#endif - return vars[pathArg] + return block["PATH"] } /// The current working directory of the process. diff --git a/Tests/TSCBasicTests/ProcessEnvTests.swift b/Tests/TSCBasicTests/ProcessEnvTests.swift index aa54742f..05c3b413 100644 --- a/Tests/TSCBasicTests/ProcessEnvTests.swift +++ b/Tests/TSCBasicTests/ProcessEnvTests.swift @@ -55,4 +55,13 @@ class ProcessEnvTests: XCTestCase { } XCTAssertNil(ProcessEnv.vars[key]) } + + func testEnvironmentKeys() throws { + XCTAssertEqual(ProcessEnvironmentKey("Key"), "Key") + #if os(Windows) + XCTAssertEqual(ProcessEnvironmentKey("Key"), "KEY") + #else + XCTAssertNotEqual(ProcessEnvironmentKey("Key"), "KEY") + #endif + } }