Skip to content

[6.0][PubGrub] Avoid resolving package versions twice in presence of prebu… #7457

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
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
1 change: 0 additions & 1 deletion Sources/PackageGraph/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ add_library(PackageGraph
PackageGraphRoot.swift
PackageModel+Extensions.swift
PackageRequirement.swift
PrebuiltPackageContainer.swift
PinsStore.swift
Resolution/PubGrub/Assignment.swift
Resolution/PubGrub/ContainerProvider.swift
Expand Down
69 changes: 0 additions & 69 deletions Sources/PackageGraph/PrebuiltPackageContainer.swift

This file was deleted.

35 changes: 16 additions & 19 deletions Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ final class ContainerProvider {
}

func removeCachedContainers(for packages: [PackageReference]) {
packages.forEach {
self.containersCache[$0] = nil
for package in packages {
self.containersCache[package] = nil
}
}

Expand All @@ -65,25 +65,13 @@ final class ContainerProvider {
/// Get the container for the given identifier, loading it if necessary.
func getContainer(
for package: PackageReference,
availableLibraries: [LibraryMetadata],
completion: @escaping (Result<PubGrubPackageContainer, Error>) -> Void
) {
// Return the cached container, if available.
if let container = self.containersCache[comparingLocation: package] {
return completion(.success(container))
}

if let metadata = package.matchingPrebuiltLibrary(in: availableLibraries) {
do {
let prebuiltPackageContainer = try PrebuiltPackageContainer(metadata: metadata)
let pubGrubContainer = PubGrubPackageContainer(underlying: prebuiltPackageContainer, pins: self.pins)
self.containersCache[package] = pubGrubContainer
return completion(.success(pubGrubContainer))
} catch {
return completion(.failure(error))
}
}

if let prefetchSync = self.prefetches[package] {
// If this container is already being prefetched, wait for that to complete
prefetchSync.notify(queue: .sharedConcurrent) {
Expand All @@ -93,15 +81,18 @@ final class ContainerProvider {
} else {
// if prefetch failed, remove from list of prefetches and try again
self.prefetches[package] = nil
return self.getContainer(for: package, availableLibraries: availableLibraries, completion: completion)
return self.getContainer(for: package, completion: completion)
}
}
} else {
// Otherwise, fetch the container from the provider
self.underlying.getContainer(
for: package,
updateStrategy: self.skipUpdate ? .never : .always, // TODO: make this more elaborate
observabilityScope: self.observabilityScope.makeChildScope(description: "getting package container", metadata: package.diagnosticsMetadata),
observabilityScope: self.observabilityScope.makeChildScope(
description: "getting package container",
metadata: package.diagnosticsMetadata
),
on: .sharedConcurrent
) { result in
let result = result.tryMap { container -> PubGrubPackageContainer in
Expand All @@ -118,7 +109,7 @@ final class ContainerProvider {
/// Starts prefetching the given containers.
func prefetch(containers identifiers: [PackageReference], availableLibraries: [LibraryMetadata]) {
let filteredIdentifiers = identifiers.filter {
return $0.matchingPrebuiltLibrary(in: availableLibraries) == nil
$0.matchingPrebuiltLibrary(in: availableLibraries) == nil
}
// Process each container.
for identifier in filteredIdentifiers {
Expand All @@ -133,13 +124,19 @@ final class ContainerProvider {
self.underlying.getContainer(
for: identifier,
updateStrategy: self.skipUpdate ? .never : .always, // TODO: make this more elaborate
observabilityScope: self.observabilityScope.makeChildScope(description: "prefetching package container", metadata: identifier.diagnosticsMetadata),
observabilityScope: self.observabilityScope.makeChildScope(
description: "prefetching package container",
metadata: identifier.diagnosticsMetadata
),
on: .sharedConcurrent
) { result in
defer { self.prefetches[identifier]?.leave() }
// only cache positive results
if case .success(let container) = result {
self.containersCache[identifier] = PubGrubPackageContainer(underlying: container, pins: self.pins)
self.containersCache[identifier] = PubGrubPackageContainer(
underlying: container,
pins: self.pins
)
}
}
}
Expand Down
Loading