Move the 'GitRepositoryProvider' to compare urls using their canonical representation, correctly accepting results where repositories only differ by '.git'

CHANGELOG.md

@@ -12,6 +12,10 @@ Swift Next
 Swift 6.0
 -----------
 
+* [#7741]
+
+  Fixed an issue where repositories would be re-cloned each build rather than using the cache due to git validation errors.
+
 * [#7507] 
 
   `swift experimental-sdk` command is deprecated with `swift sdk` command replacing it. `--experimental-swift-sdk` and

Sources/Commands/PackageCommands/ArchiveSource.swift

@@ -66,8 +66,7 @@ extension SwiftPackageCommand {
         cancellator: Cancellator?
     ) throws  {
         let gitRepositoryProvider = GitRepositoryProvider()
-        if gitRepositoryProvider.repositoryExists(at: packageDirectory) &&
-            (try? gitRepositoryProvider.isValidDirectory(packageDirectory)) == true {
+        if (try? gitRepositoryProvider.isValidDirectory(packageDirectory)) == true {
             let repository = GitRepository(path: packageDirectory, cancellator: cancellator)
             try repository.archive(to: archivePath)
         } else {

Sources/PackageModel/PackageIdentity.swift

@@ -480,6 +480,7 @@ private func computeCanonicalLocation(_ string: String) -> (description: String,
 
     // Prepend a leading slash for file URLs and paths
     if detectedScheme == "file" || string.first.flatMap(isSeparator) ?? false {
+        scheme = "file"
         description.insert("/", at: description.startIndex)
     }
 

Sources/SourceControl/GitRepository.swift

@@ -15,6 +15,8 @@ import Basics
 import Dispatch
 import class Foundation.NSLock
 
+import struct PackageModel.CanonicalPackageURL
+
 import struct TSCBasic.ByteString
 import protocol TSCBasic.DiagnosticLocation
 import struct TSCBasic.FileInfo
@@ -201,18 +203,14 @@ public struct GitRepositoryProvider: RepositoryProvider, Cancellable {
         )
     }
 
-    public func repositoryExists(at directory: Basics.AbsolutePath) -> Bool {
-        return localFileSystem.isDirectory(directory)
-    }
-
     public func isValidDirectory(_ directory: Basics.AbsolutePath) throws -> Bool {
         let result = try self.git.run(["-C", directory.pathString, "rev-parse", "--git-dir"])
         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, "config", "--get", "remote.origin.url"])
-        return remoteURL == repository.url
+        return CanonicalPackageURL(remoteURL) == CanonicalPackageURL(repository.url)
     }
 
     public func copy(from sourcePath: Basics.AbsolutePath, to destinationPath: Basics.AbsolutePath) throws {

Sources/SourceControl/Repository.swift

@@ -89,9 +89,6 @@ public protocol RepositoryProvider: Cancellable {
     /// - Throws: If there is any error fetching the repository.
     func fetch(repository: RepositorySpecifier, to path: AbsolutePath, progressHandler: FetchProgress.Handler?) throws
 
-    /// Returns true if a  repository exists at `path`
-    func repositoryExists(at path: AbsolutePath) throws -> Bool
-
     /// Open the given repository.
     ///
     /// - Parameters:

Sources/SourceControl/RepositoryManager.swift

@@ -220,7 +220,7 @@ public class RepositoryManager: Cancellable {
         // 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
-        quick: if (try? self.provider.repositoryExists(at: repositoryPath)) ?? false {
+        quick: if (try? self.provider.isValidDirectory(repositoryPath)) ?? false {
             let repository = try handle.open()
 
             guard ((try? self.provider.isValidDirectory(repositoryPath, for: repositorySpecifier)) ?? false) else {

Sources/_InternalTestSupport/InMemoryGitRepository.swift

@@ -14,6 +14,7 @@ import Basics
 import Dispatch
 import Foundation
 import SourceControl
+import struct PackageModel.CanonicalPackageURL
 
 import struct TSCBasic.ByteString
 import enum TSCBasic.FileMode
@@ -68,7 +69,7 @@ public final class InMemoryGitRepository {
     fileprivate let path: AbsolutePath
 
     /// The file system in which this repository should be installed.
-    private let fs: InMemoryFileSystem
+    fileprivate let fs: InMemoryFileSystem
 
     private let lock = NSLock()
 
@@ -438,10 +439,6 @@ public final class InMemoryGitRepositoryProvider: RepositoryProvider {
         add(specifier: RepositorySpecifier(path: path), repository: repo)
     }
 
-    public func repositoryExists(at path: AbsolutePath) throws -> Bool {
-        return fetchedMap[path] != nil
-    }
-
     public func copy(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws {
         guard let repo = fetchedMap[sourcePath] else {
             throw InternalError("unknown repo at \(sourcePath)")
@@ -482,11 +479,11 @@ public final class InMemoryGitRepositoryProvider: RepositoryProvider {
     }
 
     public func isValidDirectory(_ directory: AbsolutePath) throws -> Bool {
-        return true
+        return fetchedMap[directory] != nil || specifierMap.get().values.map(\.path).contains(directory)
     }
 
     public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
-        return true
+        return fetchedMap[directory] != nil || specifierMap[repository] != nil
     }
 
     public func cancel(deadline: DispatchTime) throws {

Tests/SourceControlTests/GitRepositoryProviderTests.swift

@@ -16,28 +16,28 @@ import _InternalTestSupport
 import XCTest
 
 class GitRepositoryProviderTests: XCTestCase {
-    func testRepositoryExists() throws {
+    func testIsValidDirectory() throws {
         try testWithTemporaryDirectory { sandbox in
             let provider = GitRepositoryProvider()
 
             // standard repository
             let repositoryPath = sandbox.appending("test")
             try localFileSystem.createDirectory(repositoryPath)
             initGitRepo(repositoryPath)
-            XCTAssertTrue(provider.repositoryExists(at: repositoryPath))
+            XCTAssertTrue(try provider.isValidDirectory(repositoryPath))
 
             // no-checkout bare repository
             let noCheckoutRepositoryPath = sandbox.appending("test-no-checkout")
             try localFileSystem.copy(from: repositoryPath.appending(".git"), to: noCheckoutRepositoryPath)
-            XCTAssertTrue(provider.repositoryExists(at: noCheckoutRepositoryPath))
+            XCTAssertTrue(try provider.isValidDirectory(noCheckoutRepositoryPath))
 
             // non-git directory
             let notGitPath = sandbox.appending("test-not-git")
-            XCTAssertFalse(provider.repositoryExists(at: notGitPath))
+            XCTAssertThrowsError(try provider.isValidDirectory(notGitPath))
 
             // non-git child directory of a git directory
             let notGitChildPath = repositoryPath.appending("test-not-git")
-            XCTAssertFalse(provider.repositoryExists(at: notGitChildPath))
+            XCTAssertThrowsError(try provider.isValidDirectory(notGitChildPath))
         }
     }
 }

Tests/SourceControlTests/GitRepositoryTests.swift

@@ -768,4 +768,142 @@ class GitRepositoryTests: XCTestCase {
             XCTAssertNoThrow(try checkoutRepo.getCurrentRevision())
         }
     }
+
+    func testValidDirectoryLocalRelativeOrigin() async throws {
+        try testWithTemporaryDirectory { tmpDir in
+            // Create a repository.
+            let packageDir = tmpDir.appending("SomePackage")
+            try localFileSystem.createDirectory(packageDir)
+
+            // Create a repository manager for it.
+            let repoProvider = GitRepositoryProvider()
+            let repositoryManager = RepositoryManager(
+                fileSystem: localFileSystem,
+                path: packageDir,
+                provider: repoProvider,
+                delegate: .none
+            )
+
+            let customRemote = "../OriginOfSomePackage.git"
+
+            // Before initializing the directory with a git repo, it is never valid.
+            XCTAssertThrowsError(try repositoryManager.isValidDirectory(packageDir))
+            XCTAssertThrowsError(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(packageDir.pathString))))
+            XCTAssertThrowsError(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(customRemote))))
+
+            initGitRepo(packageDir)
+            // Set the remote.
+            try systemQuietly([Git.tool, "-C", packageDir.pathString, "remote", "add", "origin", customRemote])
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir))
+
+            let customRemoteWithoutPathExtension = (customRemote as NSString).deletingPathExtension
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(customRemote))))
+            // We consider the directory valid even if the remote does not have the same path extension - in this case we expected '.git'.
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(customRemoteWithoutPathExtension))))
+            // We consider the directory valid even if the remote does not have the same path extension - in this case we expected '.git'.
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for:  RepositorySpecifier(url: SourceControlURL((customRemote as NSString).deletingPathExtension + "/"))))
+
+            // The following ensure that are actually checking the remote's origin.
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: AbsolutePath(validating: "/"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL("/"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: packageDir)))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(packageDir.pathString))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: packageDir.appending(extension: "git"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(packageDir.pathString.appending(".git")))))
+
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL("https://mycustomdomain/some-package.git"))))
+        }
+    }
+
+    func testValidDirectoryLocalAbsoluteOrigin() async throws {
+        try testWithTemporaryDirectory { tmpDir in
+            // Create a repository.
+            let packageDir = tmpDir.appending("SomePackage")
+            try localFileSystem.createDirectory(packageDir)
+
+            // Create a repository manager for it.
+            let repoProvider = GitRepositoryProvider()
+            let repositoryManager = RepositoryManager(
+                fileSystem: localFileSystem,
+                path: packageDir,
+                provider: repoProvider,
+                delegate: .none
+            )
+
+            let customRemote = tmpDir.appending("OriginOfSomePackage.git")
+
+            // Before initializing the directory with a git repo, it is never valid.
+            XCTAssertThrowsError(try repositoryManager.isValidDirectory(packageDir))
+            XCTAssertThrowsError(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(packageDir.pathString))))
+            XCTAssertThrowsError(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(customRemote.pathString))))
+
+            initGitRepo(packageDir)
+            // Set the remote.
+            try systemQuietly([Git.tool, "-C", packageDir.pathString, "remote", "add", "origin", customRemote.pathString])
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir))
+
+            let customRemotePath = customRemote.pathString
+            let customRemotePathWithoutPathExtension = (customRemotePath as NSString).deletingPathExtension
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: customRemote)))
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(customRemotePath))))
+            // We consider the directory valid even if the remote does not have the same path extension - in this case we expected '.git'.
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: try AbsolutePath(validating: customRemotePathWithoutPathExtension))))
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(customRemotePathWithoutPathExtension))))
+            // We consider the directory valid even if the remote does not have the same path extension - in this case we expected '.git'.
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for:  RepositorySpecifier(path: try AbsolutePath(validating: customRemotePathWithoutPathExtension + "/"))))
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for:  RepositorySpecifier(url: SourceControlURL((customRemotePath as NSString).deletingPathExtension + "/"))))
+
+            // The following ensure that are actually checking the remote's origin.
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: AbsolutePath(validating: "/"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL("/"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: packageDir)))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(packageDir.pathString))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: packageDir.appending(extension: "git"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(packageDir.pathString.appending(".git")))))
+
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL("https://mycustomdomain/some-package.git"))))
+        }
+    }
+
+    func testValidDirectoryRemoteOrigin() async throws {
+        try testWithTemporaryDirectory { tmpDir in
+            // Create a repository.
+            let packageDir = tmpDir.appending("SomePackage")
+            try localFileSystem.createDirectory(packageDir)
+
+            // Create a repository manager for it.
+            let repoProvider = GitRepositoryProvider()
+            let repositoryManager = RepositoryManager(
+                fileSystem: localFileSystem,
+                path: packageDir,
+                provider: repoProvider,
+                delegate: .none
+            )
+
+            let customRemote = try XCTUnwrap(URL(string: "https://mycustomdomain/some-package.git"))
+
+            // Before initializing the directory with a git repo, it is never valid.
+            XCTAssertThrowsError(try repositoryManager.isValidDirectory(packageDir))
+            XCTAssertThrowsError(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(customRemote))))
+
+            initGitRepo(packageDir)
+            // Set the remote.
+            try systemQuietly([Git.tool, "-C", packageDir.pathString, "remote", "add", "origin", customRemote.absoluteString])
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir))
+
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(customRemote))))
+            // We consider the directory valid even if the remote does not have the same path extension - in this case we expected '.git'.
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL("https://mycustomdomain/some-package"))))
+            // We consider the directory valid even if the remote does not have the same path extension - in this case we expected '.git'.
+            XCTAssertTrue(try repositoryManager.isValidDirectory(packageDir, for:  RepositorySpecifier(url: SourceControlURL("https://mycustomdomain/some-package/"))))
+
+            // The following ensure that are actually checking the remote's origin.
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: AbsolutePath(validating: "/"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL("/"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: packageDir)))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(packageDir.pathString))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(path: packageDir.appending(extension: "git"))))
+            XCTAssertFalse(try repositoryManager.isValidDirectory(packageDir, for: RepositorySpecifier(url: SourceControlURL(packageDir.pathString.appending(".git")))))
+        }
+    }
 }