Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cargo install not working correctly when using git repo #13259

Open
BillGoldenWater opened this issue Jan 8, 2024 · 7 comments · May be fixed by #13689
Open

cargo install not working correctly when using git repo #13259

BillGoldenWater opened this issue Jan 8, 2024 · 7 comments · May be fixed by #13689
Assignees
@hi-rustin
Labels
A-git Area: anything dealing with git C-bug Category: bug Command-install S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@BillGoldenWater
Copy link

BillGoldenWater commented Jan 8, 2024 •
edited by rustbot

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]
@BillGoldenWater BillGoldenWater added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jan 8, 2024
@weihanglo
Copy link
Member

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?

@weihanglo weihanglo added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-triage Status: This issue is waiting on initial triage. labels Jan 8, 2024
@BillGoldenWater
Copy link
Author

screenshot

@weihanglo
Copy link
Member

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
Copy link
Author

BillGoldenWater commented Jan 8, 2024 •
edited

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
Copy link
Member

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.

@weihanglo weihanglo added A-git Area: anything dealing with git Command-install S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. labels Jan 18, 2024
@hi-rustin
Copy link
Member

@rustbot claim

I will take a look.

@hi-rustin
Copy link
Member

Tested locally, I can only reproduce it. Working on it.

@hi-rustin hi-rustin linked a pull request Apr 2, 2024 that will close this issue
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hi-rustin hi-rustin

Labels
A-git Area: anything dealing with git C-bug Category: bug Command-install S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: create ephemeral workspace for git source hi-rustin/cargo
3 participants
@weihanglo @hi-rustin @BillGoldenWater