-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Switch to using gitoxide by default for listing files #13696
Conversation
This resolve an issue where the package path contains a symlink that's resolved by git
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:
@rfcbot fcp merge |
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. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
To maximize the testing window, I will merge this now and update the cargo submodule by the end of this week. Thanks :) @bors r+ |
☀️ Test successful - checks-actions |
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
This is temporary and supposed to remove before 1.79 is out. However nobody remembers this. See rust-lang#13696
fix: remove `__CARGO_GITOXIDE_DISABLE_LIST_FILES` env var ### What does this PR try to resolve? `__CARGO_GITOXIDE_DISABLE_LIST_FILES` is temporary and supposed to remove before 1.79 is out. However nobody remembers this. See #13696 ### How should we test and review this PR? The test suite should have covered it. ### Additional information <!-- homu-ignore:end -->
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.