Skip to content

improve repository cache information emmited to delegate #3705

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.

Sign up for GitHub

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 1 commit into from
Sep 2, 2021
Merged

improve repository cache information emmited to delegate #3705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions Sources/SourceControl/RepositoryManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ public class RepositoryManager {
/// - Returns: Details about the performed fetch.
@discardableResult
func fetchAndPopulateCache(handle: RepositoryHandle, repositoryPath: AbsolutePath) throws -> FetchDetails {
var updatedCache = false
var fromCache = false
var cacheUsed = false
var cacheUpdated = false

// We are expecting handle.repository.url to always be a resolved absolute path.
let isLocal = (try? AbsolutePath(validating: handle.repository.url)) != nil
Expand All @@ -248,29 +248,29 @@ public class RepositoryManager {
if (fileSystem.exists(cachedRepositoryPath)) {
let repo = try self.provider.open(repository: handle.repository, at: cachedRepositoryPath)
try repo.fetch()
cacheUsed = true
} else {
try self.provider.fetch(repository: handle.repository, to: cachedRepositoryPath)
}
updatedCache = true
cacheUpdated = true
// Copy the repository from the cache into the repository path.
try fileSystem.createDirectory(repositoryPath.parentDirectory, recursive: true)
try self.provider.copy(from: cachedRepositoryPath, to: repositoryPath)
fromCache = true
}
}
} catch {
cacheUsed = false
// Fetch without populating the cache in the case of an error.
print("Skipping cache due to an error: \(error)")
// It is possible that we already created the directory before failing, so clear leftover data if present.
try fileSystem.removeFileTree(repositoryPath)
try self.provider.fetch(repository: handle.repository, to: repositoryPath)
fromCache = false
}
} else {
// Fetch without populating the cache when no `cachePath` is set.
try self.provider.fetch(repository: handle.repository, to: repositoryPath)
fromCache = false
}
return FetchDetails(fromCache: fromCache, updatedCache: updatedCache)
return FetchDetails(fromCache: cacheUsed, updatedCache: cacheUpdated)
}

public func openWorkingCopy(at path: AbsolutePath) throws -> WorkingCheckout {
Expand Down
15 changes: 11 additions & 4 deletions Tests/SourceControlTests/RepositoryManagerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,6 @@ class RepositoryManagerTests: XCTestCase {
// Avoid a crasher that seems to happen only on macOS 10.15, but leave an environment variable for testing.
try XCTSkipUnless(ProcessEnv.vars["SWIFTPM_ENABLE_FLAKY_REPOSITORYMANAGERTESTS"] == "1", "skipping test that sometimes crashes in CI (rdar://70540298)")
}

try XCTSkipIf(true, "test became racy")

fixture(name: "DependencyResolution/External/Simple") { prefix in
let cachePath = prefix.appending(component: "cache")
Expand Down Expand Up @@ -388,6 +386,7 @@ class RepositoryManagerTests: XCTestCase {
XCTAssertEqual(delegate.didFetch[0].fetchDetails,
RepositoryManager.FetchDetails(fromCache: false, updatedCache: true))

// removing the repositories path to force re-fetch
try localFileSystem.removeFileTree(repositoriesPath)

// fetch packages from the cache
Expand All @@ -398,10 +397,11 @@ class RepositoryManagerTests: XCTestCase {
XCTAssertEqual(delegate.willFetch[1].fetchDetails,
RepositoryManager.FetchDetails(fromCache: true, updatedCache: false))
XCTAssertEqual(delegate.didFetch[1].fetchDetails,
RepositoryManager.FetchDetails(fromCache: false, updatedCache: true))
RepositoryManager.FetchDetails(fromCache: true, updatedCache: true))

try localFileSystem.removeFileTree(repositoriesPath)
// reset the state on disk
try localFileSystem.removeFileTree(cachePath)
try localFileSystem.removeFileTree(repositoriesPath)

// fetch packages and populate cache
delegate.willFetchGroup?.enter()
Expand All @@ -413,6 +413,13 @@ class RepositoryManagerTests: XCTestCase {
RepositoryManager.FetchDetails(fromCache: false, updatedCache: false))
XCTAssertEqual(delegate.didFetch[2].fetchDetails,
RepositoryManager.FetchDetails(fromCache: false, updatedCache: true))

// update packages from the cache
delegate.willUpdateGroup?.enter()
delegate.willUpdateGroup?.enter()
_ = try manager.lookup(repository: repo)
XCTAssertEqual(delegate.willUpdate[0].fileSystemIdentifier, repo.fileSystemIdentifier)
XCTAssertEqual(delegate.didUpdate[0].fileSystemIdentifier, repo.fileSystemIdentifier)
}
}

Expand Down