Show full derivation chain when encountering build failures #9108
Conversation
charliermarsh
force-pushed
the
charlie/chain
branch
7 times, most recently
from
41f35e9
to
651a074
Compare
charliermarsh
force-pushed
the
charlie/chain
branch
from
651a074
to
4561df7
Compare
charliermarsh
force-pushed
the
charlie/chain
branch
3 times, most recently
from
debb749
to
cf667fc
Compare
There was a problem hiding this comment.
This looks great; a review from @BurntSushi seems prudent since I'm not as familiar with the resolver.
| pub enum DistRef<'a> { | ||
| Built(&'a BuiltDist), | ||
| Source(&'a SourceDist), | ||
| } |
There was a problem hiding this comment.
Yay ref types.
I don't know if you saw it in my conflicting groups PR, but a useful tip if you ever need to use one of these types for key lookups in hashmaps is to use hashbrown and its Equivalent trait.
| pub fn version(&self) -> Option<&Version> { | ||
| match self { | ||
| Self::Built(wheel) => Some(wheel.version()), | ||
| Self::Source(source_dist) => source_dist.version(), | ||
| } | ||
| } | ||
|
|
||
| /// Convert this distribution into a reference. | ||
| pub fn as_ref(&self) -> DistRef { |
There was a problem hiding this comment.
Oh interesting, I thought Clippy didn't like omitting lifetimes like this. e.g., DistRef<'_>.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
These new helper constructors are a nice refactor!
| let mut queue = VecDeque::new(); | ||
| queue.push_back((target, Vec::new())); | ||
|
|
||
| // TODO(charlie): Consider respecting markers here. |
There was a problem hiding this comment.
Without markers, I think this means that you might find the shortest path, but that path might not be possible? Or is there another downside I'm not seeing?
| } | ||
| } | ||
| false | ||
| } |
| "###); | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Maybe I missed it, but do you have a test for the case where the failed dependency is not a direct dependency of the root? I was looking for that to see what the error message looks like.
There was a problem hiding this comment.
I also was hoping for a longer chain :)
There was a problem hiding this comment.
It's so annoying to test, but yeah, I definitely need to do it.
charliermarsh
force-pushed
the
charlie/graph
branch
from
18317fb
to
6efd07c
Compare
charliermarsh
force-pushed
the
charlie/chain
branch
from
cf667fc
to
3e30713
Compare
charliermarsh
force-pushed
the
charlie/graph
branch
2 times, most recently
from
93678b1
to
30691c9
Compare
More Simplify Starting to come together Reverts Trying to get the lock info from sync Rewrite resolution into a graph Add version to ResolvedDist Fix up graphs Attach to install everywhere Attach to install everywhere
charliermarsh
force-pushed
the
charlie/chain
branch
from
3e30713
to
7bb60fa
Compare
Summary
This PR adds context to our error messages to explain why a given package was included, if we fail to download or build it.
It's quite a large change, but it motivated some good refactors and improvements along the way.
Closes #8962.