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.

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

Fix error with git repo discovery and symlinks. #8186

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Apr 30, 2020

There are some cases where Cargo would generate an error when attempting to discover if a package is inside a git repo when the git repo has a symlink somewhere in its ancestor paths. One way this manifests is with cargo install --git ... where the given repo has a build.rs script. Another scenario is cargo build --manifest-path somelink/Cargo.toml where somelink is a symlink to the real thing.

The issue is that libgit2 is normalizing paths and removing symlinks, but the path Cargo uses is the path with symlinks. This was introduced in #8095.

The solution is to try to canonicalize both paths when trying to get a repo-relative path. If that fails for whatever reason, it shouldn't generate an error since this is just a "best effort" attempt to use git to list package files.

Fixes #8183

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2020
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 30, 2020

📌 Commit 5bd74c4 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 30, 2020
@bors
Copy link
Contributor

bors commented Apr 30, 2020

⌛ Testing commit 5bd74c4 with merge 4a61d1c...

@bors
Copy link
Contributor

bors commented Apr 30, 2020

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing 4a61d1c to master...

@bors bors merged commit 4a61d1c into rust-lang:master Apr 30, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request May 1, 2020
submodules: update cargo from 90931d9b3 to 258c89644

Changes:
````
Remove unnecessary loop in `maybe_spurious`
Fix error with git repo discovery and symlinks.
Allow failure when setting file mtime.
Support multiple `--target` flags on the CLI
build-std: Don't treat std like a "local" package.
Allow `cargo package --list` even for things that don't package.
````

I'd like to get rust-lang/cargo#8186 into nightly asap. :)

r? @ehuss
@ehuss ehuss added this to the 1.45.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

latest cargo update broke cargo install command in my ci
4 participants