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

User Care-Abouts

  • Separate cargo update (for lock file) and cargo upgrade (for manifest) commands is confusing to some users
  • Some application authors want to upgrade to get the latest bug fixes and features
  • Some library authors want to keep up on breaking changes but otherwise want to keep version requirements low to allow dependents to choose the version right for them (lower audit churn with fewer upgrades, workaround bugs, MSRV, etc)
  • Frequently, users will want to defer certain breaking changes because of the level of churn or missing functionality
  • Users need to be able to force a specific minimum version (and test with it) for MSRV or another need
  • Crate authors want their manifest to match their lockfile to ensure that they do not accidentally use new functionality that breaks their users building with an old version.
    • minimal (direct?) versions is another tool in solving this
    • Stablizing #[stable] attributes and warning when an API item is newer than the minimal version is being used is another tool for solving this problem
  • Crate authors want to be able to test out their changes (either a --dry-run to opt-out of changes or a --save to opt-in)
  • Crate authors want to script checking if there are any breaking changes (by checking the exit code)
  • Sometimes crate authors need to constrain (or pin) a dependency to workaround a problem and need to specifically decide when to modify the extra constraints
  • Sometimes crate authors need to rename a dependency to access an old version and a new version at the same time and don't want to upgrade the old version to the new version, negating its purpose
  • Some crate authors specifically chose their version requirement precision while authors want to ensure they use full precision always
  • Some maintainers would like to have isolated PRs for semver-compatible and semver-breaking upgrades as semver-breaking upgrades deserve a bit more scrutiny (is the crate exposed in the API making the upgrade a breaking release? were there behavior changes that might not have been caught by tests?)

Proposal

Merge cargo updates behavior into cargo upgrade and deprecate cargo update

  • cargo upgrade would unconditionally work across the entire workspace
    • This will also help when supporting [workspace.dependencies]
  • cargo upgrade would modify Cargo.toml and Cargo.lock in tandem
    • When changing the Cargo.lock file, we update the minimums in version requirements in Cargo.toml to match
    • If a user is precise on their desired upgrade (cargo upgrade -p clap@^3.2.5), the lock file will reflect that version and not aggressively upgrade it
      • cargo add should be updated to reflect this behavior
  • Replace --aggressive with --recursive=<true|false> (default true) to clarify the greater distinction being made now between direct and indirect dependencies
  • Update cargo generate-lockfile to be a more complete lockfile editor, including a --package os_str_bytes@6.3.0 6.0.0 flag for precisely controlling indirect dependencies (as cargo upgrade can only precisely specify direct dependencies)
  • cargo upgrades --package and --exclude flags would accept crate names rather than dependency names (in cases of renames) to be consistent with cargo update
  • cargo upgrade would cause git dependencies to be updated

TBD How to select between upgrading compatible, incompatible, and pinned dependencies

  • --compatible, --incompatible, and --pinned are additive but if none are specified, a default combination is selected (--compatible or --compatible --incompatible?)
  • --compatible, --incompatible, and --pinned are mutually exclusive and --compatible is the default

Evaluation

  • In effect it emulates the workflow for -Z minimal-direct-versions workflow so long as the user stays within cargo add / cargo upgrade and commits their Cargo.lock
  • Ecosystem churn in adjusting workflows from cargo update to cargo upgrade
  • Potential complications from the interactions from locking direct and indirect dependencies making it harder for the version requirement to reflect what is selected

Alternatives

Keep cargo upgrade and cargo update separate

cargo update would focus on the lockfile and compatible upgrades

  • A --save flag would be added to write-through to the manifest
  • Suggest cargo upgrade is not on the latest incompatible version

cargo upgrade would focus on incompatible upgrades and the manifest

  • Only upgrade incompatible and pinned, suggesting cargo update --save for compatible upgrades

Evaluation

Merge cargo upgrade into cargo update

  • Add --save flag to update direct dependency version requirements 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 pinned version requirements to incompatible versions
  • Provide summary table at the end for direct dependencies

TBD How to select between upgrading compatible, incompatible, and pinned dependencies without breaking compatibility

Evaluation

  • Extra ceremony to upgrade cargo update --incompatible --save
    • Some suggested to make --save implied by --incompatible but then the user sometimes needs it, sometimes doesn't
  • No way to specify precise version requirements
  • So long as the user always passes in --save, this emulates the workflow for -Z minimal-direct-versions

cargo upgrade (any variant) does not modify the repo by default

  • 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

Explicit configuration over a dependencies upgrade policy

Currently, renamed and overly constrained dependencies are considered pinned and require opt-ing in to upgrade.

In addition or as an alternative to those assumptions, we could add manifest depednency fields or cargo config fields to control the upgrade policy for a dependency

e.g. even just na = { package="nalgebra", version="0.31.1", pinned=false }, or package.update.precision = "full", or lots of other knobs which could be persistent in the manifest rather than ephemeral CLI flags.

Evaluation

  • Users can ensure exactly the policy they want
  • More features means more documentation for users to go through and more cognitive overhead in using manifests
  • So long as users use cargo add, precision = full is unneeded
  • "pinned" is redundant with a lot of version req syntax and doesn't tell the tool "how pinned" it is (what are safe compatible upgrades)

Prior Art

Prior art

Status Updates

(As of 8/1/2022)

  • Updated proposal to merge cargo-update / cargo-upgrade
6 Likes
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.

1 Like
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.

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

3 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.)

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