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

To expand, there are two reasons for minimal-direct

  • (original, unrelated reason) a practical way to move forward in testing minimum version requirements without being blocked on your transitive dependencies like with minimal
  • So a library author can run cargo upgrade --save without upgrading compatible version requirements while still getting the benefits of all of their transitive dependencies being updated
    • Thinking more about this, they are unlikely to care about using minimal versions for dev-dependencies and would want the latest MSRV compatible version for those. Unsure how best to balance that

Current temperature is that version resolution should stay unaware by default (but print a warning suggesting updating toolchains or downdating)

Maybe I missed a thread but I've seen one or two people say it should be unaware by default. I think its premature to say to convey that that is a consensus so far.

Does cargo update -p --precise (already?) allow changing major versions? Downgrading?

Upgrading across major versions errors about being incompatible with the version req. Similarly downgrading must satisfy the existing version req.

Can we reuse --precise such that --save --precise gets me full-precision dependencies?

That would be conflating two unrelated things with the flag.

I am still not in favor of editing precision. For others to catch up, see my comment

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

This is the unfortunate part, cargo update --locked errors if a lockfile update would happen.

As mentioned earlier, I see a minimal-direct resolver replacing today's cargo upgrade --to-lockfile.

How is a "pinned" dependency defined? ... Consider VersionReq which are not a single (implicit) caret operator to be pinned.

As of today, it is

  • Renamed dependency (assuming the tokio_01 case)
  • An upper bound was placed on the dependency besides ^ (e.g. =)

For expanding the definition to all non ^ operators, how is > pinning?

Does cargo update without any of the other new flags change behavior on --pinned?

--pinned opts you into modifying pinned dependencies. Since cargo update updates compatible pinned dependencies today, we can't change that. That means --pinned is restricted to only --incompatible upgrades.

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?

We upgrade to latest, whatever that is, like all other --incompatible changes.

When --pinned's upper bound is changed, what form does it take?

I believe we keep the same upper bound operator

Consider (actually[^1]) renamed dependencies as pinned.

Can you give a concrete example of how the "actually" case is different than the current behavior?

With --pinned without --incompatible

  • Update pinned dependencies as-if their top bound was a caret.

I worry about being able to clearly communicate the different versions of --pinned to the user and would prefer just one meaning.

Can someone explain to me what is the point of bulk-upgrading non-breaking releases? Under normal circumstances you only upgrade when you require a new feature or simply because the dependency made a breaking change and you need to manually resolve it (and have desire to stay up-to-date).

Only two workflows make sense to me:

  • cargo upgrade some_dep $version bumps minor version, equivalent to, but more convenient than manual edit
  • cargo upgrade --all-breaking edits all deps that released breaking change and leaves it to you to fix possible errors afterwards

Note that upgrading dependencies in Cargo.toml "just because they were released" is bad. It makes the crate less flexible and would be problematic in various situations, e.g. if one wants to upgrade a crate that depends on other crate containing a regression that was discovered just recently.

I'd actually love some kind of lint to tell people not to update patch versions. Maybe if patch version is non-zero. serde would have to be excluded ofc (or convinced to use semver).

1 Like

Making Cargo.toml reflect what you're actually using and testing and putting through CI.

Bulk upgrading non-breaking is less common. This is why it is no longer the default in cargo-upgrade.

If you are already doing bulk lockfile upgrades, you probably should keep your version reqs in sync to ensure you don't use newer capabilities. This is especially important as long as cargo install does not use the lockfile by default (to ensure you get security updates)

This isn't always needed. This is why the proposal encourages investing more time in -Zminimal-versions so you can do cargo update --minimal --incompatible --save. That is a mouthful but we have to conscious of compatibility. I suspect it might be useful to have a manifest resolver field to enable that by default with a CLI flag to override it for cargo update -p foo --save.

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