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.

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

use pygit2's blame support #1

Closed
aspiers opened this issue Jan 5, 2015 · 6 comments · Fixed by #127
Closed

use pygit2's blame support #1

aspiers opened this issue Jan 5, 2015 · 6 comments · Fixed by #127
Assignees

Comments

@aspiers
Copy link
Owner

aspiers commented Jan 5, 2015

Around the same time I originally developed git-deps, the good pygit2 folk added blame support to pygit2. However it didn't quite work. I fixed it but still need to convert git-deps to take advantage. This should result in a fairly significant performance boost.

@aspiers aspiers self-assigned this Jan 5, 2015
@aspiers aspiers added this to the post-hackweek polish milestone Jan 9, 2015
@wetneb
Copy link
Contributor

wetneb commented Sep 21, 2023

I had a go at this but it seems that migrating to pygit2's blame actually slows things down. It's unclear to me what's going on under the hood (I'd need to be able to debug libgit2 itself).

@aspiers
Copy link
Owner Author

aspiers commented Sep 21, 2023

That's strange, but thanks a lot for trying! Maybe ask the pygit2 or libgit2 community for advice?

@wetneb
Copy link
Contributor

wetneb commented Sep 23, 2023

According to my experiments the slowdown is due to libgit2 itself: libgit2/libgit2#3027.
Looking at their repository it seems that their implementation of blame is not very ripe (for instance libgit2/libgit2#6572 fixes a correctness issue and has been open for months) and is apparently unmaintained as the contributor who contributed blame support is not active there anymore.

So it's probably better to stick to the current solution.

@aspiers
Copy link
Owner Author

aspiers commented Sep 23, 2023

Well thanks a lot for trying anyway! I guess we just need to watch this space...

@RonnyPfannschmidt
Copy link

If the new impl works, would it be sensible to leave it behind a gruesomely named opt in, so a test could track when upstream libgit is doing better?

@aspiers
Copy link
Owner Author

aspiers commented Sep 23, 2023

Makes sense!

aspiers pushed a commit that referenced this issue Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants