feat(cargo): remove dependence on install-update

[InnocentZero]

Fixes the dependency of cargo update on cargo-install-update binary and also fixes the issue where packages are installed without the relevant features.

Standards checklist:

• The PR title is descriptive.
• I have read CONTRIBUTING.md
• The code compiles (cargo build)
• The code passes rustfmt (cargo fmt)
• The code passes clippy (cargo clippy)
• The code passes tests (cargo test)
• Optional: I have tested the code myself

For new steps

• Optional: Topgrade skips this step where needed
• Optional: The --dry-run option works with this step
• Optional: The --yes option works with this step if it is supported by
  the underlying command

If you developed a feature or a bug fix for someone else and you do not have the
means to test it, please tag this person here.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 60 lines in your changes are missing coverage. Please review.

Project coverage is 6.86%. Comparing base (32add8f) to head (f4954b1).
Report is 8 commits behind head on main.

❗ Current head f4954b1 differs from pull request most recent head 0cb02da

Please upload reports for the commit 0cb02da to get more accurate results.

Files	Patch %	Lines
src/steps/generic.rs	0.00%	60 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #796      +/-   ##
========================================
+ Coverage   6.60%   6.86%   +0.25%     
========================================
  Files         37      37              
  Lines      12199   11743     -456     
========================================
  Hits         806     806              
+ Misses     11393   10937     -456     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[InnocentZero]

@SteveLauC can you tell me what the failing test means? Because I cannot even understand what it is doing

[SteveLauC]

@SteveLauC can you tell me what the failing test means? Because I cannot even understand what it is doing

No worries about it, the test coverage CI is simply broken

[SteveLauC]

Hi, thanks for your interest in contributing to Topgrade!

Fixes the dependency of cargo update on cargo-install-update binary

Well, I do think it is good to get rid of an external dependency, but cargo-install-update has been pretty much the defacto option to update binaries installed through cargo, for Topgrade, using cargo-install-update makes us:

1. Easy to maintain
2. No need to reinvent the wheel

so I am not quite sure we should remove it.

For the impl of this PR, it checks the cargo version and only does a manual update if the version is greater than or equal to 1.77, so I am guessing the cargo folks have done something great to make cargo more like a package manager in that release?

And it seems that the current impl does not handle those binaries installed from git.

fixes the issue where packages are installed without the relevant features.

So IIUC, this is a bug of cargo-install-update, I think we should file an issue in the upstream repo.

[InnocentZero]

For the impl of this PR, it checks the cargo version and only does a manual update if the version is greater than or equal to 1.77, so I am guessing the cargo folks have done something great to make cargo more like a package manager in that release?

Prior to that version, cargo would error out on reinstalling a package. Now it will cleanly exit and upgrade the package if needed. In that regard this is a massive improvement. See this commit and this addition in cargo-update's README.

And it seems that the current impl does not handle those binaries installed from git.

This is a WIP PR, sorry I should have made it clear.

fixes the issue where packages are installed without the relevant features.

So IIUC, this is a bug of cargo-install-update, I think we should file an issue in the upstream repo.

Actually not! (kinda yes but also no). So cargo install-update -a command does not check the features of the packages it is updating, but it can do so individually if supplied with the appropriate flags (similar to cargo install xyz --features A,B). However to check those flags you have to read .crates2.json and NOT .crates.toml (which I suppose is being read by both this repo and upstream rn). But if you go all the way to read .crate2.json, updating it yourself using cargo install XYZ isn't really a challenge after that (just one extra command).

Apart from that, the imported dependencies are very standard and relatively small, so it did not bother me much to personally import them. serde_json will also be really helpful in the future considering a lot of package managers store their package configs as json.

[InnocentZero]

One argument made against this could be that if you have multiple packages with feature flags installed in your system, you'd spawn multiple cargo commands from within topgrade to install them but the bottleneck is the compilation anyways if only cargo step is run and topgrade in general prompts users for manual intervention if multiple steps are being run.

[SteveLauC]

Sorry for the late response, quite busy with my work on workdays.

@SteveLauC can you tell me what the failing test means? Because I cannot even understand what it is doing

No worries about it, the test coverage CI is simply broken

The CI has been fixed, you can rebase your branch.

---

Prior to that version, cargo would error out on reinstalling a package. Now it will cleanly exit and upgrade the package if needed. In that regard this is a massive improvement. See this commit and this addition in cargo-update's README.

Thanks for showing me this! I tested this locally and it was indeed changed. Though the linked commit is relatively old, would you like to show me the exact commit that made this change to 1.77? I try to find it in the CHANGELOG, but no luck.

---

Actually not! (kinda yes but also no). So cargo install-update -a command does not check the features of the packages it is updating, but it can do so individually if supplied with the appropriate flags (similar to cargo install xyz --features A,B). However to check those flags you have to read .crates2.json and NOT .crates.toml (which I suppose is being read by both this repo and upstream rn). But if you go all the way to read .crate2.json, updating it yourself using cargo install XYZ isn't really a challenge after that (just one extra command).

Well, I see this as a bug and it is a fair reason to me to implement this in Topgrade. So I am ok with the PR.

There was already issues reporting this to cargo-update: nabijaczleweli/cargo-update#251

---

Apart from that, the imported dependencies are very standard and relatively small, so it did not bother me much to personally import them. serde_json will also be really helpful in the future considering a lot of package managers store their package configs as json.

serde_json is light, it is ok to import it.

---

One argument made against this could be that if you have multiple packages with feature flags installed in your system, you'd spawn multiple cargo commands from within topgrade to install them but the bottleneck is the compilation anyways if only cargo step is run and topgrade in general prompts users for manual intervention if multiple steps are being run.

I think we should not spawn multiple child processes unless we find a good way to handle errors that happens in child processes. And, IMHO, Topgrade is not something that needs to be performant, so I think we can do the update sequentially.

[SteveLauC]

One question I have in my mind, is the JSON defined in this .crates2.json file structured? If so, we can define some concrete helper types in Topgrade, and deserialize the file to the types rather than serde_json::Value.

[SteveLauC]

BTW, looks like cargo-update #251 has just been fixed, see comments of that issue for more information.

[InnocentZero]

BTW, looks like cargo-update #251 has just been fixed, see comments of that issue for more information.

Oh, if that is the case, then idt we need to have this PR anymore since just using that would be better. I'd have tested the latest PR but I don't have any outdated binaries that need feature flags either.

[SteveLauC]

Oh, if that is the case, then idt we need to have this PR anymore since just using that would be better.

So closing:)

[glima]

Weird, because I'm pretty sure one of my packages got updated with less features (via topgrade) and I have:

❯ cargo install-update -V
cargo-install-update 14.0.0

[SteveLauC]

Weird, because I'm pretty sure one of my packages got updated with less features (via topgrade) and I have:

Hmm, that's weird, per the upstream issue, this should be fixed in cargo-install-update 14.0.0, if you can reproduce it, please report it to the upstream repo.