fix an isse where usre defined DYLD_LIBRARY_PATH gets ignored (#3773)

motivation: users should be able to set DYLD_LIBRARY_PATH

changes:
* define new utility named EnvironmentVariables
* add prependPath and appendPath helpers to EnvironmentVariables
* update call sites that take [String:String] as environment variables to use the new EnvironmentVariables
* fix the code in SwiftTestTool that stripped DYLD_LIBRARY_PATH to use EnvironmentVariables::appendPath which gives user's setting preference
* add tests for EnvironmentVariables

rdar://83607631

Author: tomerd

Sources/Basics/CMakeLists.txt

@@ -12,6 +12,7 @@ add_library(Basics
   ConcurrencyHelpers.swift
   Dictionary+Extensions.swift
   DispatchTimeInterval+Extensions.swift
+  EnvironmentVariables.swift
   Errors.swift
   FileSystem+Extensions.swift
   HTPClient+URLSession.swift

Sources/Basics/EnvironmentVariables.swift

@@ -0,0 +1,50 @@
+/*
+ This source file is part of the Swift.org open source project
+
+ Copyright (c) 2021 Apple Inc. and the Swift project authors
+ Licensed under Apache License v2.0 with Runtime Library Exception
+
+ See http://swift.org/LICENSE.txt for license information
+ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
+ */
+
+import TSCBasic
+import Foundation
+
+public typealias EnvironmentVariables = [String: String]
+
+extension EnvironmentVariables {
+
+    public static func empty() -> EnvironmentVariables {
+        return [:]
+    }
+
+    public static func process() -> EnvironmentVariables {
+        return ProcessInfo.processInfo.environment
+    }
+
+    public mutating func prependPath(_ key: String, value: String) {
+        var values = value.isEmpty ? [] : [value]
+        if let existing = self[key], !existing.isEmpty {
+            values.append(existing)
+        }
+        self.setPath(key, values)
+    }
+
+    public mutating func appendPath(_ key: String, value: String) {
+        var values = value.isEmpty ? [] : [value]
+        if let existing = self[key], !existing.isEmpty {
+            values.insert(existing, at: 0)
+        }
+        self.setPath(key, values)
+    }
+
+    private mutating func setPath(_ key: String, _ values: [String]) {
+        #if os(Windows)
+        let delimiter = ";"
+        #else
+        let delimiter = ":"
+        #endif
+        self[key] = values.joined(separator: delimiter)
+    }
+}

Sources/Build/SPMSwiftDriverExecutor.swift

@@ -28,11 +28,11 @@ final class SPMSwiftDriverExecutor: DriverExecutor {
 
   let resolver: ArgsResolver
   let fileSystem: FileSystem
-  let env: [String: String]
+  let env: EnvironmentVariables
 
   init(resolver: ArgsResolver,
        fileSystem: FileSystem,
-       env: [String: String]) {
+       env: EnvironmentVariables) {
     self.resolver = resolver
     self.fileSystem = fileSystem
     self.env = env

Sources/Commands/SwiftTestTool.swift

@@ -59,11 +59,11 @@ struct TestToolOptions: ParsableArguments {
 
         return .runSerial
     }
-    
+
     @Flag(name: .customLong("skip-build"),
           help: "Skip building the test target")
     var shouldSkipBuilding: Bool = false
-    
+
     /// If the test target should be built before testing.
     var shouldBuildTests: Bool {
         !shouldSkipBuilding
@@ -97,7 +97,7 @@ struct TestToolOptions: ParsableArguments {
         if !filter.isEmpty {
             return .regex(filter)
         }
-        
+
         return _testCaseSpecifier.map { .specific($0) } ?? .none
     }
 
@@ -115,7 +115,7 @@ struct TestToolOptions: ParsableArguments {
         if let override = testCaseSkipOverride() {
             return override
         }
-      
+
         return _testCaseSkip.isEmpty
             ? .none
             : .skip(_testCaseSkip)
@@ -197,11 +197,11 @@ public struct SwiftTestTool: SwiftCommand {
 
     @OptionGroup()
     var options: TestToolOptions
-    
+
     var shouldEnableCodeCoverage: Bool {
         swiftOptions.shouldEnableCodeCoverage
     }
-    
+
     public func run(_ swiftTool: SwiftTool) throws {
         // Validate commands arguments
         try self.validateArguments(observabilityScope: swiftTool.observabilityScope)
@@ -501,25 +501,26 @@ public struct SwiftTestTool: SwiftCommand {
     /// - Returns: Array of TestSuite
     fileprivate func getTestSuites(fromTestAt path: AbsolutePath, swiftTool: SwiftTool) throws -> [TestSuite] {
         // Run the correct tool.
-      #if os(macOS)
+        #if os(macOS)
         let data: String = try withTemporaryFile { tempFile in
             let args = [try xctestHelperPath(swiftTool: swiftTool).pathString, path.pathString, tempFile.path.pathString]
             var env = try constructTestEnvironment(toolchain: try swiftTool.getToolchain(), options: swiftOptions, buildParameters: swiftTool.buildParametersForTest())
             // Add the sdk platform path if we have it. If this is not present, we
             // might always end up failing.
             if let sdkPlatformFrameworksPath = Destination.sdkPlatformFrameworkPaths() {
-                env["DYLD_FRAMEWORK_PATH"] = sdkPlatformFrameworksPath.fwk.pathString
-                env["DYLD_LIBRARY_PATH"] = sdkPlatformFrameworksPath.lib.pathString
+                // appending since we prefer the user setting (if set) to the one we inject
+                env.appendPath("DYLD_FRAMEWORK_PATH", value: sdkPlatformFrameworksPath.fwk.pathString)
+                env.appendPath("DYLD_LIBRARY_PATH", value: sdkPlatformFrameworksPath.lib.pathString)
             }
             try Process.checkNonZeroExit(arguments: args, environment: env)
             // Read the temporary file's content.
             return try localFileSystem.readFileContents(tempFile.path).validDescription ?? ""
         }
-      #else
+        #else
         let env = try constructTestEnvironment(toolchain: try swiftTool.getToolchain(), options: swiftOptions, buildParameters: swiftTool.buildParametersForTest())
         let args = [path.description, "--dump-tests-json"]
         let data = try Process.checkNonZeroExit(arguments: args, environment: env)
-      #endif
+        #endif
         // Parse json and return TestSuites.
         return try TestSuite.parse(jsonString: data)
     }
@@ -542,12 +543,12 @@ public struct SwiftTestTool: SwiftCommand {
                 throw ExitCode.failure
             }
         }
-        
+
         if options.shouldGenerateLinuxMain {
             observabilityScope.emit(warning: "'--generate-linuxmain' option is deprecated; tests are automatically discovered on all platforms")
         }
     }
-    
+
     public init() {}
 }
 
@@ -1016,8 +1017,8 @@ fileprivate func constructTestEnvironment(
     toolchain: UserToolchain,
     options: SwiftToolOptions,
     buildParameters: BuildParameters
-) throws -> [String: String] {
-    var env = ProcessEnv.vars
+) throws -> EnvironmentVariables {
+    var env = EnvironmentVariables.process()
 
     // Add the code coverage related variables.
     if options.shouldEnableCodeCoverage {
@@ -1034,7 +1035,7 @@ fileprivate func constructTestEnvironment(
     #if !os(macOS)
     #if os(Windows)
     if let location = toolchain.configuration.xctestPath {
-        env["Path"] = "\(location.pathString);\(env["Path"] ?? "")"
+        env.prependPath("Path", value: location.pathString)
     }
     #endif
     return env

Sources/LLBuildManifest/BuildManifest.swift

@@ -8,6 +8,7 @@
  See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 */
 
+import Basics
 import struct TSCBasic.AbsolutePath
 
 public struct BuildManifest {
@@ -101,7 +102,7 @@ public struct BuildManifest {
         inputs: [Node],
         outputs: [Node],
         arguments: [String],
-        environment: [String: String] = [:],
+        environment: EnvironmentVariables = .empty(),
         workingDirectory: String? = nil,
         allowMissingInputs: Bool = false
     ) {

Sources/LLBuildManifest/Tools.swift

@@ -8,6 +8,7 @@
  See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 */
 
+import Basics
 import class Foundation.ProcessInfo
 import TSCBasic
 
@@ -97,7 +98,7 @@ public struct ShellTool: ToolProtocol {
     public var inputs: [Node]
     public var outputs: [Node]
     public var arguments: [String]
-    public var environment: [String: String]
+    public var environment: EnvironmentVariables
     public var workingDirectory: String?
     public var allowMissingInputs: Bool
 
@@ -106,7 +107,7 @@ public struct ShellTool: ToolProtocol {
         inputs: [Node],
         outputs: [Node],
         arguments: [String],
-        environment: [String: String] = [:],
+        environment: EnvironmentVariables = .empty(),
         workingDirectory: String? = nil,
         allowMissingInputs: Bool = false
     ) {

Sources/PackageLoading/ManifestLoader.swift

@@ -567,14 +567,14 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         let manifestPath: AbsolutePath
         let manifestContents: [UInt8]
         let toolsVersion: ToolsVersion
-        let env: [String: String]
+        let env: EnvironmentVariables
         let swiftpmVersion: String
         let sha256Checksum: String
 
         init (packageIdentity: PackageIdentity,
               manifestPath: AbsolutePath,
               toolsVersion: ToolsVersion,
-              env: [String: String],
+              env: EnvironmentVariables,
               swiftpmVersion: String,
               fileSystem: FileSystem
         ) throws {
@@ -598,7 +598,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
             packageIdentity: PackageIdentity,
             manifestContents: [UInt8],
             toolsVersion: ToolsVersion,
-            env: [String: String],
+            env: EnvironmentVariables,
             swiftpmVersion: String
         ) throws -> String {
             let stream = BufferedOutputByteStream()

Sources/PackageModel/ToolchainConfiguration.swift

@@ -8,6 +8,7 @@
  See http://swift.org/CONTRIBUTORS.txt for Swift project authors
  */
 
+import Basics
 import TSCBasic
 
 /// Toolchain configuration required for evaluation of swift code such as the manifests or plugins
@@ -22,7 +23,7 @@ public struct ToolchainConfiguration {
     public var swiftCompilerFlags: [String]
 
     /// Environment to pass to the Swift compiler (defaults to the inherited environment).
-    public var swiftCompilerEnvironment: [String: String]
+    public var swiftCompilerEnvironment: EnvironmentVariables
 
     /// SwiftPM library paths.
     public var swiftPMLibrariesLocation: SwiftPMLibrariesLocation
@@ -47,7 +48,7 @@ public struct ToolchainConfiguration {
     public init(
         swiftCompilerPath: AbsolutePath,
         swiftCompilerFlags: [String] = [],
-        swiftCompilerEnvironment: [String: String] = ProcessEnv.vars,
+        swiftCompilerEnvironment: EnvironmentVariables = .process(),
         swiftPMLibrariesLocation: SwiftPMLibrariesLocation? = nil,
         sdkRootPath: AbsolutePath? = nil,
         xctestPath: AbsolutePath? = nil
@@ -69,7 +70,7 @@ public struct ToolchainConfiguration {
     public init(
         swiftCompiler: AbsolutePath,
         swiftCompilerFlags: [String] = [],
-        swiftCompilerEnvironment: [String: String] = ProcessEnv.vars,
+        swiftCompilerEnvironment: EnvironmentVariables = .process(),
         libDir: AbsolutePath? = nil,
         binDir: AbsolutePath? = nil,
         sdkRoot: AbsolutePath? = nil,

Sources/SPMTestSupport/XCTAssertHelpers.swift

@@ -8,16 +8,14 @@
  See http://swift.org/CONTRIBUTORS.txt for Swift project authors
  */
 
-import XCTest
-
-import TSCBasic
-import TSCUtility
-
+import Basics
 #if os(macOS)
 import class Foundation.Bundle
 #endif
-
+import TSCBasic
 @_exported import TSCTestSupport
+import TSCUtility
+import XCTest
 
 public func XCTAssertBuilds(
     _ path: AbsolutePath,
@@ -28,7 +26,7 @@ public func XCTAssertBuilds(
     Xcc: [String] = [],
     Xld: [String] = [],
     Xswiftc: [String] = [],
-    env: [String: String]? = nil
+    env: EnvironmentVariables? = nil
 ) {
     for conf in configurations {
         do {
@@ -56,7 +54,7 @@ public func XCTAssertSwiftTest(
     _ path: AbsolutePath,
     file: StaticString = #file,
     line: UInt = #line,
-    env: [String: String]? = nil
+    env: EnvironmentVariables? = nil
 ) {
     do {
         _ = try SwiftPMProduct.SwiftTest.execute([], packagePath: path, env: env)
@@ -77,7 +75,7 @@ public func XCTAssertBuildFails(
     Xcc: [String] = [],
     Xld: [String] = [],
     Xswiftc: [String] = [],
-    env: [String: String]? = nil
+    env: EnvironmentVariables? = nil
 ) {
     do {
         _ = try executeSwiftBuild(path, Xcc: Xcc, Xld: Xld, Xswiftc: Xswiftc)

Sources/SPMTestSupport/misc.swift

@@ -147,7 +147,7 @@ public func executeSwiftBuild(
     Xcc: [String] = [],
     Xld: [String] = [],
     Xswiftc: [String] = [],
-    env: [String: String]? = nil
+    env: EnvironmentVariables? = nil
 ) throws -> (stdout: String, stderr: String) {
     let args = swiftArgs(configuration: configuration, extraArgs: extraArgs, Xcc: Xcc, Xld: Xld, Xswiftc: Xswiftc)
     return try SwiftPMProduct.SwiftBuild.execute(args, packagePath: packagePath, env: env)
@@ -162,7 +162,7 @@ public func executeSwiftRun(
     Xcc: [String] = [],
     Xld: [String] = [],
     Xswiftc: [String] = [],
-    env: [String: String]? = nil
+    env: EnvironmentVariables? = nil
 ) throws -> (stdout: String, stderr: String) {
     var args = swiftArgs(configuration: configuration, extraArgs: extraArgs, Xcc: Xcc, Xld: Xld, Xswiftc: Xswiftc)
     args.append(executable)
@@ -177,7 +177,7 @@ public func executeSwiftPackage(
     Xcc: [String] = [],
     Xld: [String] = [],
     Xswiftc: [String] = [],
-    env: [String: String]? = nil
+    env: EnvironmentVariables? = nil
 ) throws -> (stdout: String, stderr: String) {
     let args = swiftArgs(configuration: configuration, extraArgs: extraArgs, Xcc: Xcc, Xld: Xld, Xswiftc: Xswiftc)
     return try SwiftPMProduct.SwiftPackage.execute(args, packagePath: packagePath, env: env)
@@ -191,7 +191,7 @@ public func executeSwiftTest(
     Xcc: [String] = [],
     Xld: [String] = [],
     Xswiftc: [String] = [],
-    env: [String: String]? = nil
+    env: EnvironmentVariables? = nil
 ) throws -> (stdout: String, stderr: String) {
     let args = swiftArgs(configuration: configuration, extraArgs: extraArgs, Xcc: Xcc, Xld: Xld, Xswiftc: Xswiftc)
     return try SwiftPMProduct.SwiftTest.execute(args, packagePath: packagePath, env: env)