Closed CAD97 closed 2 years ago
Can you confirm which version of cargo-upgrade
you are using?
I meant to include that in my demo but it didn't make it in 🙃
〉playground〉cargo upgrade -vV
cargo-upgrade 0.10.2
〉playground〉cargo -vV
cargo 1.62.1 (a748cf5a3 2022-06-08)
release: 1.62.1
commit-hash: a748cf5a3e666bc2dcdf54f37adef8ef22196452
commit-date: 2022-06-08
host: x86_64-pc-windows-msvc
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:Schannel)
os: Windows 10.0.22000 (Windows 10 Education) [64-bit]
Oh right, forgot about the rename behavior.
If you run cargo-upgrade with --verbose
, you'll get a warning that tokio_01
was left unchanged because we assume renamred dependencies are pinned dependencies.
We hint at this behavior with the note
note: Re-run with
--pinned
to upgrade pinned version requirements
As I'm thinking through this, some things I'm going to be considering (and welcome others thoughts on)
cargo upgrade --to-lockfile --pinned
sufficient for this resolving this?--pinned
not tried and what could improve discoverability?tokio_01
as the name implying that i is meant to not be upgraded to tokio 0.2--to-lockfile
?
--to-lockfile
's presence?--to-lockfile
just imply --pinned
?--pinned
was tried... in the actual repo, not this example. --pinned
functioned properly in the test example now that I tried it.
I think why it "failed" in the actual repo is that the precision was 0.1
and not 0.1.0
, so cargo-upgrade decided to preserve that precision.
So I think the takeaway here would be
--to-lockfile
--to-lockfile
implying --pinned
is probably absolutely fine
=
pins, leave as is (assuming it matches the lockfile)<
pins, upgrade the lower bound only^
"--to-lockfile
not upgrading dependency requirements to the full precision in the lockfile.
--precise
flag to upgrade to a full major.minor.patch requirement, and note if dependency requirements weren't upgraded (just when using --to-lockfile
?) all the way to the lockfilecargo upgrade --to-lockfile
should make it so people depending on me get at least the versions in my lockfile / update --minimal-versions
does not change the versions of direct dependencies
I remembered the big reason why I was blaming cargo-upgrade for having a bug: cargo upgrade -p tokio --to-lockfile --pinned
(or equivalent) was running cleanly "no upgrade needed" despite me dancing between maximal and minimal versions in the lockfile and seeing the dependency version change. If upgrading a single dependency, upgrade absolutely should be verbose about what requirement was in the lockfile, what requirement is now in the lockfile, and what the full upgraded version is, even if no change occurs. Upgrading multiple deps should not note no-changes, but when only considering a single dep, that's the only state/information the user asked for.
Another note on renames; ideally:
In the specific case of
[dependencies."lib.name"]
package = "package.name"
this should not be considered a pinned dependency, and should be treated as a normal dependency.
There's two reasonable ways that this can come into being:
name = { package = "name"
might be a rename, because lib.name != package.nameSimilarly, renaming to __dep
maybe should be ignored, since __
is the convention to not show features even though it's publicly available. Though at the same time, __dep = { package = "dep", optional = true }
may be the pattern for "phantom dependencies," so perhaps they should be considered pinned? Probably safer to not have the special case for certain renames not being considered pinned, and just have all (actually) renamed dependencies be pinned.
e. If upgrading a single dependency, upgrade absolutely should be verbose about what requirement was in the lockfile, what requirement is now in the lockfile, and what the full upgraded version is, even if no change occurs
I think something that will help is that I'm considering adding a cargo outdated
like report which will include the reason why a dependency was not upgraded or not fully upgraded.
Similarly, renaming to dep maybe should be ignored, since is the convention to not show features even though it's publicly available
FYI the cargo team has not yet decided if it will adopt this docs.rs convention, see https://github.com/rust-lang/cargo/issues/10794#issuecomment-1189360723
I was absolutely bamboozled by --to-lockfile not upgrading dependency requirements to the full precision in the lockfile. ... Suggested resolution: add a --precise flag to upgrade to a full major.minor.patch requirement
I lean towards keeping the current behavior and not adding a flag. I think the table view will help raise visibility of what is happening to be sufficient.
If someone doesn't like the existing behavior, its a one time change to their lock file (set full precision)
A flag effectively would automate that one-time change and doesn't seem to justify the cost of increasing the surface area of the UX.
If someone doesn't like the existing behavior, its a one time change to their lock file [sic; manifest] (set full precision)
A flag effectively would automate that one-time change and doesn't seem to justify the cost of increasing the surface area of the UX.
While it is true it is a one-time cost to make the migration, in a workspace such as https://github.com/tokio-rs/tracing/pull/2246, it's not a trivial change to do by hand due to there being multiple manifests involved. For that PR I did a s/= "(\d+\.\d+)"/$1.0/g
search-replace to increase precision across the workspace.
A more difficult task would be doing so for a single dependency rather than all dependencies. Ideally, in such a minver correctness diff, instead of {upgrade to lockfile; downgrade to msrv} you'd upgrade each dependency only as-needed, which is significantly more work, especially if it involves manual steps[^1] (e.g. to increase dependency precision).
Personally I would prefer --to-lockfile
to increase precision (such that the dependency specification actually requires the version in the lockfile), but I can at least understand the choice otherwise :)
[^1]: If cargo-upgrade supported increasing the precision, the process would be automatable. (Though perhaps it'd be better off using toml-edit directly rather than going through cargo-upgrade?) Roughly
- for features in [`--no-default-features`, ` `, `--all-features`]
- while !`cargo minimal-versions check $features`
- extract what crate failed to compile
- if !`cargo update -p $failed; cargo upgrade $failed --to-lockfile`
- find the direct dependency edge: `cargo minimal-versions tree -i $failed`
- if !`cargo update -p $direct; cargo upgrade $direct --to-lockfile`
- throw "manual intervention required to avoid minver-incorrect $failed"
One of these days I'm going to get annoyed enough to write this tool.
Example (from nushell):