Implement semver ranges for install --vers #4229
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matklad (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
src/cargo/ops/cargo_install.rs
Outdated
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Looks like a couple of accidentally added blank lines :)
There was a problem hiding this comment.
Oops, yes! I will correct that right away :)
|
Looks great, @azerupi ! Hope we don't forget to remove the fallback after a couple of releases :) |
|
Looks great, and there are already @bors r+ |
|
📌 Commit d9afb2d has been approved by |
Implement semver ranges for install --vers This implements the design discussed in #4207 It allows to specify semver ranges on `cargo install ... --vers` 1. The first character of the `--vers` value is checked, if there is none we return an error. 2. If it is one of `<`, `>`, `=`, `^`, `~` we parse the value as a `VersionReq`, otherwise we parse it as a `Version`. 3. If the parsing as a `Version` fails but parsing as `VersionReq` succeeds, we add a note to the warning mentioning that a qualifier should be used to specify a semver range. This catches versions with less than tree digits. Otherwise, the previous behaviour is preserved with the warning of backwards compatibility. This means that - `cargo install ... --vers "^1.2.3"` will be parsed as a range - `cargo install ... --vers 1.2.3` will be parsed as a version - `cargo install ... --vers 1.2` will be parsed as a version for backwards compatibility reasons, fail and be passed through as is,**but** we will add a note `if you want to specify semver range, add an explicit qualifier, like ^1.2` - `cargo install ... --vers blah` will be parsed as a version for backwards compatibility reasons (which is weird because it is not even a valid semver range) and produce an `unknown error` down the line. I have left this behaviour untouched because it worked like that before, but I can easily make it error sooner with a better message.
|
💥 Test timed out |
|
@bors retry |
Implement semver ranges for install --vers This implements the design discussed in #4207 It allows to specify semver ranges on `cargo install ... --vers` 1. The first character of the `--vers` value is checked, if there is none we return an error. 2. If it is one of `<`, `>`, `=`, `^`, `~` we parse the value as a `VersionReq`, otherwise we parse it as a `Version`. 3. If the parsing as a `Version` fails but parsing as `VersionReq` succeeds, we add a note to the warning mentioning that a qualifier should be used to specify a semver range. This catches versions with less than tree digits. Otherwise, the previous behaviour is preserved with the warning of backwards compatibility. This means that - `cargo install ... --vers "^1.2.3"` will be parsed as a range - `cargo install ... --vers 1.2.3` will be parsed as a version - `cargo install ... --vers 1.2` will be parsed as a version for backwards compatibility reasons, fail and be passed through as is,**but** we will add a note `if you want to specify semver range, add an explicit qualifier, like ^1.2` - `cargo install ... --vers blah` will be parsed as a version for backwards compatibility reasons (which is weird because it is not even a valid semver range) and produce an `unknown error` down the line. I have left this behaviour untouched because it worked like that before, but I can easily make it error sooner with a better message.
|
☀️ Test successful - status-appveyor, status-travis |
This implements the design discussed in #4207
It allows to specify semver ranges on
cargo install ... --vers--versvalue is checked, if there is none we return an error.<,>,=,^,~we parse the value as aVersionReq, otherwise we parse it as aVersion.Versionfails but parsing asVersionReqsucceeds, we add a note to the warning mentioning that a qualifier should be used to specify a semver range. This catches versions with less than tree digits.Otherwise, the previous behaviour is preserved with the warning of backwards compatibility.
This means that
cargo install ... --vers "^1.2.3"will be parsed as a rangecargo install ... --vers 1.2.3will be parsed as a versioncargo install ... --vers 1.2will be parsed as a version for backwards compatibility reasons, fail and be passed through as is,but we will add a noteif you want to specify semver range, add an explicit qualifier, like ^1.2cargo install ... --vers blahwill be parsed as a version for backwards compatibility reasons (which is weird because it is not even a valid semver range) and produce anunknown errordown the line. I have left this behaviour untouched because it worked like that before, but I can easily make it error sooner with a better message.