-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
A way for users to bulk upgrade across incompatible versions #12425
Comments
Below is some background on what we are using to help come up with a design Care AboutsPriorities
Primary use cases:
Secondary use cases are:
Some open questions we had
ContextCurrently,
Version requirement editing is different in that
And as a reminder of the CLI: cargo update -p foo # select a non-ambiguous `foo` and update it
cargo update -p foo@ver # selects a specific `foo` to update
cargo update -p foo --aggressive # also update dependencies
cargo update -p foo --precise <ver> # select a specific version
cargo update --locked # fail if the lockfile will change Note: Some design tools we can consider include
InterjectionThrough this, I realized that the core of my concern with our previous attempts at a single command is that it feels like we are shoehorning new behaviors into
I also realized that my Windows Updates vs Windows Upgrades analogy for |
Proposal:
|
fix(update): Tweak CLI behavior ### What does this PR try to resolve? When looking at `cargo update` for #12425, I noticed that the two flags related to `--package` were not next to it or each other. I figured grouping them like that would make things easier to browse. When looking into that, I noticed that the two flags conflict and figured we'd provide a better error message if we did that through clap. ### How should we test and review this PR? Looking per commit will help show the behavior changes. ### Additional information I wanted to scope this to being simple, non-controversial, low effort, incremental improvements with this change so I did not look into the history of `--aggressive` not requiring `--package` like `--precise` does and figure out if there is any consistency we can be working towards.
This proposal has been up here and on internals for a bit now without any major concerns raised. @rfcbot fcp merge |
Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Having been fairly involved in this discussion via the internals thread, I'm happy to see this move forward in a direction that I wholeheartedly support. @epage thanks for pushing this forward! |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Can |
Yes, we'd update the version requirement, if an incompatible version exists, and then run the normal code. |
When working on cargo-upgrade, I found the meaning of `--aggressive` confusing and named it `--recursive` there. Renaming this in `cargo update` (with a backwards compatible alias) was referenced in rust-lang#12425.
This is probably stating the obvious. But if a dependency is imprecise, current Let's say a dependency is The separation of the tools and the clarity on each one's remit makes this, if not immediately predictable, at least easily understandable. But if this will no longer be the case, then this distinction, or a change from that behavior, should be made clear. For what it's worth, I don't like this behavior, and I'll implement |
FYI on zulip I brought up the idea of a |
Generally, cargo avoids positional arguments. Mostly for the commands that might forward arguments to another command, like `cargo test`. It also allows some flexibility in turning flags into options. For `cargo add` and `cargo remove`, we decided to accept positionals because the motivations didn't seem to apply as much (similar to `cargo install`). This applies the pattern to `cargo update` as well which is in the same category of commands as `cargo add` and `cargo remove`. Switching to a positional for `cargo update` (while keeping `-p` for backwards compatibility) was referenced in rust-lang#12425.
Generally, cargo avoids positional arguments. Mostly for the commands that might forward arguments to another command, like `cargo test`. It also allows some flexibility in turning flags into options. For `cargo add` and `cargo remove`, we decided to accept positionals because the motivations didn't seem to apply as much (similar to `cargo install`). This applies the pattern to `cargo update` as well which is in the same category of commands as `cargo add` and `cargo remove`. As for `--help` formatting, I'm mixed on whether `[SPEC]...` should be at the top like other positionals or should be relegated to "Package selection". I went with the latter mostly to make it easier to visualize the less common choice. Switching to a positional for `cargo update` (while keeping `-p` for backwards compatibility) was referenced in rust-lang#12425.
Generally, cargo avoids positional arguments. Mostly for the commands that might forward arguments to another command, like `cargo test`. It also allows some flexibility in turning flags into options. For `cargo add` and `cargo remove`, we decided to accept positionals because the motivations didn't seem to apply as much (similar to `cargo install`). This applies the pattern to `cargo update` as well which is in the same category of commands as `cargo add` and `cargo remove`. As for `--help` formatting, I'm mixed on whether `[SPEC]...` should be at the top like other positionals or should be relegated to "Package selection". I went with the latter mostly to make it easier to visualize the less common choice. Switching to a positional for `cargo update` (while keeping `-p` for backwards compatibility) was referenced in rust-lang#12425.
When working on cargo-upgrade, I found the meaning of `--aggressive` confusing and named it `--recursive` there. Renaming this in `cargo update` (with a backwards compatible alias) was referenced in rust-lang#12425.
Generally, cargo avoids positional arguments. Mostly for the commands that might forward arguments to another command, like `cargo test`. It also allows some flexibility in turning flags into options. For `cargo add` and `cargo remove`, we decided to accept positionals because the motivations didn't seem to apply as much (similar to `cargo install`). This applies the pattern to `cargo update` as well which is in the same category of commands as `cargo add` and `cargo remove`. As for `--help` formatting, I'm mixed on whether `[SPEC]...` should be at the top like other positionals or should be relegated to "Package selection". I went with the latter mostly to make it easier to visualize the less common choice. Switching to a positional for `cargo update` (while keeping `-p` for backwards compatibility) was referenced in rust-lang#12425.
Really hope yarn is one of the nodejs package managers, and it supports yarn 1 https://classic.yarnpkg.com/lang/en/docs/cli/upgrade-interactive/ yarn 4 https://yarnpkg.com/cli/upgrade-interactive Not similar to For example, which supports Press <up>/<down> to select packages. Press <enter> to install.
Press <left>/<right> to select versions. Press <ctrl+c> to abort.
? Pick the packages you want to upgrade. Current Range Latest
> @types/node --------------------------------- ◉ 16 ----------- ◯ 16.18.91 ----- ◯ 20.11.30 -----
typescript ---------------------------------- ◉ 3.7 ---------- ◯ 3.7.7 -------- ◯ 5.4.3 -------- |
I'm working on this. I should have a draft PR up soon. |
@torhovland which part of this are you working on? In a lot of ways, Any high level details of what your understanding is for resolving the half you are working on? |
@epage I've been looking into So far I have introduced an That is probably a little too simplistic, though. In particular, it won't work well with If you have any thoughts about this, feel free to share. |
The other problem with modifying the manifests before anything else is that we will leave the workspace in a half-updated state on failure. My assumption was that we'd edit the |
Yes, I believe the feature as specified says we shouldn't do compatible updates. We probably want the semantics of |
I submitted a draft PR now. I've changed the code so it can mutate the manifests without needing to write out intermediate files and resetting the workspace. But this means there are two kinds of manifest mutations going on, one for the manifests in memory, and one for preserving formatting when writing out modified manifest files. See the PR for details. I haven't changed I'll happily take any feedback. |
Add `cargo update --breaking` Related to #12425. There are two kinds of manifest mutations here. In `upgrade_manifests` we have to mutate `cargo::core::manifest::Manifest` so that `resolve_ws` does what it needs to do, and outputs a correct lock file. Then, in `write_manifest_upgrades` we mutate `cargo::util::toml_mut::manifest::Manifest`, because that is how we can preserve the existing formatting in the manifest files on disk. Some of the code here is copied from `cargo-edit`. # Comparison with `cargo upgrade` Running on the Cargo source itself gives the following: ``` ❯ cargo upgrade --dry-run --incompatible allow --compatible ignore Updating 'https://github.com/rust-lang/crates.io-index' index Checking benchsuite's dependencies Checking capture's dependencies Checking cargo's dependencies name old req compatible latest new req note ==== ======= ========== ====== ======= ==== anstream 0.6.13 0.6.14 0.6.14 0.6.13 compatible anstyle 1.0.6 1.0.7 1.0.7 1.0.6 compatible anyhow 1.0.82 1.0.86 1.0.86 1.0.82 compatible itertools 0.12.1 0.12.1 0.13.0 0.13.0 libc 0.2.154 0.2.155 0.2.155 0.2.154 compatible opener 0.7.0 0.7.1 0.7.1 0.7.0 compatible openssl 0.10.57 0.10.64 0.10.64 0.10.57 compatible openssl-sys =0.9.92 0.9.92 0.9.102 =0.9.92 pinned pulldown-cmark 0.10.3 0.10.3 0.11.0 0.11.0 security-framework 2.10.0 2.11.0 2.11.0 2.10.0 compatible semver 1.0.22 1.0.23 1.0.23 1.0.22 compatible serde 1.0.199 1.0.203 1.0.203 1.0.199 compatible serde-untagged 0.1.5 0.1.6 0.1.6 0.1.5 compatible serde_json 1.0.116 1.0.117 1.0.117 1.0.116 compatible tar 0.4.40 0.4.41 0.4.41 0.4.40 compatible thiserror 1.0.59 1.0.61 1.0.61 1.0.59 compatible toml 0.8.12 0.8.14 0.8.14 0.8.12 compatible toml_edit 0.22.12 0.22.14 0.22.14 0.22.12 compatible unicode-width 0.1.12 0.1.13 0.1.13 0.1.12 compatible Checking cargo-credential's dependencies Checking cargo-credential-1password's dependencies Checking cargo-credential-libsecret's dependencies Checking cargo-credential-macos-keychain's dependencies Checking cargo-credential-wincred's dependencies Checking cargo-platform's dependencies Checking cargo-test-macro's dependencies Checking cargo-test-support's dependencies Checking cargo-util's dependencies Checking cargo-util-schemas's dependencies Checking crates-io's dependencies Checking home's dependencies Checking mdman's dependencies Checking resolver-tests's dependencies Checking rustfix's dependencies Checking semver-check's dependencies Checking xtask-build-man's dependencies Checking xtask-bump-check's dependencies Checking xtask-stale-label's dependencies note: Re-run with `--pinned` to upgrade pinned version requirements note: Re-run with `--verbose` to show all dependencies local: cargo unchanged: annotate-snippets, base64, bytesize, cargo-credential, cargo-credential-libsecret, cargo-credential-macos-keychain, cargo-credential-wincred, cargo-platform, cargo-test-macro, cargo-test-support, cargo-util, cargo-util-schemas, cargo_metadata, clap, color-print, core-foundation, crates-io, criterion, curl, curl-sys, filetime, flate2, git2, git2-curl, gix, glob, handlebars, hex, hmac, home, http-auth, humantime, ignore, im-rc, indexmap, jobserver, lazycell, libgit2-sys, libloading, memchr, miow, os_info, pasetors, pathdiff, percent-encoding, pkg-config, proptest, rand, regex, rusqlite, rustfix, same-file, serde-value, serde_ignored, sha1, sha2, shell-escape, similar, snapbox, supports-hyperlinks, supports-unicode, tempfile, time, tracing, tracing-chrome, tracing-subscriber, unicase, unicode-xid, url, varisat, walkdir, windows-sys warning: aborting upgrade due to dry run ❯ target/debug/cargo update --breaking --dry-run -Zunstable-options Updating crates.io index Upgrading itertools ^0.12.1 -> ^0.13.0 Upgrading pulldown-cmark ^0.10.3 -> ^0.11.0 Updating crates.io index Locking 3 packages to latest compatible versions Updating itertools v0.12.1 -> v0.13.0 Updating pulldown-cmark v0.10.3 -> v0.11.0 Updating pulldown-cmark-escape v0.10.0 -> v0.11.0 warning: not updating any files due to dry run ``` In both cases we see an upgrade of `itertools` to `^0.13.0` and `pulldown-cmark` to `^0.11.0`. The diff to the manifest (when it isn't a dry run) is as follows: ``` --- a/Cargo.toml +++ b/Cargo.toml `@@` -57,7 +57,7 `@@` humantime = "2.1.0" ignore = "0.4.22" im-rc = "15.1.0" indexmap = "2.2.6" -itertools = "0.12.1" +itertools = "0.13.0" jobserver = "0.1.31" lazycell = "1.3.0" libc = "0.2.154" `@@` -74,7 +74,7 `@@` pathdiff = "0.2.1" percent-encoding = "2.3.1" pkg-config = "0.3.30" proptest = "1.4.0" -pulldown-cmark = { version = "0.10.3", default-features = false, features = ["html"] } +pulldown-cmark = { version = "0.11.0", default-features = false, features = ["html"] } rand = "0.8.5" regex = "1.10.4" rusqlite = { version = "0.31.0", features = ["bundled"] } ``` # TODO - [x] In the case of `--incompatible`, we also need to let `update_lockfile` use `upgrades` in order to only update the incompatible dependencies. - [x] Testing all the different cases of package sources, version requirements, pinned versions, renamed dependencies, inherited workspace dependencies, multiple versions of the same dependency, selecting a subset `--package`, etc. - [x] Passing tests. - [x] Implement suggestions from reviews. - [x] The preservation of formatting in manifest files should be improved. - [x] Compare with `cargo upgrade`.
I have started working on the |
Thanks! I've added to the task list another test case we need (and to pin down the semantics of): |
All right. We already cover |
We cover those for It might also be of interest to have a mixed workspace, where one package depends on |
This bug will need a separate PR: #14049 (comment) |
More identified tasks
|
Not sure this makes sense to me? Does |
The non-breaking update does not fail, but outputs something like this:
The breaking update currently only outputs this:
So maybe the breaking one should add:
as well as a feature to list the other packages using |
#14140 is making output consistent between breaking and non-breaking updates (both now use the same So a breaking update that ends up not upgrading anything will now output this:
|
More `update --breaking` tests Related to #12425 (comment) in #12425.
would it be possible / is it currently planned to also update non-breaking versions in the
i don't know if this is a desired feature, but since |
That is covered in #12425 (comment)
|
For my use case of Quoting myself from #14204 (apparently @epage prefers feedback in this issue instead):
As such it seems I will probably have to port cargo-upgrade from cargo-edit to sparse registry myself at some point. When and if I have time. As |
Problem
A lot of incompatible upgrades have a small enough of breakages that a "upgrade all of my incompatible version requirements" would make it easier to go through this process.
This is currently exposed in the
cargo upgrade
commandProposed Solution
Tasks
cargo update --breaking
#13979cargo update --breaking only-compatible renamed non-semver-operator transitive-dep
--precise <breaking>
-p
more convenient by being positional #12545Deferred
See #12425 (comment)
Previously, https://internals.rust-lang.org/t/feedback-on-cargo-upgrade-to-prepare-it-for-merging/17101/141
Notes
Related
The text was updated successfully, but these errors were encountered: