diff --git a/Sources/SPMTestSupport/InMemoryGitRepository.swift b/Sources/SPMTestSupport/InMemoryGitRepository.swift index 659697eb80d..7e595d59681 100644 --- a/Sources/SPMTestSupport/InMemoryGitRepository.swift +++ b/Sources/SPMTestSupport/InMemoryGitRepository.swift @@ -379,7 +379,7 @@ extension InMemoryGitRepository: WorkingCheckout { } } - public func isAlternateObjectStoreValid() -> Bool { + public func isAlternateObjectStoreValid(expected: AbsolutePath) -> Bool { return true } diff --git a/Sources/SourceControl/GitRepository.swift b/Sources/SourceControl/GitRepository.swift index 73d15923e7b..8f654eb17f0 100644 --- a/Sources/SourceControl/GitRepository.swift +++ b/Sources/SourceControl/GitRepository.swift @@ -597,7 +597,7 @@ public final class GitRepository: Repository, WorkingCheckout { } /// Returns true if there is an alternative object store in the repository and it is valid. - public func isAlternateObjectStoreValid() -> Bool { + public func isAlternateObjectStoreValid(expected: AbsolutePath) -> Bool { let objectStoreFile = self.path.appending(components: ".git", "objects", "info", "alternates") guard let bytes = try? localFileSystem.readFileContents(objectStoreFile) else { return false @@ -606,7 +606,11 @@ public final class GitRepository: Repository, WorkingCheckout { guard let firstLine = ByteString(split[0]).validDescription else { return false } - return (try? localFileSystem.isDirectory(AbsolutePath(validating: firstLine))) == true + guard let objectsPath = try? AbsolutePath(validating: firstLine), localFileSystem.isDirectory(objectsPath) else { + return false + } + let repositoryPath = objectsPath.parentDirectory + return expected == repositoryPath } /// Returns true if the file at `path` is ignored by `git` diff --git a/Sources/SourceControl/Repository.swift b/Sources/SourceControl/Repository.swift index 76d247e2f8a..f7dcfb798c0 100644 --- a/Sources/SourceControl/Repository.swift +++ b/Sources/SourceControl/Repository.swift @@ -273,7 +273,7 @@ public protocol WorkingCheckout { func checkout(newBranch: String) throws /// Returns true if there is an alternative store in the checkout and it is valid. - func isAlternateObjectStoreValid() -> Bool + func isAlternateObjectStoreValid(expected: AbsolutePath) -> Bool /// Returns true if the file at `path` is ignored by `git` func areIgnored(_ paths: [AbsolutePath]) throws -> [Bool] diff --git a/Sources/SourceControl/RepositoryManager.swift b/Sources/SourceControl/RepositoryManager.swift index 2b9f602d5d5..220f5835168 100644 --- a/Sources/SourceControl/RepositoryManager.swift +++ b/Sources/SourceControl/RepositoryManager.swift @@ -361,10 +361,18 @@ public class RepositoryManager: Cancellable { return FetchDetails(fromCache: cacheUsed, updatedCache: cacheUpdated) } + /// Open a working copy checkout at a path public func openWorkingCopy(at path: AbsolutePath) throws -> WorkingCheckout { try self.provider.openWorkingCopy(at: path) } + /// Validate a working copy check is aligned with its repository setup + public func isValidWorkingCopy(_ workingCopy: WorkingCheckout, for repository: RepositorySpecifier) throws -> Bool { + let relativePath = try repository.storagePath() + let repositoryPath = self.path.appending(relativePath) + return workingCopy.isAlternateObjectStoreValid(expected: repositoryPath) + } + /// Open a repository from a handle. private func open(_ handle: RepositoryHandle) throws -> Repository { try self.provider.open( diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 7506ba9ce4c..315a573c9d8 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -3309,8 +3309,13 @@ extension Workspace { observabilityScope: ObservabilityScope ) throws -> AbsolutePath { let repository = try package.makeRepositorySpecifier() - // first fetch the repository. - let checkoutPath = try self.fetchRepository(package: package, observabilityScope: observabilityScope) + + // first fetch the repository + let checkoutPath = try self.fetchRepository( + package: package, + at: checkoutState.revision, + observabilityScope: observabilityScope + ) // Check out the given revision. let workingCopy = try self.repositoryManager.openWorkingCopy(at: checkoutPath) @@ -3378,37 +3383,40 @@ extension Workspace { /// /// - Returns: The path of the local repository. /// - Throws: If the operation could not be satisfied. - private func fetchRepository(package: PackageReference, observabilityScope: ObservabilityScope) throws -> AbsolutePath { + private func fetchRepository( + package: PackageReference, + at revision: Revision, + observabilityScope: ObservabilityScope + ) throws -> AbsolutePath { + let repository = try package.makeRepositorySpecifier() + // If we already have it, fetch to update the repo from its remote. // also compare the location as it may have changed if let dependency = self.state.dependencies[comparingLocation: package] { - let path = self.location.repositoriesCheckoutSubdirectory(for: dependency) + let checkoutPath = self.location.repositoriesCheckoutSubdirectory(for: dependency) - // Make sure the directory is not missing (we will have to clone again - // if not). - fetch: if self.fileSystem.isDirectory(path) { + // Make sure the directory is not missing (we will have to clone again if not). + // This can become invalid if the build directory is moved. + fetch: if self.fileSystem.isDirectory(checkoutPath) { // Fetch the checkout in case there are updates available. - let workingCopy = try self.repositoryManager.openWorkingCopy(at: path) + let workingCopy = try self.repositoryManager.openWorkingCopy(at: checkoutPath) // Ensure that the alternative object store is still valid. - // - // This can become invalid if the build directory is moved. - guard workingCopy.isAlternateObjectStoreValid() else { + guard try self.repositoryManager.isValidWorkingCopy(workingCopy, for: repository) else { + observabilityScope.emit(debug: "working copy at '\(checkoutPath)' does not align with expected local path of '\(repository)'") break fetch } - // The fetch operation may update contents of the checkout, so // we need do mutable-immutable dance. - try self.fileSystem.chmod(.userWritable, path: path, options: [.recursive, .onlyFiles]) + try self.fileSystem.chmod(.userWritable, path: checkoutPath, options: [.recursive, .onlyFiles]) try workingCopy.fetch() - try? self.fileSystem.chmod(.userUnWritable, path: path, options: [.recursive, .onlyFiles]) + try? self.fileSystem.chmod(.userUnWritable, path: checkoutPath, options: [.recursive, .onlyFiles]) - return path + return checkoutPath } } // If not, we need to get the repository from the checkouts. - let repository = try package.makeRepositorySpecifier() // FIXME: this should not block let handle = try temp_await { self.repositoryManager.lookup( @@ -3423,24 +3431,24 @@ extension Workspace { } // Clone the repository into the checkouts. - let path = self.location.repositoriesCheckoutsDirectory.appending(component: repository.basename) + let checkoutPath = self.location.repositoriesCheckoutsDirectory.appending(component: repository.basename) // Remove any existing content at that path. - try self.fileSystem.chmod(.userWritable, path: path, options: [.recursive, .onlyFiles]) - try self.fileSystem.removeFileTree(path) + try self.fileSystem.chmod(.userWritable, path: checkoutPath, options: [.recursive, .onlyFiles]) + try self.fileSystem.removeFileTree(checkoutPath) // Inform the delegate that we're about to start. - self.delegate?.willCreateWorkingCopy(package: package.identity, repository: handle.repository.location.description, at: path) + self.delegate?.willCreateWorkingCopy(package: package.identity, repository: handle.repository.location.description, at: checkoutPath) let start = DispatchTime.now() // Create the working copy. - _ = try handle.createWorkingCopy(at: path, editable: false) - + _ = try handle.createWorkingCopy(at: checkoutPath, editable: false) + // Inform the delegate that we're done. let duration = start.distance(to: .now()) - self.delegate?.didCreateWorkingCopy(package: package.identity, repository: handle.repository.location.description, at: path, duration: duration) + self.delegate?.didCreateWorkingCopy(package: package.identity, repository: handle.repository.location.description, at: checkoutPath, duration: duration) - return path + return checkoutPath } /// Removes the clone and checkout of the provided specifier. diff --git a/Tests/SourceControlTests/GitRepositoryTests.swift b/Tests/SourceControlTests/GitRepositoryTests.swift index d48ffc2486f..8dfdd8a7c70 100644 --- a/Tests/SourceControlTests/GitRepositoryTests.swift +++ b/Tests/SourceControlTests/GitRepositoryTests.swift @@ -655,11 +655,14 @@ class GitRepositoryTests: XCTestCase { let checkoutRepo = try provider.createWorkingCopy(repository: repoSpec, sourcePath: testClonePath, at: checkoutPath, editable: false) // The object store should be valid. - XCTAssertTrue(checkoutRepo.isAlternateObjectStoreValid()) + XCTAssertTrue(checkoutRepo.isAlternateObjectStoreValid(expected: testClonePath)) + + // Wrong path + XCTAssertFalse(checkoutRepo.isAlternateObjectStoreValid(expected: testClonePath.appending(UUID().uuidString))) // Delete the clone (alternative object store). try localFileSystem.removeFileTree(testClonePath) - XCTAssertFalse(checkoutRepo.isAlternateObjectStoreValid()) + XCTAssertFalse(checkoutRepo.isAlternateObjectStoreValid(expected: testClonePath)) } } diff --git a/Tests/SourceControlTests/RepositoryManagerTests.swift b/Tests/SourceControlTests/RepositoryManagerTests.swift index 834ea8be98c..caaf7dec27c 100644 --- a/Tests/SourceControlTests/RepositoryManagerTests.swift +++ b/Tests/SourceControlTests/RepositoryManagerTests.swift @@ -787,7 +787,7 @@ private class DummyRepositoryProvider: RepositoryProvider { fatalError("not implemented") } - func isAlternateObjectStoreValid() -> Bool { + func isAlternateObjectStoreValid(expected: AbsolutePath) -> Bool { fatalError("not implemented") }