cleanup

Author: tomerd

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -36,20 +36,14 @@ extension PackageGraph {
 
         // Create a map of the manifests, keyed by their identity.
         let rootManifestsMap = root.manifests
-        //let externalManifestsMap = externalManifests.map{ (identityResolver.resolveIdentity(for: $0.packageLocation), $0) }
-        //let manifestMap = rootManifestsMap.merging(externalManifests, uniquingKeysWith: { lhs, rhs in
-          //  return lhs
-        //})
-
-        // prefer roots
         var manifestMap = rootManifestsMap
         externalManifests.forEach {
+            // prefer roots
             if !manifestMap.keys.contains($0.key) {
                 manifestMap[$0.key] = $0.value
             }
         }
 
-
         let successors: (GraphLoadingNode) -> [GraphLoadingNode] = { node in
             node.requiredDependencies().compactMap{ dependency in
                 return manifestMap[dependency.identity].map { manifest in

Sources/Workspace/ManagedDependency.swift

@@ -154,11 +154,13 @@ extension Workspace {
         }
 
         public subscript(identity: PackageIdentity) -> ManagedDependency? {
-            //self.dependencies.values.first(where: { $0.packageRef.identity == identity })
             return self.dependencies[identity]
         }
 
-        public subscript(package: PackageReference) -> ManagedDependency? {
+        // When loading manifests in Workspace, there are cases where we must also compare the location
+        // as it may attempt to load manifests for dependencies that have the same identity but from a different location
+        // (e.g. dependency is changed to  a fork with the same identity)
+        public subscript(comparingLocation package: PackageReference) -> ManagedDependency? {
             if let dependency = self.dependencies[package.identity], dependency.packageRef.location == package.location {
                 return dependency
             }

Sources/Workspace/Workspace.swift

@@ -1176,7 +1176,6 @@ extension Workspace {
             _ = try clone(package: dependency.packageRef, at: checkoutState)
         } else {
             // The original dependency was removed, update the managed dependency state.
-            //self.state.dependencies.remove(forURL: dependency.packageRef.location)
             self.state.dependencies.remove(dependency.packageRef.identity)
             try self.state.save()
         }
@@ -1562,8 +1561,6 @@ extension Workspace {
 
     /// Loads the given manifests, if it is present in the managed dependencies.
     private func loadManagedManifests(for packages: [PackageReference], diagnostics: DiagnosticsEngine, completion: @escaping (Result<[PackageIdentity: Manifest], Error>) -> Void) {
-        // this is allot of boilerplate code but its is important for performance
-        //let lock = Lock()
         let sync = DispatchGroup()
         let manifests = ThreadSafeKeyValueStore<PackageIdentity, Manifest>()
         Set(packages).forEach { package in
@@ -1588,7 +1585,7 @@ extension Workspace {
         // dependencies that have the same identity but from a different location
         // which is an error case we diagnose an report about in the GraphLoading part which
         // is prepared to handle the case where not all manifest are available
-        guard let managedDependency = self.state.dependencies[package] else {
+        guard let managedDependency = self.state.dependencies[comparingLocation: package] else {
             return completion(.none)
         }
 
@@ -2039,8 +2036,8 @@ extension Workspace {
         // We require cloning if there is no checkout or if the checkout doesn't
         // match with the pin.
         let requiredPins = pinsStore.pins.filter{ pin in
-            //guard let dependency = state.dependencies[forURL: pin.packageRef.location] else {
-            guard let dependency = state.dependencies[pin.packageRef] else {
+            // also compare the location in case it has changed
+            guard let dependency = state.dependencies[comparingLocation: pin.packageRef] else {
                 return true
             }
             switch dependency.state {
@@ -2416,8 +2413,8 @@ extension Workspace {
             if case .edited(let basedOn, _) = currentDependency?.state, let originalReference = basedOn?.packageRef {
                 packageStateChanges[originalReference.identity] = (originalReference, .unchanged)
             } else {
-                // if not edited, also lookup by location since it may have changed
-                currentDependency = self.state.dependencies[packageRef]
+                // if not edited, also compare by location since it may have changed
+                currentDependency = self.state.dependencies[comparingLocation: packageRef]
             }
 
             switch binding {
@@ -2659,7 +2656,8 @@ extension Workspace {
     /// - Throws: If the operation could not be satisfied.
     private func fetch(package: PackageReference) throws -> AbsolutePath {
         // If we already have it, fetch to update the repo from its remote.
-        if let dependency = self.state.dependencies[package] {
+        // also compare the location as it may have changed
+        if let dependency = self.state.dependencies[comparingLocation: package] {
             let path = self.location.repositoriesCheckoutsDirectory.appending(dependency.subpath)
 
             // Make sure the directory is not missing (we will have to clone again
@@ -2797,7 +2795,7 @@ extension Workspace {
 
     /// Removes the clone and checkout of the provided specifier.
     fileprivate func remove(package: PackageReference) throws {
-        guard let dependency = self.state.dependencies[package] else {
+        guard let dependency = self.state.dependencies[package.identity] else {
             throw InternalError("trying to remove \(package.identity) which isn't in workspace")
         }
 

Sources/Workspace/WorkspaceState.swift

@@ -188,7 +188,6 @@ fileprivate struct WorkspaceStateStorage {
                     state: state,
                     subpath: subpath
                 )
-
             }
 
             func encode(to encoder: Encoder) throws {