feat(add): improve cargo add X@latest error message#16406
Conversation
0xPoe
changed the title
cargo add X@latestcargo add X@latest
0xPoe
force-pushed
the
poe-add-latest
branch
from
20221de to
c913c6d
Compare
0xPoe
force-pushed
the
poe-add-latest
branch
2 times, most recently
from
d741881 to
72ea89c
Compare
0xPoe
changed the title
cargo add X@latestcargo add X@latest
0xPoe
force-pushed
the
poe-add-latest
branch
5 times, most recently
from
8f6212b to
2c8d357
Compare
0xPoe
force-pushed
the
poe-add-latest
branch
3 times, most recently
from
2f63114 to
b80ebde
Compare
0xPoe
changed the title
cargo add X@latestcargo add X@latest error message
0xPoe
changed the title
cargo add X@latest error messagecargo add X@latest error message
Signed-off-by: 0xPoe <techregister@pm.me>
Signed-off-by: 0xPoe <techregister@pm.me>
There was a problem hiding this comment.
🔢 Self-check (PR reviewed by myself and ready for feedback)
-
Code compiles successfully
-
Unit tests added
-
No AI-generated elegant nonsense in PR.
-
Comments added where necessary
-
PR title and description updated
-
Documentation PR created (or confirmed not needed)
-
PR size is reasonable
Some changes I would like to discuss with you first. What are your thoughts before I implement them?
r? @epage
rustbot
added
the
S-waiting-on-review
0xPoe
changed the title
cargo add X@latest error messagecargo add X@latest error message
0xPoe
force-pushed
the
poe-add-latest
branch
3 times, most recently
from
d963531 to
bba51a3
Compare
rustbot
removed
the
S-waiting-on-review
0xPoe
force-pushed
the
poe-add-latest
branch
4 times, most recently
from
e7fc3dc to
21d9cba
Compare
| let request_latest = crate_spec | ||
| .as_ref() | ||
| .is_some_and(|crate_spec| matches!(crate_spec.version(), Some(VersionSpec::Latest))); |
There was a problem hiding this comment.
how come you pulled this out up here?
There was a problem hiding this comment.
It is used in the following if-else statement.
| // Get the exact version that `cargo add <name>` would resolve to, | ||
| // respecting MSRV and existing version constraints. |
There was a problem hiding this comment.
What other constraints are there?
There was a problem hiding this comment.
I believe it includes:
- any existing version requirements
- the selected registry or source
- registry resolution behavior (path, yanked filtering, prefer stable version)
Basically, it's just what we would select by running the command without the latest version selector.
| // Get the exact version that `cargo add <name>` would resolve to, | ||
| // respecting MSRV and existing version constraints. |
There was a problem hiding this comment.
Why are we reporting the MSRV-compatible version?
There was a problem hiding this comment.
From #10741 (comment)
$ cargo add clap@latest
error: invalid version requirement `latest`
help: to use X, run `cargo add clap`
help: to use the latest version, run `cargo add clap@4.5.38`I thought we usually selected X by running cargo add clap?
rustbot
added
the
S-waiting-on-review
| let resolved_version = resolved | ||
| .version() | ||
| .expect("resolved dependency should have version"); | ||
| // Get the actual latest non-prerelease, non-yanked version from the registry, |
There was a problem hiding this comment.
I think this comment is not entirely accurate. I discovered that the behavior of get_latest_dependency is to prefer the stable version. Only if there is no stable version available will it select the pre-release versions.
@epage Form you comment: #10741 (comment)
Yes, I was suggesting we suggest the latest version as that would be the literal meaning of latest and would therefore match the users intent. The user could have done this themselves. We likely should skip pre-release and yanked as we generally don't consider them the latest.
It may also make sense to inherit the same behavior: if there is no stable release, we suggest the pre-release version here?
0xPoe
force-pushed
the
poe-add-latest
branch
3 times, most recently
from
184fb4e to
9ec528b
Compare
Signed-off-by: 0xPoe <techregister@pm.me>
3 participants
View all comments
What does this PR try to resolve?
refs #10741
This PR improves the error message for
cargo add <name>@latest.cargo add <name>@<latest>for the latest published version, excluding pre-releases and yanked versionsOriginal proposal: #10741 (comment)
How to test and review this PR?
Review it commit by commit.