Open epage opened 1 year ago
Below is some background on what we are using to help come up with a design
Priorities
cargo update
Cargo.lock
and Cargo.toml
Primary use cases:
cargo update
)Secondary use cases are:
Some open questions we had
tokio_03
is pinned or not?
--exclude
these dependencies but that might be a bit of a pain to always remember to doCurrently, cargo update
is focused solely on Cargo.lock
editing
name@version
Version requirement editing is different in that
And as a reminder of the CLI:
cargo update -p foo # select a non-ambiguous `foo` and update it
cargo update -p foo@ver # selects a specific `foo` to update
cargo update -p foo --aggressive # also update dependencies
cargo update -p foo --precise <ver> # select a specific version
cargo update --locked # fail if the lockfile will change
Note: cargo add --locked
will also fail if the manifest will change
Some design tools we can consider include
cargo <cmd> --help
and cargo --list
will point people to the new namefoo@ver
syntax for versions and version requirementsCargo.lock
mostly align with Cargo.toml
, making it easier to conflate the two commands (whether merging them or keeping separate but de-emphasizing update
)Through this, I realized that the core of my concern with our previous attempts at a single command is that it feels like we are shoehorning new behaviors into cargo update
rather than making the behavior cohesive.
cargo update --incompatible
on a blog post, can I predict what will happen if you do cargo update
? No, because --save
is needed to match behavior--package
be both for package IDs and dependency names to make some of the cargo upgrade
workflows work--precise
to allow control over version requirementsI also realized that my Windows Updates vs Windows Upgrades analogy for cargo update
and cargo upgrade
breaks down a little because cargo upgrade
can do "upgrades" that are on the level of cargo update
(say we call it cargo upgrade --compatible
). The difference is in the target audience (yourself vs your dependents)
cargo update
only changes version requirements as a side effectThe primary role of cargo update
has been to update your active dependencies (ie Cargo.lock
). We do not plan to change that role but give the user control to force it to update in situations that were previously unsupported, particularly updating the Cargo.toml
if need be.
Behavior:
cargo update --incompatible
/ cargo update -i
will force (ie update version reqs) update unpinned, incompatible versions (no other dependencies)
--breaking
or something else. The name depends on what expresses the concept clearly especially in light of any pinning behavior we have-i
unused would keep the door for a --interactive
to be addedcargo update -p foo --precise ver
will force the update to happen, only erroring if we can't (don't own relevant version reqs, is pinned), even if its incompatible but unpinned. Version requirement is only changed on incompatible.
Somewhere between deferred and rejected (speaking for myself): Support in cargo update
for writing to the manifest for non-breaking changes, like bulk compatible upgrades of version requirements (ie a -save
flag) which was one of our lower priority workflows. A --save
flag is more about updating versions for your dependents, which while important for having valid lower-bounds on version requirements, doesn't fit with the existing model of cargo update
. Maybe in the future we can find a way to express this in cargo update
that fits with how it works or maybe another command can take on this role. We just aren't wanting to distract our efforts for handling most of the use cases to handle this one. See #10498
While this tells a cohesive story, a part of me is somewhat concerned that this goes beyond the name update
.
cargo update
improvements--aggressive
to --recursive
(make --aggressive
an alias for the new name) (#12544)foo@x.y.z
so that foo@x
will work so long as its unambiguous (which it should be), much like cargo install foo@x.y.z
supports auto-selecting the y
and z
fields (picks latest) (#12614)cargo upgrade
into cargo update
(#13372)
cargo outdated
experience (#4309)package
argument to cargo update
, removing the need for --package
(#12545)
0..=1
valuesThese are alternatives I had considered that help give an idea of what I mean by fitting into cargo.
cargo update
always modifies Cargo.toml
We deprecate cargo update
and a cargo upgrade
always updates both files
We migrate to minimal-version resolution by default
cargo update
becomes less useful and we move it out of the spot light.cargo upgrade
is added that is focused on editing version requirementsSeparate commands for Cargo.lock
(update
) and Cargo.toml
(upgrade
)
apt dist-upgrade
, maybe it could be cargo req-update
?^
as we document them as being the same thing and sometimes people use ^
just becausecargo update && cargo update --incompatible
vs cargo update --incompatible
doing both as there are people who do want them separated and running two commands, while annoying, allows us to cover more use cases.This proposal has been up here and on internals for a bit now without any major concerns raised.
@rfcbot fcp merge
Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
Having been fairly involved in this discussion via the internals thread, I'm happy to see this move forward in a direction that I wholeheartedly support. @epage thanks for pushing this forward!
:bell: This is now entering its final comment period, as per the review above. :bell:
Can --incompatible
be used in conjunction with --package
?
Just recall https://github.com/rust-lang/cargo/issues/11974 and not sure about the implication of cargo update --incompat -p <pkgid>
.
Can
--incompatible
be used in conjunction with--package
? Just recall https://github.com/rust-lang/cargo/issues/11974 and not sure about the implication ofcargo update --incompat -p <pkgid>
.
Yes, we'd update the version requirement, if an incompatible version exists, and then run the normal code.
cargo upgrade
can do "upgrades" that are on the level of cargo update (say we call it cargo upgrade --compatible). The difference is in the target audience (yourself vs your dependents)
This is probably stating the obvious. But if a dependency is imprecise, current cargo upgrade
may have no effect where current cargo update
would.
Let's say a dependency is serde_derive = "1"
in Cargo.toml
. And it's version = "1.0.183"
in Cargo.lock
. Then current cargo upgrade
would change nothing locally, and wouldn't force dependants to get the latest version.
The separation of the tools and the clarity on each one's remit makes this, if not immediately predictable, at least easily understandable. But if this will no longer be the case, then this distinction, or a change from that behavior, should be made clear.
For what it's worth, I don't like this behavior, and I'll implement --force-precise
for personal use. Especially since I only used imprecise deps in old projects.
FYI on zulip I brought up the idea of a pedantic
machine-applicable (ie --fix
able) lint to flag imprecise dependencies.
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
- Keeping
-i
unused would keep the door for a--interactive
to be added
Really hope --interactive
.
yarn is one of the nodejs package managers, and it supports upgrade-interactive
.
yarn 1
https://classic.yarnpkg.com/lang/en/docs/cli/upgrade-interactive/
yarn 4
https://yarnpkg.com/cli/upgrade-interactive
Not similar to cargo update
, but similar to cargo-edit's cargo upgrade
, but have a terminal ui.
For example, which supports
Press <up>/<down> to select packages. Press <enter> to install.
Press <left>/<right> to select versions. Press <ctrl+c> to abort.
? Pick the packages you want to upgrade. Current Range Latest
> @types/node --------------------------------- ◉ 16 ----------- ◯ 16.18.91 ----- ◯ 20.11.30 -----
typescript ---------------------------------- ◉ 3.7 ---------- ◯ 3.7.7 -------- ◯ 5.4.3 --------
I'm working on this. I should have a draft PR up soon.
@torhovland which part of this are you working on? In a lot of ways, --precise
vs --breaking
are very different and I suspect they would best be handled as separate PRs.
Any high level details of what your understanding is for resolving the half you are working on?
@epage I've been looking into --incompatible
(--breaking
).
So far I have introduced an ops::update_manifests()
in front of ops::update_lockfile()
. It reuses some code from cargo-edit to go through all dependencies, query latest versions, check if they are incompatible, update the manifest TOML if they are, then write the modified TOML to file. Then I'm simply resetting the workspace before calling ops::update_lockfile()
.
That is probably a little too simplistic, though. In particular, it won't work well with --dry-run
, because ops::update_lockfile()
will need the updated manifests. But maybe this can be improved so we don't have to reset the workspace. Also, I suppose we shouldn't do a full ops::update_lockfile()
, but only update the dependencies that got modified by ops::update_manifests()
.
If you have any thoughts about this, feel free to share.
The other problem with modifying the manifests before anything else is that we will leave the workspace in a half-updated state on failure.
My assumption was that we'd edit the Dependency
s for workspace members (maybe all path sources?) and on success, we'd find all Cargo.toml
all dependency blocks, including workspace.dependencies
, and edit them then.
. Also, I suppose we shouldn't do a full ops::update_lockfile(), but only update the dependencies that got modified by ops::update_manifests().
Yes, I believe the feature as specified says we shouldn't do compatible updates. We probably want the semantics of cargo check
after editing Cargo.toml
in case a requirement change causes a cascade of compatible updates.
I submitted a draft PR now. I've changed the code so it can mutate the manifests without needing to write out intermediate files and resetting the workspace. But this means there are two kinds of manifest mutations going on, one for the manifests in memory, and one for preserving formatting when writing out modified manifest files. See the PR for details.
I haven't changed update_lockfile()
yet.
I'll happily take any feedback.
I have started working on the --precise
part of this issue too.
Thanks!
I've added to the task list another test case we need (and to pin down the semantics of): cargo update --breaking only-compatible renamed non-semver-operator transitive-dep
All right. We already cover only-compatible
and renamed
, though. Unless you mean they are important in that new, specific test case for some reason?
We cover those for cargo update --breaking
but I'm not seeing those covered for cargo update --breaking [SPEC]
. We only test just-foo shared ws
which are all candidates for upgrading. I'm concerned about the semantics of a user explicitly naming a package that --breaking
doesn't apply to.
It might also be of interest to have a mixed workspace, where one package depends on renamed
without a rename while another depends on it with a rename. This would need to be a separate test as this wouldn't error but the other case could potentially (depending on what semantics we want to give it)
This bug will need a separate PR: https://github.com/rust-lang/cargo/pull/14049#discussion_r1636021228
cargo update --breaking compatible@1.0.10
(where compatible@1.0.10
does not exist), but there is no lock file yet. This is consistent with the non-breaking update. See https://github.com/rust-lang/cargo/pull/14049#discussion_r1645783714.semver_ext::matches_prerelease
needs some improvement in order to support precise updates to prerelease versions. Fixed in #14140. See the new tests precise_prerelease
and update_precise_breaking_pre_release_explicit_version_req
.update_precise_breaking_shared_non_ws
.UPGRADING
and UPDATING
messages and if it is OK that the former outputs version requirements (^1.0
) while the latter outputs versions (v1.0.0
). See https://github.com/rust-lang/cargo/pull/14140#discussion_r1658829398.Cargo.toml
or Cargo.lock
changes when --locked
is passed in2.0.0-beta.1
to 2.0.0-beta.2
2.0.0-beta.1
to 2.0.0
2.0.0-beta.1
to 3.0.0-beta.1
should worksConsider an error message if the command completed without doing any upgrades.
Not sure this makes sense to me? Does cargo update
fail when there's nothing to update?
The non-breaking update does not fail, but outputs something like this:
[UPDATING] `dummy-registry` index
[LOCKING] 0 packages to latest compatible versions
[NOTE] pass `--verbose` to see 3 unchanged dependencies behind latest
The breaking update currently only outputs this:
[UPDATING] `dummy-registry` index
So maybe the breaking one should add:
[UPGRADING] 0 packages to latest compatible versions
as well as a feature to list the other packages using --verbose
?
ops::update_lockfile()
).So a breaking update that ends up not upgrading anything will now output this:
[UPDATING] `[..]` index
[LOCKING] 0 packages to latest compatible versions
[NOTE] pass `--verbose` to see 3 unchanged dependencies behind latest
would it be possible / is it currently planned to also update non-breaking versions in the Cargo.toml
file?
cargo upgrade
from cargo-edit currently supports that
name old req compatible latest new req
==== ======= ========== ====== =======
ratatui 0.26.1 0.26.1 0.27.0 0.27.0 // this is already supported in nightly
serde_json 1.0.117 1.0.118 1.0.118 1.0.118 // this is missing currently
i don't know if this is a desired feature, but since cargo upgrade
already does it i am just wondering if this hasn't been discussed yet, has been discussed and has been decided against or has discussed and decided for, but just hasn't been implemented yet or if it is in another issue entirely?
That is covered in https://github.com/rust-lang/cargo/issues/12425#issuecomment-1659453818
Somewhere between deferred and rejected (speaking for myself): Support in cargo update for writing to the manifest for non-breaking changes, like bulk compatible upgrades of version requirements (ie a -save flag) which was one of our lower priority workflows. A --save flag is more about updating versions for your dependents, which while important for having valid lower-bounds on version requirements, doesn't fit with the existing model of cargo update. Maybe in the future we can find a way to express this in cargo update that fits with how it works or maybe another command can take on this role. We just aren't wanting to distract our efforts for handling most of the use cases to handle this one
These are alternatives I had considered that help give an idea of what I mean by fitting into cargo.
cargo update
always modifiesCargo.toml
- This would be a breaking change
- This would get in the way of people intentionally keeping separate versions from version requirements
For my use case of cargo upgrade
(which cargo update --breaking
has been advertised as a replacement of) this would be the mode I want. I'm fine with it being an optional flag to do so, but currently cargo update --breaking
is a step back functionality wise (performance is of course better though).
Quoting myself from https://github.com/rust-lang/cargo/issues/14204 (apparently @epage prefers feedback in this issue instead):
I was led to believe that
cargo +nightly -Zunstable-options update --breaking
should be a drop-in replacement forcargo upgrade -i
. But it isn't. Specifically it doesn't seem to upgradeCargo.toml
unless there is a breaking change.I use
cargo upgrade
to upgrade to new minor versions inCargo.toml
too. There are several reasons for this:
- As I'm on the latest version when developing, I can't be sure I'm not relying on some new API that didn't exist in the version that is declared in
Cargo.toml
. While I believe there is some "min-version" flag to test with, I prefer to just force everything the latest. (No I don't care about LTS distros like Debian etc, not one bit, nor about people who are not on the most recent stable Rust. My MSRV is N.)- If there are any security issues in a dependency I'd also prefer to force the latest version there too.
As such it seems I will probably have to port cargo-upgrade from cargo-edit to sparse registry myself at some point. When and if I have time. As cargo update --breaking
just doesn't cut it for my use case.
Personally I don't know if we took the right approach to pre-releases in #14250 — I left more of a comment on that issue (https://github.com/rust-lang/cargo/issues/14178#issuecomment-2275688972), but my overall proposal is:
This, what I would consider a bug, showed up with derive-more
when cargo upgrade --breaking
first failed to upgrade from 1.0.0-beta.6
to 1.0.0-beta.7
, and again (more egregiously) from 1.0.0-beta.7
to 1.0.0
.
Let me know if I'm misguided here / if you'd like me to try to submit a PR!
Personally I don't know if we took the right approach to pre-releases in #14250 — I left more of a comment on that issue (#14178 (comment)), but my overall proposal is:
- If version is currently stable / non-pre-release, then ignore pre-releases
- If version is currently a pre-release, upgrade it as normal — including upgrading to the stable version!
This, what I would consider a bug, showed up with
derive-more
whencargo upgrade --breaking
first failed to upgrade from1.0.0-beta.6
to1.0.0-beta.7
, and again (more egregiously) from1.0.0-beta.7
to1.0.0
.Let me know if I'm misguided here / if you'd like me to try to submit a PR!
Isn't this the last checkbox of https://github.com/rust-lang/cargo/issues/12425#issuecomment-2186198258?
Should it also upgrade from 1.0.0-alpha.1
to 1.0.0-beta.1
? How about from 1.0.0-beta.1
to 1.0.0-pre.1
?
I think all of those should be in scope for --breaking
.
I would say yes. If you decide to go with a prerelease, it usually means you want the latest thing. Also, I remember having read somewhere that prereleases are incompatible between each other, so --breaking
should bump to the latest because each version is incompatible.
But what if 1.0.0-pre.1
is older than 1.0.0-beta.1
?
I mean, you're not going to have --breaking
upgrade from 1.2.3 to 1.4.5 if 2.0.0 is also available, right, independent of age?
Should it also upgrade from
1.0.0-alpha.1
to1.0.0-beta.1
? How about from1.0.0-beta.1
to1.0.0-pre.1
? And But what if 1.0.0-pre.1 is older than 1.0.0-beta.1?
I reckon we should try to stick to semver's rules here (https://semver.org/#semantic-versioning-specification-semver):
Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows:
Identifiers consisting of only digits are compared numerically.
Identifiers with letters or hyphens are compared lexically in ASCII sort order.
Numeric identifiers always have lower precedence than non-numeric identifiers.
A larger set of pre-release fields has a higher precedence than a smaller set, if all of the preceding identifiers are equal.
Example: 1.0.0-alpha < 1.0.0-alpha.1 < 1.0.0-alpha.beta < 1.0.0-beta < 1.0.0-beta.2 < 1.0.0-beta.11 < 1.0.0-rc.1 < 1.0.0.
And would agree with @djc that moving to a higher version is better than taking into account age — though really there should probably be a lint for anyone trying to push a lower prerelease version than is already published (if there isn't already?)
And just to have this all in the same place, from https://github.com/rust-lang/cargo/issues/14178#issuecomment-2275791649:
I am not sure about if 2.0.0-beta.1 to 3.0.0-beta.1 should works
Perhaps that's better as 2.0.0-beta.1
to 3.0.0
? That way you have to opt into each pre-release series, but you never "skip over" a chance to return to stable?
So this upgrading between pre-releases only happens when all of the pre-releases have that same MAJOR, MINOR, and PATCH versions, and otherwise it looks for a newer stable version to upgrade to?
Yes, I agree the logic should be:
(Note that even though I'm writing that, that's not what I would like to use. I would like something like #14372. But it could be a separate flag.)
From what I understand of the current semantics, they are what they should be.
--breaking
is a "force" flag to update a dependency when a ^
operator would otherwise prevent the update.
At this time, updating through pre-releases is not a breaking change, so --breaking
does not apply (see #6016, #2222, #3263). If you work around that with =
, then --breaking
still does not apply.
To update pre-releases that use ^
, then we need the feature that is left out of this Issue, a way to bump the minimum of version reqs to what is in the lockfile (see #10498).
`"^2.0.0-beta.1" when "^2.0.0-beta.2" is available
cargo update --breaking
should not change compatible version requirements
"^2.0.0-beta.1"
when `"^2.0.0" is available
cargo update --breaking
should not change compatible version requirements
`"^2.0.0-beta.1" when "^3.0.0-beta.1"
We need to clarify the semantics for how to handle this case
Problem
A lot of incompatible upgrades have a small enough of breakages that a "upgrade all of my incompatible version requirements" would make it easier to go through this process.
This is currently exposed in the
cargo upgrade
commandProposed Solution
Tasks
cargo update --breaking only-compatible renamed non-semver-operator transitive-dep
--precise <breaking>
Deferred
See https://github.com/rust-lang/cargo/issues/12425#issuecomment-1659453818
Previously, https://internals.rust-lang.org/t/feedback-on-cargo-upgrade-to-prepare-it-for-merging/17101/141
Unresolved questions
--breaking
to focus on semver (as this is limited to^
)--incompatible
applies to all version req operators and doesn't have a good short flag (-i
is generally assumed to be "interactive")--major-versions
though we have the issue of "absolute major" (x.0.0
) and "relative major (also0.x.0
,0.0.x
).--major
--latest
because of a "latest" tagNotes
Related
13908
10498