Feedback on `cargo-upgrade` to prepare it for merging

tools and infrastructure cargo
#1

Like with cargo-add, I'm interested in merging cargo upgrade into cargo. There are some open questions on the design and, with how successful the cargo-add thread was, I'm hopeful we can have a productive brainstorming session on resolving them to finalize the design for merging into cargo. I especially want to call attention to the first point of concern below as its the most critical, undefined, and yet could have the biggest repercussions.

Background

cargo upgrade performs bulk edits to a Cargo.toml's version requirements.

Example run on cargo

Screenshot 2022-07-29 112628
Screenshot 2022-07-29 1126281155×371 20.8 KB
(ignore "Finished"/"Running", I'm running the command out of the repo)

Notes

  • It preserves existing version requirement precision, where possible
  • By default, it deals with breaking changes only. To instead upgrade minimum versions, pass --to-lockfile
  • By default it ignores dependencies that are most likely pinned, requiring --pinned to upgrade them also. Besides the version requirement itself (e.g. using =), we assume renamed dependencies are pinned dependencies since a common use case is to have multiple incompatible versions of a depednency
  • Output is formatted into a table modeled after cargo outdated but summarizes "uninteresting" cases. The goal is to (1) build trust that its working by communicating why we made a choice, (2) show information for users to make decisions, while (3) not overwhelming the user especially in large crates or workspaces
  • When selecting dependencies to upgrade (cargo upgrade serde), it accepts the renamed version of that dependency (cargo upgrade tokio_01) to avoid selecting items intentionally held back (which is somewhat redundant with not upgrading pinned by default

Known Points of Concern

1. Distinction between cargo update and cargo upgrade may be confusing

The verbs "update" and "upgrade" can be synonymous to users. How can we clarify this?

Tools available

  • Naming of the commands
  • Finding a way to merge the commands, reconciling
    • upgrade deals with breaking changes while update deals with compatible changes
    • upgrade deals with direct dependencies while update deals with direct and transitive dependencies
    • upgrade selects dependencies by their key (ie if renamed) while update selects dependencies by their crate name and optionally version
    • upgrade interacts with workspace members (manifests) while update interacts with the workspace itself (lock file)
  • Cross-linking commands (e.g. cargo upgrade could suggest running cargo update if a direct dependency in the lockfile is stale)
  • Learn from prior art. I've not found another build system that deals with these concepts. Extending out to linux package managers, apt has update and upgrade. Maybe people have experience with what they do to help clarify these or how other distributions avoid the problem.

Proposal Merge cargo upgrade into cargo update

  • Add --save flag to update direct dependencies to corresponding lockfile versions
  • Add --incompatible that forces the lockfile to upgrade across incompatible versions for unpinned version requirements
    • Error if this mismatches with manifest (ie --save is not passed in)
  • Add --pinned flag to update unpinned version requirements
  • Provide summary table at the end for direct dependencies
  • Add -Z minimal-direct-versions that causes the resolver to update all transitive dependencies to latest but direct dependencies to the earliest version. This will help with libs that want to leave their version requirements low
  • Consider we mimicking poetry and go get by supporting cargo add <pkg>@latest
  • Hack this behavior into cargo upgrade so we can collect feedback on whether there are workflows this is leaving behind

Comparing this with commands today:

  • cargo update: same as today
  • cargo update --save: like cargo update && cargo upgrade --workspace --to-lockfile
  • cargo update --incompatible: error if we try to upgrade to an incompatible version, suggesting adding --save
  • cargo update --incompatible --save: like cargo upgrade --workspace && cargo update && cargo upgrade --workspace --to-lockfile
  • --pinned would still exist but require --incompatible to be present (i.e. we'd start updating pinned version reqs to the lock file)
  • Provide the direct dependency summary table at the end

2. How to select packages to upgrade vs dependencies within packages

Normally in cargo commands --workspace, --package <PKGID>, and --exclude <PKGID> are used to select which packages to operate on.

cargo upgrade has two dimensions to operate on

  • Manifests to modify
  • Dependencies within those manifests

Currently, we mix up the flags between these cases

  • --workspace and --package <PKGID> select manifests to modify
  • <DEP_ID> (a positional argument) and --exclude <DEP_ID> select which dependencies within those manifests to modify

Proposal Rely on cargo updates behavior

3. How do we represent "upgrade minimum compatible versions" vs "error if upgrades needed"

Currently

  • Upgrade compatible minimums: --to-lockfile
  • Error if upgrades needed: --locked

--locked generally means "avoid or error if something will modify the lock file" which might match "Upgrade minimums" but we then need a way to fail if upgrades are needed.

There is --dry-run but there are multiple, valid, approaches to error reporting for that

  • Error only if a non-dry-run will fail
  • Error if the --dry-run was prevented from doing something
  • Always error

A more out there alternative to --to-lockfile is we actually prompt the user for each upgrade and have a --choice <none|locked|latest> to bypass the prompts with a specific choice.

Proposal Rely on cargo updates behavior

4. Should we require require the repo to be clean, like cargo publish or cargo fix

Proposal No change

  • cargo update already doesn't do this and would be weird to have --save only care
  • This is also consistent with cargo add and cargo rm
  • Like the above commands, the developer might iterate on what is happening and requiring --allow-dirty would be disruptive

Prior Art

Prior art

Status Updates

(As of 8/1/2022)

  • Updated proposal to merge cargo-update / cargo-upgrade
4 Likes
Version-based visibility to make the language&ecosystem more stable
#2

It would be nice to be able to list dependencies you don't want upgrade in the package metadata section. Using a = constraint prevents you from picking up patches to the old release that you'd probably want.

It would also be great if cargo upgrade could update the lockfile while running. If the lockfile is checked in to the repo you currently need to do a separate cargo update or cargo fetch or whatever after the upgrade call.

#3

=3.1 will still allow upgrading to patch releases.

That doesn't necessarily mean we don't provide a way to explicitly declare pinned dependencies. It does reduce the need for it which, for me, raises the bar for justifying the extra user-facing complexity from configuration.

A recent release fixed this.

#4

Huh, TIL! Does that mean that = is basically equivalent to ~ then?

I don't think that's documented in the Cargo book, FWIW: Dependency Resolution - The Cargo Book.

Oh, yay!

#5

I feel like once concern is that cargo upgrade is too easily too destructive. It could

  • like cargo fix refuse to work unless the working directory has no uncommited changes, or --allow-dirty is provided, and/or
  • do a dry run by default, requiring an additional flag (name TBD) to actually do anything

These changes would also solve part of the problem of users confusing this with cargo update, since the worst consequence of this confusion is probably to accidentally do cargo upgrade when you didn’t want to. If it doesn’t do anything by default and/or doesn’t accidentally have you lose uncommited changes, then consequences of a mixup are sufficiently limited.

4 Likes
#6

Looking at the code, it appears = and wildcards are the same while ~ differs in the handling of patch and pre fields.

#7

apt's distinction between update and upgrade isn't relevant here; apt update is the equivalent of our implicit "download the index" step (which will be largely evaporating with the HTTP registry work).

The more I think about it, the more I think we shouldn't have two separate commands for this. I think it makes sense to have cargo update do compatible upgrades by default, and have an option (with short-option version) to cargo update to do incompatible upgrades. Perhaps cargo update -i if we stick with the "incompatible" terminology, or cargo update -m for --major.

7 Likes
#8

Yes, the model is different but in effect making apt install and apt upgrade limited by what is downloaded (--offline in cargo terms), it acts kind of like a lock file which is why I was hoping we can learn something from it despite the differences.

1 Like
#9

This still leaves several questions open

  • Are there reasonable workflows we are excluding?
  • For example, if I'm developing a [lib] without checking in my Cargo.lock, should we still modify the manifest if a developer decides to update dependencies for themself?
  • How do we reconcile the behavior changes? Make a new command and deprecate the old one?
  • How do we deal with their two different models (workspace-wide version editing vs per-manifest version requirement editing)?
  • Cargo.lock and Cargo.toml can still drift apart
    • Cargo.lock isn't checked will always have CI running at latest
    • If someone upgrades in one workspace member but not others

While this doesn't solve everything, to help the conversation move forward, here is a straw man proposal

  • cargo upgrade replaces cargo update which will be deprecated
  • cargo upgrade has --workspace, --package, and --exclude flags for selecting manifests to modify
  • cargo upgrade accepts positional arguments for dependencies to upgrade
    • Note: We stop accepting the dependency table keys and instead accept crate names, relying on the "don't touch pinned" default (since this was partly to allow precision in selecting renamed vs not)
  • When upgrading a dependency's version requirement, we only update the locked versions for all of that dependency's transitive dependencies, so the new cargo upgrade does cargo upgrade && cargo update while cargo upgrade clap will only touch clap's transitive dependencies
    • Note: Unsure how well cargo will be able to filter out updating parts of the resolved dependency graph like this
1 Like
#10

apt does have two upgrade modes though, it's just that update isn't one of them.

  • upgrade installs new versions of installed packages, but doesn't remove packages or install new ones
  • dist-upgrade (aka full-upgrade) will go further and install new dependencies or remove current dependencies if needed for a newer version of an installed package

As far as lock-file analogies, there's pinning to set the relative preference of packages (which in practice I think is mainly used to remove certain upgrades from consideration), and also the ability to hold a package (so that it isn't upgraded by default).

1 Like
#11

Personally, after many years of using cargo upgrade I feel like the current naming scheme works perfectly and there is enough of a distinction between "updating the lockfile" and "upgrading the dependency requirements"; but maybe that's just the stockholm syndrome talking.

2 Likes
#12

Add upgrade behavior to cargo update as an option, and either deprecate cargo upgrade or make it an alias for cargo update --the-new-option?

Require --workspace for the former just as we do for cargo update today? And potentially consider potential future transitions in which --workspace becomes the default?

Why not the reverse? I don't think there's any cargo upgrade behavior that's fundamentally incompatible with being an option of cargo update.

1 Like
#13

This does remind me, the npm equivalents are cargo update = npm update and cargo upgrade --to-lockfile = npm update --save (but then there's also npm update --save-{dev,prod,optional,peer} which.... all appear to do nothing?).

1 Like
#14

I was originally going off of your comment about manifest-editing being a default option.

Based on your comments combined with the prior art that Nemo157 brought up, it sounds like the proposal would be summarized as

  • cargo update: same as today
  • cargo update --save: like cargo update && cargo upgrade --workspace --to-lockfile
  • cargo update --incompatible: error if we try to upgrade to an incompatible version, suggesting adding --save
  • cargo update --incompatible --save: like cargo upgrade --workspace && cargo update && cargo upgrade --workspace --to-lockfile
  • --pinned would still exist but require --incompatible to be present (i.e. we'd start updating pinned version reqs to the lock file)
  • Provide the direct dependency summary table at the end

This seems to provide a coherent story though I haven't fully thought through --package / --precises behavior.

Trying to think through different use cases

  • There seemed to be general interest in keeping library version requirements low within a major version but to upgrade across major versions. This wouldn't be supported today but if we created a -Z minimal-direct-versions like I've seen ehuss talk about, then that would be a route forward.
  • For upgrading individual versions (e.g. cargo upgrade serde), we could always mimic poetry and go get by supporting cargo add <pkg>@latest
1 Like
#15

Making up flags, I think roughly
cargo update is cargo upgrade --compatible --lockfile-only
cargo upgrade is cargo update --allow-breaking
cargo upgrade --compatible is cargo update --required[1]

If this is used as (partial) justification for a choice, we should consider refusing to run (or perhaps just doing a --dry-run) if VCS is dirty, like how cargo publish and cargo fix do unless you --allow-dirty.

Idea kernel: we use --pinned to allow updating dependencies we consider pinned; why not --compatible to cause the update to chose the latest compatible?

Even more wild would be to just have --locked mean --to-lockfile. I argue that this may be coherent: cargo upgrade --locked with the normal "ignore compatible" rules is contradictory, since you're saying "upgrade my dependencies but don't change what dependencies I'm using."

This kinda feels like the difference between --frozen and --locked to me, though perhaps only inasmuch as I don't really understand the difference between them. upgrade imho should semantically include updating the lockfile to the upgraded version (cargo update -p upgraded...), so --to-lockfile is as much as it can do without editing the lockfile or accessing the network (--offline?).

I admit not fully understanding what --frozen adds to --locked (--offline just for version resolution, perhaps?) but it does feel similar to what's desired as it's more locked than --locked.

When does cargo publish --dry-run error? Answer: if publishing would fail before upload; it ends with a warning: aborted upload due to dry run and a 0 exit code. Following that example, cargo upgrade --dry-run should exit with the same exit code as a non-dry-run if everything succeeded except for skipped changes to the manifest(s)/lockfile.

If verifying no change is considered necessary to support (question: is it?), e.g. cargo upgrade --verify-unchanged is a reasonable way of explicitly requesting this.

--interactive is a great mode to have available, but it probably should be opt-in since none of the other builtin cargo subcommands are interactive by default.

Updating, yes, but upgrade --pinned without --to-lockfile will only see compatible versions, and with --to-lockfile you're still not changing the manifest's precision, so the only thing that would change is the lockfile (which doesn't with --to-lockfile; you'd need --compatible.)

  1. Updates to the lockfile are "optional," as the lockfile is ignored by default for libraries (default VCS ignore ignores the lockfile), always for dependencies, and when using cargo install (unless you use --locked; we should invert this imho). Updates to the manifest are "required" for any downstream user to provide, since you've upgraded your dependency version specification/requirement. ↩︎

1 Like
#16

I quite strongly feel that upgrade functionality should become an option to the update command. I'm thinking here mainly of future Rust users who are not familiar with the current situation, where I think updating dependencies is the overriding theme for both of these and the difference is too small to warrant a separate command. So far I haven't thought of a great option name, I think --major might be confusing because in our ecosystem today this will often be updating the minor version, not the major one. --allow-breaking also seems overly focused on the downside of upgrading across a semver compatibility barrier. --incompatible seems like a decent option, I guess? To me, the concept I'm usually really looking for is maybe closer to --all.

I can see how we'd want to stop upgrade from running by default for the dirty Git workdir by default.

FWIW, I use cargo upgrade every week and every week I find it annoying that I have to use --workspace for it -- I'd much rather have it default to --workspace, partially because update already does so.

(I also think we should optimize for applications more than libraries, similar to how cargo init has already decided that --bin should be the default over --lib.)

2 Likes
#17

I've updated the proposal with the latest thoughts

I've not seen this concern raised for any cargo-edit command before. I think it'd be better to not have an --allow-dirty because

  • The other edit commands do not use it
  • If we merge into cargo update, I would find it weird for cargo update to sometimes require --allow-dirty but sometimes not
  • It would be disruptive. Just today, I did multiple runs of cargo upgrade on the dirty repo.
1 Like
#18

Could you expand on these? Its unclear to me which side is meant to be today and the future since both use cargo update and cargo upgrade and neither use flags that exist today.

Also, what are your thoughts on the proposed plan for merging cargo upgrade into cargo update? How does your proposal compare in terms of user workflows?

#19

What are your thoughts on the proposed plan for merging cargo upgrade into cargo update? I've copied it from the comments into the original comment to make it more discoverable.

#20

The table was more about bridging vocabulary than any concrete proposal.

In general, I do like the general direction for cargo update --save. I just have a few notes and clarifying questions:

  • --resolver=minimal-direct is useful and avoids the main issue with --resolver=minimal (relying on transitive deps to be minimal-versions-correct) by making it local.
    • It's a good step towards ecosystem minimal-versions-correctness, but I worry it could end up replacing testing of minimal-versions, meaning that
      • cargo add should always work (produce a successful compilation), barring insufficient rust-version.
      • cargo update -p me should always work (produce a successful compilation), barring insufficient rust-version.
      • add/update results in a tested configuration or later.
        • Testing all configurations is temporally impossible, so we just have to assume semver-compatible updates are okay, and that no semver-minor breakage happens temporarily between versions.
      • rust-version/MSRV support isolated from (transitive) upstream.
        • minimal-versions is actually a hack here, since rust-version can go down in an update. However, it's typically sufficient,.and since transitive correctness often requires updating just for that anyway, you can update to that dependency version if you're aware it exists. Properly solved by version resolution being (optionally) rust-version aware.
    • I'm somewhat doubtful it could be presented not as a global resolver flag. Globally minimal version resolution could be e.g. update --minimal and --locked on other operations, but while I suppose update --minimal-direct works, it feels less right, as update applies to all transitive dependencies.
    • Is there ever a reason for --direct=maximal, --transitive=minimal?
    • What about flagging for rust-version-awareness? Current temperature is that version resolution should stay unaware by default (but print a warning suggesting updating toolchains or downdating) and selecting the version respecting rust-version compatibility should be an opt-in flag.
    • It feels like we're going to rapidly approach exponential resolver state selection blowup (and/or forbid otherwise reasonable combinations) unless we make these separate switches.
    • Related to cargo [update|upgrade] usability, but orthogonal work.
  • Does cargo update -p --precise (already?) allow changing major versions? Downgrading?
  • Can we reuse --precise such that --save --precise gets me full-precision dependencies?
    • Is flag repurposing like this something clap supports?
  • What is --save --locked? Is the manifest locked (causing an error; perhaps better spelled --verify), or is only the lockfile locked (giving upgrade --to-lockfile)?
  • The behavior of --pinned is now somewhat unclear, and should be clarified.
    • How is a "pinned" dependency defined?
      • VersionReq with an operator other than ^?
      • VersionReq with an elaborated upper-bound operator other than <MAJOR.0.0, <0.MAJOR.0, or <0.0.MAJOR?
      • Implicit pinning for renamed dependencies?
    • Does cargo update without any of the other new flags change behavior on --pinned?
    • Does --pinned without --incompatible only update within the pin region (i.e. only changing the lower bound), or does it update within the same major version (^)?
    • Does --pinned --incompatible opt into updating outside of the pinned region but within the same major version, or does it allow upgrading across different major versions?
    • When --pinned's upper bound is changed, what form does it take?
      • = the new selected version
      • keep the existing upper bound operator
        • matches the "don't change precision" rule
  • upgrade should for a while accept removed flags and give a nice error pointing at the new flag semantics and a migration guide before transitioning to a simple pointer to cargo update --save (and ideally still pointing to the migration guide).

Proposal refinement for --pinned:

  • Consider (actually[^1]) renamed dependencies as pinned.
  • Consider VersionReq which are not a single (implicit) caret operator to be pinned.
    • Warn on use of operators which could be expressed as (implicit) caret operators.
    • Warn on use of operators covering multiple major versions; this almost certainly doesn't do what people intend it to do (allow unifying with any major version in range; it's possible via lockfile editing but only barely)
  • Without --pinned
    • On cargo update, update the lockfile within the pinned VersionReq. (No change.)
    • On cargo update --save, bump the VersionReq to require >= the selected version.
      • If there is a single operator that expresses the current top bound and the new lower bound, use that instead of an additional lower bound operator.
      • waffling about "don't change precision"
  • With --pinned without --incompatible
    • Update pinned dependencies as-if their top bound was a caret.
    • Don't upgrade the top bound unless it changes.
  • With --pinned --incompatible
    • Show a warning that/if pinned major versions can/are changing.
    • Upgrade pinned dependencies as if they weren't pinned.
    • Keep the pinned style of top bound.

[^1] A package is actually renamed where [self package].dependencies.name != [dependency package].lib.name (after normalizing for -_); remember that lib.name and package.name are only conventionally/by-default related, and may be different.