Provide extra context on a query failure. #7934
Conversation
|
r? @Eh2406 (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
This is just an experiment, and I wanted to get feedback if anyone thinks it looks like a good idea. I'm also open to changing the wording. |
|
seems reasonable. |
|
It sounds like the goal here is to sort of figure out why a package was included, right? If so I think 1 package as a parent helps a lot, but perhaps we could go all the way and do the |
|
A full path might be nice, but I don't know how to do that at this location. I assume you're referring to |
|
Yes, I think something like describe_path(
&cx.parents.path_to_bottom(&parent.package_id()),
)Will make a message that matches our other errors. Looks like we can pass Or perhaps that can be added as part of a cc #6199 |
|
I think that if the error context was shifted to around here it may be easier to call that |
|
Hm, I pushed a change that moves the context up so that it can get the path. Unfortunately I'm not really sure it's worth it, it seems to complicate things a little. What do you think? |
|
Hmmm, I was more thinking of the smaller change like master...Eh2406:pull/7934#diff-995a0a0a73b467f7c6b0cb34d8fba39d (I did not fix the tests.) |
That looks good. (The link is broken, but I think this link works: Eh2406@d3f712a) I'll try it out tomorrow. |
ehuss
force-pushed
the
query-error-context
branch
from
4e8091e
to
1eca786
Compare
|
Thanks, updated with @Eh2406 suggestion, it looks much better. |
|
Looks great! Thanks for adding the new test! |
|
📌 Commit 1eca786 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
Update cargo, clippy Closes #69601 ## cargo 16 commits in e57bd02999c9f40d52116e0beca7d1dccb0643de..bda50510d1daf6e9c53ad6ccf603da6e0fa8103f 2020-02-21 20:20:10 +0000 to 2020-03-02 18:05:34 +0000 - Fix rare failure in collision_export test. (rust-lang/cargo#7956) - Ignore broken Cargo.toml in git sources (rust-lang/cargo#7947) - Add more fingerprint mtime debug logging. (rust-lang/cargo#7952) - Fix plugin tests for latest nightly. (rust-lang/cargo#7955) - Simplified usage code of SipHasher (rust-lang/cargo#7945) - Add a special case for git config discovery inside tests (rust-lang/cargo#7944) - Fixes issue rust-lang/cargo#7543 (rust-lang/cargo#7946) - Filter out cfgs which should not be used during build (rust-lang/cargo#7943) - Provide extra context on a query failure. (rust-lang/cargo#7934) - Try to clarify `cargo metadata`'s relationship with the workspace. (rust-lang/cargo#7927) - Update libgit2 dependency (rust-lang/cargo#7939) - Fix link in comment (rust-lang/cargo#7936) - Enable `cargo doc --open` tests on macos. (rust-lang/cargo#7932) - build-std: remove sysroot probe (rust-lang/cargo#7931) - Try to clarify how feature flags work on the "current" package. (rust-lang/cargo#7928) - Add extra details in the new feature resolver doc comment. (rust-lang/cargo#7918) ## clippy 6 commits in fc5d0cc..8b7f7e6 2020-02-24 05:58:17 +0000 to 2020-03-02 20:00:31 +0000 - Rustup to #69469 (rust-lang/rust-clippy#5254) - Some rustups (rust-lang/rust-clippy#5247) - Update git2 to 0.12 (rust-lang/rust-clippy#5232) - Rustup to #61812 (rust-lang/rust-clippy#5231) - Add lint to improve floating-point expressions (rust-lang/rust-clippy#4897) - Do not run deploy action on other repos (rust-lang/rust-clippy#5222)
This adds error context when a query fails, primarily to tell you which parent package included the dependency that failed. For example, imagine deep within your dependency graph you have a
gitdependency, and it fails to download. The current error doesn't tell you where in the graph thatgitdependency was included.I also slightly tweaked the
failed to load sourceerror message. I felt like the existing wording could be misinterpreted that it was an error loading the dependency for the given package. I felt like there were multiple ways to interpret it, so I tried to simplify it to avoid that possibility.