cargo icon indicating copy to clipboard operation
cargo copied to clipboard

A way for users to bulk upgrade across incompatible versions

Open epage opened this issue 2 years ago • 42 comments
trafficstars

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 command

Proposed Solution

Tasks

  • [ ] update breaking
    • [x] #13979
      • [ ] Test cases for cargo update --breaking only-compatible renamed non-semver-operator transitive-dep
    • [ ] --precise <breaking>
    • [ ] Call for testing
    • [ ] Stabilization
  • [x] #12544
  • [x] #12614
  • [x] #13372
  • [x] #12545
  • [ ] bugs and identified tasks

Deferred

  • Modifying version requirements for compatible upgrades

See https://github.com/rust-lang/cargo/issues/12425#issuecomment-1659453818

Previously, https://internals.rust-lang.org/t/feedback-on-cargo-upgrade-to-prepare-it-for-merging/17101/141

Unresolved questions

  • Flag name
    • --breaking to focus on semver (as this is limited to ^)
    • --incompatible applies to all version req operators and doesn't have a good short flag (-i is generally assumed to be "interactive")
    • Dart calls it --major-versions though we have the issue of "absolute major" (x.0.0) and "relative major (also 0.x.0, 0.0.x).
    • pnpm calls it --latest because of a "latest" tag

Notes

Related

  • #13908
  • #10498

epage avatar Aug 01 '23 01:08 epage

Below is some background on what we are using to help come up with a design

Care Abouts

Priorities

  • Don't break behavior on cargo update
  • Don't write out incompatible Cargo.lock and Cargo.toml
  • Focus is on end-users solving common problems and not on being a general programmatic CLI that is meant to cover every case
  • Be predictable and understandable
    • Can someone unfamiliar with Rust, reading a blog post, predict what different command invocations will do?
    • Preference for not having too similarly named commands
  • When higher priorities allow, avoid errors that make users go "if you know exactly what I was asking for then why didn't you just do it?"; those are a sign of issues with the UX.
  • Don't be hassle when dealing with intentionally held back dependencies

Primary use cases:

  • Want to have simple workflow for "upgrade incompatible dependencies only".
  • Want to have simple workflow for bulk lock to latest compatible (already exists as cargo update)
  • (medium priority) Selective modify one dependency's version requirement to latest compatible, latest incompatible
  • (lower priority) Want to have simple-ish workflow for bulk upgrade to latest compatible
  • (lower priority) bulk upgrade all dependencies (could just be two command invocations)

Secondary use cases are:

  • Selective modify version requirement to user-provided value
  • Upgrade explicitly pinned version requirements

Some open questions we had

  • How do we tell when a renamed dependency like tokio_03 is pinned or not?
    • We could just assume all renamed are pinned
    • We could add a dependency field but I'm a bit leery of adding that kind of bookkeeping to the manifest
    • We could force users to --exclude these dependencies but that might be a bit of a pain to always remember to do
    • We could only skip renamed if multiple dependencies exist that point to the same package

Context

Currently, cargo update is focused solely on Cargo.lock editing

  • Spans entire dependency tree
  • Multiple versions of a package may exist, referenced by name@version
  • Deals with exact versions and not version ranges
  • Only affects you and not your dependents

Version requirement editing is different in that

  • Workspace members only
  • May want differences between members
  • Supports alternative names for packages
  • Affects dependents

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: cargo add --locked will also fail if the manifest will change

Some design tools we can consider include

  • Renaming a command, making the old name an alias
    • Even if there isn't a culture shift to use the new name, cargo <cmd> --help and cargo --list will point people to the new name
  • Versions without the build metadata field is a subset of version requirement syntax, we may be able to do some mixing of them
    • Precedence: using the same foo@ver syntax for versions and version requirements
  • Minimal-version resolution being the default mode would make Cargo.lock mostly align with Cargo.toml, making it easier to conflate the two commands (whether merging them or keeping separate but de-emphasizing update)

Interjection

Through 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 cargo update rather than making the behavior cohesive.

  • If I see a cargo update --incompatible on a blog post, can I predict what will happen if you do cargo update? No, because --save is needed to match behavior
  • We were trying to make --package be both for package IDs and dependency names to make some of the cargo upgrade workflows work
  • We were trying to overload --precise to allow control over version requirements

I also realized that my Windows Updates vs Windows Upgrades analogy for cargo update and cargo upgrade breaks down a little because cargo upgrade can do "upgrades" that are on the level of cargo update (say we call it cargo upgrade --compatible). The difference is in the target audience (yourself vs your dependents)

epage avatar Aug 01 '23 02:08 epage

Proposal: cargo update only changes version requirements as a side effect

The primary role of cargo update has been to update your active dependencies (ie Cargo.lock). We do not plan to change that role but give the user control to force it to update in situations that were previously unsupported, particularly updating the Cargo.toml if need be.

Behavior:

  • By default, only "safe" updates are allowed (today's behavior)
  • cargo update --incompatible / cargo update -i will force (ie update version reqs) update unpinned, incompatible versions (no other dependencies)
    • Yes, this could potentially be called --breaking or something else. The name depends on what expresses the concept clearly especially in light of any pinning behavior we have
    • Keeping -i unused would keep the door for a --interactive to be added
  • cargo update -p foo --precise ver will force the update to happen, only erroring if we can't (don't own relevant version reqs, is pinned), even if its incompatible but unpinned. Version requirement is only changed on incompatible.
    • Maybe the error on pinned could be relaxed

Somewhere between deferred and rejected (speaking for myself): Support in cargo update for writing to the manifest for non-breaking changes, like bulk compatible upgrades of version requirements (ie a -save flag) which was one of our lower priority workflows. A --save flag is more about updating versions for your dependents, which while important for having valid lower-bounds on version requirements, doesn't fit with the existing model of cargo update. Maybe in the future we can find a way to express this in cargo update that fits with how it works or maybe another command can take on this role. We just aren't wanting to distract our efforts for handling most of the use cases to handle this one. See #10498

While this tells a cohesive story, a part of me is somewhat concerned that this goes beyond the name update.

Potential related cargo update improvements

  • "Rename" --aggressive to --recursive (make --aggressive an alias for the new name) (#12544)
  • Specialize the parsing of [email protected] so that foo@x will work so long as its unambiguous (which it should be), much like cargo install [email protected] supports auto-selecting the y and z fields (picks latest) (#12614)
  • Consider applying some of the output formatting from cargo upgrade into cargo update (#13372)
    • Provides some of the cargo outdated experience (#4309)
    • Is an alternative to warnings (#7167)
  • Add a positional package argument to cargo update, removing the need for --package (#12545)
    • Should be safe because there are no trailing var args for a wrapped child process and no (and little chance of) flags that accept 0..=1 values

Alternatives

These are alternatives I had considered that help give an idea of what I mean by fitting into cargo.

cargo update always modifies Cargo.toml

  • This would be a breaking change
  • This would get in the way of people intentionally keeping separate versions from version requirements

We deprecate cargo update and a cargo upgrade always updates both files

  • This would get in the way of people intentionally keeping separate versions from version requirements

We migrate to minimal-version resolution by default

  • cargo update becomes less useful and we move it out of the spot light.
  • A cargo upgrade is added that is focused on editing version requirements

Separate commands for Cargo.lock (update) and Cargo.toml (upgrade)

  • Names don't clarify the role each fills
  • Much like Debian has apt dist-upgrade, maybe it could be cargo req-update?

Misc Notes

  • I don't see us making a distinction between default operator and ^ as we document them as being the same thing and sometimes people use ^ just because
  • We are erring on the side of needing cargo update && cargo update --incompatible vs cargo update --incompatible doing both as there are people who do want them separated and running two commands, while annoying, allows us to cover more use cases.

epage avatar Aug 01 '23 02:08 epage

This proposal has been up here and on internals for a bit now without any major concerns raised.

@rfcbot fcp merge

epage avatar Aug 08 '23 17:08 epage

Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members:

  • [x] @Eh2406
  • [x] @Muscraft
  • [x] @arlosi
  • [x] @ehuss
  • [x] @epage
  • [x] @joshtriplett
  • [x] @weihanglo

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.

rfcbot avatar Aug 08 '23 17:08 rfcbot

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!

djc avatar Aug 08 '23 19:08 djc

:bell: This is now entering its final comment period, as per the review above. :bell:

rfcbot avatar Aug 14 '23 20:08 rfcbot

Can --incompatible be used in conjunction with --package? Just recall https://github.com/rust-lang/cargo/issues/11974 and not sure about the implication of cargo update --incompat -p <pkgid>.

weihanglo avatar Aug 16 '23 12:08 weihanglo

Can --incompatible be used in conjunction with --package? Just recall https://github.com/rust-lang/cargo/issues/11974 and not sure about the implication of cargo update --incompat -p <pkgid>.

Yes, we'd update the version requirement, if an incompatible version exists, and then run the normal code.

epage avatar Aug 16 '23 12:08 epage

cargo upgrade can do "upgrades" that are on the level of cargo update (say we call it cargo upgrade --compatible). The difference is in the target audience (yourself vs your dependents)

This is probably stating the obvious. But if a dependency is imprecise, current cargo upgrade may have no effect where current cargo update would.

Let's say a dependency is serde_derive = "1" in Cargo.toml. And it's version = "1.0.183" in Cargo.lock. Then current cargo upgrade would change nothing locally, and wouldn't force dependants to get the latest version.

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 --force-precise for personal use. Especially since I only used imprecise deps in old projects.

MoSal avatar Aug 23 '23 15:08 MoSal

FYI on zulip I brought up the idea of a pedantic machine-applicable (ie --fixable) lint to flag imprecise dependencies.

epage avatar Aug 23 '23 15:08 epage

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

rfcbot avatar Aug 24 '23 20:08 rfcbot

  • Keeping -i unused would keep the door for a --interactive to be added

Really hope --interactive.

yarn is one of the nodejs package managers, and it supports upgrade-interactive.

yarn 1

https://classic.yarnpkg.com/lang/en/docs/cli/upgrade-interactive/

yarn 4

https://yarnpkg.com/cli/upgrade-interactive

Not similar to cargo update, but similar to cargo-edit's cargo upgrade, but have a terminal ui.

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 --------

loynoir avatar Mar 25 '24 00:03 loynoir

I'm working on this. I should have a draft PR up soon.

torhovland avatar May 28 '24 11:05 torhovland

@torhovland which part of this are you working on? In a lot of ways, --precise vs --breaking are very different and I suspect they would best be handled as separate PRs.

Any high level details of what your understanding is for resolving the half you are working on?

epage avatar May 28 '24 17:05 epage

@epage I've been looking into --incompatible (--breaking).

So far I have introduced an ops::update_manifests() in front of ops::update_lockfile(). It reuses some code from cargo-edit to go through all dependencies, query latest versions, check if they are incompatible, update the manifest TOML if they are, then write the modified TOML to file. Then I'm simply resetting the workspace before calling ops::update_lockfile().

That is probably a little too simplistic, though. In particular, it won't work well with --dry-run, because ops::update_lockfile() will need the updated manifests. But maybe this can be improved so we don't have to reset the workspace. Also, I suppose we shouldn't do a full ops::update_lockfile(), but only update the dependencies that got modified by ops::update_manifests().

If you have any thoughts about this, feel free to share.

torhovland avatar May 28 '24 18:05 torhovland

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 Dependencys for workspace members (maybe all path sources?) and on success, we'd find all Cargo.toml all dependency blocks, including workspace.dependencies, and edit them then.

epage avatar May 28 '24 18:05 epage

. Also, I suppose we shouldn't do a full ops::update_lockfile(), but only update the dependencies that got modified by ops::update_manifests().

Yes, I believe the feature as specified says we shouldn't do compatible updates. We probably want the semantics of cargo check after editing Cargo.toml in case a requirement change causes a cascade of compatible updates.

epage avatar May 28 '24 18:05 epage

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 update_lockfile() yet.

I'll happily take any feedback.

torhovland avatar May 29 '24 12:05 torhovland

I have started working on the --precise part of this issue too.

torhovland avatar Jun 11 '24 08:06 torhovland

Thanks!

I've added to the task list another test case we need (and to pin down the semantics of): cargo update --breaking only-compatible renamed non-semver-operator transitive-dep

epage avatar Jun 11 '24 18:06 epage

All right. We already cover only-compatible and renamed, though. Unless you mean they are important in that new, specific test case for some reason?

torhovland avatar Jun 11 '24 19:06 torhovland

We cover those for cargo update --breaking but I'm not seeing those covered for cargo update --breaking [SPEC]. We only test just-foo shared ws which are all candidates for upgrading. I'm concerned about the semantics of a user explicitly naming a package that --breaking doesn't apply to.

It might also be of interest to have a mixed workspace, where one package depends on renamed without a rename while another depends on it with a rename. This would need to be a separate test as this wouldn't error but the other case could potentially (depending on what semantics we want to give it)

epage avatar Jun 11 '24 20:06 epage

This bug will need a separate PR: https://github.com/rust-lang/cargo/pull/14049#discussion_r1636021228

torhovland avatar Jun 19 '24 08:06 torhovland

More identified tasks

  • [x] Handle dependencies that are both renamed/pinned and not in the same workspace. See https://github.com/rust-lang/cargo/pull/14049#discussion_r1636021228. Fixed in #14049.
  • [ ] Consider an error message if the command completed without doing any upgrades. See https://github.com/rust-lang/cargo/pull/14049#discussion_r1636367041. Improved in #14140. See https://github.com/rust-lang/cargo/issues/12425#issuecomment-2193912809. The breaking update will now have similar behaviour and output as the non-breaking update. The question still stands if the breaking update needs an explicit error message. See discussion below.
  • [ ] Consider if it is OK to not error when doing cargo update --breaking [email protected] (where [email protected] does not exist), but there is no lock file yet. This is consistent with the non-breaking update. See https://github.com/rust-lang/cargo/pull/14049#discussion_r1645783714.
  • [ ] semver_ext::matches_prerelease needs some improvement in order to support precise updates to prerelease versions. Fixed in #14140. See the new tests precise_prerelease and update_precise_breaking_pre_release_explicit_version_req.
  • [ ] Can we limit updates more in case of breaking upgrades? See https://github.com/rust-lang/cargo/pull/14140#discussion_r1656927375 and the test update_precise_breaking_shared_non_ws.
  • [x] Improve an error message. See https://github.com/rust-lang/cargo/pull/14049#discussion_r1656929921. Fixed in #14279.
  • [ ] Decide if we need both UPGRADING and UPDATING messages and if it is OK that the former outputs version requirements (^1.0) while the latter outputs versions (v1.0.0). See https://github.com/rust-lang/cargo/pull/14140#discussion_r1658829398.
  • [x] Don't downgrade if pre-release was used — https://github.com/rust-lang/cargo/issues/14178#issuecomment-2203083535
  • [ ] Tests verifying that each mode fails if Cargo.toml or Cargo.lock changes when --locked is passed in
  • [ ] Upgrade from one pre-release version to the other, specifically
    • 2.0.0-beta.1 to 2.0.0-beta.2
    • 2.0.0-beta.1 to 2.0.0
    • I am not sure about if 2.0.0-beta.1 to 3.0.0-beta.1 should works
    • https://github.com/rust-lang/cargo/pull/14250#discussion_r1686796460

torhovland avatar Jun 24 '24 10:06 torhovland

Consider an error message if the command completed without doing any upgrades.

Not sure this makes sense to me? Does cargo update fail when there's nothing to update?

djc avatar Jun 24 '24 11:06 djc

The non-breaking update does not fail, but outputs something like this:

[UPDATING] `dummy-registry` index
[LOCKING] 0 packages to latest compatible versions
[NOTE] pass `--verbose` to see 3 unchanged dependencies behind latest

The breaking update currently only outputs this:

[UPDATING] `dummy-registry` index

So maybe the breaking one should add:

[UPGRADING] 0 packages to latest compatible versions

as well as a feature to list the other packages using --verbose?

torhovland avatar Jun 24 '24 11:06 torhovland

#14140 is making output consistent between breaking and non-breaking updates (both now use the same ops::update_lockfile()).

So a breaking update that ends up not upgrading anything will now output this:

[UPDATING] `[..]` index
[LOCKING] 0 packages to latest compatible versions
[NOTE] pass `--verbose` to see 3 unchanged dependencies behind latest

torhovland avatar Jun 27 '24 06:06 torhovland

would it be possible / is it currently planned to also update non-breaking versions in the Cargo.toml file?

cargo upgrade from cargo-edit currently supports that

name       old req compatible latest  new req
====       ======= ========== ======  =======
ratatui    0.26.1  0.26.1     0.27.0  0.27.0  // this is already supported in nightly
serde_json 1.0.117 1.0.118    1.0.118 1.0.118 // this is missing currently

i don't know if this is a desired feature, but since cargo upgrade already does it i am just wondering if this hasn't been discussed yet, has been discussed and has been decided against or has discussed and decided for, but just hasn't been implemented yet or if it is in another issue entirely?

m4rch3n1ng avatar Jun 27 '24 18:06 m4rch3n1ng

That is covered in https://github.com/rust-lang/cargo/issues/12425#issuecomment-1659453818

Somewhere between deferred and rejected (speaking for myself): Support in cargo update for writing to the manifest for non-breaking changes, like bulk compatible upgrades of version requirements (ie a -save flag) which was one of our lower priority workflows. A --save flag is more about updating versions for your dependents, which while important for having valid lower-bounds on version requirements, doesn't fit with the existing model of cargo update. Maybe in the future we can find a way to express this in cargo update that fits with how it works or maybe another command can take on this role. We just aren't wanting to distract our efforts for handling most of the use cases to handle this one

epage avatar Jun 28 '24 01:06 epage

These are alternatives I had considered that help give an idea of what I mean by fitting into cargo.

cargo update always modifies Cargo.toml

  • This would be a breaking change
  • This would get in the way of people intentionally keeping separate versions from version requirements

For my use case of cargo upgrade (which cargo update --breaking has been advertised as a replacement of) this would be the mode I want. I'm fine with it being an optional flag to do so, but currently cargo update --breaking is a step back functionality wise (performance is of course better though).

Quoting myself from https://github.com/rust-lang/cargo/issues/14204 (apparently @epage prefers feedback in this issue instead):

I was led to believe that cargo +nightly -Zunstable-options update --breaking should be a drop-in replacement for cargo upgrade -i. But it isn't. Specifically it doesn't seem to upgrade Cargo.toml unless there is a breaking change.

I use cargo upgrade to upgrade to new minor versions in Cargo.toml too. There are several reasons for this:

  • As I'm on the latest version when developing, I can't be sure I'm not relying on some new API that didn't exist in the version that is declared in Cargo.toml. While I believe there is some "min-version" flag to test with, I prefer to just force everything the latest. (No I don't care about LTS distros like Debian etc, not one bit, nor about people who are not on the most recent stable Rust. My MSRV is N.)
  • If there are any security issues in a dependency I'd also prefer to force the latest version there too.

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 cargo update --breaking just doesn't cut it for my use case.

VorpalBlade avatar Jul 06 '24 22:07 VorpalBlade