`cargo install` not working correctly when using git repo

[BillGoldenWater]

Problem

when using cargo install to install a different commit from a git repo, the executable doesn't gets replaced to target version

Steps

1. cargo install --git https://github.com/BillGoldenWater/Tools-rs --rev b6c330a video_in_waveform
2. get the hash of .cargo/bin/video_in_waveform
3. cargo install --git https://github.com/BillGoldenWater/Tools-rs --rev e25e95d video_in_waveform
4. compare the hash of .cargo/bin/video_in_waveform to previously, it's the same

Possible Solution(s)

No response

Notes

maybe related to workspace

tried --force, +nightly and uninstall then install

after run cargo clean, it will install the proper version

the nightly version:

cargo 1.77.0-nightly (2ce45605d 2024-01-04)
release: 1.77.0-nightly
commit-hash: 2ce45605d9db521b5fd6c1211ce8de6055fdb24e
commit-date: 2024-01-04
host: aarch64-apple-darwin
libgit2: 1.7.1 (sys:0.18.1 vendored)
libcurl: 8.4.0 (sys:0.4.70+curl-8.5.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Mac OS 14.2.1 [64-bit]

Version

cargo 1.75.0 (1d8b05cdd 2023-11-20)
release: 1.75.0
commit-hash: 1d8b05cdd1287c64467306cf3ca2c8ac60c11eb0
commit-date: 2023-11-20
host: aarch64-apple-darwin
libgit2: 1.7.1 (sys:0.18.1 vendored)
libcurl: 8.4.0 (sys:0.4.68+curl-8.4.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Mac OS 14.2.1 [64-bit]

[weihanglo]

It doesn't seem to be reproducible to me. What was the console output you've got in the beginning and the end? It is supposed to have

    Updating git repository `https://github.com/BillGoldenWater/Tools-rs`
  Installing video_in_waveform v0.1.0 (https://github.com/BillGoldenWater/Tools-rs?rev=b6c330a#b6c330a8)
...
...
   Replacing /Users/rust/.cargo/bin/video_in_waveform
    Replaced package `video_in_waveform v0.1.0 (https://github.com/BillGoldenWater/Tools-rs?rev=e25e95d#e25e95df)` with `video_in_waveform v0.1.0 (https://github.com/BillGoldenWater/Tools-rs?rev=b6c330a#b6c330a8)` (executable `video_in_waveform`)

And you shall see the git revision.

Also, how did you verify the binaries are the same? Could you provide the exact step you build and verify it?

[BillGoldenWater]

[weihanglo]

So the build cache was shared, as cargo clean somehow invalidated those cache….
Could you check if any of the CARGO_TARGET_DIR related settings is set? You can get the information by echoing those environment variables or running cargo +nightly -Zunstable-options config get build.target-dir.

It does look like a bug if the target-dir is shared across builds.

[BillGoldenWater]

it's indeed shared, set by using .cargo/config.toml

[build]
target-dir = "/Users/golden_water/.cargo/shared-target

output of cargo +nightly -Zunstable-options config get build.target-dir

build.target-dir = "/Users/golden_water/.cargo/shared-target"

[weihanglo]

Dug into it a bit.

It seems that cargo install creates a non-ephemeral workspace for git sources, that would try finding all member packages in that git repo, and add them as path dependencies.

cargo/src/cargo/ops/cargo_install.rs

Lines 826 to 827 in ddec308

	let mut ws = if source_id.is_git() || source_id.is_path() {
	Workspace::new(pkg.manifest_path(), config)?

This doesn't seem wrong. However, when computing fingerprints to determine if a rebuild is needed, Cargo doesn't hash git revision into fingerprint directly. Instead, it checks if source file path has changed. For git source, it is an absolute path to ~/.cargo/git/checkouts/<pkg-hash>/<short-commit-sha>, so commit SHA is always there. For path dependency, it is relative to workspace root, so nothing change and Cargo doesn't rebuild even it is from a different git revision.

cargo/src/cargo/core/compiler/fingerprint/mod.rs

Lines 587 to 589 in 9574120

	/// Hash of the path to the base source file. This is relative to the
	/// workspace root for path members, or absolute for other sources.
	path: u64,

The possible fix might be cargo install creating ephemeral workspaces for git sources. Haven't yet checked other consequences of that, but it is something worthy of exploration.

[Rustin170506]

@rustbot claim

I will take a look.