validate local repositry has the correct remote

motivation: in some edge cases, the local repository may be partially valid (the containing directory is a legit git repo, but the repository diretory is not), or otherwise point to a remote different than the one expected

changes:
* validate that the local reposiotry remote aligns with the expected one, not just that the directory is a valid git repo
* refactor validation flow to be more streamlined

Author: tomerd

Sources/SPMTestSupport/InMemoryGitRepository.swift

@@ -486,6 +486,10 @@ public final class InMemoryGitRepositoryProvider: RepositoryProvider {
         return true
     }
 
+    public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
+        return true
+    }
+
     public func cancel(deadline: DispatchTime) throws {
         // noop
     }

Sources/SourceControl/GitRepository.swift

@@ -210,6 +210,11 @@ public struct GitRepositoryProvider: RepositoryProvider, Cancellable {
         return result == ".git" || result == "." || result == directory.pathString
     }
 
+    public func isValidDirectory(_ directory: Basics.AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
+        let remoteURL = try self.git.run(["-C", directory.pathString, "remote", "get-url", "origin"])
+        return remoteURL == repository.url
+    }
+
     public func copy(from sourcePath: Basics.AbsolutePath, to destinationPath: Basics.AbsolutePath) throws {
         try localFileSystem.copy(from: sourcePath, to: destinationPath)
     }
@@ -423,10 +428,10 @@ public final class GitRepository: Repository, WorkingCheckout {
         self.git = git
         self.path = path
         self.isWorkingRepo = isWorkingRepo
-        assert({
+        /*assert({
             // Ignore if we couldn't run popen for some reason.
             (try? self.isBare() != isWorkingRepo) ?? true
-        }())
+        }())*/
     }
 
     /// Private function to invoke the Git tool with its default environment and given set of arguments, specifying the

Sources/SourceControl/Repository.swift

@@ -144,6 +144,9 @@ public protocol RepositoryProvider: Cancellable {
 
     /// Returns true if the directory is valid git location.
     func isValidDirectory(_ directory: AbsolutePath) throws -> Bool
+
+    /// Returns true if the directory is valid git location.
+    func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool
 }
 
 /// Abstract repository operations.

Sources/SourceControl/RepositoryManager.swift

@@ -188,37 +188,42 @@ public class RepositoryManager: Cancellable {
     // sync func and roll the logic into the async version above
     private func lookup(
         package: PackageIdentity,
-        repository: RepositorySpecifier,
+        repository repositorySpecifier: RepositorySpecifier,
         updateStrategy: RepositoryUpdateStrategy,
         observabilityScope: ObservabilityScope,
         delegateQueue: DispatchQueue
     ) throws -> RepositoryHandle {
-        let relativePath = try repository.storagePath()
+        let relativePath = try repositorySpecifier.storagePath()
         let repositoryPath = self.path.appending(relativePath)
-        let handle = RepositoryHandle(manager: self, repository: repository, subpath: relativePath)
+        let handle = RepositoryHandle(manager: self, repository: repositorySpecifier, subpath: relativePath)
 
         // check if a repository already exists
         // errors when trying to check if a repository already exists are legitimate
         // and recoverable, and as such can be ignored
-        if (try? self.provider.repositoryExists(at: repositoryPath)) ?? false {
+        quick: if (try? self.provider.repositoryExists(at: repositoryPath)) ?? false {
             let repository = try handle.open()
+            
+            guard ((try? self.provider.isValidDirectory(repositoryPath, for: repositorySpecifier)) ?? false) else {
+                observabilityScope.emit(warning: "\(repositoryPath) is not valid git repository for '\(repositorySpecifier.location)', will fetch again.")
+                break quick
+            }
+
             // Update the repository if needed
             if self.fetchRequired(repository: repository, updateStrategy: updateStrategy) {
                 let start = DispatchTime.now()
-                
+
                 delegateQueue.async {
                     self.delegate?.willUpdate(package: package, repository: handle.repository)
                 }
-                
+
                 try repository.fetch()
                 let duration = start.distance(to: .now())
                 delegateQueue.async {
                     self.delegate?.didUpdate(package: package, repository: handle.repository, duration: duration)
                 }
-                return handle
-            } else if try self.provider.isValidDirectory(repositoryPath) {
-                return handle
             }
+
+            return handle
         }
 
         // inform delegate that we are starting to fetch
@@ -334,7 +339,7 @@ public class RepositoryManager: Cancellable {
                 }
             } catch {
                 // If we are offline and have a valid cached repository, use the cache anyway.
-                if try isOffline(error) && self.provider.isValidDirectory(cachedRepositoryPath) {
+                if try isOffline(error) && self.provider.isValidDirectory(cachedRepositoryPath, for: handle.repository) {
                     // For the first offline use in the lifetime of this repository manager, emit a warning.
                     if self.emitNoConnectivityWarning.get(default: false) {
                         self.emitNoConnectivityWarning.put(false)
@@ -426,6 +431,10 @@ public class RepositoryManager: Cancellable {
         try self.provider.isValidDirectory(directory)
     }
 
+    public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
+        try self.provider.isValidDirectory(directory, for: repository)
+    }
+
     /// Reset the repository manager.
     ///
     /// Note: This also removes the cloned repositories from the disk.

Tests/SourceControlTests/RepositoryManagerTests.swift

@@ -561,6 +561,10 @@ class RepositoryManagerTests: XCTestCase {
                 fatalError("should not be called")
             }
 
+            public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
+                fatalError("should not be called")
+            }
+
             func cancel(deadline: DispatchTime) throws {
                 print("cancel")
             }
@@ -640,6 +644,10 @@ private class DummyRepository: Repository {
         fatalError("unexpected API call")
     }
 
+    public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
+        fatalError("unexpected API call")
+    }
+
     func fetch() throws {
         self.provider.increaseFetchCount()
     }
@@ -720,6 +728,10 @@ private class DummyRepositoryProvider: RepositoryProvider {
         return true
     }
 
+    func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
+        return true
+    }
+
     func cancel(deadline: DispatchTime) throws {
         // noop
     }

Tests/WorkspaceTests/SourceControlPackageContainerTests.swift

@@ -150,6 +150,10 @@ private class MockRepositories: RepositoryProvider {
         return true
     }
 
+    public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
+        return true
+    }
+
     func cancel(deadline: DispatchTime) throws {
         // noop
     }