filter transient env variables from manifest and plugin cache keys (#7056)

motivation: When changing environment variables manifest loader and
plugin cache reset too aggressivly

changes:
* introduce a static list of env variables that are transiant in nature
* filter transient env variables from cache keys used in manfiest loader
and plugin caches
* adjust and add tests

rdar://107029374

Author: tomerd

Sources/Basics/EnvironmentVariables.swift

@@ -10,7 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import Foundation
+import class Foundation.ProcessInfo
 
 public typealias EnvironmentVariables = [String: String]
 
@@ -58,3 +58,30 @@ extension EnvironmentVariables {
         return self[pathArg]
     }
 }
+
+// filter env variable that should not be included in a cache as they change
+// often and should not be considered in business logic
+// rdar://107029374
+extension EnvironmentVariables {
+    // internal for testing
+    internal static let nonCachableKeys: Set<String> = [
+        "TERM",
+        "TERM_PROGRAM",
+        "TERM_PROGRAM_VERSION",
+        "TERM_SESSION_ID",
+        "ITERM_PROFILE",
+        "ITERM_SESSION_ID",
+        "SECURITYSESSIONID",
+        "LaunchInstanceID",
+        "LC_TERMINAL",
+        "LC_TERMINAL_VERSION",
+        "CLICOLOR",
+        "LS_COLORS",
+        "VSCODE_IPC_HOOK_CLI",
+        "HYPERFINE_RANDOMIZED_ENVIRONMENT_OFFSET",
+    ]
+
+    public var cachable: EnvironmentVariables {
+        return self.filter { !Self.nonCachableKeys.contains($0.key) }
+    }
+}

Sources/PackageLoading/ManifestLoader.swift

@@ -497,7 +497,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
                 packageLocation: packageLocation,
                 manifestPath: path,
                 toolsVersion: toolsVersion,
-                env: ProcessEnv.vars,
+                env: ProcessEnv.cachableVars,
                 swiftpmVersion: SwiftVersion.current.displayString,
                 fileSystem: fileSystem
             )
@@ -1210,3 +1210,9 @@ extension ManifestLoader {
         }
     }
 }
+
+extension ProcessEnv {
+    fileprivate static var cachableVars: EnvironmentVariables {
+        Self.vars.cachable
+    }
+}

Sources/Workspace/DefaultPluginScriptRunner.swift

@@ -270,7 +270,7 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
         /// Persisted information about the last time the compiler was invoked.
         struct PersistedCompilationState: Codable {
             var commandLine: [String]
-            var environment: [String:String]
+            var environment: EnvironmentVariables
             var inputHash: String?
             var output: String
             var result: Result
@@ -364,7 +364,7 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
                 // Save the persisted compilation state for possible reuse next time.
                 let compilationState = PersistedCompilationState(
                     commandLine: commandLine,
-                    environment: toolchain.swiftCompilerEnvironment,
+                    environment: toolchain.swiftCompilerEnvironment.cachable,
                     inputHash: compilerInputHash,
                     output: compilerOutput,
                     result: .init(process.exitStatus))

Tests/PackageLoadingTests/ManifestLoaderCacheTests.swift

@@ -10,13 +10,14 @@
 //
 //===----------------------------------------------------------------------===//
 
-import Basics
+@testable import Basics
 @testable import PackageLoading
 import PackageModel
 import SPMTestSupport
 import XCTest
 
 import class TSCBasic.InMemoryFileSystem
+import enum TSCBasic.ProcessEnv
 import func TSCTestSupport.withCustomEnv
 
 class ManifestLoaderCacheTests: XCTestCase {
@@ -297,6 +298,21 @@ class ManifestLoaderCacheTests: XCTestCase {
                 delegate: delegate
             )
 
+            try check(loader: manifestLoader, expectCached: false)
+            try check(loader: manifestLoader, expectCached: true)
+
+            try withCustomEnv(["SWIFTPM_MANIFEST_CACHE_TEST": "1"]) {
+                try check(loader: manifestLoader, expectCached: false)
+                try check(loader: manifestLoader, expectCached: true)
+            }
+
+            try withCustomEnv(["SWIFTPM_MANIFEST_CACHE_TEST": "2"]) {
+                try check(loader: manifestLoader, expectCached: false)
+                try check(loader: manifestLoader, expectCached: true)
+            }
+
+            try check(loader: manifestLoader, expectCached: true)
+
             func check(loader: ManifestLoader, expectCached: Bool) throws {
                 delegate.clear()
                 delegate.prepare(expectParsing: !expectCached)
@@ -315,18 +331,65 @@ class ManifestLoaderCacheTests: XCTestCase {
                 XCTAssertEqual(manifest.displayName, "Trivial")
                 XCTAssertEqual(manifest.targets[0].name, "foo")
             }
+        }
+    }
 
-            try check(loader: manifestLoader, expectCached: false)
-            try check(loader: manifestLoader, expectCached: true)
+    func testCacheDoNotInvalidationExpectedEnv() throws {
+        try testWithTemporaryDirectory { path in
+            let fileSystem = InMemoryFileSystem()
+            let observability = ObservabilitySystem.makeForTesting()
 
-            try withCustomEnv(["SWIFTPM_MANIFEST_CACHE_TEST": "1"]) {
-                try check(loader: manifestLoader, expectCached: false)
-                try check(loader: manifestLoader, expectCached: true)
+            let manifestPath = path.appending(components: "pkg", "Package.swift")
+            try fileSystem.createDirectory(manifestPath.parentDirectory, recursive: true)
+            try fileSystem.writeFileContents(
+                manifestPath,
+                string: """
+                    import PackageDescription
+                    let package = Package(
+                        name: "Trivial",
+                        targets: [
+                            .target(
+                                name: "foo",
+                                dependencies: []),
+                        ]
+                    )
+                    """
+            )
+
+            let delegate = ManifestTestDelegate()
+
+            let manifestLoader = ManifestLoader(
+                toolchain: try UserToolchain.default,
+                cacheDir: path,
+                delegate: delegate
+            )
+
+            func check(loader: ManifestLoader, expectCached: Bool) throws {
+                delegate.clear()
+                delegate.prepare(expectParsing: !expectCached)
+
+                let manifest = try XCTUnwrap(loader.load(
+                    manifestPath: manifestPath,
+                    packageKind: .root(manifestPath.parentDirectory),
+                    toolsVersion: .current,
+                    fileSystem: fileSystem,
+                    observabilityScope: observability.topScope
+                ))
+
+                XCTAssertNoDiagnostics(observability.diagnostics)
+                XCTAssertEqual(try delegate.loaded(timeout: .now() + 1), [manifestPath])
+                XCTAssertEqual(try delegate.parsed(timeout: .now() + 1).count, expectCached ? 0 : 1)
+                XCTAssertEqual(manifest.displayName, "Trivial")
+                XCTAssertEqual(manifest.targets[0].name, "foo")
             }
 
-            try withCustomEnv(["SWIFTPM_MANIFEST_CACHE_TEST": "2"]) {
-                try check(loader: manifestLoader, expectCached: false)
-                try check(loader: manifestLoader, expectCached: true)
+            try check(loader: manifestLoader, expectCached: false)
+            try check(loader: manifestLoader, expectCached: true)
+
+            for key in EnvironmentVariables.nonCachableKeys {
+                try withCustomEnv([key: UUID().uuidString]) {
+                    try check(loader: manifestLoader, expectCached: true)
+                }
             }
 
             try check(loader: manifestLoader, expectCached: true)