Skip to content

Commit 4953d32

Browse files
tomerdtomerd
authored
add delegate to track events from the dependency resolver (#3420) (#3425)
motivation: * significant part of pacakge resolution is spent in the dependency resolver which does not emit events we can use to tell the user what is happening * a follow up PR would use these new events to emit inforamtion when package depedencies are resolved, e.g when running swift package update chnages: * define DependencyResolverDelegate * replace logging in PubgrubDependencyResolver with calling the relevant delegate methods * add TracingDependencyResolverDelegate which back-fills the existing logging behavior in PubgrubDependencyResolver * adjust call-sites
1 parent f1c30ab commit 4953d32

File tree

6 files changed

+180
-65
lines changed

6 files changed

+180
-65
lines changed

‎Sources/PackageGraph/DependencyResolver.swift

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,16 @@
1111
import TSCBasic
1212
import PackageModel
1313

14+
public protocol DependencyResolver {
15+
typealias Binding = (package: PackageReference, binding: BoundVersion, products: ProductFilter)
16+
typealias Delegate = DependencyResolverDelegate
17+
}
18+
1419
public enum DependencyResolverError: Error, Equatable {
1520
/// A revision-based dependency contains a local package dependency.
1621
case revisionDependencyContainsLocalPackage(dependency: String, localPackage: String)
1722
}
1823

19-
public class DependencyResolver {
20-
public typealias Binding = (container: PackageReference, binding: BoundVersion, products: ProductFilter)
21-
}
22-
2324
extension DependencyResolverError: CustomStringConvertible {
2425
public var description: String {
2526
switch self {
@@ -28,3 +29,71 @@ extension DependencyResolverError: CustomStringConvertible {
2829
}
2930
}
3031
}
32+
33+
public protocol DependencyResolverDelegate {
34+
func willResolve(term: Term)
35+
func didResolve(term: Term, version: Version)
36+
37+
func derived(term: Term)
38+
func conflict(conflict: Incompatibility)
39+
func satisfied(term: Term, by: Assignment, incompatibility: Incompatibility)
40+
func partiallySatisfied(term: Term, by: Assignment, incompatibility: Incompatibility, difference: Term)
41+
func failedToResolve(incompatibility: Incompatibility)
42+
func computed(bindings: [DependencyResolver.Binding])
43+
}
44+
45+
public struct TracingDependencyResolverDelegate: DependencyResolverDelegate {
46+
private let stream: OutputByteStream
47+
48+
public init (path: AbsolutePath) throws {
49+
self.stream = try LocalFileOutputByteStream(path, closeOnDeinit: true, buffered: false)
50+
}
51+
52+
public init (stream: OutputByteStream) {
53+
self.stream = stream
54+
}
55+
56+
public func willResolve(term: Term) {
57+
self.log("resolving: \(term.node.package.location)")
58+
}
59+
60+
public func didResolve(term: Term, version: Version) {
61+
self.log("resolved: \(term.node.package.location) @ \(version)")
62+
}
63+
64+
public func derived(term: Term) {
65+
self.log("derived: \(term.node.package.location)")
66+
}
67+
68+
public func conflict(conflict: Incompatibility) {
69+
self.log("conflict: \(conflict)")
70+
}
71+
72+
public func failedToResolve(incompatibility: Incompatibility) {
73+
self.log("failed to resolve: \(incompatibility)")
74+
}
75+
76+
public func satisfied(term: Term, by assignment: Assignment, incompatibility: Incompatibility) {
77+
log("CR: \(term) is satisfied by \(assignment)")
78+
log("CR: which is caused by \(assignment.cause?.description ?? "")")
79+
log("CR: new incompatibility \(incompatibility)")
80+
}
81+
82+
public func partiallySatisfied(term: Term, by assignment: Assignment, incompatibility: Incompatibility, difference: Term) {
83+
log("CR: \(term) is partially satisfied by \(assignment)")
84+
log("CR: which is caused by \(assignment.cause?.description ?? "")")
85+
log("CR: new incompatibility \(incompatibility)")
86+
}
87+
88+
public func computed(bindings: [DependencyResolver.Binding]) {
89+
self.log("solved:")
90+
for (container, binding, _) in bindings {
91+
self.log("\(container) \(binding)")
92+
}
93+
}
94+
95+
private func log(_ message: String) {
96+
self.stream <<< message <<< "\n"
97+
self.stream.flush()
98+
}
99+
}

‎Sources/PackageGraph/Pubgrub/PartialSolution.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public struct PartialSolution {
2929

3030
/// Union of all negative assignments for a package.
3131
///
32-
/// Only present if a package has no postive assignment.
32+
/// Only present if a package has no positive assignment.
3333
public private(set) var _negative: [DependencyResolutionNode: Term] = [:]
3434

3535
/// The current decision level.

‎Sources/PackageGraph/Pubgrub/PubgrubDependencyResolver.swift

Lines changed: 20 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -111,40 +111,22 @@ public struct PubgrubDependencyResolver {
111111
/// Should resolver prefetch the containers.
112112
private let isPrefetchingEnabled: Bool
113113

114-
// Log stream
115-
private let traceStream: OutputByteStream?
114+
/// Resolver delegate
115+
private let delegate: DependencyResolverDelegate?
116116

117117
public init(
118118
provider: PackageContainerProvider,
119119
pinsMap: PinsStore.PinsMap = [:],
120120
isPrefetchingEnabled: Bool = false,
121121
skipUpdate: Bool = false,
122-
traceFile: AbsolutePath? = nil,
123-
traceStream: OutputByteStream? = nil
122+
delegate: DependencyResolverDelegate? = nil
124123
) {
125124
self.packageContainerProvider = provider
126125
self.pinsMap = pinsMap
127126
self.isPrefetchingEnabled = isPrefetchingEnabled
128127
self.skipUpdate = skipUpdate
129-
if let stream = traceStream {
130-
self.traceStream = stream
131-
} else {
132-
self.traceStream = traceFile.flatMap { file in
133-
// FIXME: Emit a warning if this fails.
134-
try? LocalFileOutputByteStream(file, closeOnDeinit: true, buffered: false)
135-
}
136-
}
137128
self.provider = ContainerProvider(provider: self.packageContainerProvider, skipUpdate: self.skipUpdate, pinsMap: self.pinsMap)
138-
}
139-
140-
public init(
141-
provider: PackageContainerProvider,
142-
pinsMap: PinsStore.PinsMap = [:],
143-
isPrefetchingEnabled: Bool = false,
144-
skipUpdate: Bool = false,
145-
traceFile: AbsolutePath? = nil
146-
) {
147-
self.init(provider: provider, pinsMap: pinsMap, isPrefetchingEnabled: isPrefetchingEnabled, skipUpdate: skipUpdate, traceFile: traceFile, traceStream: nil)
129+
self.delegate = delegate
148130
}
149131

150132
/// Execute the resolution algorithm to find a valid assignment of versions.
@@ -245,7 +227,7 @@ public struct PubgrubDependencyResolver {
245227
var finalAssignments: [DependencyResolver.Binding]
246228
= flattenedAssignments.keys.sorted(by: { $0.name < $1.name }).map { package in
247229
let details = flattenedAssignments[package]!
248-
return (container: package, binding: details.binding, products: details.products)
230+
return (package: package, binding: details.binding, products: details.products)
249231
}
250232

251233
// Add overriden packages to the result.
@@ -256,7 +238,7 @@ public struct PubgrubDependencyResolver {
256238
finalAssignments.append((identifier, override.version, override.products))
257239
}
258240

259-
self.log(finalAssignments)
241+
self.delegate?.computed(bindings: finalAssignments)
260242

261243
return (finalAssignments, state)
262244
}
@@ -517,7 +499,7 @@ public struct PubgrubDependencyResolver {
517499
return .conflict
518500
}
519501

520-
log("derived: \(unsatisfiedTerm.inverse)")
502+
self.delegate?.derived(term: unsatisfiedTerm.inverse)
521503
state.derive(unsatisfiedTerm.inverse, cause: incompatibility)
522504

523505
return .almostSatisfied(node: unsatisfiedTerm.node)
@@ -527,7 +509,7 @@ public struct PubgrubDependencyResolver {
527509
// https://github.com/dart-lang/pub/tree/master/doc/solver.md#conflict-resolution
528510
// https://github.com/dart-lang/pub/blob/master/lib/src/solver/version_solver.dart#L201
529511
internal func resolve(state: State, conflict: Incompatibility) throws -> Incompatibility {
530-
log("conflict: \(conflict)")
512+
self.delegate?.conflict(conflict: conflict)
531513

532514
var incompatibility = conflict
533515
var createdIncompatibility = false
@@ -594,12 +576,16 @@ public struct PubgrubDependencyResolver {
594576
)
595577
createdIncompatibility = true
596578

597-
log("CR: \(mostRecentTerm?.description ?? "") is\(difference != nil ? " partially" : "") satisfied by \(_mostRecentSatisfier)")
598-
log("CR: which is caused by \(_mostRecentSatisfier.cause?.description ?? "")")
599-
log("CR: new incompatibility \(incompatibility)")
579+
if let term = mostRecentTerm {
580+
if let diff = difference {
581+
self.delegate?.partiallySatisfied(term: term, by: _mostRecentSatisfier, incompatibility: incompatibility, difference: diff)
582+
} else {
583+
self.delegate?.satisfied(term: term, by: _mostRecentSatisfier, incompatibility: incompatibility)
584+
}
585+
}
600586
}
601587

602-
log("failed: \(incompatibility)")
588+
self.delegate?.failedToResolve(incompatibility: incompatibility)
603589
throw PubgrubError._unresolvable(incompatibility, state.incompatibilities)
604590
}
605591

@@ -649,8 +635,10 @@ public struct PubgrubDependencyResolver {
649635
let counts = try result.get()
650636
// forced unwraps safe since we are testing for count and errors above
651637
let pkgTerm = undecided.min { counts[$0]! < counts[$1]! }!
638+
self.delegate?.willResolve(term: pkgTerm)
652639
// at this point the container is cached
653640
let container = try self.provider.getCachedContainer(for: pkgTerm.node.package)
641+
654642
// Get the best available version for this package.
655643
guard let version = try container.getBestAvailableVersion(for: pkgTerm) else {
656644
state.addIncompatibility(try Incompatibility(pkgTerm, root: state.root, cause: .noAvailableVersion), at: .decisionMaking)
@@ -670,7 +658,7 @@ public struct PubgrubDependencyResolver {
670658
// Add the incompatibility to our partial solution.
671659
state.addIncompatibility(incompatibility, at: .decisionMaking)
672660

673-
// Check if this incompatibility will statisfy the solution.
661+
// Check if this incompatibility will satisfy the solution.
674662
haveConflict = haveConflict || incompatibility.terms.allSatisfy {
675663
// We only need to check if the terms other than this package
676664
// are satisfied because we _know_ that the terms matching
@@ -682,7 +670,7 @@ public struct PubgrubDependencyResolver {
682670

683671
// Decide this version if there was no conflict with its dependencies.
684672
if !haveConflict {
685-
self.log("decision: \(pkgTerm.node.package)@\(version)")
673+
self.delegate?.didResolve(term: pkgTerm, version: version)
686674
state.decide(pkgTerm.node, at: version)
687675
}
688676

@@ -692,20 +680,6 @@ public struct PubgrubDependencyResolver {
692680
}
693681
}
694682
}
695-
696-
private func log(_ assignments: [(container: PackageReference, binding: BoundVersion, products: ProductFilter)]) {
697-
log("solved:")
698-
for (container, binding, _) in assignments {
699-
log("\(container) \(binding)")
700-
}
701-
}
702-
703-
private func log(_ message: String) {
704-
if let traceStream = traceStream {
705-
traceStream <<< message <<< "\n"
706-
traceStream.flush()
707-
}
708-
}
709683
}
710684

711685
private struct DiagnosticReportBuilder {

‎Sources/Workspace/Workspace.swift

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ extension Workspace {
580580
}
581581

582582
// Resolve the dependencies.
583-
let resolver = self.createResolver(pinsMap: pinsMap)
583+
let resolver = try self.createResolver(pinsMap: pinsMap)
584584
self.activeResolver = resolver
585585

586586
let updateResults = resolveDependencies(
@@ -1924,7 +1924,7 @@ extension Workspace {
19241924
constraints += try graphRoot.constraints() + extraConstraints
19251925

19261926
// Perform dependency resolution.
1927-
let resolver = createResolver(pinsMap: pinsStore.pinsMap)
1927+
let resolver = try createResolver(pinsMap: pinsStore.pinsMap)
19281928
self.activeResolver = resolver
19291929

19301930
let result = resolveDependencies(
@@ -2285,14 +2285,15 @@ extension Workspace {
22852285
}
22862286

22872287
/// Creates resolver for the workspace.
2288-
fileprivate func createResolver(pinsMap: PinsStore.PinsMap) -> PubgrubDependencyResolver {
2289-
let traceFile = enableResolverTrace ? self.dataPath.appending(components: "resolver.trace") : nil
2288+
fileprivate func createResolver(pinsMap: PinsStore.PinsMap) throws -> PubgrubDependencyResolver {
2289+
let delegate = self.enableResolverTrace ? try TracingDependencyResolverDelegate(path: self.dataPath.appending(components: "resolver.trace")) : nil
22902290

22912291
return PubgrubDependencyResolver(
22922292
provider: containerProvider,
22932293
pinsMap: pinsMap,
22942294
isPrefetchingEnabled: isResolverPrefetchingEnabled,
2295-
skipUpdate: skipUpdate, traceFile: traceFile
2295+
skipUpdate: skipUpdate,
2296+
delegate: delegate
22962297
)
22972298
}
22982299

@@ -2301,7 +2302,7 @@ extension Workspace {
23012302
resolver: PubgrubDependencyResolver,
23022303
constraints: [PackageContainerConstraint],
23032304
diagnostics: DiagnosticsEngine
2304-
) -> [(container: PackageReference, binding: BoundVersion, products: ProductFilter)] {
2305+
) -> [(package: PackageReference, binding: BoundVersion, products: ProductFilter)] {
23052306

23062307
os_signpost(.begin, log: .swiftpm, name: SignpostName.resolution)
23072308
let result = resolver.solve(constraints: constraints)

‎Tests/PackageGraphPerformanceTests/DependencyResolverPerfTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ class DependencyResolverRealWorldPerfTests: XCTestCasePerf {
6262
XCTFail("Unexpected result")
6363
return nil
6464
}
65-
return ($0.container, version)
65+
return ($0.package, version)
6666
}
6767
graph.checkResult(result)
6868
case .failure:

0 commit comments

Comments
 (0)