Try to improve "version not found" error #6112
Conversation
|
r? @matklad (rust_highfive has picked a reviewer for you, use r? to override) |
| required by package `bar v0.0.1` | ||
| ... which is depended on by `foo [..]` |
There was a problem hiding this comment.
Why did this change? I am not objecting, just mechanically, what changed making us print which is depended on by?
There was a problem hiding this comment.
Oh I just expanded the error message a bit here, only a subset of the output was previously tested with with_stderr_contains (vs with_stderr)
| } else { | ||
| 1024 // but locally try and find it in the one build | ||
| }, | ||
| cases: 256, |
There was a problem hiding this comment.
Why did you choose to change this?
I assume it was running to slowly locally. If that is correct can we atleast add back in a comment on how running repeatedly or increasing this number will increase the chance of finding a bug at the expense of time?
There was a problem hiding this comment.
Ah yeah I found this was taking ~4m or so on my local machine which made a full test run sort of unbearable unfortunately :(. And sure I'll add a comment!
There was a problem hiding this comment.
That makes sense. It is new and we are still experimenting with how it fits into the workflow. I am ok with running fewer iterations, for the better ergonomics of running the test.
| @@ -683,3 +683,44 @@ fn workspace_different_locations() { | |||
| ", | |||
| ).run(); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn version_missing() { | |||
There was a problem hiding this comment.
In general the error messages are not nearly adequately tested. I'd like one day to be sure we have all the code branches tested. Thank you for adding this test!
src/cargo/core/resolver/mod.rs
Outdated
| location searched: {}\n\ | ||
| versions found: {}\n", | ||
| dep.version_req(), | ||
| "failed to select a version for `{}` satisfying the requirement `{}`\n \ |
There was a problem hiding this comment.
I found this sentence structure convoluted the first time I looked at this PR, but it has grown on me. I thought about putting the concepts in the other order, but that leads to the existing wording which many people report as confusing.
A more radical thought is to use the cargo.toml syntax:
failed to select a version satisfying the requirement 'foo = "2"'
But I do not know if that is a good thought.
There was a problem hiding this comment.
Oh I like that actually, everyone's quite familiar with it!
This commit tweaks the wording for when versions aren't found in a source, notably including more location information other than the source id but rather also taking into account replaced sources and such. It's hoped that this gives more information to make it a bit more clear what's going on. Closes rust-lang#6111
alexcrichton
force-pushed
the
better-errors
branch
from
fc34ba1 to
20cfb41
Compare
|
Ok updated! |
|
Lgtm. cc the last discussion of the error messages in #5452, although I can't quite map how our thoughts from then apply now. |
|
Ok thanks! I'm all for any improvements to our error messages, so I'd be ok to review that again :) @bors: r=Eh2406 |
|
📌 Commit 20cfb41 has been approved by |
|
⌛ Testing commit 20cfb41 with merge 2f0b0d4ccb30435ac6872caa8e7e57c874d23ce3... |
|
💔 Test failed - status-travis |
|
@bors: retry |
Try to improve "version not found" error This commit tweaks the wording for when versions aren't found in a source, notably including more location information other than the source id but rather also taking into account replaced sources and such. It's hoped that this gives more information to make it a bit more clear what's going on. Closes #6111
|
☀️ Test successful - status-appveyor, status-travis |
This commit tweaks the wording for when versions aren't found in a
source, notably including more location information other than the
source id but rather also taking into account replaced sources and such.
It's hoped that this gives more information to make it a bit more clear
what's going on.
Closes #6111