Skip to content

refactor(gitrepo): drop FetchRemoteCommit global lock#38105

Open
lunny wants to merge 6 commits into
go-gitea:mainfrom
lunny:lunny/remove_fetch_lock
Open

refactor(gitrepo): drop FetchRemoteCommit global lock#38105
lunny wants to merge 6 commits into
go-gitea:mainfrom
lunny:lunny/remove_fetch_lock

Conversation

@lunny

@lunny lunny commented Jun 14, 2026

Copy link
Copy Markdown
Member

FetchRemoteCommit only needs fetched objects to exist locally for
merge-base and compare operations. It does not rely on FETCH_HEAD or
local ref updates.

Use git fetch --no-write-fetch-head --no-auto-maintenance to avoid
shared FETCH_HEAD side effects and reduce concurrent maintenance risk,
then remove the repo-wide global lock.

Add concurrent tests for fetching the same commit and different commits.

@lunny lunny added the performance/speed performance issues with slow downs label Jun 14, 2026
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 14, 2026
Comment thread modules/gitrepo/fetch.go
Comment thread modules/gitrepo/fetch_test.go Outdated
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 15, 2026
Comment thread modules/gitrepo/fetch.go
AddDynamicArguments(commitID))
})
// Avoid shared FETCH_HEAD and auto-maintenance side effects because callers only need the object data.
return RunCmd(ctx, repo, gitcmd.NewCommand("fetch", "--no-tags", "--no-write-fetch-head", "--no-auto-maintenance").

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this requires git > 2.29 for --no-write-fetch-head - we should then require it in the modules/git/git.go too

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged performance/speed performance issues with slow downs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants