[PubGrub] Avoid resolving package versions twice in presence of prebuilt libraries

Integrate information about versions of prebuilt libraries into the solver
and prioritize these versions when making decisions if they meet all of the
requirements of undecided terms.

This helps solver to avoid fetching and at the same time allows partual
matches on prebuilt libraries with fallback to package provided versions.

Author: xedin

Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift

@@ -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) {
@@ -93,7 +81,7 @@ 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 {

Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift

@@ -103,6 +103,9 @@ public struct PubGrubDependencyResolver {
     /// Reference to the pins store, if provided.
     private let pins: PinsStore.Pins
 
+    /// The packages that are available in a prebuilt form in SDK or a toolchain
+    private let availableLibraries: [LibraryMetadata]
+
     /// The container provider used to load package containers.
     private let provider: ContainerProvider
 
@@ -121,13 +124,15 @@ public struct PubGrubDependencyResolver {
     public init(
         provider: PackageContainerProvider,
         pins: PinsStore.Pins = [:],
+        availableLibraries: [LibraryMetadata] = [],
         skipDependenciesUpdates: Bool = false,
         prefetchBasedOnResolvedFile: Bool = false,
         observabilityScope: ObservabilityScope,
         delegate: DependencyResolverDelegate? = nil
     ) {
         self.packageContainerProvider = provider
         self.pins = pins
+        self.availableLibraries = availableLibraries
         self.skipDependenciesUpdates = skipDependenciesUpdates
         self.prefetchBasedOnResolvedFile = prefetchBasedOnResolvedFile
         self.provider = ContainerProvider(
@@ -140,11 +145,7 @@ public struct PubGrubDependencyResolver {
     }
 
     /// Execute the resolution algorithm to find a valid assignment of versions.
-    public func solve(constraints: [Constraint], availableLibraries: [LibraryMetadata], preferPrebuiltLibraries: Bool) -> Result<[DependencyResolverBinding], Error> {
-        if !preferPrebuiltLibraries {
-            self.provider.removeCachedContainers(for: availableLibraries.flatMap { $0.identities.map { $0.ref } })
-        }
-
+    public func solve(constraints: [Constraint]) -> Result<[DependencyResolverBinding], Error> {
         // the graph resolution root
         let root: DependencyResolutionNode
         if constraints.count == 1, let constraint = constraints.first, constraint.package.kind.isRoot {
@@ -159,26 +160,24 @@ public struct PubGrubDependencyResolver {
         }
 
         do {
-            // Use empty `availableLibraries` for the rest of resolving if we don't prefer them.
-            let availableLibraries = preferPrebuiltLibraries ? availableLibraries : []
-            // strips state
-            let bindings = try self.solve(root: root, constraints: constraints, availableLibraries: availableLibraries).bindings.filter {
-                return $0.package.matchingPrebuiltLibrary(in: availableLibraries) == nil
+            // Strip packages that have prebuilt libraries only if they match library version.
+            //
+            // FIXME: This is built on assumption that libraries are part of the SDK and are
+            //        always available in include/library paths, but what happens if they are
+            //        part of a toolchain instead? Builder needs an indicator that certain path
+            //        has to be included when building packages that depend on prebuilt libraries.
+            let bindings = try self.solve(root: root, constraints: constraints).bindings.filter {
+                if case let .version(version) = $0.boundVersion {
+                    if let library = $0.package.matchingPrebuiltLibrary(in: self.availableLibraries) {
+                        return .init(stringLiteral: library.version) != version
+                    }
+                }
+                return true
             }
             return .success(bindings)
         } catch {
             // If version solving failing, build the user-facing diagnostic.
             if let pubGrubError = error as? PubgrubError, let rootCause = pubGrubError.rootCause, let incompatibilities = pubGrubError.incompatibilities {
-                let incompatiblePackages = incompatibilities.map({ $0.key.package })
-                if incompatiblePackages.contains(where: { $0.matchingPrebuiltLibrary(in: availableLibraries) != nil }) {
-                    return .failure(
-                        PubgrubError.potentiallyUnresovableDueToPrebuiltLibrary(
-                            incompatiblePackages,
-                            pubGrubError.description
-                        )
-                    )
-                }
-
                 do {
                     var builder = DiagnosticReportBuilder(
                         root: root,
@@ -199,12 +198,12 @@ public struct PubGrubDependencyResolver {
     /// Find a set of dependencies that fit the given constraints. If dependency
     /// resolution is unable to provide a result, an error is thrown.
     /// - Warning: It is expected that the root package reference has been set  before this is called.
-    internal func solve(root: DependencyResolutionNode, constraints: [Constraint], availableLibraries: [LibraryMetadata] = []) throws -> (
+    internal func solve(root: DependencyResolutionNode, constraints: [Constraint]) throws -> (
         bindings: [DependencyResolverBinding],
         state: State
     ) {
         // first process inputs
-        let inputs = try self.processInputs(root: root, with: constraints, availableLibraries: availableLibraries)
+        let inputs = try self.processInputs(root: root, with: constraints)
 
         // Prefetch the containers if prefetching is enabled.
         if self.prefetchBasedOnResolvedFile {
@@ -214,7 +213,7 @@ public struct PubGrubDependencyResolver {
             let pins = self.pins.values
                 .map(\.packageRef)
                 .filter { !inputs.overriddenPackages.keys.contains($0) }
-            self.provider.prefetch(containers: pins, availableLibraries: availableLibraries)
+            self.provider.prefetch(containers: pins, availableLibraries: self.availableLibraries)
         }
 
         let state = State(root: root, overriddenPackages: inputs.overriddenPackages)
@@ -230,7 +229,7 @@ public struct PubGrubDependencyResolver {
             state.addIncompatibility(incompatibility, at: .topLevel)
         }
 
-        try self.run(state: state, availableLibraries: availableLibraries)
+        try self.run(state: state)
 
         let decisions = state.solution.assignments.filter(\.isDecision)
         var flattenedAssignments: [PackageReference: (binding: BoundVersion, products: ProductFilter)] = [:]
@@ -250,7 +249,7 @@ public struct PubGrubDependencyResolver {
             let products = assignment.term.node.productFilter
 
             // TODO: replace with async/await when available
-            let container = try temp_await { provider.getContainer(for: assignment.term.node.package, availableLibraries: availableLibraries, completion: $0) }
+            let container = try temp_await { provider.getContainer(for: assignment.term.node.package, completion: $0) }
             let updatePackage = try container.underlying.loadPackageReference(at: boundVersion)
 
             if var existing = flattenedAssignments[updatePackage] {
@@ -272,7 +271,7 @@ public struct PubGrubDependencyResolver {
         // Add overridden packages to the result.
         for (package, override) in state.overriddenPackages {
             // TODO: replace with async/await when available
-            let container = try temp_await { provider.getContainer(for: package, availableLibraries: availableLibraries, completion: $0) }
+            let container = try temp_await { provider.getContainer(for: package, completion: $0) }
             let updatePackage = try container.underlying.loadPackageReference(at: override.version)
             finalAssignments.append(.init(
                     package: updatePackage,
@@ -288,8 +287,7 @@ public struct PubGrubDependencyResolver {
 
     private func processInputs(
         root: DependencyResolutionNode,
-        with constraints: [Constraint],
-        availableLibraries: [LibraryMetadata]
+        with constraints: [Constraint]
     ) throws -> (
         overriddenPackages: [PackageReference: (version: BoundVersion, products: ProductFilter)],
         rootIncompatibilities: [Incompatibility]
@@ -331,10 +329,10 @@ public struct PubGrubDependencyResolver {
                 // Process dependencies of this package.
                 //
                 // We collect all version-based dependencies in a separate structure so they can
-                // be process at the end. This allows us to override them when there is a non-version
+                // be processed at the end. This allows us to override them when there is a non-version
                 // based (unversioned/branch-based) constraint present in the graph.
                 // TODO: replace with async/await when available
-                let container = try temp_await { provider.getContainer(for: node.package, availableLibraries: availableLibraries, completion: $0) }
+                let container = try temp_await { provider.getContainer(for: node.package, completion: $0) }
                 for dependency in try container.underlying.getUnversionedDependencies(productFilter: node.productFilter) {
                     if let versionedBasedConstraints = VersionBasedConstraint.constraints(dependency) {
                         for constraint in versionedBasedConstraints {
@@ -382,7 +380,7 @@ public struct PubGrubDependencyResolver {
             // Process dependencies of this package, similar to the first phase but branch-based dependencies
             // are not allowed to contain local/unversioned packages.
             // TODO: replace with async/await when avail
-            let container = try temp_await { provider.getContainer(for: package, availableLibraries: availableLibraries, completion: $0) }
+            let container = try temp_await { provider.getContainer(for: package, completion: $0) }
 
             // If there is a pin for this revision-based dependency, get
             // the dependencies at the pinned revision instead of using
@@ -465,7 +463,7 @@ public struct PubGrubDependencyResolver {
     /// decisions if nothing else is left to be done.
     /// After this method returns `solution` is either populated with a list of
     /// final version assignments or an error is thrown.
-    private func run(state: State, availableLibraries: [LibraryMetadata]) throws {
+    private func run(state: State) throws {
         var next: DependencyResolutionNode? = state.root
 
         while let nxt = next {
@@ -474,13 +472,13 @@ public struct PubGrubDependencyResolver {
             // initiate prefetch of known packages that will be used to make the decision on the next step
             self.provider.prefetch(
                 containers: state.solution.undecided.map(\.node.package),
-                availableLibraries: availableLibraries
+                availableLibraries: self.availableLibraries
             )
 
             // If decision making determines that no more decisions are to be
             // made, it returns nil to signal that version solving is done.
             // TODO: replace with async/await when available
-            next = try temp_await { self.makeDecision(state: state, availableLibraries: availableLibraries, completion: $0) }
+            next = try temp_await { self.makeDecision(state: state, completion: $0) }
         }
     }
 
@@ -658,7 +656,6 @@ public struct PubGrubDependencyResolver {
 
     private func computeCounts(
         for terms: [Term],
-        availableLibraries: [LibraryMetadata],
         completion: @escaping (Result<[Term: Int], Error>) -> Void
     ) {
         if terms.isEmpty {
@@ -670,7 +667,7 @@ public struct PubGrubDependencyResolver {
 
         terms.forEach { term in
             sync.enter()
-            provider.getContainer(for: term.node.package, availableLibraries: availableLibraries) { result in
+            provider.getContainer(for: term.node.package) { result in
                 defer { sync.leave() }
                 results[term] = result.flatMap { container in Result(catching: { try container.versionCount(term.requirement) }) }
             }
@@ -687,7 +684,6 @@ public struct PubGrubDependencyResolver {
 
     internal func makeDecision(
         state: State,
-        availableLibraries: [LibraryMetadata] = [],
         completion: @escaping (Result<DependencyResolutionNode?, Error>) -> Void
     ) {
         // If there are no more undecided terms, version solving is complete.
@@ -696,9 +692,33 @@ public struct PubGrubDependencyResolver {
             return completion(.success(nil))
         }
 
+        // If prebuilt libraries are available, let's attempt their versions first before going for
+        // the latest viable version in the package. This way we archive multiple goals - prioritize
+        // prebuilt libraries if they satisfy all requirements, avoid counting and building package
+        // manifests and avoid (re-)building packages.
+        //
+        // Since the conflict resolution learns from incorrect terms this wouldn't be re-attempted.
+        if !self.availableLibraries.isEmpty {
+            let start = DispatchTime.now()
+            for pkgTerm in undecided {
+                let package = pkgTerm.node.package
+                guard let library = package.matchingPrebuiltLibrary(in: self.availableLibraries) else {
+                    continue
+                }
+
+                let version = Version(stringLiteral: library.version)
+
+                if pkgTerm.requirement.contains(version) {
+                    self.delegate?.didResolve(term: pkgTerm, version: version, duration: start.distance(to: .now()))
+                    state.decide(pkgTerm.node, at: version)
+                    return completion(.success(pkgTerm.node))
+                }
+            }
+        }
+
         // Prefer packages with least number of versions that fit the current requirements so we
         // get conflicts (if any) sooner.
-        self.computeCounts(for: undecided, availableLibraries: availableLibraries) { result in
+        self.computeCounts(for: undecided) { result in
             do {
                 let start = DispatchTime.now()
                 let counts = try result.get()
@@ -762,16 +782,13 @@ public extension PubGrubDependencyResolver {
     enum PubgrubError: Swift.Error, CustomStringConvertible {
         case _unresolvable(Incompatibility, [DependencyResolutionNode: [Incompatibility]])
         case unresolvable(String)
-        case potentiallyUnresovableDueToPrebuiltLibrary([PackageReference], String)
 
         public var description: String {
             switch self {
             case ._unresolvable(let rootCause, _):
                 return rootCause.description
             case .unresolvable(let error):
                 return error
-            case .potentiallyUnresovableDueToPrebuiltLibrary(_, let error):
-                return error
             }
         }
 
@@ -781,8 +798,6 @@ public extension PubGrubDependencyResolver {
                 return rootCause
             case .unresolvable:
                 return nil
-            case .potentiallyUnresovableDueToPrebuiltLibrary:
-                return nil
             }
         }
 
@@ -792,8 +807,6 @@ public extension PubGrubDependencyResolver {
                 return incompatibilities
             case .unresolvable:
                 return nil
-            case .potentiallyUnresovableDueToPrebuiltLibrary:
-                return nil
             }
         }
     }

Sources/SPMTestSupport/Resolver.swift

@@ -44,19 +44,14 @@ struct ResolverPrecomputationProvider: PackageContainerProvider {
     /// The tools version currently in use.
     let currentToolsVersion: ToolsVersion
 
-    /// The available libraries in the SDK.
-    let availableLibraries: [LibraryMetadata]
-
     init(
         root: PackageGraphRoot,
         dependencyManifests: Workspace.DependencyManifests,
-        currentToolsVersion: ToolsVersion = ToolsVersion.current,
-        availableLibraries: [LibraryMetadata]
+        currentToolsVersion: ToolsVersion = ToolsVersion.current
     ) {
         self.root = root
         self.dependencyManifests = dependencyManifests
         self.currentToolsVersion = currentToolsVersion
-        self.availableLibraries = availableLibraries
     }
 
     func getContainer(
@@ -89,15 +84,6 @@ struct ResolverPrecomputationProvider: PackageContainerProvider {
                 return completion(.success(container))
             }
 
-            // Match against available prebuilt libraries.
-            if let matchingPrebuiltLibrary = package.matchingPrebuiltLibrary(in: availableLibraries) {
-                do {
-                    return completion(.success(try PrebuiltPackageContainer(metadata: matchingPrebuiltLibrary)))
-                } catch {
-                    return completion(.failure(error))
-                }
-            }
-
             // As we don't have anything else locally, error out.
             completion(.failure(ResolverPrecomputationError.missingPackage(package: package)))
         }