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 build crash in project with git Split Index #10150

Closed
de-husk opened this issue Dec 3, 2021 · 5 comments · Fixed by #13696
Closed

Cargo build crash in project with git Split Index #10150

de-husk opened this issue Dec 3, 2021 · 5 comments · Fixed by #13696
Labels
A-git Area: anything dealing with git C-bug Category: bug S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix

Comments

@de-husk
Copy link

de-husk commented Dec 3, 2021 •
edited
Loading

Problem

Cargo build is breaking when a build.rs file is present in a project with a git repo using split indexes.

$ cargo build

error: failed to determine package fingerprint for build script for split_index_bug v0.1.0 (/.../split_index_bug)

Caused by:
  failed to determine the most recently modified file in /.../split_index_bug

Caused by:
  failed to determine list of files in /.../split_index_bug

Caused by:
  failed to open git index at /.../split_index_bug/.git/

Caused by:
  unsupported mandatory extension: 'link'; class=Index (10)

Steps to Reproduce

I have made a small repo where you can reproduce the issue https://github.com/Samangan/cargo-split-index-bug

Version

cargo 1.56.0 (4ed5d137b 2021-10-04)
release: 1.56.0
commit-hash: 4ed5d137baff5eccf1bae5a7b2ae4b57efad4a7d
commit-date: 2021-10-04
@de-husk de-husk added the C-bug Category: bug label Dec 3, 2021
@de-husk de-husk changed the title Cargo build crash in project with git Split Index Cargo build always crashes in project with git Split Index Dec 3, 2021
@de-husk de-husk changed the title Cargo build always crashes in project with git Split Index Cargo build crash in project with git Split Index Dec 3, 2021
@weihanglo
Copy link
Member

weihanglo commented Dec 4, 2021 •
edited
Loading

Confirmed that libgit2 v1.3.0, the C library wrapped by Cargo's git2 dependency, doesn't yet support split-index extension at this moment.

https://github.com/libgit2/libgit2/blob/v1.3.0/src/index.c#L2604-L2616

@ehuss ehuss added A-git Area: anything dealing with git S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix labels Dec 6, 2021
@weihanglo weihanglo mentioned this issue Mar 9, 2023
Open
15 tasks
@arlosi arlosi mentioned this issue Feb 27, 2024
Closed
@arlosi
Copy link
Contributor

arlosi commented Feb 28, 2024 •
edited
Loading

Bringing the discussion from #13498 back into the issue.

This issue is preventing me from vendoring crates into a repo that uses unsupported git extensions. When attempting to build any crate that has a build.rs file, the error is encountered.

Potential solutions:

  1. Ignore the error (log it to tracing and fall back to the non-git heuristic approach): proposed in fix: reduce failing to open a git repo to a tracing warning #13498
    • As @weihanglo pointed out, this goes against Cargo documentation.
  2. Ignore the error, but only for building, not for publishing (where it would still be an error)
  3. Do the .gitignore processing in Cargo, rather than using libgit2 (either always, or only if libgit2 fails)
  4. Have a config or CLI option that disables using git when calculating includes/excludes for a package.
  5. Require build.rs scripts to use rerun-if-changed as needed in the 2024 edition so that enumerating the files in a path source is no longer required to build.

@arlosi arlosi added the I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting label Feb 29, 2024
@ehuss ehuss moved this to Nominated in Cargo status tracker Mar 1, 2024
@epage
Copy link
Contributor

epage commented Mar 5, 2024

iirc jj is looking to switch to ignore.

@Byron
Copy link
Member

Byron commented Mar 10, 2024 •
edited
Loading

It looks like this code segment in Cargo is involved in triggering this behaviour as suggested by Weihanglo. A gitoxide based implementation of that should be available soon, which could serve as a workaround.

Please note that in order to be fully Git compatible, one has to consider the Git index, which as far as I know ignore does not, and which is the reason git2 fails in the first place. However, maybe using ignore in the git2 track if it failed would still be a viable trade-off.

PS: Here is the PR that added ignored to jj, I found the 60% speed boost quite impressive.

@ehuss ehuss removed the I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting label Mar 11, 2024
@ehuss
Copy link
Contributor

ehuss commented Mar 11, 2024

Update from the meeting last week: @arlosi is going to try to investigate if it is feasible to have a git CLI fallback in cases where libgit2 is unable to open the repository. If that is a relatively small change, that might be something we will consider for at least a short-term fix.

@Byron Byron mentioned this issue Mar 15, 2024
Merged
5 tasks
bors added a commit that referenced this issue Mar 24, 2024
@bors
Auto merge of #13592 - Byron:gix-for-list-files-git, r=arlosi
a5b31eb 
Use `gitoxide` for `list_files_git`

Related to #10150.

### Tasks

* [x] update `gix` to v0.60
* [x] assure this is tested (currently only git-tests run with `git2` and `gitoxide`)
* [x] allow `list_files_git` to use `gitoxide` if it is enabled as feature.
* [x] use dirwalk iterator
* [x] use new release of `gix` with necessary updates

### Review Notes

As this PR has come a long way, I decided to keep a few of the steps leading up to the final state, showing the PR's evolution in the hope it helps the review.

* Would it be better to simply use `gitoxide` for this without a switch? I don't think
  it will cause more trouble than `git2`, and if there is an issue I will fix it with priority.
* In one test, the walk resolves a symlink to a submodule to individual files, including the `.git/*` folder contained in the submodule which is ignored by the walk, i.e. `submodule/*` does not contain it, but `submodule-link/*` does. This is fixed in the gitoxide version, and the `git2` version.
* I noticed that symlinks are resolved for packaging *and* are allowed to point to anywhere, even outside of package root. I left it, but felt that maybe this should be reconsidered.

### Remarks

* I love the test-suite! It's incredibly exhaustive to the point where it uncovers shortcomings in `gitoxide`, which I greatly appreciate.
* I also love `git2` as it's API for many things leads to pretty idiomatic code, and sometimes I really have to work to match it. The example here is the initial `dirwalk()` method which requires a delegate as it doesn't just collect into a `Vec` like `git2` does (for good reason). Turning that into an iterator via `dirwalk_iter()` makes it far more usable, and will definitely be good for performance as the dirwalk work is offloaded into its own thread.
@arlosi arlosi mentioned this issue Apr 3, 2024
Merged
@bors bors closed this as completed in 81ca704 Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-git Area: anything dealing with git C-bug Category: bug S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix
Projects
Cargo status tracker
Archived in project
Milestone
No milestone
6 participants
@ehuss @epage @Byron @arlosi @de-husk @weihanglo