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

I'm curious to hear your thoughts on this. I've actually been meaning to propose the opposite: we should always check-in lockfiles, not just for binaries.

  • I recently had to bisect a project without a lockfile and failed to find the offending change and am left uncertain if it came from the one of many dependencies
  • Testing for MSRV is nearly impossible without one
5 Likes

Hey, I'm also interested in your argument against committing lockfiles.

My opinion is that all projects should always commit their lockfiles. The reason is reproducibility.

  • A lockfile allows you to checkout any commit and be guaranteed that the dependencies will allow you to successfully compile the project (assuming the registry is immutable, which is the case for crates.io).
  • A lockfile does not prevent you from checking what would happen with the latest dependencies: you can delete it at any time to force a refresh (or just use cargo update).
  • On the other hand, if you don't commit the lockfile, it's virtually impossible to recover the state at the time of the commit.

In my opinion the benefits of long-term reproducible builds far outweigh the need to be explicit when you want to check behavior with the latest compatible dependencies. In particular they are invaluable when investigating regressions or trouble-shooting build errors. If a crate fails to build locally, but I see a commit with a lockfile and green CI it narrows down the source of errors to factors outside of the control of cargo.

To be honest I'm still a bit baffled by this recommendation in the cargo book:

Why do binaries have Cargo.lock in version control, but not libraries?

[...]

For libraries the situation is somewhat different. A library is not only used by the library developers, but also any downstream consumers of the library. Users dependent on the library will not inspect the library’s Cargo.lock (even if it exists). This is precisely because a library should not be deterministically recompiled for all users of the library.

A Cargo.lock in a lib repo is obviously not for users but for lib devs. A library still has binaries, they're just called through cargo test instead of cargo run; why should they be non-deterministic by default? A Cargo.lock does not prevent running your tests against the latest dependency versions, it's easy. What's hard is manually resolving the dependencies at a given time when you forgot to commit the lock file in the first place.

This is also the stance of Yarn, which is IMO the most reliable package manager for Node:

Which files should be gitignored?

[...]

Yarn actually goes even further and recommends storing the Yarn version itself in the project. I recommend also reading their arguments (they have other ones, such as a missing lock file not being sufficient to ensure you use the freshest dependencies anyway). cargo can piggyback off rust-version so we probably don't need a cargo-version.


Also to have a message on the topic of this thread. A few months ago I posted a comment about a previous version of cargo upgrade complaining that it was harder to pull the latest version to test against them. Since then, cargo-edit was updated a few times and I'm very happy with the current iteration of cargo upgrade. Thank you for your work.

3 Likes

I'm a huge fan of this even though it seems relatively unpopular. Bisection is often difficult/impossible without a Cargo.lock. Even getting a clean build on a stale repo can be problematic.

This has lead to requests for a Cargo time machine that could try to do the resolution based on a historical index state, but there's a lot simpler solution: just check in Cargo.lock.

2 Likes

This is the approach Go's modules take, and I think lockfiles for libraries are maximally powerful combined with Go's notion of Minimum Version Selection (research!rsc: Minimal Version Selection (Go & Versioning, Part 4)). Obviously that's a much bigger change than simply including them.

2 Likes

I'm very curious to hear about this as well.

1 Like

cargo-edit 0.12.0 is out. I have not (yet) revamped the UI but did smaller steps

  • --recursive changed from being on by default to the default being based on if compatible upgrades are being done
  • We now choose MSRV-compatible version requirements
    • Obviously, the resolver itself is not (yet) MSRV aware
    • The message around MSRV-incompatible versions being available is not great. It was going to make the code fairly messy while in contrast I hope the UX changes will simplify the existing code also making improving this simplerr
1 Like

Committing a lockfile doesn't mean it must be used. People are always free to rm it, do cargo clean, and build from scratch.

The lockfile is more like a documentation: it last worked with this particular set of dependencies. Useful for debugging all sort of problems with dependencies, like regressions and semver violations.

1 Like

Looking to summarize a recent discussion on where to go from here. I've tried to go back and include some details from the thread but I likely missed some.

Expected user operations across cargo update and cargo upgrade

  • Selective upgrade to latest compatible, latest incompatible, or user-provided version req
  • Selective locking to a specific version
  • Bulk upgrade to latest compatible
  • Bulk upgrade to latest incompatible
    • Selectively ignoring renamed dependencies as they are likely meant to be used with a specific major version (e.g. tokio_03)
    • Selectively ignoring non-default version requirement operators as the user might have intended to pin things

Note: "compatible" and "incompatible" are relative to the version requirements and not semver (though thats the default version requirement operator)

cargo update today

  • Works at workspace level
  • Edits Cargo.lock, not Cargo.toml
  • cargo update -p foo@ver, @ver is used to disambiguate foo within the dependency tree (must be full version)
    • --precise ver takes a full version for replacing ver in the lockfile and must remain compatible
    • --aggresive to recursively update foo

Considerations:

  • What do we optimize for (ie default)? For some workflows:
    • [[bin]] maintainers may want bulk updates of everything, most likely with a focus on compatible as incompatible might need hand updates
    • [lib] normal/build deps: maintainers may want compatible updates by hand. incompatible bulk updates are good for checking of it works but sometimes they will need to be done by hand
    • [lib] dev-only deps: like [[bin]]
  • Upcoming MSRV-aware resolver (personally leaning towards this always being enabled)
  • -Zdirect-minimal-versions
    • Users may want to keep their lockfile and requirements in-sync so what gets tested locally is at least what your dependents will use
    • This either requires a sticky -Zdirect-minimal-versions or a way to update requirements without updating the lockfile

Proposal 1: Deprecate cargo update in favor of cargo upgrade

  • cargo upgrade (formerly cargo update && cargo upgrade --to-lockfile)
  • cargo upgrade --incompatible / cargo upgrade -i
    • Change version requirements to latest incompatible, leaving compatible versions the same
    • Non-recursively updates lock file
    • Ignores pinned dependencies
  • cargo upgrade -p foo
    • Change dep names "foo" (not dependency package names)
    • Can be used with -i, --pinned
  • cargo upgrade -p foo@verreq
    • Change dep names "foo" to the specified version req
    • Open question: is --pinned needed?
  • Open question: what command handles the role of cargo update -p foo --precise ver?

Proposal 2: Separate Commands

  • cargo upgrade only does incompatible (formerly cargo upgrade --incompatible allow --compatible ignore)
  • cargo update --save does compatible

Proposal 3: Merge Upgrade into Update

  • cargo update stays the same except...
  • cargo update --save (formerly cargo upgrade --incompatible false --compatible true)
    • Concern: Inconsistency on whether --save is needed feels off to me
  • cargo update --incompatible (formerly cargo upgrade --incompatible true --compatible false)
  • cargo update -p tokio_03 de-sugars the dep name to package name + version (tokio@0.3.12)
  • cargo update -p foo --precise ver
    • Open question do we make a version requirement out of --precise and not allow controlling the precision or operators, do we hack up --precises behavior, or find a new flag?
    • Open question if ver is incompatible, do your need --save, -i, or either?
  • Open question can add support for cargo update --precise --save to fully specify all version requirements
  • cargo update && cargo update --save --locked would be an error, mirroring cargo adds behavior which has --locked apply to the manifest as well

Note:

  • All of these switch from being able to upgrade both compatible and incompatible in a single command to requiring two invocations. We are assuming that doing both is a low enough occurrence that the simplified set of flags for the more common cases justifies it
  • We have not addressed users limiting actions to normal+build vs dev
    • Cargo tree does this through the --edges flag which works when talking about graphs but not really in this context
  • Can we correctly limit upgrades that would cause incompatible links?
  • Do we bother allowing upgrading of pinned dependencies? Leaning towards "no"
  • When only upgrading incompatible version requirements, if that forces a direct dependency to do a compatible upgrade, should we also update the version requirements?

I think the next step is updating cargo-edit to one of the proposals for us to gain first-hand experience with how it works out

  • (1) and (2) would be easy to implement for just getting something better into people's hands
  • (3) would be the most different in workflow and I suspect the one that we would learn the most from people using it even if I personally hope we don't go this route
1 Like

I still don't think we should have --compatible true and --incompatible true; let's just spell those --compatible/-c and --incompatible/-i. If you pass either one, you just get that one; if you want both, pass both.

Of the various reasons given to not merge upgrade into update, the main one seems like the inconsistency of having to give --save to upgrade the manifest for compatible upgrades but not for incompatible upgrades.

I think that's worth not having two commands or a deprecated command, though, because I think it's a natural consequence:

  • Any upgrade that includes incompatible packages has to upgrade the manifest, so --save would be redundant there.
  • With a new command we could switch the default to save compatible versions in the manifest, as well, but I don't actually think that's the right default: "update the lockfile" still seems like a common action and shouldn't become more verbose.
  • We could make it consistent by adding a -c/--compatible option, and then having cargo update -c and cargo update -i both save changes to the manifest.

So:

  • cargo update stays the same
  • cargo update -c upgrades compatible and saves to the manifest
  • cargo update -i upgrades incompatible and saves to the manifest
  • cargo upgrade -c -i upgrades both compatible and incompatible and saves to the manifest.

The options I gave dropped --compatible <value> / --incompatible <value>. The difference is in the details of what happens if no flags are passed in and whether both flags exist.

I disagree when it is as destructively described as --save.

However I think this is subject to how the flag is framed and framing it a different way (your --compatible) bypasses that. Sorry, I had forgotten to call this out when doing my write up.

I'm still a little cautious of this approach as there is still a gap in the concepts of --compatible and --incompatible . This isn't a "no", just a "eh".

Then I misunderstood your parentheticals; I thought you were giving "abbreviated form (long form)" rather than "new form (old form)".

Thanks for pointing out that unclear point; I've tried to clarify it

I kinda disagree here. For bins, if you're doing an update --incompatible at all, the goal is most likely to get all dependencies on their latest version, and there's no reason to not do so for dependencies that haven't released a breaking bump.

If you're being more precise about it, then you'd (either do so originally or back up and) probably do each incompatible dependency upgrade in an independent step and commit, not attempt to do all of them at the same time.

I still don't really see --incompatible true --compatible false ever really being a desirable workflow[1]; either you want to update all dependencies in one step, or you want to update single (trees of[2]) dependencies at a time to minimize the incremental patches needed to resolve the upgrade.

So my counteroffer would be I guess roughly

  • update writes to lockfile (as today)
  • update --save writes to manifest
  • update --breaking is an error, suggesting one of the below
  • update --breaking --save writes to manifest, upgrades allowed to cross compatibility ranges
  • update --breaking --precise implies --save for just the specified dependencies (any transitive updates are lockfile only and not saved unless --save is also specified)

A interestingly meaningful command might then be cargo update -bsp tracing-subscriber; this would upgrade your manifest dependencies on tracing-subscriber and tracing to the most up to date potentially breaking release.

Interesting (but potentially poor) idea: differentiate between version = "1.0.0" and version = "^1.0.0"; --breaking will update "default bound" version requirements across compatibility ranges but not move requirements using an operator across compatibility ranges. Rationale: using a non-default requirement mode is an advanced use case (which required manual editing of the manifest at some point rather than the use of cargo add and upgrade) and the user probably knows what they're doing and want better than the tool can guess it. As an escape hatch, a --force flag will change back to the default bound requirement again (if an update outside of the bound exists) and do the update.

That idea is why I used --breaking instead of --incompatible; I'm focusing on the fact that an incompatible upgrade of the default requirement mode is potentially breaking (and thus not done without an opt-in), rather than that it's incompatible with the version requirement.


  1. Even for libs that want to keep low requirements within compatibility ranges, I'd describe what they want not as "upgrade incompatible to the most recent" but "upgrade all to the lowest compatible in their most recent compatibility range;" it's not a difference in the upgrade they're doing but in the version selection preference. ↩︎

  2. Things are a bit more interesting when multiple dependencies related by a public/peer dependency are involved, as you do want/need to upgrade them together. ↩︎

With the current cargo-edit version is there any way to upgrade all dependencies for one crate within a workspace? Or even upgrade a version requirement in one crate but not another when multiple in the workspace have the same dependency? It seems like it's only capable of operating in --workspace mode currently. (I have a few repositories containing separate library and binary crates so I commonly want to treat their dependencies differently).

Currently, everything is workspace-wide. The motivations were

  • Avoid UX confusion over the user filtering their workspace vs selecting which packages get upgraded
  • Align with cargo-update

I think this is the first time since that change that someone has expressed interest in handling upgrades per-workspace-member. Feel free to come up with a counter proposal for how it can work. I'm assuming supporting this would push us away from merging cargo-upgrade and cargo-update.