Use `gix` for `cargo package`

[Byron]

trafficstars

This should also help fixing these spurious "cannot package because some excluded file is untracked" issues.

Tasks

• [x] step-by-step conversion of vcs.rs
• [x] use proper feature toggle
• [x] ~~cleanup~~ final check by myself
• [ ] move split & rename into its own commit. Probably squash all changes except for the gix upgrade.
• [ ] upgrade to a gix release including https://github.com/GitoxideLabs/gitoxide/pull/2016

Notes for the Reviewer

• This implementation is both faster and more correct, thus affects #15416 and #14955.
• Maybe this would be an avenue for getting gitoxide to the people, to start on nightly, and then switch it over for everyone with an emergency-switch to use git2, to finally remove the git2 codepath.
  • This might be something we could do more often then, it's also a motivation for me

Related to https://github.com/GitoxideLabs/gitoxide/issues/106.

[rustbot]

:umbrella: The latest upstream changes (possibly #15557) made this pull request unmergeable. Please resolve the merge conflicts.

[weihanglo]

While the PR is not ready to merge yet, I am kicking off the process to get a consensus on how we want to merge this.

@rfcbot fcp merge

Proposal: Switch to gix for cargo package VCS dirtiness check completely, without any fallback to libgit2.

Reasons:

• This is mostly a performance fix. Not really something fundamentally changed.
• Unlike #13696, VCS dirtiness check informational only, as the team have agreed on.
• There is already --allow-dirty users can opt out the functionality completely.

[rfcbot]

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

• [ ] @0xPoe
• [ ] @Eh2406
• [x] @Muscraft
• [x] @arlosi
• [x] @ehuss
• [x] @epage
• [x] @joshtriplett
• [x] @weihanglo

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rustbot]

:umbrella: The latest upstream changes (possibly a9e8aa2c65091b520519fd1065f41d156d45f9dd) made this pull request unmergeable. Please resolve the merge conflicts.

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[Byron]

It's too bad this had to wait for so long but I decided to finally get it off my queue, and replaced the git2 implementation with the gix one entirely as suggested.

Did I miss anything that was part of the review?

Further, the Cargo test-suite discovered a bug that makes submodule listings fail.

I will fix that in gix, but also feel like it could be caught here to not further delay merging and wait until gix is released on the 22nd or so. What do you think?

[weihanglo]

If it is an existing bug, we can wait and fix it later. Regardless, do whatever makes sense and is easy to follow on both review and implementation side.

[Byron]

I realised that the gitoxide issue was already fixed 2 months ago, and made a release instead.

Unfortunately, there still is a failure on Windows related to how gix status sees symlinks - apparently it doesn't detect it's a link and walks right through, somewhat rightfully considering the file in the submodule untracked.

For today I am out of time, but I'd hope to debug this more tomorrow.

[rustbot]

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

[Byron]

Without me doing anything, the checks have passed.

My last commit (fix test that fails on CI, now removed) added some debug output to gather more dat for today, and that just passed for no good reason.

Let's see how prone it now is to failure. The test-setup is simple as well and it is something that should be reproducible in gitoxide as well, it's just the question how much effort it takes to reproduce on a local Windows VM, or if it reproduces at all. In the worst case, flakiness on CI only, it would be much tougher to fix, particularly because the issue had a smell of "filesystem race condition".

I marked the PR as ready for review nonetheless, in case CI passes, and will be back to investigate this phantom otherwise.

[Byron]

Thanks again for the review and the additional performance testing, @weihanglo, it's much appreciated. It's good to see it's also faster on Linux, albeit by a smaller margin.

I have squashed the PR down into 3 commits, one for the gix update, then the main implementation, and a last one for the performance optimisation hoping that it might prevent someone from thinking the previous version could be faster or simpler.

Can't wait for this to finally land :).

[weihanglo]

It's good to see it's also faster on Linux, albeit by a smaller margin.

I was using c6a.24xlarge which is fairly not representative 😬