refactor(gitrepo): drop FetchRemoteCommit global lock

This commit is contained in:
Lunny Xiao
2026-06-13 19:25:52 -07:00
parent aab9737651
commit 5c60a3a16f
5 changed files with 133 additions and 20 deletions
+4 -6
View File
@@ -7,7 +7,6 @@ import (
"context"
"gitea.dev/modules/git/gitcmd"
"gitea.dev/modules/globallock"
)
// FetchRemoteCommit fetches a specific commit and its related objects from a remote
@@ -20,9 +19,8 @@ import (
// This behavior is sufficient for temporary operations, such as determining the
// merge base between commits.
func FetchRemoteCommit(ctx context.Context, repo, remoteRepo Repository, commitID string) error {
return globallock.LockAndDo(ctx, getRepoWriteLockKey(repo.RelativePath()), func(ctx context.Context) error {
return RunCmd(ctx, repo, gitcmd.NewCommand("fetch", "--no-tags").
AddDynamicArguments(repoPath(remoteRepo)).
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").
AddDynamicArguments(repoPath(remoteRepo)).
AddDynamicArguments(commitID))
}
+123
View File
@@ -0,0 +1,123 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package gitrepo
import (
"fmt"
"os"
"path/filepath"
"strings"
"sync"
"testing"
"gitea.dev/modules/git/gitcmd"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestFetchRemoteCommitConcurrentSameCommit(t *testing.T) {
sourceRepo, commitIDs := createFetchRemoteCommitSourceRepo(t, 8)
targetRepo := createBareRepo(t, "target.git")
commitID := commitIDs[len(commitIDs)-1]
const fetchers = 8
errCh := make(chan error, fetchers)
var wg sync.WaitGroup
for range fetchers {
wg.Go(func() {
errCh <- FetchRemoteCommit(t.Context(), targetRepo, sourceRepo, commitID)
})
}
wg.Wait()
close(errCh)
for err := range errCh {
require.NoError(t, err)
}
assertFetchedCommitPresent(t, targetRepo, commitID)
assertNoFetchHead(t, targetRepo)
assertNoFetchedRefs(t, targetRepo)
}
func TestFetchRemoteCommitConcurrentDifferentCommits(t *testing.T) {
sourceRepo, commitIDs := createFetchRemoteCommitSourceRepo(t, 8)
targetRepo := createBareRepo(t, "target.git")
errCh := make(chan error, len(commitIDs))
var wg sync.WaitGroup
for _, commitID := range commitIDs {
wg.Add(1)
go func(commitID string) {
defer wg.Done()
errCh <- FetchRemoteCommit(t.Context(), targetRepo, sourceRepo, commitID)
}(commitID)
}
wg.Wait()
close(errCh)
for err := range errCh {
require.NoError(t, err)
}
for _, commitID := range commitIDs {
assertFetchedCommitPresent(t, targetRepo, commitID)
}
assertNoFetchHead(t, targetRepo)
assertNoFetchedRefs(t, targetRepo)
}
func createFetchRemoteCommitSourceRepo(t *testing.T, commitCount int) (*mockRepository, []string) {
t.Helper()
repoDir := filepath.Join(t.TempDir(), "source")
require.NoError(t, gitcmd.NewCommand("init").AddDynamicArguments(repoDir).Run(t.Context()))
require.NoError(t, gitcmd.NewCommand("config", "user.name", "User").WithDir(repoDir).Run(t.Context()))
require.NoError(t, gitcmd.NewCommand("config", "user.email", "user@example.com").WithDir(repoDir).Run(t.Context()))
for i := range commitCount {
fileName := filepath.Join(repoDir, fmt.Sprintf("file-%d.txt", i))
require.NoError(t, os.WriteFile(fileName, fmt.Appendf(nil, "content %d\n", i), 0o644))
require.NoError(t, gitcmd.NewCommand("add", ".").WithDir(repoDir).Run(t.Context()))
require.NoError(t, gitcmd.NewCommand("commit", "-m").AddDynamicArguments(fmt.Sprintf("commit %d", i)).WithDir(repoDir).Run(t.Context()))
}
stdout, _, runErr := gitcmd.NewCommand("rev-list", "--reverse", "HEAD").WithDir(repoDir).RunStdString(t.Context())
require.NoError(t, runErr)
commitIDs := strings.Fields(stdout)
require.Len(t, commitIDs, commitCount)
return &mockRepository{path: repoDir}, commitIDs
}
func createBareRepo(t *testing.T, name string) *mockRepository {
t.Helper()
repoDir := filepath.Join(t.TempDir(), name)
require.NoError(t, gitcmd.NewCommand("init", "--bare").AddDynamicArguments(repoDir).Run(t.Context()))
return &mockRepository{path: repoDir}
}
func assertFetchedCommitPresent(t *testing.T, repo *mockRepository, commitID string) {
t.Helper()
err := gitcmd.NewCommand("cat-file", "-e").WithDir(repo.path).AddDynamicArguments(commitID + "^{commit}").Run(t.Context())
require.NoError(t, err)
}
func assertNoFetchHead(t *testing.T, repo *mockRepository) {
t.Helper()
_, err := os.Stat(filepath.Join(repo.path, "FETCH_HEAD"))
assert.ErrorIs(t, err, os.ErrNotExist)
}
func assertNoFetchedRefs(t *testing.T, repo *mockRepository) {
t.Helper()
stdout, _, runErr := gitcmd.NewCommand("for-each-ref", "--format=%(refname)").WithDir(repo.path).RunStdString(t.Context())
require.NoError(t, runErr)
assert.Empty(t, strings.TrimSpace(stdout))
}
-10
View File
@@ -1,10 +0,0 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package gitrepo
// getRepoWriteLockKey returns the global lock key for write operations on the repository.
// Parallel write operations on the same git repository should be avoided to prevent data corruption.
func getRepoWriteLockKey(repoStoragePath string) string {
return "repo-write:" + repoStoragePath
}
+3 -2
View File
@@ -69,8 +69,9 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito
return compareInfo, errors.Join(err1, err2)
}
// if they are not the same repository, then we need to fetch the base commit into the head repository
// because we will use headGitRepo in the following code
// If they are not the same repository, fetch the base commit object into the head repository
// because the following comparison code only operates on headGitRepo.
// This only needs object presence; it does not rely on FETCH_HEAD or a local ref update.
if baseRepo.ID != headRepo.ID {
exist := headGitRepo.IsReferenceExist(compareInfo.BaseCommitID)
if !exist {
+3 -2
View File
@@ -90,8 +90,9 @@ func checkPullRequestMergeableByMergeTree(ctx context.Context, pr *issues_model.
}
}
// 4. fetch head commit id into the current repository
// it will be checked in 2 weeks by default from git if the pull request created failure.
// 4. Fetch the head commit object into the base repository so merge-base/merge-tree
// can operate locally. This only needs object presence; no FETCH_HEAD or local ref is used.
// It will be checked in 2 weeks by default from git if the pull request created failure.
if !pr.IsSameRepo() {
if !baseGitRepo.IsReferenceExist(pr.HeadCommitID) {
if err := gitrepo.FetchRemoteCommit(ctx, pr.BaseRepo, pr.HeadRepo, pr.HeadCommitID); err != nil {