Skip to content

refactor!: remove go-git backend - #39487

Draft
silverwind wants to merge 4 commits into
go-gitea:mainfrom
silverwind:remove-gogit
Draft

refactor!: remove go-git backend#39487
silverwind wants to merge 4 commits into
go-gitea:mainfrom
silverwind:remove-gogit

Conversation

@silverwind

@silverwind silverwind commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Removes the go-git backend so every build uses the git CLI backend. Its Windows performance advantage is gone, it lacks SHA-256 support, and it breaks repositories on Windows.

Bug: go-git keeps pack files open, so when git's auto-maintenance repacks after a push, which is the default since Git 2.54, Windows cannot delete the old .pack and go-git fails every read with packfile not found. TAGS=gogit still builds but has no effect.

  • Detect the object format through the running cat-file instead of spawning git hash-object per repository
  • Skip --numstat and an unused rev-list --count for activity top authors
  • Skip git log restarts for cold directory listings while the history ahead is linear, fewer git processes for the same result
  • Ignore log.follow in that walk, it made renamed entries lose their last commit

Median ms from gogit to this PR, small is https://gitea.com/gitea/tea, large is this repository, Windows without Defender:

Page Windows macOS Linux
Small home 64 → 46 (-28%) 35 → 27 (-23%) 11 → 8 (-27%)
Small activity 73 → 65 (-11%) 36 → 27 (-25%) 5 → 3 (-40%)
Small cold home 104 → 146 (+40%) 58 → 71 (+22%) 41 → 48 (+17%)
Large home 135 → 51 (-62%) 70 → 29 (-59%) 49 → 13 (-73%)
Large activity 514 → 218 (-58%) 297 → 130 (-56%) 359 → 133 (-63%)
Large cold home 1042 → 489 (-53%) 365 → 289 (-21%) 328 → 185 (-44%)

Fixes #38359
Fixes #34694

@silverwind
refactor!: remove go-git backend
81671ba 
The gogit build tag was kept for Windows performance, but the git CLI
backend is now at least as fast on Windows. go-git also keeps pack files
open, so on Windows git's post-push repack leaves `.pack` files without
their `.idx`, after which go-git fails every read of the repository with
"packfile not found".

Detect the object format through the cat-file process a request already
uses instead of spawning `git hash-object` per repository, and stop running
`--numstat` and an unused commit count for the activity page's top authors.

Fixes: go-gitea#38359
Fixes: go-gitea#34694

Assisted-by: Claude Code:claude-opus-5-5
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 30, 2026
@github-actions github-actions Bot added docs-update-needed The document needs to be updated synchronously pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/refactoring Existing code has been cleaned up. There should be no new functionality. labels Sep 30, 2026
@silverwind

silverwind commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Looking into this "Small cold home" regression on Windows and Mac, all other benchmark cases are pure improvements.

@silverwind
Merge remote-tracking branch 'origin/main' into remove-gogit
7b8d3ee 
* origin/main:
  fix(actions): keep runs order after auto refresh (go-gitea#39479)
  ci: Also release for other versions than 1 majors (go-gitea#39475)
  docs: Add changelog for 28.0.0 (go-gitea#39474)
  [skip ci] Updated translations via Crowdin
  fix(git): reject fsck-invalid objects on push (go-gitea#39472)

Assisted-by: Claude Code:claude-opus-5-5

# Conflicts:
#	modules/git/repo_base_gogit.go
wxiaoguang
wxiaoguang requested changes Sep 30, 2026
View reviewed changes

@wxiaoguang wxiaoguang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to do that so fast. Wait for more user feedbacks.

There are far more important bugs and PRs need to handle.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 30, 2026
@silverwind

silverwind commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

There are far more important bugs and PRs need to handle.

go-git breaking repos sounds important enough and there's even two open issues about it.

My initial trigger was go-git/go-git#2439, by the way which necessiated a ugly workaround. go-git is not a good library imho.

@wxiaoguang

wxiaoguang commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

There are far more important bugs and PRs need to handle.

go-git breaking repos sounds important enough and there's even two open issues about it.

As long as we won't provide gogit build, these issues are already "fixed".

@silverwind

silverwind commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Its presence still affects development. I had to waste Claude tokens to produce that workaround for this shitty library which if it were gone would not have been needed.

@TheFox0x7

TheFox0x7 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Sure. Just now you'll have to do it either way for a backport which will still have gogit. Why not wait some time so we can work on 28 and merge this later when it'll be mostly 29 work?

@silverwind

silverwind commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

As far as I understand, main branch already targets v29 as of today, correct? This change is meant for v29 only.

@TheFox0x7

TheFox0x7 commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Most likely yes. Process isn't ironed out yet - we all know that.

This change is meant for v29 only.

I'll take it you'll make sure all the backports that touch git files which have gogit alternative will be fixed in gogit too?
If no, why not leave it for later so it's less of a hassle?

@silverwind

silverwind commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

The release/v28 branch exists now, only backports go into v28.

Merging later in the release cycle will produce less conflict work, but ultimately I don't see conflicts as a problem, AI solves them flawlessly. I'm happy to resolve any that arise.

@TheFox0x7

TheFox0x7 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

It's not about conflicts....

Say you change a function after your PR that had gogit version. Now a backport would introduce drift between the two which either we'll forget about OR have to manually solve.
So you save nothing by introducing breaking change this early and IMO make backports more difficult to perform.
If you wanted to drop gogit - sure. Now is not the right time.

@wxiaoguang

wxiaoguang commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Just wait for a few weeks, maybe to 28.2 (or 28.0.2)

@wxiaoguang
wxiaoguang marked this pull request as draft September 30, 2026 11:27
@silverwind

silverwind commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

I don't see a big problem, but sure keep this open for a while and merge it once we are closer to v29. Just don't forget about it.

@silverwind silverwind added this to the 29.0.0 milestone Sep 30, 2026
@silverwind
chore: drop superfluous TAGS env from Windows lint
21e44a8 
golangci-lint does not read TAGS, the build tags come from --build-tags.

Assisted-by: Claude Code:claude-opus-5-5
@silverwind
perf(git): skip git log restarts on linear history in directory listings
63bc8a8 
A cold directory listing walks history with git log and restarts it with a
narrower pathspec whenever enough entries are resolved. On a small repository
that is around ten git processes, which dominates the listing on Windows.

A narrower pathspec only changes what git log lists where history is not a
chain of commits with decreasing commit times. Restarts inside such a chain
are now recorded instead of run, and the last recorded restart runs once the
log leaves the chain, so the result stays identical. One rev-list call finds
the chain.

The walk also ignores log.follow now. It made git follow renames once a
restart narrowed the pathspec to one path, and renamed entries lost their
last commit.

Assisted-by: Claude Code:claude-opus-5-5
@silverwind

silverwind commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Found some more optimizations and cold cache repo homepage got faster, table in #39487 is up to date. Many operations are now 2-3x faster than on main branch.

If we want I could extract those perf tweaks from this PR and they could land independently.

@silverwind

silverwind commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

https://github.com/go-gitea/gitea/actions/runs/36721877142/job/109908823684 this flake is directly related to previously undiscivered go-git bugs exposed by #39426. It's not free keeping go-git in the tree. The sooner it's gone, the better.

@silverwind silverwind mentioned this pull request Sep 30, 2026
Open
@silverwind

silverwind commented Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

https://github.com/go-gitea/gitea/actions/runs/36721877142/job/109908823684 this flake is directly related to previously undiscivered go-git bugs exposed by #39426. It's not free keeping go-git in the tree. The sooner it's gone, the better.

Next workaround for go-git in #39510, I'm getting tired of it already, would prefer we go ahead with the removal now and stop dealing with this crappy library. Forgejo removed it a long time ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wxiaoguang wxiaoguang wxiaoguang requested changes

Assignees

No one assigned

Labels

docs-update-needed The document needs to be updated synchronously lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Milestone

29.0.0

4 participants

@silverwind @wxiaoguang @TheFox0x7 @GiteaBot