diff --git a/modules/gitrepo/fetch.go b/modules/gitrepo/fetch.go index b50571e5b2..a0549aabc2 100644 --- a/modules/gitrepo/fetch.go +++ b/modules/gitrepo/fetch.go @@ -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)) } diff --git a/modules/gitrepo/fetch_test.go b/modules/gitrepo/fetch_test.go new file mode 100644 index 0000000000..7e2d551ecb --- /dev/null +++ b/modules/gitrepo/fetch_test.go @@ -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)) +} diff --git a/modules/gitrepo/repo_lock.go b/modules/gitrepo/repo_lock.go deleted file mode 100644 index 2eb89ce807..0000000000 --- a/modules/gitrepo/repo_lock.go +++ /dev/null @@ -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 -} diff --git a/services/git/compare.go b/services/git/compare.go index 23f6ffe75f..4d2b08ea51 100644 --- a/services/git/compare.go +++ b/services/git/compare.go @@ -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 { diff --git a/services/pull/merge_tree.go b/services/pull/merge_tree.go index 10bdcf99f9..e0c2952e01 100644 --- a/services/pull/merge_tree.go +++ b/services/pull/merge_tree.go @@ -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 {