cleanup identity cache mapping impl; assure transformation mode is .swizzle

bripeticca · bripeticca · commit 8ff43cb1166b · 2026-04-16T15:25:28.000-04:00
diff --git a/Sources/PackageLoading/ManifestLoader.swift b/Sources/PackageLoading/ManifestLoader.swift
@@ -126,10 +126,6 @@ public protocol ManifestLoaderProtocol {
         fileSystem: FileSystem,
         observabilityScope: ObservabilityScope,
         delegateQueue: DispatchQueue,
-        identityLookupCache: ThreadSafeKeyValueStore<
-        SourceControlURL,
-        (result: Result<PackageIdentity?, Error>, expirationTime: DispatchTime)
-    >
     ) async throws -> Manifest
 
     /// Reset any internal cache held by the manifest loader.
@@ -203,10 +199,6 @@ extension ManifestLoaderProtocol {
         fileSystem: FileSystem,
         observabilityScope: ObservabilityScope,
         delegateQueue: DispatchQueue,
-        identityLookupCache: ThreadSafeKeyValueStore<
-        SourceControlURL,
-        (result: Result<PackageIdentity?, Error>, expirationTime: DispatchTime)
-    >
     ) async throws -> Manifest {
         // find the manifest path and parse it's tools-version
         let manifestPath = try ManifestLoader.findManifest(
@@ -233,8 +225,7 @@ extension ManifestLoaderProtocol {
             dependencyMapper: dependencyMapper,
             fileSystem: fileSystem,
             observabilityScope: observabilityScope,
-            delegateQueue: delegateQueue,
-            identityLookupCache: identityLookupCache
+            delegateQueue: delegateQueue
         )
     }
 }
@@ -320,10 +311,6 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         fileSystem: FileSystem,
         observabilityScope: ObservabilityScope,
         delegateQueue: DispatchQueue,
-        identityLookupCache: ThreadSafeKeyValueStore<
-        SourceControlURL,
-        (result: Result<PackageIdentity?, Error>, expirationTime: DispatchTime)
-    >
     ) async throws -> Manifest {
         // Inform the delegate.
         let start = DispatchTime.now()
diff --git a/Sources/Workspace/PackageContainer/FileSystemPackageContainer.swift b/Sources/Workspace/PackageContainer/FileSystemPackageContainer.swift
@@ -89,8 +89,7 @@ public struct FileSystemPackageContainer: PackageContainer {
                 dependencyMapper: self.dependencyMapper,
                 fileSystem: self.fileSystem,
                 observabilityScope: self.observabilityScope,
-                delegateQueue: .sharedConcurrent,
-                identityLookupCache: .init()
+                delegateQueue: .sharedConcurrent
             )
         }
     }
diff --git a/Sources/Workspace/PackageContainer/RegistryPackageContainer.swift b/Sources/Workspace/PackageContainer/RegistryPackageContainer.swift
@@ -30,7 +30,7 @@ public class RegistryPackageContainer: PackageContainer {
     private let manifestLoader: ManifestLoaderProtocol
     private let currentToolsVersion: ToolsVersion
     private let observabilityScope: ObservabilityScope
-    private let identityLookupCache: Workspace.SCMToRegistryMap
+    private let identityLookupCache: Workspace.IdentityLookupCache
 
     private var knownVersionsCache = AsyncThrowingValueMemoizer<[Version]>()
     private var toolsVersionsCache = ThrowingAsyncKeyValueMemoizer<Version, ToolsVersion>()
@@ -46,7 +46,7 @@ public class RegistryPackageContainer: PackageContainer {
         manifestLoader: ManifestLoaderProtocol,
         currentToolsVersion: ToolsVersion,
         observabilityScope: ObservabilityScope,
-        identityLookupCache: Workspace.SCMToRegistryMap
+        identityLookupCache: Workspace.IdentityLookupCache
     ) {
         self.package = package
         self.identityResolver = identityResolver
@@ -152,7 +152,6 @@ public class RegistryPackageContainer: PackageContainer {
                 fileSystem: result.fileSystem,
                 observabilityScope: self.observabilityScope,
                 delegateQueue: .sharedConcurrent,
-                identityLookupCache: self.identityLookupCache
             )
         }
 
diff --git a/Sources/Workspace/PackageContainer/SourceControlPackageContainer.swift b/Sources/Workspace/PackageContainer/SourceControlPackageContainer.swift
@@ -72,7 +72,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
     private var knownVersionsCache = ThreadSafeBox<[Version: String]?>()
     private var manifestsCache = ThrowingAsyncKeyValueMemoizer<String, Manifest>()
     private var toolsVersionsCache = ThreadSafeKeyValueStore<Version, ToolsVersion>()
-    private var identityLookupCache: Workspace.SCMToRegistryMap
+    private var identityLookupCache: Workspace.IdentityLookupCache
 
     /// This is used to remember if tools version of a particular version is
     /// valid or not.
@@ -89,7 +89,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
         fingerprintStorage: PackageFingerprintStorage?,
         fingerprintCheckingMode: FingerprintCheckingMode,
         observabilityScope: ObservabilityScope,
-        identityLookupCache: Workspace.SCMToRegistryMap
+        identityLookupCache: Workspace.IdentityLookupCache
     ) throws {
         self.package = package
         self.identityResolver = identityResolver
@@ -412,7 +412,6 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
             fileSystem: fileSystem,
             observabilityScope: self.observabilityScope,
             delegateQueue: .sharedConcurrent,
-            identityLookupCache: self.identityLookupCache
         )
     }
 
diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift
@@ -373,28 +373,41 @@ extension Workspace {
             )
         }
 
+        // Populate the identity lookup cache via the Package.resolved.
+        // This will only ever be done if the user has passed in `--force-resolved-versions`
+        self.identityLookupCache.deriveCache(
+            from: resolvedPackagesStore.resolvedPackages,
+            self.configuration.sourceControlToRegistryDependencyTransformation
+        )
         // Create the identity mapping between the registry and scm, if applicable.
-        for resolvedPackage in resolvedPackagesStore.resolvedPackages.values {
-            if case let .registry(id, .some(url)) = resolvedPackage.packageRef.kind {
-                self.identityLookupCache[url] = (
-                    result: .success(id),
-                    expirationTime: .now().advanced(by: .seconds(300))
-                )
-            }
-        }
+//        for resolvedPackage in resolvedPackagesStore.resolvedPackages.values {
+//            if case let .registry(id, .some(url)) = resolvedPackage.packageRef.kind {
+//                self.identityLookupCache[storing: url] = .success(id)
+//            }
+//        }
 
-        // Track SCM packages that don't have a mapped registry equivalent;
-        // this will avoid having to make any unnecessary calls to registry
-        // endpoints.
-        for resolvedPackage in resolvedPackagesStore.resolvedPackages.values {
-            if case .remoteSourceControl(let url) = resolvedPackage.packageRef.kind,
-               self.identityLookupCache[url] == nil {
-                self.identityLookupCache[url] = (
-                    result: .success(nil),
-                    expirationTime: .now().advanced(by: .seconds(300))
-                )
-            }
-        }
+//        let transformationMode = self.configuration.sourceControlToRegistryDependencyTransformation
+
+        // Track SCM packages that don't have a mapped registry equivalent,
+        // only if the transformation mode is `.swizzle`; this will avoid
+        // having to make any unnecessary calls to registry endpoints.
+        //
+        // Additionally for cases where `--use-registry-identity-for-scm` is used,
+        // the package kind remains that of a remote source control while using a
+        // registry identity to de-duplicate references to this package elsewhere in
+        // the package graph. Here, we do not want to keep track of any mapping since
+        // it still may have a registry equivalent.
+//        if transformationMode == .swizzle {
+//            for resolvedPackage in resolvedPackagesStore.resolvedPackages.values {
+//                if case .remoteSourceControl(let url) = resolvedPackage.packageRef.kind,
+//                   self.identityLookupCache[url] == nil {
+//                    self.identityLookupCache[url] = (
+//                        result: .success(nil),
+//                        expirationTime: .now().advanced(by: .seconds(300))
+//                    )
+//                }
+//            }
+//        }
 
         // Request all the containers to fetch them in parallel.
         //
diff --git a/Sources/Workspace/Workspace+Manifests.swift b/Sources/Workspace/Workspace+Manifests.swift
@@ -859,8 +859,7 @@ extension Workspace {
                 dependencyMapper: self.dependencyMapper,
                 fileSystem: fileSystem,
                 observabilityScope: manifestLoadingScope,
-                delegateQueue: .sharedConcurrent,
-                identityLookupCache: self.identityLookupCache
+                delegateQueue: .sharedConcurrent
             )
         } catch {
             let duration = start.distance(to: .now())
diff --git a/Sources/Workspace/Workspace+Registry.swift b/Sources/Workspace/Workspace+Registry.swift
@@ -56,21 +56,18 @@ extension Workspace {
         private let underlying: ManifestLoaderProtocol
         private let registryClient: RegistryClient
         private let transformationMode: TransformationMode
-
-        private let cacheTTL = DispatchTimeInterval.seconds(300) // 5m
-//        private let identityLookupCache = ThreadSafeKeyValueStore<
-//            SourceControlURL,
-//            (result: Result<PackageIdentity?, Error>, expirationTime: DispatchTime)
-//        >()
+        private let identityLookupCache: Workspace.IdentityLookupCache
 
         init(
             underlying: ManifestLoaderProtocol,
             registryClient: RegistryClient,
-            transformationMode: TransformationMode
+            transformationMode: TransformationMode,
+            identityLookupCache: Workspace.IdentityLookupCache
         ) {
             self.underlying = underlying
             self.registryClient = registryClient
             self.transformationMode = transformationMode
+            self.identityLookupCache = identityLookupCache
         }
 
         func load(
@@ -85,10 +82,6 @@ extension Workspace {
             fileSystem: any FileSystem,
             observabilityScope: ObservabilityScope,
             delegateQueue: DispatchQueue,
-            identityLookupCache: ThreadSafeKeyValueStore<
-            SourceControlURL,
-            (result: Result<PackageIdentity?, Error>, expirationTime: DispatchTime)
-            >
         ) async throws -> Manifest {
             let manifest = try await self.underlying.load(
                 manifestPath: manifestPath,
@@ -101,13 +94,11 @@ extension Workspace {
                 dependencyMapper: dependencyMapper,
                 fileSystem: fileSystem,
                 observabilityScope: observabilityScope,
-                delegateQueue: delegateQueue,
-                identityLookupCache: .init()
+                delegateQueue: delegateQueue
             )
             return try await self.transformSourceControlDependenciesToRegistry(
                 manifest: manifest,
                 transformationMode: transformationMode,
-                identityLookupCache: identityLookupCache,
                 observabilityScope: observabilityScope
             )
         }
@@ -123,10 +114,6 @@ extension Workspace {
         private func transformSourceControlDependenciesToRegistry(
             manifest: Manifest,
             transformationMode: TransformationMode,
-            identityLookupCache: ThreadSafeKeyValueStore<
-            SourceControlURL,
-            (result: Result<PackageIdentity?, Error>, expirationTime: DispatchTime)
-        >,
             observabilityScope: ObservabilityScope
         ) async throws -> Manifest {
             var transformations = [PackageDependency: PackageIdentity]()
@@ -138,7 +125,6 @@ extension Workspace {
                             do {
                                 let identity = try await self.mapRegistryIdentity(
                                     url: url,
-                                    identityLookupCache: identityLookupCache,
                                     observabilityScope: observabilityScope
                                 )
                                 return (dependency, identity)
@@ -343,37 +329,31 @@ extension Workspace {
 
         private func mapRegistryIdentity(
             url: SourceControlURL,
-            identityLookupCache: ThreadSafeKeyValueStore<
-            SourceControlURL,
-            (result:
-                Result<PackageIdentity?, Error>,
-             expirationTime: DispatchTime
-            )>,
             observabilityScope: ObservabilityScope
         ) async throws -> PackageIdentity? {
             if let cached = identityLookupCache[url], cached.expirationTime > .now() {
                 switch cached.result {
                 case .success(let identity):
-                    return identity;
+                    return identity
+                case .notApplicable:
+                    // scm package does not have a valid registry mapping
+                    return nil
                 case .failure:
                     // server error, do not try again
                     return nil
                 }
             }
 
             do {
-                // TODO bp :
-                // - possibly propogate the resolvedFileStrategy here?
-                // - ensure that the cache is up to date
                 let identities = try await self.registryClient.lookupIdentities(
                     scmURL: url,
                     observabilityScope: observabilityScope
                 )
                 let identity = identities.sorted().first
-                identityLookupCache[url] = (result: .success(identity), expirationTime: .now() + self.cacheTTL)
+                identityLookupCache[storing: url] = .success(identity)
                 return identity
             } catch {
-                identityLookupCache[url] = (result: .failure(error), expirationTime: .now() + self.cacheTTL)
+                identityLookupCache[storing: url] = .failure(error)
                 throw error
             }
         }
@@ -469,3 +449,71 @@ extension Workspace {
         try registryDownloadsManager.remove(package: dependency.packageRef.identity)
     }
 }
+
+extension Workspace {
+    public class IdentityLookupCache {
+        public typealias Key = SourceControlURL
+        public typealias Value = CacheResult
+        public typealias CacheResult = (result: IdentityMapResult<PackageIdentity?, Error>, expirationTime: DispatchTime)
+
+
+        public enum IdentityMapResult<Success, Failure> {
+            /// Represents a successful mapping of a source control URL to its registry identity
+            case success(Success)
+            /// Represents a failure to retrieve an identity from the registry
+            case failure(Failure)
+            /// Represents a scenario wherein a scm package has no valid registry identity mapping
+            case notApplicable
+        }
+
+        /// Tracks the mapping of scm packages to their registry identities
+        private let cache = ThreadSafeKeyValueStore<SourceControlURL, CacheResult>()
+        private let cacheTTL = DispatchTimeInterval.seconds(300) // 5m
+
+        public subscript(key: Key) -> CacheResult? {
+            get { self.cache[key] }
+            set { self.cache[key] = newValue }
+        }
+
+        public subscript(storing key: Key) -> IdentityMapResult<PackageIdentity?, Error>? {
+            get { self.cache[key]?.result }
+            set {
+                guard let newValue else {
+                    cache.removeValue(forKey: key)
+                    return
+                }
+                self.cache[key] = (
+                    result: newValue,
+                    expirationTime: .now() + self.cacheTTL
+                )
+            }
+        }
+
+        /// Derives an identity lookup cache from the Package.resolved file if applicable.
+        /// Asserts against the transformation mode set in the Workspace to determine how to store
+        /// source control packages and their identity mappings.
+        public func deriveCache(from resolvedPackages: ResolvedPackagesStore.ResolvedPackages, _ transformationMode: WorkspaceConfiguration.SourceControlToRegistryDependencyTransformation) {
+            guard transformationMode != .disabled else {
+                return
+            }
+
+            // First run, create a mapping between registry packages and their scm location, if applicable.
+            for resolvedPackage in resolvedPackages.values {
+                if case let .registry(id, .some(url)) = resolvedPackage.packageRef.kind {
+                    self[storing: url] = .success(id)
+                }
+            }
+
+            // Second pass; identify source control packages that may have had their
+            // identifiers modified to that of a registry id
+            if transformationMode == .swizzle {
+                for resolvedPackage in resolvedPackages.values {
+                    if case .remoteSourceControl(let url) = resolvedPackage.packageRef.kind,
+                        self.cache[url] == nil {
+                        self[storing: url] = .notApplicable
+                    }
+                }
+            }
+        }
+    }
+}
diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift
diff --git a/Sources/_InternalTestSupport/MockManifestLoader.swift b/Sources/_InternalTestSupport/MockManifestLoader.swift

Original file line number	Diff line number	Diff line change
`@@ -89,8 +89,7 @@ public struct FileSystemPackageContainer: PackageContainer {`
`89`	`89`	`dependencyMapper: self.dependencyMapper,`
`90`	`90`	`fileSystem: self.fileSystem,`
`91`	`91`	`observabilityScope: self.observabilityScope,`
`92`		`- delegateQueue: .sharedConcurrent,`
`93`		`- identityLookupCache: .init()`
	`92`	`+ delegateQueue: .sharedConcurrent`
`94`	`93`	`)`
`95`	`94`	`}`
`96`	`95`	`}`