cargo
cargo copied to clipboard
fix: create ephemeral workspace for git source
What does this PR try to resolve?
close https://github.com/rust-lang/cargo/issues/13259
Root cause
The root cause of this problem is 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.
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/
Sloution
If any time we create an ephemeral workspace, then we will update the git source every time and that would help us use the right code and rebuild the binary correctly.
There are some historical PRs related to this change:
- 4 years ago, we tried to all use ephemeral workspace in https://github.com/rust-lang/cargo/pull/5350. But it caused https://github.com/rust-lang/cargo/issues/5662 so we revert that change in https://github.com/rust-lang/cargo/pull/5685.
- https://github.com/rust-lang/cargo/pull/7768 In this PR, we try to search the root manifest by using
Workspace::new
, but it also caused the https://github.com/rust-lang/cargo/issues/13259.
So in my fix, I just rollback to use ephemera workspace but using find_root
to achieve the same goal of searching the root manifest. You can find more in the test: git_install_reads_workspace_manifest
How should we test and review this PR?
- [x] manually test https://github.com/rust-lang/cargo/pull/13689#issuecomment-2053473575
- [x] unit test
Additional information
None
r? @weihanglo
rustbot has assigned @weihanglo. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r?
to explicitly pick a reviewer
Debugging note:
- The failed case was caused by my change.
- The reason is that we don't check the package content in
Workspace::ephemera
function, but inWorkspace::new
we will try to load the package again. This way, it can bail out the correct error from the manifest. - Perhaps we should explicitly check the manifest in the ephemera function or find a way to abort the process before
make_ws_rustc_target
.
Tested locally:
- Add a new config
[build]
target-dir = "/Volumes/t7/code/cargo/target/tmp1"
- Install the video_in_waveform with new cargo build:
./target/debug/cargo install --git https://github.com/hi-rustin/cargo-information --rev 5fd64bb6afe37092ee1dd7c518f4fc4865175c22
Compiling bytesize v1.3.0
Compiling cargo-information v0.4.2 (https://github.com/hi-rustin/cargo-information?rev=5fd64bb6afe37092ee1dd7c518f4fc4865175c22#5fd64bb6)
Finished `release` profile [optimized] target(s) in 1m 26s
Installing /Users/hi-rustin/.cargo/bin/cargo-info
Installed package `cargo-information v0.4.2 (https://github.com/hi-rustin/cargo-information?rev=5fd64bb6afe37092ee1dd7c518f4fc4865175c22#5fd64bb6)` (executable `cargo-info`)
- Calculate the hash:
md5 ~/.cargo/bin/cargo-info
MD5 (/Users/hi-rustin/.cargo/bin/cargo-info) = 3a7ccae20527098730fda436790ddf72
- Install it again with a different version:
./target/debug/cargo install --git https://github.com/hi-rustin/cargo-information --rev e8b3419bc033d3252ad19081f516946c94e0c02e
Updating git repository `https://github.com/hi-rustin/cargo-information`
Updating crates.io index
Compiling cargo-information v0.4.2 (https://github.com/hi-rustin/cargo-information?rev=e8b3419bc033d3252ad19081f516946c94e0c02e#e8b3419b)
Finished `release` profile [optimized] target(s) in 7.85s
Replacing /Users/hi-rustin/.cargo/bin/cargo-info
Replaced package `cargo-information v0.4.2 (https://github.com/hi-rustin/cargo-information?rev=5fd64bb6afe37092ee1dd7c518f4fc4865175c22#5fd64bb6)` with `cargo-information v0.4.2 (https://github.com/hi-rustin/cargo-information?rev=e8b3419bc033d3252ad19081f516946c94e0c02e#e8b3419b)` (executable `cargo-info`)
- Calculate the hash:
md5 ~/.cargo/bin/cargo-info
MD5 (/Users/hi-rustin/.cargo/bin/cargo-info) = abd82eed7d6f3f88cf7a5371bf75d469
@weihanglo Could you please take a look? Thanks! 💚 💙 💜 💛 ❤️
:umbrella: The latest upstream changes (presumably #13769) made this pull request unmergeable. Please resolve the merge conflicts.