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

Regarding terminology, would it be possible to avoid the word "pinned"? It clashes with Pin. Current Cargo docs don't use the term "pin", but refer to = as "exact" version.

2 Likes

Avoiding upgrading renamed dependencies is clever! But apart from that, I actually like current cargo upgrade behavior of just bumping everything to latest, and I think it's the most robust way when projects don't actually test with -Z minimal-versions:

But some people are concerned with updating to very latest version, because it could be yanked. So upgrade to "latest - 1" maybe?

1 Like

For some crates latest - 1 could be years old. A more reliable indicator could be some sort of probation period, but I’d expect update and add to use the same behaviour for that; and again the appropriate period could vary wildly depending on the crate.

What version of cargo-upgrade are you using? Because it's not the default anymore; the default is currently to not upgrade within compatible versions.

Also @epage that PSA post is another argument for at least allowing upgrade to change precision to patch. I doubt cargo wants to carry a list of misbehaved questionably semantically versioned crates to treat specially, and not specifying patch on such crates is a really bad idea.

I want some cargo upgrade command I can run such that both

  • I don't downgrade from what's currently in my lockfile; and
  • My manifest requires the direct dependency versions in my lockfile.

So far you're offering the second but not the first. This is sufficient if I'm already direct-minimal-versions correct, but if I'm not, now I've overwritten my lockfile witness of what version I was working with and have to manually bisect to find it again, or more likely just give up and require the most recent of all my dependencies.

(As for yank avoidance: I absolutely think we should push to make it convention to publish a newer version if the most recent version has to be yanked. It can be a copy of the before version, but the goal is to always allow updating out of a yank unless the whole version has to be yanked for being unsalvageable.)

This makes me believe Cargo.lock should be committed even for libraries. (gotta run will reply to others when I'm back)

2 Likes

I don't understand this phrase. Nothing keeps you from using newer capabilities. Did you mean to say to ensure you do use new capabilities?

This is kinda ironic because if someone actually reviewed their deps and ensured the hashes in Cargo.lock match you trade supposed security fixes for supply chain attacks. The situation is not great and the solution isn't obvious.

That sounds like a good thing, yes.

How about set patch to zero unless the dependency is known for not following semver (e.g. serde)? I myself tend to check when a feature I want to use was added, but I guess not everyone has the motivation to do that.

Anyway, reading these posts seems to indicate that my idea of hiding new items is even more useful than I thought.

The problem is that cargo doesn't want to be carrying around a list of misbehaved crates. A third party crate could do this, but this is not something cargo should be in the business of doing.

The purpose of upgrading the manifest is such that you don't use capabilities introduced in versions included in the lockfile but not guaranteed by the manifest, by making the manifest match the lockfile, ensuring that the new capabilities are actually available.

Maybe complaints from users could make the crates behave? :slight_smile: I'd be perfectly OK with managing it manually for serde.

Not being in sync is a crate bug though. Checking with minimal versions should be the standard, maybe even the default cargo publish behavior.

That assumes you’re not relying on bugs being fixed in a patch release.

That's what we have lockfiles for, isn't it?

Yes, definitely. I never understood why the usual recommendation is to not commit. I want the ability to go back to some old git commit and know which exact dependencies CI and maintainer used to test this code. That's literally what lockfiles are for. Cargo.toml cannot replace this, since dependencies could have released new patch releases since then.

8 Likes

I've started to check in my Cargo.lock to help with MSRV testing. Unless/until we improve that, I wonder if we should change the recommendation.

3 Likes

If you are updating your lockfile, its likely to use new functionality and you should update your version requirements to match so that, for libs, your dependencies won't unintentionally use old versions. This direct people to avoid too-low of minimal versions while -Zminimal-versions testing in CI would be the enforcer

When the request came in for preserving precision, I had originally leaned against doing it for this reason. After mulling over it, I realized that this was too prescriptive. cargo add acts as the encouragement for fully specifying version requirements by unconditionally doing it. When cargo upgrade is run, I did not want to force things away from what the user might have done so explicitly, something they'd have to undo on every run as compared to someone who wanted fully explicit version reqs (and didn't use cargo-add) who'd just need to do it once.

A one time specifying of precision when dealing with a dependency not added by cargo-add does not seem to justify the cost to me in the UX. In part the cost is the hit to usability by overwhelming the user with options especially since this is meant to be a convenience especially for new users; users always have the option of doing things manually. The other cost is the supportability for the cargo team. Every new piece of functionality adds to the support burden of cargo and we need to be cognizant of that when adding commands, especially with the cargo team being under water at the moment. When merging cargo-add I kept the scope intentionally narrow to limit the support burden for the cargo team. One decision specifically was that these commands would not be for adjacent fixing up of styling or policy (e.g. cargo-add used to have a flag to force the sorting of dependencies). I feel editing of precision to be something different than what the user already specified in the manifest falls under that category,

In terms of issue trackers or reddit discussions, I've not seen people complain about "update/upgrade to latest" before.

To not re-hash the whole "what is semver" discussion, we also have to keep in mind that dependencies following semver to the T can still not be safe to downgrade within patch releases because crates may be relying on bug fixes. There is not a safe heuristic for picking anything other than latest, independent of what crates like serde do.

1 Like

I personally hope that cargo update will keep doing exactly what it does now (updating every version in Cargo.toml to the latest compatible version), and that cargo update --incompatible (or however we abbreviate that) will update everything to the latest available version even if it's a semver bump. --skip-compatible would be nice to have as well.

4 Likes

+1 for adding flags to cargo update rather than having cargo update and cargo upgrade doing subtly different things. This is pretty confusing. And also not at all consistent across ecosystems. I would suggest making a cargo upgrade an alias for cargo update.

Regarding flags, I think --major, --minor, --patch would be good, being both easy to remember and short to remember.

I would also suggest that the command should take any number of positional arguments where the value of the argument is either a crate name, or a crate name with a specified version (where that version can optionally be latest, or a prefix of a version (only specifying x.y rathe than x,y.z). So the following would all be valid invocations:

  • cargo update --minor
  • cargo update --minor regex
  • cargo update --patch serde serde_json
  • cargo update csv@latest
  • cargo update rand@latest time@0.3

The above with cargo upgrade would also be valid. And it would be possible to specify a lower version to downgrade as well as upgrade with the same syntax.

I do think --compatible and --incompatible or incompatible could be nice, but I can't imagine myself wanting to type them interactively if I could avoid it

1 Like

I'll have to look more at your proposal but my concern with these fields is I suspect most people do not care about bumping the absolute position but care about bumping the relative position. I want to upgrade across incompatible releases, regardless of whether the field is major or minor.

We could make --major be relative but I would be concerned with the 0.x to 1.y jump and with user confusion who would expect it to be absolute. With the 0.x to 1.y jump, I'm also putting on my cargo-release hat which has to have support for absolute positioning to make the 0.x to 1.0 jump and I'd rather not have the confusion over --major having different semantics depending on what tool is being used. While cargo-release isn't on the path to integration, I think it and tools like it are still visible enough and important enough to consider.

1 Like

This is also a bit of a terminology thing, since cargo extends the semver spec (in a defacto common way).

There's two ways of describing the extension. In my personally preferred way, it's the "0.MAJOR.MINOR extension," where the second position is the major version number while the first, the actual major version, is still 0. (This also allows distinguishing between 0.MAJOR.MINOR and 0.MAJOR.PATCH conventions; whether you're bumping the second position just for breaking changes or for all feature updates, respectively.)

More correct, though, is to say that MAJOR is still 0, and that it's the "0.MINOR compatibility extension". The semver version 2 spec says that all 0.* versions are to be considered arbitrarily incompatible. The extension is to say that all 0.N.* versions are considered compatible (unless N=0). Importantly, this avoids conflating the real MAJOR version of 0 with the nonzero version fields.

The former is perhaps a concession to the use of zer0ver, and the latter more strictly semver.

I do think cargo should strive to stick to semver-accurate naming (and thus the latter isomorphic-up-to-naming extension here). Thus update --major should refer to the semver MAJOR version number, not the leading nonzero version number (which may be the MINOR or the PATCH version).

1 Like

I suspect there is a large enough subset of users that would appreciate a --patch flag, whatever it gets called

Most cargo commands avoid positional arguments

  • To allow users to see an option's values by making the value optional (cargo update -p)
  • To avoid ambiguity in parsing the above (which is unlikely to be a problem in practice as its most likely to be the trailing arg)

As for csv@latest, did you have in mind that it would be the same as csv or would it allow upgrading across incompatible versions?

And it would be possible to specify a lower version to downgrade as well as upgrade with the same syntax.

I do like the idea of allowing downgrades. Maybe that would be a better way to describe the --incompatible flag is "force to work regardless of current version req". Now, what to summarize that for a flag, I'm unsure

  • --incompatible could refer to the version req rather than semver
  • --force but there are too many different things you could be forcing and it would still fail if you don't include --save (at least in the current plan)
  • --ignore-req but I'd like us to preserve precision, so we wouldn't be completely ignoring it

Unless someone has a better name, I think I prefer --incompatible.

I do think --compatible and --incompatible or incompatible could be nice, but I can't imagine myself wanting to type them interactively if I could avoid it

Because of length, chance for typos, or something else?

1 Like

I'm just going to echo one more time: --patch --force which upgrades version specification precision?

= "(\d+\.\d+)"/= "$1.0"/g functions, and restricting it to **/Cargo.toml may even be near[1] free from false hits, but I really do hold it as axiomatic that there should be a way to upgrade --to-lockfile such that cargo upgrade --to-lockfile --magic && rm Cargo.lock && cargo generate-lockfile --resolver=minimal-direct gives you the exact same direct dependency lockfile versions, and that this should function without downgrading the currently used versions, no matter what the current Cargo.toml state is in.

Cargo should be imho be opinionated that caret dependencies should specify the patch version. Not bumping to the most latest patch version needlessly is preferable for ecosystem compatibility, but not specifying the current patch version when adding a dependency is a misguided approach to increasing compatibility. (And IIRC cargo add always specifies the patch version and doesn't allow requesting lower precision without just directly specifying the used versionreq.)

Personally, I'd be fine with cargo upgrade always upgrading precision (iff the version is actually upgraded), and answering requests to preserve precision on upgrades with "no, that's an antipattern to underspecify caret version requirements," but I understand that I'm likely in the minority here.


  1. It could have a false hit in the metadata table, but I don't think elsewhere in the real Cargo metadata. ↩︎

1 Like