Separate error code for cargo install failing due to binary already installed

[djeedai]

Problem

I'm trying to use cargo install in CI on GitHub, to install cargo-tarpaulin. However cargo install might fail if the CI agent already has the version of cargo-tarpaulin that I need. Since it takes a bit to install, I also want to use GitHub caching, so want to avoid --force. However, I still want to fail CI if the install fails for any other reason than "the version is already installed". Unfortunately cargo returns a single error code so there's no way for me to discriminate those two failures, and no easy way that I know of either to check if a binary is installed (cargo install --list is hard to parse via script due to its nested listing which outputs both the package and its binaries, which are often the same).

Proposed Solution

Have cargo return a different error code when not using --force and the version of the binary is already installed.

Notes

Briefly discussed on Discord here.

[weihanglo]

Out of curiosity, which version are you using? IIRC from Rust 1.41, cargo install automatically upgrade installed binaries by default. It should work as the following:

$ cargo install --list
cargo-expand v1.0.36:
    cargo-expand
$ cargo install cargo-expand
    Updating crates.io index
     Ignored package `cargo-expand v1.0.36` is already installed, use --force to override
$ echo $?
0

[djeedai]

This is on latest update on 2022-12-15, rust version 1.66.0 (69f9c33d7 2022-12-12).

But I suspect then that there's a bad interaction with the actions/cache@v2 of the GitHub Action, which probably restores the binary but not some config file where that install is logged, yielding a "genuine" error of "cargo didn't expect that binary to be installed, but found it on disk anyway". Here's the GitHub cache action, which incidentally doesn't cache .crates.toml nor .crates2.json:

https://github.com/djeedai/bevy_hanabi/blob/1713e0cd951fdf265bc9f987a4801b4593138bef/.github/workflows/ci.yaml#L76-L84

[ehuss]

I would recommend updating the cache action to include the .crates2.json file in order for Cargo to know whether or not it should try to reinstall. There's more documentation of which files cargo uses in https://doc.rust-lang.org/cargo/guide/cargo-home.html.

[djeedai]

@ehuss that documentation explicitly lists the things to cache and neither .crates.toml nor .crates2.json are in that list. Is the documentation incorrect then?

[weihanglo]

It is telling caching things to avoid redownload not rebuild executables. I guess that's the difference. Perhaps we can expand the paragraph with some sentenses about how to cache installed binaries.

[djeedai]

Perhaps we can expand the paragraph with some sentenses about how to cache installed binaries.

I think that would be beneficial yes. I've actually copied my own CI settings from another project so I'm not alone making that mistake. How can we determine the set of files which is necessary to cache for installed binaries? I don't even understand .crates.toml and .crates2.json and why there's 2 files seemingly doing the same thing just in a different format.

[weihanglo]

.crates2.json is the second version of the tracking file of installed info, which actually does what you originally requested (see #6797). These two files are in sync and have sufficient information to cache installed binaries I believe (see config value install.root).

cargo/src/cargo/ops/common_for_install_and_uninstall.rs

Lines 23 to 35 in 7fb01c6

	/// On-disk tracking for which package installed which binary.
	///
	/// v1 is an older style, v2 is a new style that tracks more information, and
	/// is both backwards and forwards compatible. Cargo keeps both files in sync,
	/// updating both v1 and v2 at the same time. Additionally, if it detects
	/// changes in v1 that are not in v2 (such as when an older version of Cargo
	/// is used), it will automatically propagate those changes to v2.
	///
	/// This maintains a filesystem lock, preventing other instances of Cargo from
	/// modifying at the same time. Drop the value to unlock.
	///
	/// It is intended that v1 should be retained for a while during a longish
	/// transition period, and then v1 can be removed.

It has been three years since the feature got stabilized. Maybe it's about time to remove v1 tracking file.

[weihanglo]

It is telling caching things to avoid redownload not rebuild executables. I guess that's the difference. Perhaps we can expand the paragraph with some sentenses about how to cache installed binaries.

For those interested in helping this, here is the place we need a new paragraph added. Could be something like “If binaries installed via cargo install need to be cached, the following files should be added to the CI cache as well to track what has been installed.”

cargo/src/doc/src/guide/cargo-home.md

Lines 54 to 66 in a5d47a7

	## Caching the Cargo home in CI
	To avoid redownloading all crate dependencies during continuous integration, you can cache the `$CARGO_HOME` directory.
	However, caching the entire directory is often inefficient as it will contain downloaded sources twice.
	If we depend on a crate such as `serde 1.0.92` and cache the entire `$CARGO_HOME` we would actually cache the sources twice, the `serde-1.0.92.crate` inside `registry/cache` and the extracted `.rs` files of serde inside `registry/src`.
	That can unnecessarily slow down the build as downloading, extracting, recompressing and reuploading the cache to the CI servers can take some time.
	It should be sufficient to only cache the following directories across builds:
	* `bin/`
	* `registry/index/`
	* `registry/cache/`
	* `git/db/`

[jofas]

@rustbot claim

[weihanglo]

Close as #11592 added a tip to cache those two tracking files. Thank you everyone for participating the discussion :)

[djeedai]

Thanks for the fix!

[djeedai]

Just to circle back, I've finally updated the CI of my project and confirmed that even without the --force on the cargo install the binary is properly cached when the GitHub cache action includes .crates.toml and .crates2.json, slashing 3 minutes in the CI build. Thanks everyone.