refactor: migrate git2 to gix

[tisonkun]

@Byron Thanks a lot for pointing me that we can check ignore with gix.

I'm still investigating how to get the Platform struct but draft here for further conversation.

Said we'd like to call AttributeStack::at_path and then Platform::is_excluded. The first lacking thing is how to get Attribute. I found:

impl Repository {
pub fn attributes(
    &self,
    index: &State,
    attributes_source: Source,
    ignore_source: Source,
    exclude_overrides: Option<Search>
) -> Result<AttributeStack<'_>, Error> { ... }
}

I can get a repository now, but have no ideas on the other params.

Also some issues I ever encountered with git2:

• Repository is not owned by this user; often happen with Docker on CI. Hopefully we don't have to check the permission ..

[Byron]

Thanks for giving it a shot!

It really depends how many files this exclusion check is currently run on, as the version I see here probably won't be much faster than git2, maybe even slower, I never tried.

If it's not slower, it might already be OK to keep it, as making this fast is going to need some refactoring. select_files_with_git() should keep the state it needs on the stack, instead of putting it into a separate GitHelper structure (it's not helping in this case). Taking the gix::Repository directly should do the trick.

[Byron]

To not have to deal with the index, I suggested repo.worktree()?.excludes() previously. Otherwise, the index can be opened with repo.index()?.

However, doing this is no better, performance-wise, than git2 as it will have to do a lot of repetitive work each time the path is queried.

It's probably OK to keep it at first just to get it to work.

[tisonkun]

Yes. I found that the lifecycle mark make it hard to hold the AttributeStack and I'm trying to get rid of git2 first.

If we can make it work as better as current, we can make further improvements.

[Byron]

The path should be prefix-stripped with the worktree directory to become relative to it. gix can't do that reliably and with the desired performance, as only the caller knows which root the input paths are relative to.
Without that knowledge, they need to be canonicalized which is very expensive.

[tisonkun]

I work it around with pathdiff and pseudo code:

let path = path.canonicalize();
let workdir = self.repo.work_dir().canonicalize();
let at_path = pathdiff::diff_paths(path, workdir);

It works as expected now. But perhaps we can improve it.

[Byron]

When GitHelper is gone, improving it will be quite straightforward I think.

[tisonkun]

Taking the gix::Repository directly should do the trick.

Make sense. This code is ported from Java JNI one. So you know, a wrapper class/object is normal, lol.

[tisonkun]

But I'm not quite familiar with "keep the state it needs on the stack".

All things work well now. I'll try to reduce the Docker layer and merge this one. If you can give some hints how I can leverage gix better, I can give it a try. Or I'm glad to review/collaborate on a draft PR also..

[Byron]

I had a feeling this could be the case…😁.

[Byron]

But I'm not quite familiar with "keep the state it needs on the stack".

All things work well now. I'll try to reduce the Docker layer and merge this one. If you can give some hints how I can leverage gix better, I can give it a try. Or I'm glad to review/collaborate on a draft PR also..

If it works for you as is, it's fine to merge for sure. I might just make the changes myself later to make it fast.

[tisonkun]

Thank you!

I guess you mean flatten the git code from GitHelper struct to a few functions and keep the repository on stack. So that we don't have to make a struct to hold the state and thus handle the lifetime things, but let the stack handle it while we pass the AttributeStack / Worktree as a param to those top-level functions.

[tisonkun]

Merging...

Thanks for your information again and "nudge" me to make it happen :D

[Byron]

Thank you!

I guess you mean flatten the git code from GitHelper struct to a few functions and keep the repository on stack. So that we don't have to make a struct to hold the state and thus handle the lifecycle things, but let the stack handle it while we pass the AttributeStack / Worktree as a param to those top-level functions.

Yes, that's quite what I had in mind. Let me try a PR real quick. (It's here: #126)