improve dependency resolution diagnostics when resolving non-versioned dependencies #4030
Conversation
|
@swift-ci please smoke test |
tomerd
added
the
ready
|
ci infra issues causing build timeout? |
|
@swift-ci please smoke test |
tomerd
added
WIP
tomerd
changed the title
|
@swift-ci please smoke test |
tomerd
changed the title
tomerd
added
ready
|
@swift-ci please test package compatibility |
| // FIXME: This is duplicated and wrong. | ||
| private func isFailure(_ incompatibility: Incompatibility) -> Bool { | ||
| return incompatibility.terms.count == 1 && incompatibility.terms.first?.node.package.identity == .plain("<synthesized-root>") | ||
| } |
|
not able to reproduce the package compatibility failures reported, could be from a different cause |
|
package compatibility failures: swiftlang/swift#40877 |
|
@swift-ci please test package compatibility |
| identity: .plain("<synthesized-root>"), | ||
| path: .root | ||
| )) | ||
| // 👀 is this the right thing to do? |
There was a problem hiding this comment.
This seems like an improvement for now, especially for CLI. For IDEs such as Xcode, where there can be multiple roots and also package dependencies that don't come from packages, I think we'll want to add more information to PackageGraphRootInput that can be used in diagnostics. That should be easy for the IDE to provide and would let you know where the "root pointers" into the package graph are (either a set of root packages or some other external dependency on a package, such as from an iOS app Xcode target to a package). That could of course be done in a separate PR.
There was a problem hiding this comment.
also discussed with @neonichu and we agreed this is an improvement
| // Some of these might be overridden as we discover local and branch-based references. | ||
| var versionBasedDependencies: [DependencyResolutionNode: [VersionBasedConstraint]] = [:] | ||
| var versionBasedDependencies = OrderedDictionary<DependencyResolutionNode, [VersionBasedConstraint]>() |
There was a problem hiding this comment.
I think switching to ordered collections is great where it makes sense — it will often help understandability if we can make the order of diagnostics stable from run to run, and also, when there's no better order, to show entities in the same order that users specify them (not always possible of course, but sometimes it is).
…d depedendencies motivation: when resolving graphs with non-versioned depedendencies (such as branch dependencies), the dependencies get "flattened" onto the root node which looses the original association with the dependency's parent yielding incorrect diagnostics changes: * more accuretly track the underlying dependency when computing root dependencies from non-versioned depedendencie * peek into the "real" cause when producing depedency based diagnostics for the root node * add a bunch of tests rdar://87486528
tomerd
force-pushed
the
radar/87486528
branch
from
07e161d to
e26a79e
Compare
|
@swift-ci please smoke test |
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
error handling cleanup ^^
motivation: when resolving graphs with non-versioned depedendencies (such as branch dependencies), the dependencies get "flattened" onto the root node which looses the original association with the dependency's parent yielding incorrect diagnostics
changes:
rdar://87486528
rdar://64226666