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

Switch to using gitoxide by default for listing files #13696

Merged
merged 2 commits into from Apr 4, 2024
Merged

Switch to using gitoxide by default for listing files #13696

merged 2 commits into from Apr 4, 2024

Conversation

arlosi
Copy link
Contributor

@arlosi arlosi commented Apr 3, 2024

What does this PR try to resolve?

Uses gitoxide by for listing the contents of a git repository by default. Fixes #10150

It's possible out-opt of this change with the environment variable __CARGO_GITOXIDE_DISABLE_LIST_FILES=1. This opt-out mechanism is temporary and will be removed before the next release.

How should we test and review this PR?

The newly added test fails with the git2 implementation.

@rustbot
Copy link
Collaborator

rustbot commented Apr 3, 2024

r? @ehuss

rustbot has assigned @ehuss.
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

@rustbot rustbot added A-unstable Area: nightly unstable support S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2024
@arlosi
Use strip_path_canonical for getting the relative path
312e2aa 
This resolve an issue where the package path contains a symlink that's resolved by git
@weihanglo weihanglo added the T-cargo Team: Cargo label Apr 3, 2024
@weihanglo
Copy link
Member

The conclusion from the team meeting is to have both git2 and gitoxide work in parallel, and see if there is a divergence. However, during the exploration, Arlo found that the new gitoxide implementation is more "correct" than the old one.

So, before reviewing it, I'd like to make sure the team is on the same page:

  • Make gitoxide file listing the default.
  • Have __CARGO_GITOXIDE_DISABLE_LIST_FILES as a escape hatch before the next release

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 3, 2024 •
edited by Muscraft

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

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.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels Apr 3, 2024
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 3, 2024

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

@weihanglo
Copy link
Member

To maximize the testing window, I will merge this now and update the cargo submodule by the end of this week. Thanks :)

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 4, 2024

📌 Commit 312e2aa has been approved by weihanglo

It is now in the queue for this repository.

@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 4, 2024
@bors
Copy link
Collaborator

bors commented Apr 4, 2024

⌛ Testing commit 312e2aa with merge 81ca704...

@bors
Copy link
Collaborator

bors commented Apr 4, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 81ca704 to master...

@bors bors merged commit 81ca704 into rust-lang:master Apr 4, 2024
21 checks passed
@bors bors mentioned this pull request Apr 4, 2024
Open
@weihanglo weihanglo mentioned this pull request Apr 6, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2024
@bors
Auto merge of rust-lang#123527 - weihanglo:update-cargo, r=weihanglo
23d47db 
Update cargo

9 commits in 0637083df5bbdcc951845f0d2eff6999cdb6d30a..28e7b2bc0a812f90126be30f48a00a4ada990eaa
2024-04-02 23:55:05 +0000 to 2024-04-05 19:31:01 +0000
- refactor(toml): Decouple target discovery from Target creation (rust-lang/cargo#13701)
- Don't depend on `?` affecting type inference in weird ways (rust-lang/cargo#13706)
- test(metadata): Show behavior with TOML-specific types (rust-lang/cargo#13703)
- fix: adjust tracing verbosity in list_files_git (rust-lang/cargo#13704)
- doc: comments on `PackageRegistry` (rust-lang/cargo#13698)
- Switch to using gitoxide by default for listing files (rust-lang/cargo#13696)
- Allow precise update to prerelease. (rust-lang/cargo#13626)
- refactor(toml): Split out an explicit step to resolve `Cargo.toml` (rust-lang/cargo#13693)
- chore(deps): update rust crate base64 to 0.22.0 (rust-lang/cargo#13675)

r? ghost
@rustbot rustbot added this to the 1.79.0 milestone Apr 6, 2024
@rfcbot rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@ehuss ehuss

Labels
A-unstable Area: nightly unstable support Command-package disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
None yet
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.

Cargo build crash in project with git Split Index
6 participants
@arlosi @rustbot @weihanglo @rfcbot @bors @ehuss