-
-
Notifications
You must be signed in to change notification settings - Fork 581
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
Add LogWalkerWithoutFilter, using gitoxide #2275
Add LogWalkerWithoutFilter, using gitoxide #2275
Conversation
@cruessler one other solution if not ideal is to pass a git2::Repo AND the gix::Repo into the logwalker to keep the interface and use the one for walking and the other for filtering? |
@extrawurst I considered this option, but I thought it might be better not to mix both because I didn’t know whether either of them expected to have exclusive access to the |
It's very exciting to see this work happening in Regarding the
There won't be any issues. Not even concurrent writes can shake a Git repository if every implementation uses lock-files correctly, or just writes to the object store. Doing traversal with I hope that assorted knowledge helps in some way :). |
@extrawurst Using @Byron Thanks for the context, that’s very much appreciated! :-) There is already a PR open for updating |
Thanks for checking and linking the |
I just compared binary sizes for the release build between this PR and current Detailed numbers## Binary sizes on the PR https://github.com/extrawurst/gitui/actions/runs/9742727520 ubuntu-latest macos-latest windows-latest -a--- 7/1/2024 10:46 AM 7464960 gitui.exe -rwxr-xr-x 2 runner docker 14716232 Jul 1 10:41 ./target/x86_64-unknown-linux-musl/release/gitui Binary sizes on masterhttps://github.com/extrawurst/gitui/actions/runs/9787466979 ubuntu-latest macos-latest windows-latest -a--- 7/4/2024 2:20 AM 6657536 gitui.exe -rwxr-xr-x 2 runner docker 13706160 Jul 4 02:17 ./target/x86_64-unknown-linux-musl/release/gitui Comparison14579096 / 13550192 = 1.075932798590603 14716232 / 13706160 = 1.07369474747121 |
I am happy to pay that price in the meantime. the endgoal of reaching independence from |
This PR is split from #2269. It is focuces on the single use case of walking the log when there is no filter.
The current
LogWalker
implementation accepts afilter
of typeOption<SharedCommitFilterFn>
withtype SharedCommitFilterFn = Arc<Box<dyn Fn(&Repository, &CommitId) -> Result<bool> + Send + Sync>
. As you can see, this function takes agit2::Repository
as an option. Unfortunately, this does not work withgix
.When using
gitx
to walk the log, we open the repository throughgix::open
which returns agix::Repository
, a struct that is incompatible withgit2::Repository
. In order to make filtering work withgix
, we would have to rewrite quite a bit of code incommit_filter
. In particular, we would have to makeget_commit_diff
work withgix::Repository
.In order to limit this PR’s impact as well as the implementation effort, I thought it might be better to postpone this work and focus on the case of searching without a filter first.
It changes the following:
LogWalkerWithoutFilter
, capable of walking the log usinggix
.I followed the checklist:
make check
without errors