Skip to content

Rewrite in N-API and update libgit2#6

Open
savetheclocktower wants to merge 20 commits intopulsar-edit:masterfrom
savetheclocktower:context-aware-napi-and-libgit2-upgrade
Open

Rewrite in N-API and update libgit2#6
savetheclocktower wants to merge 20 commits intopulsar-edit:masterfrom
savetheclocktower:context-aware-napi-and-libgit2-upgrade

Conversation

@savetheclocktower
Copy link

git-utils is failing to build altogether on my work laptop (which runs macOS 26, despite my best efforts), so I figured this was a good time to bump libgit2.

This also adopts the needed rewrite of git-utils in N-API; this should probably fix pulsar#1468.

The tests pass for me locally, but I'll open this as a draft to start just to see what CI thinks of it on the other platforms.

@savetheclocktower savetheclocktower force-pushed the context-aware-napi-and-libgit2-upgrade branch from 2509503 to 1723dfa Compare February 28, 2026 00:45
@savetheclocktower
Copy link
Author

The size of the diff should be explained.

We used to explicitly vendor libgit2 into deps/libgit2. When I did my N-API rewrite about 16 months ago, I converted that back into a submodule. If the submodule approach works, I'd much prefer it; if not, I can switch back to vendoring.

Either way, this PR would touch a lot of files; if the submodule approach works, then future libgit2 bumps would go much more smoothly.

@savetheclocktower
Copy link
Author

OK, CI currently only tests in Linux. Good to know Linux passes, but I should add Windows to the matrix or else I can't be certain that this is a safe change to make! I'll revisit this tomorrow.

@savetheclocktower
Copy link
Author

PR #5 has more context about why libgit2 was vendorized, and why it's likely fine to reintroduce the submodule:

Separately: @mauricioszabo indicated that his original reason for vendorizing libgit2 had something to do with yarn. It occurs to me that he was likely running into problems that were specific to how yarn handled dependencies that were git repositories instead of NPM packages. But now that we have a package-publishing strategy, there should be no need for vendorizing, and we can return to the submodule strategy.

@savetheclocktower
Copy link
Author

Hey! Got a passing test suite on all platforms. I think some of those Windows bugs had been around for a while.

Taking this out of draft!

@savetheclocktower savetheclocktower marked this pull request as ready for review February 28, 2026 06:40
@savetheclocktower savetheclocktower mentioned this pull request Feb 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Occasional Git-related crashes on window reload

1 participant