Skip to content

validate local repository has the correct remote #7079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Sources/SPMTestSupport/InMemoryGitRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 5 additions & 0 deletions Sources/SourceControl/GitRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will the remote always be origin? (just wondering)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes iiuc, in this case it us the one that SwiftPM sets

return remoteURL == repository.url
}

public func copy(from sourcePath: Basics.AbsolutePath, to destinationPath: Basics.AbsolutePath) throws {
try localFileSystem.copy(from: sourcePath, to: destinationPath)
}
Expand Down
3 changes: 3 additions & 0 deletions Sources/SourceControl/Repository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 for the specified repository
func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool
}

/// Abstract repository operations.
Expand Down
30 changes: 20 additions & 10 deletions Sources/SourceControl/RepositoryManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -426,6 +431,11 @@ public class RepositoryManager: Cancellable {
try self.provider.isValidDirectory(directory)
}

/// Returns true if the directory is valid git location for the specified repository
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.
Expand Down
213 changes: 172 additions & 41 deletions Tests/SourceControlTests/RepositoryManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -561,11 +561,134 @@ 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")
}
}
}

func testInvalidRepositoryOnDisk() throws {
let fileSystem = localFileSystem
let observability = ObservabilitySystem.makeForTesting()

try testWithTemporaryDirectory { path in
let repositoriesDirectory = path.appending("repositories")
try fileSystem.createDirectory(repositoriesDirectory, recursive: true)

let testRepository = RepositorySpecifier(url: .init("test-\(UUID().uuidString)"))
let provider = MockRepositoryProvider(repository: testRepository)

let manager = RepositoryManager(
fileSystem: fileSystem,
path: repositoriesDirectory,
provider: provider,
delegate: nil
)

_ = try manager.lookup(repository: testRepository, observabilityScope: observability.topScope)
testDiagnostics(observability.diagnostics) { result in
result.check(
diagnostic: .contains("is not valid git repository for '\(testRepository)', will fetch again"),
severity: .warning
)
}
}

class MockRepositoryProvider: RepositoryProvider {
let repository: RepositorySpecifier
var fetch: Int = 0

init(repository: RepositorySpecifier) {
self.repository = repository
}

func fetch(repository: RepositorySpecifier, to path: AbsolutePath, progressHandler: ((FetchProgress) -> Void)?) throws {
assert(repository == self.repository)
self.fetch += 1
}

func repositoryExists(at path: AbsolutePath) throws -> Bool {
// the directory exists
return true
}

func open(repository: RepositorySpecifier, at path: AbsolutePath) throws -> Repository {
return MockRepository()
}

func createWorkingCopy(repository: RepositorySpecifier, sourcePath: AbsolutePath, at destinationPath: AbsolutePath, editable: Bool) throws -> WorkingCheckout {
fatalError("should not be called")
}

func workingCopyExists(at path: AbsolutePath) throws -> Bool {
fatalError("should not be called")
}

func openWorkingCopy(at path: AbsolutePath) throws -> WorkingCheckout {
fatalError("should not be called")
}

func copy(from sourcePath: AbsolutePath, to destinationPath: AbsolutePath) throws {
fatalError("should not be called")
}

func isValidDirectory(_ directory: AbsolutePath) throws -> Bool {
fatalError("should not be called")
}

public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
assert(repository == self.repository)
// the directory is not valid
return false
}

func cancel(deadline: DispatchTime) throws {
fatalError("should not be called")
}
}

class MockRepository: Repository {
func getTags() throws -> [String] {
fatalError("unexpected API call")
}

func resolveRevision(tag: String) throws -> Revision {
fatalError("unexpected API call")
}

func resolveRevision(identifier: String) throws -> Revision {
fatalError("unexpected API call")
}

func exists(revision: Revision) -> Bool {
fatalError("unexpected API call")
}

func isValidDirectory(_ directory: AbsolutePath) throws -> Bool {
fatalError("unexpected API call")
}

public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
fatalError("unexpected API call")
}

func fetch() throws {
// noop
}

func openFileView(revision: Revision) throws -> FileSystem {
fatalError("unexpected API call")
}

public func openFileView(tag: String) throws -> FileSystem {
fatalError("unexpected API call")
}
}
}
}

extension RepositoryManager {
Expand Down Expand Up @@ -613,46 +736,6 @@ private enum DummyError: Swift.Error {
case invalidRepository
}

private class DummyRepository: Repository {
unowned let provider: DummyRepositoryProvider

init(provider: DummyRepositoryProvider) {
self.provider = provider
}

func getTags() throws -> [String] {
["1.0.0"]
}

func resolveRevision(tag: String) throws -> Revision {
fatalError("unexpected API call")
}

func resolveRevision(identifier: String) throws -> Revision {
fatalError("unexpected API call")
}

func exists(revision: Revision) -> Bool {
fatalError("unexpected API call")
}

func isValidDirectory(_ directory: AbsolutePath) throws -> Bool {
fatalError("unexpected API call")
}

func fetch() throws {
self.provider.increaseFetchCount()
}

func openFileView(revision: Revision) throws -> FileSystem {
fatalError("unexpected API call")
}

public func openFileView(tag: String) throws -> FileSystem {
fatalError("unexpected API call")
}
}

private class DummyRepositoryProvider: RepositoryProvider {
private let fileSystem: FileSystem

Expand Down Expand Up @@ -720,6 +803,10 @@ private class DummyRepositoryProvider: RepositoryProvider {
return true
}

func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
return true
}

func cancel(deadline: DispatchTime) throws {
// noop
}
Expand Down Expand Up @@ -795,7 +882,7 @@ private class DummyRepositoryProvider: RepositoryProvider {
}
}

private class DummyRepositoryManagerDelegate: RepositoryManager.Delegate {
fileprivate class DummyRepositoryManagerDelegate: RepositoryManager.Delegate {
private var _willFetch = ThreadSafeArrayStore<(repository: RepositorySpecifier, details: RepositoryManager.FetchDetails)>()
private var _didFetch = ThreadSafeArrayStore<(repository: RepositorySpecifier, result: Result<RepositoryManager.FetchDetails, Error>)>()
private var _willUpdate = ThreadSafeArrayStore<RepositorySpecifier>()
Expand Down Expand Up @@ -870,3 +957,47 @@ private class DummyRepositoryManagerDelegate: RepositoryManager.Delegate {
self.group.leave()
}
}

fileprivate class DummyRepository: Repository {
unowned let provider: DummyRepositoryProvider

init(provider: DummyRepositoryProvider) {
self.provider = provider
}

func getTags() throws -> [String] {
["1.0.0"]
}

func resolveRevision(tag: String) throws -> Revision {
fatalError("unexpected API call")
}

func resolveRevision(identifier: String) throws -> Revision {
fatalError("unexpected API call")
}

func exists(revision: Revision) -> Bool {
fatalError("unexpected API call")
}

func isValidDirectory(_ directory: AbsolutePath) throws -> Bool {
fatalError("unexpected API call")
}

public func isValidDirectory(_ directory: AbsolutePath, for repository: RepositorySpecifier) throws -> Bool {
fatalError("unexpected API call")
}

func fetch() throws {
self.provider.increaseFetchCount()
}

func openFileView(revision: Revision) throws -> FileSystem {
fatalError("unexpected API call")
}

public func openFileView(tag: String) throws -> FileSystem {
fatalError("unexpected API call")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down