-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --precise
flag for cargo-upgrade
#200
Conversation
Fixes killercup#177. If this flag is provided, all dependencies upgraded will be blindly upgraded to the version specified. Also, there has been substantial refactoring. The new `Manifests` and `Dependencies` structs now hold the logic for upgrade. This allows us to commonise the cases of a single upgrade and full workspace upgrade quite considerably. This should also make it fairly simple to extend to permit providing a list of packages to confine upgrades to. Also does some tidying up for the new `--precise` option. Most significantly, we now have a test for this functionality.
And an old bit of oddness that we can now get rid of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @bjgill!
I have 2 concerns:
- There is a little inconsistency between
cargo add
andcargo upgrade
flags: incargo add
you can specify versions in 2 ways:docopt@~0.9.0
ordocopt --vers ~0.9.0
, and withcargo upgrade
we now havedocopt --precise ~0.9.0
. - With
cargo upgrade
you can't specify version for each dependency individually.
Maybe we should just support docopt@~0.9.0
variant in cargo upgrade
as well instead of the --precise
flag?
Also, does this PR fix #201 (where do we assume crates.io
is the only registry)?
I'll look at adding support for the No, this does not fix #201. I don't believe it makes matters any worse, though. |
OK - I've added support for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(where do we assume crates.io is the only registry)
Ah, we have crates.io
url hardcoded:
Line 14 in c582f30
const REGISTRY_HOST: &str = "https://crates.io"; |
I agree that we should fix it in a separate PR.
Now that we support dep@vers
syntax, what is the use case for --precise
flag: when do you need to specify same version for multiple dependencies, maybe don't need this flag anymore?
The `dep@vers' mechanism seems likely to be more generally useful
I agree - I don't think we need |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify in README.md
that it is possible to specify arbitrary version requirements with @
syntax, e.g. "serde_json@>=0.9,<2.0"
or docopt@~0.9
?
Other than that, LGTM 💯
src/crate_name.rs
Outdated
semver::VersionReq::parse(version).chain_err(|| "Invalid crate version requirement")?; | ||
|
||
Ok(Some(Dependency::new(name).set_version(version))) | ||
// Ok(Some((name, version))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: old stuff
Thanks. I've made that clearer in the readme and help text. bors r+ |
200: Add `--precise` flag for cargo-upgrade r=bjgill a=bjgill Fixes #177. If this flag is provided, all dependencies upgraded will be blindly upgraded to the version specified. Added a test for this new feature. Also, there has been substantial refactoring. The new `Manifests` and `Dependencies` structs now hold the logic for upgrade. This allows us to commonise the cases of a single and full workspace upgrade quite considerably. EDIT: this also fixes #182 (I'll open a new issue for alternate registries)
Fixes #177. If this flag is provided, all dependencies upgraded will be blindly upgraded to the
version specified. Added a test for this new feature.
Also, there has been substantial refactoring. The new
Manifests
andDependencies
structs nowhold the logic for upgrade. This allows us to commonise the cases of a single and full
workspace upgrade quite considerably.
EDIT: this also fixes #182 (I'll open a new issue for alternate registries)