Bump libgit2 from v0.28.5 to v1.9.4 by hannesa2 · Pull Request #641 · gitx/gitx

hannesa2 · 2026-09-14T17:52:19Z

It includes

Bump libgit2 to 1.8.5 Bump libgit2 to 1.8.5 #509 and
Bump libgit2 to 1.0.0 Bump libgit2 to 1.0.0 #513

hannesa2 · 2026-09-14T18:17:25Z

Screenshot differs: emulatorApi=arm64 with 4 screenshot(s)

setpoint (old) | diff | actual screenshot (new)

00-commit-context-menu.png

01-history-view.png

02-main-window.png

03-staging-view.png

Copilot

🟡 Changes recommended

CI coverage can disappear, invalid reset modes can raise an exception, and the history controller should restore its own-header import.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates the ObjectiveGit/libgit2 integration to libgit2 v1.9.4 and reduces transitive Objective-C dependencies.

Changes:

Updates ObjectiveGit/libgit2 compatibility.
Replaces umbrella imports with targeted imports and forward declarations.
Adjusts reset API typing and CI test execution.

File summaries

File	Reviewed change
`Classes/Views/PBGitRevisionCell.m`	Adds the repository dependency.
`Classes/GitX-Bridging-Header.h`	Removes the ObjectiveGit umbrella import.
`Classes/git/PBGitRepository.m`	Updates reset handling. Moderate (1 vote): invalid modes can leave `modeParam` nil and raise an exception.
`Classes/git/PBGitRepository.h`	Updates the reset API parameter type.
`Classes/git/PBGitRepository_PBGitBinarySupport.m`	Adds a direct repository import.
`Classes/git/PBGitCommit.h`	Adds ObjectiveGit forward declarations.
`Classes/Controllers/PBWebHistoryController.m`	Adds the repository import.
`Classes/Controllers/PBWebChangesController.m`	Adds the window-controller import.
`Classes/Controllers/PBViewController.m`	Adds the repository import.
`Classes/Controllers/PBViewController.h`	Uses forward declarations.
`Classes/Controllers/PBRepositoryDocumentController.m`	Adds the repository import.
`Classes/Controllers/PBHistorySearchController.m`	Adds the window-controller import.
`Classes/Controllers/PBGitWindowController.m`	Adds the repository import.
`Classes/Controllers/PBGitSidebarController.m`	Adds explicit model and controller imports.
`Classes/Controllers/PBGitHistoryController.m`	Adds direct dependencies. Nit (2 votes): restore the implementation’s own-header import.
`Classes/Controllers/PBGitHistoryController.h`	Adds search-mode imports and declarations.
`Classes/Controllers/PBGitCommitController.m`	Adds explicit model and controller imports.
`.github/workflows/BuildPR.yml`	Removes the standalone unit-test step. Moderate (3 votes): retain equivalent no-signing tests for pushes and unsigned pull requests.

Review details

Suppressed comments (1)

Classes/git/PBGitRepository.m:1030

Changing the parameter from the reset enum to NSInteger widens the accepted input, but this switch still has no default. Any value other than the three supported modes leaves modeParam nil, and the array literal below raises an exception instead of returning a failure. Reject invalid modes before constructing the command (or keep a typed project enum).

- (BOOL)resetRefish:(NSInteger)mode to:(id<PBGitRefish>)ref error:(NSError **)error

Files reviewed: 19/19 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+#import "PBGitRepository.h"
+#import "PBGitWindowController.h"


goneng · 2026-09-14T21:15:56Z

Nice work! the bump itself looks good.
I built it and ran the suite CI isn't running: 105 tests, 0 failures.
libgit2 pinned at v1.9.4, submodule on gitx/objective-git, header changes look right.

One blocker:
1ac226a3 Temporary remove unit test job (Don't merge) is still in the diff.
It deletes both unit test steps, so this PR's green checks never ran the tests, and
merging removes them from master.
It also drops the retention-days line from #623.

Two notes, neither blocking:

a657d5ed restores pre-1.0 symbols via git2/deprecated.h rather than migrating.
Fine for now, worth an issue so the debt is recorded.
PBGitWindowWiringTests is flaky under CPU load, but it fails on master too (PBGitRevList.m:264),
so not from this PR. I have filed it separately as issue PBGitWindowWiringTests fails when the machine is under load #643

hannesa2 · 2026-09-15T05:25:43Z

Screenshot differs: emulatorApi=arm64 with 4 screenshot(s)

setpoint (old) | diff | actual screenshot (new)

00-commit-context-menu.png

01-history-view.png

02-main-window.png

03-staging-view.png

goneng · 2026-09-15T08:05:22Z

@hannesa2 , drop the comment below on your AI-tool - let's see what it thinks of the following review 🙂

goneng · 2026-09-15T08:16:33Z

Blocker is cleared, thanks. .github/workflows/BuildPR.yml is now byte-identical
to master (so the retention-days line is back too), and CI on 9e7cdbe really
ran the suite this time: 105 tests, 0 failures on both arches.

I rebuilt locally against the new head and the GitX side is green as well:
105 unit + 4 UI tests, 0 failures.

Two defects in the submodule, though. Both are real, both compile-visible, and
neither is caught by CI because gitx/objective-git only runs Archive project
and never executes ObjectiveGitTests.

1. Stack buffer overflow in GTRepository+Merging.m

The git_merge_file_init_input -> git_merge_file_input_init rename was applied
to three call sites but the middle one got the wrong function:

https://github.com/gitx/objective-git/blob/068ad8daf52952ec0260273820bba479a77b5f4b/ObjectiveGit/GTRepository+Merging.m#L200-L206

git_merge_file_options_init takes a git_merge_file_options *, not a
git_merge_file_input *. Against the pinned libgit2 v1.9.4 headers,
sizeof(git_merge_file_input) is 40 and sizeof(git_merge_file_options) is 48,
and the init is a memcpy of the full template, so this writes 8 bytes past a
stack object. GIT_MERGE_FILE_INPUT_VERSION and GIT_MERGE_FILE_OPTIONS_VERSION
are both 1, so the version check passes and it fails silently.

clang does flag it, in my build of this branch:

GTRepository+Merging.m:202:41: warning: incompatible pointer types passing
  'git_merge_file_input *' to parameter of type 'git_merge_file_options *'
  [-Wincompatible-pointer-types]

Not reachable from GitX (nothing calls contentsOfDiffWithAncestor:), but
ObjectiveGitTests/GTRepositorySpec.m:263 does cover it. Fix is to match the
other two sites: git_merge_file_input_init(&ourInput, ...).

2. GTCredential declares a method that does not exist

The header now promises git_credential:

https://github.com/gitx/objective-git/blob/068ad8daf52952ec0260273820bba479a77b5f4b/ObjectiveGit/GTCredential.h#L89-L90

but the implementation still only has git_cred, now confined to the class
extension:

https://github.com/gitx/objective-git/blob/068ad8daf52952ec0260273820bba479a77b5f4b/ObjectiveGit/GTCredential.m#L44

No .m in the framework mentions git_credential at all. So the public accessor
was removed and its replacement is an unrecognized selector. Also compile-visible:

GTCredential.m:47:17: warning: method definition for 'git_credential' not found
  [-Wincomplete-implementation]

Again not called from GitX, but it is a source-breaking change to the framework's
public API rather than an API migration.

Merge order

External/objective-git points at 068ad8da, which exists only on the open
branch of gitx/objective-git#101. If that branch gets force-pushed the submodule
pointer dangles, so #101 needs to land first and the pointer be moved to the
merged commit before this one goes in.

Two smaller things, non-blocking

resetRefish: was widened from GTRepositoryResetType to NSInteger. I think
this is leftover scaffolding: once PBViewController.h switched to forward
declarations, PBGitRepository.h is no longer reachable from the Swift bridging
header at all, so the typed enum has nothing to trip over. I reverted both lines
locally and it builds clean, 105 tests 0 failures. Worth putting back, because
as written the switch has no default:, so an out-of-range mode leaves
modeParam nil and @[@"reset", modeParam, refName] raises.

gitx/Classes/git/PBGitRepository.m

Lines 1030 to 1047 in 9e7cdbe

    
           - (BOOL)resetRefish:(NSInteger)mode to:(id<PBGitRefish>)ref error:(NSError **)error 
        
           { 
        
           	if (!ref) 
        
           		return NO; 
        
           	NSString *refName = [ref refishName]; 
        
           	NSString *modeParam; 
        
           	switch (mode) { 
        
           		case GTRepositoryResetTypeSoft: 
        
           			modeParam = @"--soft"; 
        
           			break; 
        
           		case GTRepositoryResetTypeMixed: 
        
           			modeParam = @"--mixed"; 
        
           			break; 
        
           		case GTRepositoryResetTypeHard: 
        
           			modeParam = @"--hard"; 
        
           			break;

PBGitHistoryController.m dropped the import of its own header. It still
compiles only because PBWebHistoryController.h pulls it in transitively, which
is the exact pattern this PR is otherwise removing.

gitx/Classes/Controllers/PBGitHistoryController.m

Lines 9 to 12 in 9e7cdbe

    
           #import <Quartz/Quartz.h> 
        
           #import "PBGitRepository.h" 
        
           #import "PBGitWindowController.h"

One observation on the angle-bracket include conversion

My build of this branch emits 1431 -Wquoted-include-in-framework-header
warnings; I measured 994 on master back in August. Switching the wrapper headers
to angle brackets promotes them to framework-header status during the framework's
own build, which exposes the git2/*.h quoted includes still inside them. I tried
the same conversion previously for warning reduction and backed it out for this
reason. Not a blocker, and the bump is worth having regardless, but the conversion
is not a net cleanup on this metric.

Bump objective-git to include libgit2 1.0.0 deprecated-symbol fix Use angle-bracket includes for framework/sibling headers Modulemap

fix: import ObjectiveGit in public git headers fix: avoid ObjectiveGit import in Swift bridging header dependency graph fix: decouple PBGitRepository header from ObjectiveGit umbrella import Co-authored-by: hannesa2 <[email protected]>

hannesa2 · 2026-09-15T08:46:15Z

Merge order

I'm aware of this and will not change history here anymore. When this is merged, I'll merge gitx/objective-git#101 as well

The other stuff, maybe you put a commit on top, or make a new pull request after this merge. It's up to you

hannesa2 · 2026-09-15T08:50:57Z

Screenshot differs: emulatorApi=arm64 with 4 screenshot(s)

setpoint (old) | diff | actual screenshot (new)

00-commit-context-menu.png

01-history-view.png

02-main-window.png

03-staging-view.png

hannesa2 added the dependencies Pull requests that update a dependency file label Sep 14, 2026

hannesa2 force-pushed the libgit2-1.9.4 branch from 087dab9 to 0963ad6 Compare September 14, 2026 18:08

hannesa2 changed the title ~~Bump libgit2 1.9.4~~ Bump libgit2 from v0.28.5 to v1.9.4 Sep 14, 2026

hannesa2 requested review from goneng and a lite review from Copilot September 14, 2026 18:57

Copilot started reviewing on behalf of hannesa2 September 14, 2026 18:58

View session

Copilot AI reviewed Sep 14, 2026

View reviewed changes

Comment thread .github/workflows/BuildPR.yml Resolved

Comment thread Classes/Controllers/PBGitHistoryController.m

Comment on lines +11 to +12

#import "PBGitRepository.h"

#import "PBGitWindowController.h"

hannesa2 force-pushed the libgit2-1.9.4 branch from 0963ad6 to 9e7cdbe Compare September 15, 2026 05:13

hannesa2 and others added 5 commits September 15, 2026 10:32

Bump libgit2 to 1.0.0

6ae43b0

Bump objective-git to include libgit2 1.0.0 deprecated-symbol fix Use angle-bracket includes for framework/sibling headers Modulemap

Fix tests

69f56ff

Bump libgit2 to 1.8.5

a11a368

Bump libgit2 to 1.9.4

ecf00ce

hannesa2 force-pushed the libgit2-1.9.4 branch from 9e7cdbe to ecf00ce Compare September 15, 2026 08:33

goneng mentioned this pull request Sep 15, 2026

objective-git still relies on libgit2's deprecated API shims #652

Closed

hannesa2 merged commit eb46a29 into master Sep 15, 2026
3 checks passed

hannesa2 deleted the libgit2-1.9.4 branch September 15, 2026 19:29

This was referenced Sep 15, 2026

Bump Libgit2 1.9.4 gitx/objective-git#101

Merged

build: update libgit2 to 1.9.4 gitx/objective-git#98

Closed

This was referenced Sep 15, 2026

Restore the typed reset mode and the history controller's own import #659

Merged

fix(git): support relative worktree repositories #559

Closed

hannesa2 mentioned this pull request Oct 4, 2026

gitx segmentation fault on 15.7 #556

Closed

Navigation Menu

Repository navigation

Bump libgit2 from v0.28.5 to v1.9.4 - #641

Bump libgit2 from v0.28.5 to v1.9.4#641
hannesa2 merged 5 commits into
masterfrom
libgit2-1.9.4

hannesa2 commented Sep 14, 2026 •

edited

Loading

Uh oh!

hannesa2 commented Sep 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

goneng commented Sep 14, 2026 •

edited

Loading

Uh oh!

hannesa2 commented Sep 15, 2026

Uh oh!

goneng commented Sep 15, 2026 •

edited

Loading

Uh oh!

goneng commented Sep 15, 2026

Uh oh!

hannesa2 commented Sep 15, 2026 •

edited

Loading

Uh oh!

hannesa2 commented Sep 15, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		#import "PBGitRepository.h"
		#import "PBGitWindowController.h"

Conversation

hannesa2 commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hannesa2 commented Sep 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

goneng commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hannesa2 commented Sep 15, 2026

Uh oh!

goneng commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goneng commented Sep 15, 2026

Uh oh!

hannesa2 commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hannesa2 commented Sep 15, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hannesa2 commented Sep 14, 2026 •

edited

Loading

goneng commented Sep 14, 2026 •

edited

Loading

goneng commented Sep 15, 2026 •

edited

Loading

hannesa2 commented Sep 15, 2026 •

edited

Loading