Skip to content

Commit a368b64

Browse files
tomerdtomerd
authored
emit information diagnostics for dependency resolution (#3835)
motivation: provide transperancy / debug information when needed changes: * add a new dependency resolver that emits debug diagnostics for resover callbacks, wire it into the observability system * this information will only show when running in verbose mode * deprecate TracingDependencyResolverDelegate which was a bespose solution serving the same need * adjust and add tests
1 parent 6aabf67 commit a368b64

File tree

9 files changed

+126
-45
lines changed

9 files changed

+126
-45
lines changed

‎Sources/Commands/Options.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,8 @@ public struct SwiftToolOptions: ParsableArguments {
273273
var shouldEnableParseableModuleInterfaces: Bool = false
274274

275275
/// Write dependency resolver trace to a file.
276-
@Flag(name: .customLong("trace-resolver"))
277-
var enableResolverTrace: Bool = false
276+
@Flag(name: .customLong("trace-resolver"), help: .hidden)
277+
var _deprecated_enableResolverTrace: Bool = false
278278

279279
/// The number of jobs for llbuild to start (aka the number of schedulerLanes)
280280
@Option(name: .shortAndLong, help: "The number of jobs to spawn in parallel during the build process")

‎Sources/Commands/SwiftTool.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,11 @@ public class SwiftTool {
467467
if options._deprecated_netrcOptional {
468468
observabilityScope.emit(warning: "'--netrc-optional' option is deprecated; .netrc files are located by default")
469469
}
470+
471+
if options._deprecated_enableResolverTrace {
472+
observabilityScope.emit(warning: "'--enableResolverTrace' option is deprecated; use --verbose flag to log resolver output")
473+
}
474+
470475
}
471476

472477
private func editsDirectory() throws -> AbsolutePath {
@@ -660,7 +665,6 @@ public class SwiftTool {
660665
additionalFileRules: isXcodeBuildSystemEnabled ? FileRuleDescription.xcbuildFileTypes : FileRuleDescription.swiftpmFileTypes,
661666
resolverUpdateEnabled: !options.skipDependencyUpdate,
662667
resolverPrefetchingEnabled: options.shouldEnableResolverPrefetching,
663-
resolverTracingEnabled: options.enableResolverTrace,
664668
sharedRepositoriesCacheEnabled: self.options.useRepositoriesCache,
665669
delegate: delegate
666670
)

‎Sources/PackageGraph/DependencyResolver.swift

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
99
*/
1010

11+
import Basics
1112
import Dispatch
1213
import PackageModel
1314
import TSCBasic
@@ -43,6 +44,7 @@ public protocol DependencyResolverDelegate {
4344
func solved(result: [DependencyResolver.Binding])
4445
}
4546

47+
@available(*, deprecated, message: "user verbosity flags instead")
4648
public struct TracingDependencyResolverDelegate: DependencyResolverDelegate {
4749
private let stream: OutputByteStream
4850

@@ -99,6 +101,53 @@ public struct TracingDependencyResolverDelegate: DependencyResolverDelegate {
99101
}
100102
}
101103

104+
public struct ObservabilityDependencyResolverDelegate: DependencyResolverDelegate {
105+
private let observabilityScope: ObservabilityScope
106+
107+
public init (observabilityScope: ObservabilityScope) {
108+
self.observabilityScope = observabilityScope.makeChildScope(description: "DependencyResolver")
109+
}
110+
111+
public func willResolve(term: Term) {
112+
self.debug("resolving '\(term.node.package.identity)'")
113+
}
114+
115+
public func didResolve(term: Term, version: Version, duration: DispatchTimeInterval) {
116+
self.debug("resolved '\(term.node.package.identity)' @ '\(version)'")
117+
}
118+
119+
public func derived(term: Term) {
120+
self.debug("derived '\(term.node.package.identity)' requirement '\(term.requirement)'")
121+
}
122+
123+
public func conflict(conflict: Incompatibility) {
124+
self.debug("conflict: \(conflict)")
125+
}
126+
127+
public func failedToResolve(incompatibility: Incompatibility) {
128+
self.debug("failed to resolve '\(incompatibility)'")
129+
}
130+
131+
public func satisfied(term: Term, by assignment: Assignment, incompatibility: Incompatibility) {
132+
self.debug("'\(term)' is satisfied by '\(assignment)', which is caused by '\(assignment.cause?.description ?? "unknown cause")'. new incompatibility: '\(incompatibility)'")
133+
}
134+
135+
public func partiallySatisfied(term: Term, by assignment: Assignment, incompatibility: Incompatibility, difference: Term) {
136+
self.debug("\(term) is partially satisfied by '\(assignment)', which is caused by '\(assignment.cause?.description ?? "unknown cause")'. new incompatibility \(incompatibility)")
137+
}
138+
139+
public func solved(result: [DependencyResolver.Binding]) {
140+
for (package, binding, _) in result {
141+
self.debug("solved '\(package.identity)' (\(package.locationString)) at '\(binding)'")
142+
}
143+
self.debug("dependency resolution complete!")
144+
}
145+
146+
private func debug(_ message: String) {
147+
self.observabilityScope.emit(debug: "[DependencyResolver] \(message)")
148+
}
149+
}
150+
102151
public struct MultiplexResolverDelegate: DependencyResolverDelegate {
103152
private let underlying: [DependencyResolverDelegate]
104153

‎Sources/SPMTestSupport/Observability.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,21 @@ public struct TestingObservability {
7777
}
7878
}
7979

80-
81-
public func XCTAssertNoDiagnostics(_ diagnostics: [Basics.Diagnostic], file: StaticString = #file, line: UInt = #line) {
80+
public func XCTAssertNoDiagnostics(_ diagnostics: [Basics.Diagnostic], problemsOnly: Bool = true, file: StaticString = #file, line: UInt = #line) {
81+
let diagnostics = problemsOnly ? diagnostics.filter({ $0.severity >= .warning }) : diagnostics
8282
if diagnostics.isEmpty { return }
8383
let description = diagnostics.map({ "- " + $0.description }).joined(separator: "\n")
8484
XCTFail("Found unexpected diagnostics: \n\(description)", file: file, line: line)
8585
}
8686

8787
public func testDiagnostics(
8888
_ diagnostics: [Basics.Diagnostic],
89+
problemsOnly: Bool = true,
8990
file: StaticString = #file,
9091
line: UInt = #line,
9192
handler: (DiagnosticsTestResult) throws -> Void
9293
) {
94+
let diagnostics = problemsOnly ? diagnostics.filter({ $0.severity >= .warning }) : diagnostics
9395
let testResult = DiagnosticsTestResult(diagnostics)
9496

9597
do {

‎Sources/Workspace/Workspace.swift

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -225,9 +225,6 @@ public class Workspace {
225225
/// Update containers while fetching them.
226226
fileprivate let resolverUpdateEnabled: Bool
227227

228-
/// Write dependency resolver trace to a file.
229-
fileprivate let resolverTracingEnabled: Bool
230-
231228
fileprivate let additionalFileRules: [FileRuleDescription]
232229

233230
// state
@@ -262,7 +259,6 @@ public class Workspace {
262259
/// - additionalFileRules: File rules to determine resource handling behavior.
263260
/// - resolverUpdateEnabled: Enables the dependencies resolver automatic version update check. Enabled by default. When disabled the resolver relies only on the resolved version file
264261
/// - resolverPrefetchingEnabled: Enables the dependencies resolver prefetching based on the resolved version file. Enabled by default..
265-
/// - resolverTracingEnabled: Enables the dependencies resolver tracing. Disabled by default..
266262
/// - sharedRepositoriesCacheEnabled: Enables the shared repository cache. Enabled by default..
267263
/// - delegate: Delegate for workspace events
268264
public init(
@@ -283,7 +279,6 @@ public class Workspace {
283279
additionalFileRules: [FileRuleDescription]? = .none,
284280
resolverUpdateEnabled: Bool? = .none,
285281
resolverPrefetchingEnabled: Bool? = .none,
286-
resolverTracingEnabled: Bool? = .none,
287282
sharedRepositoriesCacheEnabled: Bool? = .none,
288283
delegate: WorkspaceDelegate? = .none
289284
) throws {
@@ -325,7 +320,6 @@ public class Workspace {
325320
let additionalFileRules = additionalFileRules ?? []
326321
let resolverUpdateEnabled = resolverUpdateEnabled ?? true
327322
let resolverPrefetchingEnabled = resolverPrefetchingEnabled ?? false
328-
let resolverTracingEnabled = resolverTracingEnabled ?? false
329323

330324
// initialize
331325
self.fileSystem = fileSystem
@@ -355,7 +349,6 @@ public class Workspace {
355349
self.additionalFileRules = additionalFileRules
356350
self.resolverUpdateEnabled = resolverUpdateEnabled
357351
self.resolverPrefetchingEnabled = resolverPrefetchingEnabled
358-
self.resolverTracingEnabled = resolverTracingEnabled
359352

360353
self.state = WorkspaceState(dataPath: self.location.workingDirectory, fileSystem: fileSystem)
361354
}
@@ -412,8 +405,7 @@ public class Workspace {
412405
customChecksumAlgorithm: checksumAlgorithm,
413406
additionalFileRules: additionalFileRules,
414407
resolverUpdateEnabled: skipUpdate.map{ !$0 },
415-
resolverPrefetchingEnabled: isResolverPrefetchingEnabled,
416-
resolverTracingEnabled: enableResolverTrace
408+
resolverPrefetchingEnabled: isResolverPrefetchingEnabled
417409
)
418410
if let toolsVersionLoader = toolsVersionLoader {
419411
self.toolsVersionLoader = toolsVersionLoader
@@ -748,7 +740,7 @@ extension Workspace {
748740
}
749741

750742
// Resolve the dependencies.
751-
let resolver = try self.createResolver(pinsMap: pinsMap)
743+
let resolver = self.createResolver(pinsMap: pinsMap, observabilityScope: ObservabilitySystem(diagnosticEngine: diagnostics).topScope)
752744
self.activeResolver = resolver
753745

754746
let updateResults = resolveDependencies(
@@ -2388,7 +2380,7 @@ extension Workspace {
23882380
computedConstraints += try graphRoot.constraints() + constraints
23892381

23902382
// Perform dependency resolution.
2391-
let resolver = try createResolver(pinsMap: pinsStore.pinsMap)
2383+
let resolver = self.createResolver(pinsMap: pinsStore.pinsMap, observabilityScope: ObservabilitySystem(diagnosticEngine: diagnostics).topScope)
23922384
self.activeResolver = resolver
23932385

23942386
let result = self.resolveDependencies(
@@ -2855,15 +2847,17 @@ extension Workspace {
28552847
}
28562848

28572849
/// Creates resolver for the workspace.
2858-
fileprivate func createResolver(pinsMap: PinsStore.PinsMap) throws -> PubgrubDependencyResolver {
2859-
var delegates = [DependencyResolverDelegate]()
2850+
fileprivate func createResolver(pinsMap: PinsStore.PinsMap, observabilityScope: ObservabilityScope) -> PubgrubDependencyResolver {
2851+
var delegate: DependencyResolverDelegate
2852+
let observabilityDelegate = ObservabilityDependencyResolverDelegate(observabilityScope: observabilityScope)
28602853
if let workspaceDelegate = self.delegate {
2861-
delegates.append(WorkspaceDependencyResolverDelegate(workspaceDelegate))
2862-
}
2863-
if self.resolverTracingEnabled {
2864-
delegates.append(try TracingDependencyResolverDelegate(path: self.location.workingDirectory.appending(components: "resolver.trace")))
2854+
delegate = MultiplexResolverDelegate([
2855+
observabilityDelegate,
2856+
WorkspaceDependencyResolverDelegate(workspaceDelegate),
2857+
])
2858+
} else {
2859+
delegate = observabilityDelegate
28652860
}
2866-
let delegate = !delegates.isEmpty ? MultiplexResolverDelegate(delegates) : nil
28672861

28682862
return PubgrubDependencyResolver(
28692863
provider: self,

‎Tests/BasicsTests/ObservabilitySystemTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ final class ObservabilitySystemTest: XCTestCase {
108108
emitter.emit(debug: "debug")
109109
emitter.emit(.debug("debug 2"))
110110

111-
testDiagnostics(collector.diagnostics) { result in
111+
testDiagnostics(collector.diagnostics, problemsOnly: false) { result in
112112
result.check(diagnostic: "error", severity: .error, metadata: metadata)
113113
result.check(diagnostic: "error 2", severity: .error, metadata: metadata)
114114
result.check(diagnostic: "error 3", severity: .error, metadata: metadata)

‎Tests/PackageGraphTests/PubgrubTests.swift

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,14 @@
88
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
99
*/
1010

11-
import XCTest
12-
13-
import TSCBasic
11+
import Basics
12+
@testable import PackageGraph
1413
import PackageLoading
1514
@testable import PackageModel
16-
@testable import PackageGraph
1715
import SourceControl
1816
import SPMTestSupport
19-
20-
17+
import TSCBasic
18+
import XCTest
2119

2220
// There's some useful helper utilities defined below for easier testing:
2321
//
@@ -1898,6 +1896,44 @@ final class PubGrubBacktrackTests: XCTestCase {
18981896
("d", .version("2.0.0")),
18991897
])
19001898
}
1899+
1900+
func testLogging() {
1901+
builder.serve("a", at: "1.0.0")
1902+
builder.serve("a", at: "2.0.0")
1903+
builder.serve("b", at: "1.0.1", with: [
1904+
"b": ["a": (.versionSet(.exact("1.0.0")), .specific(["a"]))],
1905+
])
1906+
builder.serve("c", at: "1.5.2", with: [
1907+
"c": ["b": (.versionSet(.range("0.0.0"..<"3.0.0")), .specific(["b"]))],
1908+
])
1909+
builder.serve("d", at: "1.0.1")
1910+
builder.serve("d", at: "2.3.0")
1911+
1912+
let observability = ObservabilitySystem.makeForTesting()
1913+
1914+
let resolver = builder.create(pinsMap: [:], delegate: ObservabilityDependencyResolverDelegate(observabilityScope: observability.topScope))
1915+
let dependencies = builder.create(dependencies: [
1916+
"a": (.versionSet(.range("1.0.0"..<"4.0.0")), .specific(["a"])),
1917+
"c": (.versionSet(.range("1.0.0"..<"4.0.0")), .specific(["c"])),
1918+
"d": (.versionSet(.range("1.0.0"..<"4.0.0")), .specific(["d"])),
1919+
])
1920+
1921+
let result = resolver.solve(constraints: dependencies)
1922+
1923+
AssertResult(result, [
1924+
("a", .version("1.0.0")),
1925+
("b", .version("1.0.1")),
1926+
("c", .version("1.5.2")),
1927+
("d", .version("2.3.0")),
1928+
])
1929+
1930+
observability.diagnostics.forEach { print("\($0)") }
1931+
1932+
XCTAssertTrue(observability.diagnostics.contains(where: { $0.message == "[DependencyResolver] resolved 'a' @ '1.0.0'" }))
1933+
XCTAssertTrue(observability.diagnostics.contains(where: { $0.message == "[DependencyResolver] resolved 'b' @ '1.0.1'" }))
1934+
XCTAssertTrue(observability.diagnostics.contains(where: { $0.message == "[DependencyResolver] resolved 'c' @ '1.5.2'" }))
1935+
XCTAssertTrue(observability.diagnostics.contains(where: { $0.message == "[DependencyResolver] resolved 'd' @ '2.3.0'" }))
1936+
}
19011937
}
19021938

19031939
fileprivate extension CheckoutState {
@@ -2212,12 +2248,8 @@ class DependencyGraphBuilder {
22122248
return store
22132249
}
22142250

2215-
func create(pinsMap: PinsStore.PinsMap = [:], log: Bool = false) -> PubgrubDependencyResolver {
2216-
let delegate = log ? TracingDependencyResolverDelegate(stream: TSCBasic.stdoutStream) : nil
2217-
return self.create(pinsMap: pinsMap, delegate: delegate)
2218-
}
22192251

2220-
func create(pinsMap: PinsStore.PinsMap = [:], delegate: DependencyResolverDelegate?) -> PubgrubDependencyResolver {
2252+
func create(pinsMap: PinsStore.PinsMap = [:], delegate: DependencyResolverDelegate? = .none) -> PubgrubDependencyResolver {
22212253
defer {
22222254
self.containers = [:]
22232255
self.references = [:]

‎Tests/PackageLoadingTests/TargetSourcesBuilderTests.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ class TargetSourcesBuilderTests: XCTestCase {
278278
"/Resources/Sub/foo.txt"
279279
)
280280

281-
build(target: target, toolsVersion: .v5_3, fs: fs) { _, _, _, _, identity, location, path, diagnostics in
281+
build(target: target, toolsVersion: .v5_3, fs: fs, checkOnlyProblems: false) { _, _, _, _, identity, location, path, diagnostics in
282282
var expectedMetadata = ObservabilityMetadata.packageMetadata(identity: identity, location: location, path: path)
283283
expectedMetadata.targetName = target.name
284284
diagnostics.check(diagnostic: "multiple resources named 'foo.txt' in target 'Foo'", severity: .error, metadata: expectedMetadata)
@@ -300,7 +300,7 @@ class TargetSourcesBuilderTests: XCTestCase {
300300
"/Copied/foo.txt"
301301
)
302302

303-
build(target: target, toolsVersion: .v5_3, fs: fs) { _, _, _, _, identity, location, path, diagnostics in
303+
build(target: target, toolsVersion: .v5_3, fs: fs, checkOnlyProblems: false) { _, _, _, _, identity, location, path, diagnostics in
304304
var expectedMetadata = ObservabilityMetadata.packageMetadata(identity: identity, location: location, path: path)
305305
expectedMetadata.targetName = target.name
306306
diagnostics.check(diagnostic: "multiple resources named 'foo.txt' in target 'Foo'", severity: .error, metadata: expectedMetadata)
@@ -623,6 +623,7 @@ class TargetSourcesBuilderTests: XCTestCase {
623623
additionalFileRules: [FileRuleDescription] = [],
624624
toolsVersion: ToolsVersion,
625625
fs: FileSystem,
626+
checkOnlyProblems: Bool = false,
626627
file: StaticString = #file,
627628
line: UInt = #line,
628629
checker: (Sources, [Resource], [AbsolutePath], [AbsolutePath], PackageIdentity, String, AbsolutePath, DiagnosticsTestResult) -> ()
@@ -644,7 +645,7 @@ class TargetSourcesBuilderTests: XCTestCase {
644645
do {
645646
let (sources, resources, headers, others) = try builder.run()
646647

647-
testDiagnostics(observability.diagnostics, file: file, line: line) { diagnostics in
648+
testDiagnostics(observability.diagnostics, problemsOnly: checkOnlyProblems, file: file, line: line) { diagnostics in
648649
checker(sources, resources, headers, others, builder.packageIdentity, builder.packageLocation, builder.packagePath, diagnostics)
649650
}
650651
} catch {

‎Tests/WorkspaceTests/WorkspaceTests.swift

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4410,7 +4410,7 @@ final class WorkspaceTests: XCTestCase {
44104410
XCTAssertFalse(fs.isDirectory(AbsolutePath("/tmp/ws/.build/artifacts/B/B.xcframework")))
44114411

44124412
try workspace.checkPackageGraph(roots: ["Foo"]) { graph, diagnostics in
4413-
XCTAssertTrue(diagnostics.isEmpty, diagnostics.description)
4413+
XCTAssertNoDiagnostics(diagnostics)
44144414

44154415
// Ensure that the artifacts have been properly extracted
44164416
XCTAssertTrue(fs.isDirectory(AbsolutePath("/tmp/ws/.build/artifacts/A/A1.xcframework")))
@@ -4665,7 +4665,7 @@ final class WorkspaceTests: XCTestCase {
46654665
try fs.createDirectory(workspace.artifactsDir.appending(components: "A", a4FrameworkName, "local-archived"), recursive: true)
46664666

46674667
try workspace.checkPackageGraph(roots: ["Foo"]) { graph, diagnostics in
4668-
XCTAssertTrue(diagnostics.isEmpty, diagnostics.description)
4668+
XCTAssertNoDiagnostics(diagnostics)
46694669

46704670
// Ensure that the original archives have been untouched
46714671
XCTAssertTrue(fs.exists(a1FrameworkArchivePath))
@@ -4773,9 +4773,7 @@ final class WorkspaceTests: XCTestCase {
47734773
try fs.writeFileContents(aFrameworksPath.appending(component: "archived-artifact-does-not-match-target-name.zip"), bytes: ByteString([0xA1]))
47744774

47754775
workspace.checkPackageGraphFailure(roots: ["Foo"]) { diagnostics in
4776-
testDiagnostics(diagnostics) { result in
4777-
XCTAssertTrue(diagnostics.isEmpty, diagnostics.description)
4778-
}
4776+
XCTAssertNoDiagnostics(diagnostics)
47794777
}
47804778
}
47814779

@@ -5514,8 +5512,9 @@ final class WorkspaceTests: XCTestCase {
55145512
)
55155513

55165514
workspace.checkPackageGraphFailure(roots: ["Foo"]) { diagnostics in
5517-
XCTAssertEqual(diagnostics.map { $0.message },
5518-
["downloaded archive of binary target 'A3' does not contain expected binary artifact 'A3'"])
5515+
testDiagnostics(diagnostics) { result in
5516+
result.check(diagnostic: "downloaded archive of binary target 'A3' does not contain expected binary artifact 'A3'", severity: .error)
5517+
}
55195518
XCTAssert(fs.isDirectory(AbsolutePath("/tmp/ws/.build/artifacts/B")))
55205519
XCTAssert(!fs.exists(AbsolutePath("/tmp/ws/.build/artifacts/A/A3.xcframework")))
55215520
XCTAssert(!fs.exists(AbsolutePath("/tmp/ws/.build/artifacts/A/A4.xcframework")))

0 commit comments

Comments
 (0)