diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index f60696a76378d..8d26f1e9e7304 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -18,32 +18,6 @@ import ( "code.gitea.io/gitea/modules/git/gitcmd" ) -// GetMergeBase checks and returns merge base of two branches and the reference used as base. -func (repo *Repository) GetMergeBase(tmpRemote, base, head string) (string, string, error) { - if tmpRemote == "" { - tmpRemote = "origin" - } - - if tmpRemote != "origin" { - tmpBaseName := RemotePrefix + tmpRemote + "/tmp_" + base - // Fetch commit into a temporary branch in order to be able to handle commits and tags - _, _, err := gitcmd.NewCommand("fetch", "--no-tags"). - AddDynamicArguments(tmpRemote). - AddDashesAndList(base + ":" + tmpBaseName). - WithDir(repo.Path). - RunStdString(repo.Ctx) - if err == nil { - base = tmpBaseName - } - } - - stdout, _, err := gitcmd.NewCommand("merge-base"). - AddDashesAndList(base, head). - WithDir(repo.Path). - RunStdString(repo.Ctx) - return strings.TrimSpace(stdout), base, err -} - type lineCountWriter struct { numLines int } diff --git a/modules/gitrepo/fetch.go b/modules/gitrepo/fetch.go new file mode 100644 index 0000000000000..0474d6111e66a --- /dev/null +++ b/modules/gitrepo/fetch.go @@ -0,0 +1,28 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "context" + + "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/globallock" +) + +// FetchRemoteCommit fetches a specific commit and its related objects from a remote +// repository into the managed repository. +// +// If no reference (branch, tag, or other ref) points to the fetched commit, it will +// be treated as unreachable and cleaned up by `git gc` after the default prune +// expiration period (2 weeks). Ref: https://www.kernel.org/pub/software/scm/git/docs/git-gc.html +// +// 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)) + }) +} diff --git a/modules/gitrepo/merge.go b/modules/gitrepo/merge.go new file mode 100644 index 0000000000000..b0fae88d136db --- /dev/null +++ b/modules/gitrepo/merge.go @@ -0,0 +1,22 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "context" + "fmt" + "strings" + + "code.gitea.io/gitea/modules/git/gitcmd" +) + +// MergeBase checks and returns merge base of two commits. +func MergeBase(ctx context.Context, repo Repository, baseCommitID, headCommitID string) (string, error) { + mergeBase, err := RunCmdString(ctx, repo, gitcmd.NewCommand("merge-base"). + AddDashesAndList(baseCommitID, headCommitID)) + if err != nil { + return "", fmt.Errorf("get merge-base of %s and %s failed: %w", baseCommitID, headCommitID, err) + } + return strings.TrimSpace(mergeBase), nil +} diff --git a/modules/gitrepo/repo_lock.go b/modules/gitrepo/repo_lock.go new file mode 100644 index 0000000000000..2eb89ce807933 --- /dev/null +++ b/modules/gitrepo/repo_lock.go @@ -0,0 +1,10 @@ +// 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/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index a97f32e01819c..7670660e31125 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -495,7 +495,7 @@ func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) { pull := issue.PullRequest ctx.Data["WillSign"] = false if ctx.Doer != nil { - sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, ctx.Repo.GitRepo, pull.BaseBranch, pull.GetGitHeadRefName()) + sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, ctx.Repo.GitRepo) ctx.Data["WillSign"] = sign ctx.Data["SigningKeyMergeDisplay"] = asymkey_model.GetDisplaySigningKey(key) if err != nil { diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index d778ff8918be6..cffefe08ae6f6 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -271,13 +271,22 @@ Loop: } // SignMerge determines if we should sign a PR merge commit to the base repository -func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, gitRepo *git.Repository, baseCommit, headCommit string) (bool, *git.SigningKey, *git.Signature, error) { +func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, gitRepo *git.Repository) (bool, *git.SigningKey, *git.Signature, error) { if err := pr.LoadBaseRepo(ctx); err != nil { log.Error("Unable to get Base Repo for pull request") return false, nil, nil, err } repo := pr.BaseRepo + baseCommit, err := gitRepo.GetCommit(pr.BaseBranch) + if err != nil { + return false, nil, nil, err + } + headCommit, err := gitRepo.GetCommit(pr.GetGitHeadRefName()) + if err != nil { + return false, nil, nil, err + } + signingKey, signer := gitrepo.GetSigningKey(ctx) if signingKey == nil { return false, nil, nil, &ErrWontSign{noKey} @@ -319,38 +328,26 @@ Loop: return false, nil, nil, &ErrWontSign{approved} } case baseSigned: - commit, err := gitRepo.GetCommit(baseCommit) - if err != nil { - return false, nil, nil, err - } - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, baseCommit) if !verification.Verified { return false, nil, nil, &ErrWontSign{baseSigned} } case headSigned: - commit, err := gitRepo.GetCommit(headCommit) - if err != nil { - return false, nil, nil, err - } - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, headCommit) if !verification.Verified { return false, nil, nil, &ErrWontSign{headSigned} } case commitsSigned: - commit, err := gitRepo.GetCommit(headCommit) - if err != nil { - return false, nil, nil, err - } - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, headCommit) if !verification.Verified { return false, nil, nil, &ErrWontSign{commitsSigned} } // need to work out merge-base - mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit) + mergeBaseCommit, err := gitrepo.MergeBase(ctx, pr.BaseRepo, baseCommit.ID.String(), headCommit.ID.String()) if err != nil { return false, nil, nil, err } - commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit) + commitList, err := headCommit.CommitsBeforeUntil(mergeBaseCommit) if err != nil { return false, nil, nil, err } diff --git a/services/git/compare.go b/services/git/compare.go index ee397fe947714..6c49fff26acef 100644 --- a/services/git/compare.go +++ b/services/git/compare.go @@ -6,14 +6,10 @@ package git import ( "context" "fmt" - "strconv" - "time" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" - "code.gitea.io/gitea/modules/graceful" - logger "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" ) @@ -48,25 +44,6 @@ func (ci *CompareInfo) DirectComparison() bool { // GetCompareInfo generates and returns compare information between base and head branches of repositories. func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Repository, headGitRepo *git.Repository, baseRef, headRef git.RefName, directComparison, fileOnly bool) (_ *CompareInfo, err error) { - var ( - remoteBranch string - tmpRemote string - ) - - // We don't need a temporary remote for same repository. - if baseRepo.ID != headRepo.ID { - // Add a temporary remote - tmpRemote = strconv.FormatInt(time.Now().UnixNano(), 10) - if err = gitrepo.GitRemoteAdd(ctx, headRepo, tmpRemote, baseRepo.RepoPath()); err != nil { - return nil, fmt.Errorf("GitRemoteAdd: %w", err) - } - defer func() { - if err := gitrepo.GitRemoteRemove(graceful.GetManager().ShutdownContext(), headRepo, tmpRemote); err != nil { - logger.Error("GetPullRequestInfo: GitRemoteRemove: %v", err) - } - }() - } - compareInfo := &CompareInfo{ BaseRepo: baseRepo, BaseRef: baseRef, @@ -76,47 +53,49 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito CompareSeparator: util.Iif(directComparison, "..", "..."), } + compareInfo.BaseCommitID, err = gitrepo.GetFullCommitID(ctx, baseRepo, baseRef.String()) + if err != nil { + return nil, err + } compareInfo.HeadCommitID, err = gitrepo.GetFullCommitID(ctx, headRepo, headRef.String()) if err != nil { - compareInfo.HeadCommitID = headRef.String() + return nil, err } - // FIXME: It seems we don't need mergebase if it's a direct comparison? - compareInfo.MergeBase, remoteBranch, err = headGitRepo.GetMergeBase(tmpRemote, baseRef.String(), headRef.String()) - if err == nil { - compareInfo.BaseCommitID, err = gitrepo.GetFullCommitID(ctx, headRepo, remoteBranch) - if err != nil { - compareInfo.BaseCommitID = remoteBranch - } - separator := "..." - baseCommitID := compareInfo.MergeBase - if directComparison { - separator = ".." - baseCommitID = compareInfo.BaseCommitID + // 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 baseRepo.ID != headRepo.ID { + exist := headGitRepo.IsReferenceExist(compareInfo.BaseCommitID) + if !exist { + if err := gitrepo.FetchRemoteCommit(ctx, headRepo, baseRepo, compareInfo.BaseCommitID); err != nil { + return nil, fmt.Errorf("FetchRemoteCommit: %w", err) + } } + } - // We have a common base - therefore we know that ... should work - if !fileOnly { - compareInfo.Commits, err = headGitRepo.ShowPrettyFormatLogToList(ctx, baseCommitID+separator+headRef.String()) - if err != nil { - return nil, fmt.Errorf("ShowPrettyFormatLogToList: %w", err) - } - } else { - compareInfo.Commits = []*git.Commit{} + if !directComparison { + compareInfo.MergeBase, err = gitrepo.MergeBase(ctx, headRepo, compareInfo.BaseCommitID, compareInfo.HeadCommitID) + if err != nil { + return nil, fmt.Errorf("MergeBase: %w", err) } } else { - compareInfo.Commits = []*git.Commit{} - compareInfo.MergeBase, err = gitrepo.GetFullCommitID(ctx, headRepo, remoteBranch) + compareInfo.MergeBase = compareInfo.BaseCommitID + } + + // We have a common base - therefore we know that ... should work + if !fileOnly { + compareInfo.Commits, err = headGitRepo.ShowPrettyFormatLogToList(ctx, compareInfo.BaseCommitID+compareInfo.CompareSeparator+compareInfo.HeadCommitID) if err != nil { - compareInfo.MergeBase = remoteBranch + return nil, fmt.Errorf("ShowPrettyFormatLogToList: %w", err) } - compareInfo.BaseCommitID = compareInfo.MergeBase + } else { + compareInfo.Commits = []*git.Commit{} } // Count number of changed files. // This probably should be removed as we need to use shortstat elsewhere // Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly - compareInfo.NumFiles, err = headGitRepo.GetDiffNumChangedFiles(remoteBranch, headRef.String(), directComparison) + compareInfo.NumFiles, err = headGitRepo.GetDiffNumChangedFiles(compareInfo.BaseCommitID, compareInfo.HeadCommitID, directComparison) if err != nil { return nil, err } diff --git a/services/issue/pull.go b/services/issue/pull.go index 43d5aa645d71e..2fcf3860d031d 100644 --- a/services/issue/pull.go +++ b/services/issue/pull.go @@ -7,35 +7,16 @@ import ( "context" "fmt" "slices" - "time" issues_model "code.gitea.io/gitea/models/issues" org_model "code.gitea.io/gitea/models/organization" - repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" - "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" ) -func getMergeBase(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) { - // Add a temporary remote - tmpRemote := fmt.Sprintf("mergebase-%d-%d", pr.ID, time.Now().UnixNano()) - if err := gitrepo.GitRemoteAdd(ctx, repo, tmpRemote, gitRepo.Path); err != nil { - return "", fmt.Errorf("GitRemoteAdd: %w", err) - } - defer func() { - if err := gitrepo.GitRemoteRemove(graceful.GetManager().ShutdownContext(), repo, tmpRemote); err != nil { - log.Error("getMergeBase: GitRemoteRemove: %v", err) - } - }() - - mergeBase, _, err := gitRepo.GetMergeBase(tmpRemote, baseBranch, headBranch) - return mergeBase, err -} - type ReviewRequestNotifier struct { Comment *issues_model.Comment IsAdd bool @@ -99,11 +80,10 @@ func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullReque } // get the mergebase - mergeBase, err := getMergeBase(ctx, pr.BaseRepo, repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName()) + mergeBase, err := gitrepo.MergeBase(ctx, pr.BaseRepo, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName()) if err != nil { return nil, err } - // https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed // between the merge base and the head commit but not the base branch and the head commit changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitHeadRefName()) diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 4b50e86b12cee..f32c095d76a9f 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -732,7 +732,7 @@ func (g *GiteaLocalUploader) newPullRequest(ctx context.Context, pr *base.PullRe if pr.Base.Ref != "" && pr.Head.SHA != "" { // A PR against a tag base does not make sense - therefore pr.Base.Ref must be a branch // TODO: should we be checking for the refs/heads/ prefix on the pr.Base.Ref? (i.e. are these actually branches or refs) - pr.Base.SHA, _, err = g.gitRepo.GetMergeBase("", git.BranchPrefix+pr.Base.Ref, pr.Head.SHA) + pr.Base.SHA, err = gitrepo.MergeBase(ctx, g.repo, git.BranchPrefix+pr.Base.Ref, pr.Head.SHA) if err != nil { log.Error("Cannot determine the merge base for PR #%d in %s/%s. Error: %v", pr.Number, g.repoOwner, g.repoName, err) } diff --git a/services/pull/check.go b/services/pull/check.go index 691ce9da9f9c2..f1d72cc7c6e87 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -238,7 +238,7 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer } defer closer.Close() - sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, gitRepo, pr.BaseBranch, pr.GetGitHeadRefName()) + sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, gitRepo) return sign, err } diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index 6f752c351d369..d2aa967b5e28b 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -19,6 +19,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" asymkey_service "code.gitea.io/gitea/services/asymkey" ) @@ -105,7 +106,7 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque mergeCtx.sig = doer.NewGitSig() mergeCtx.committer = mergeCtx.sig - gitRepo, err := git.OpenRepository(ctx, mergeCtx.tmpBasePath) + gitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo) if err != nil { defer cancel() return nil, nil, fmt.Errorf("failed to open temp git repo for pr[%d]: %w", mergeCtx.pr.ID, err) @@ -113,7 +114,7 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque defer gitRepo.Close() // Determine if we should sign - sign, key, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, gitRepo, "HEAD", trackingBranch) + sign, key, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, gitRepo) if sign { mergeCtx.signKey = key if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { diff --git a/services/pull/pull.go b/services/pull/pull.go index 9b84e5c7215d8..ff5c56200111b 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -521,14 +521,7 @@ func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, } defer cancel() - tmpRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath) - if err != nil { - return false, "", fmt.Errorf("OpenRepository: %w", err) - } - defer tmpRepo.Close() - - // Find the merge-base - mergeBase, _, err = tmpRepo.GetMergeBase("", "base", "tracking") + mergeBase, err = gitrepo.MergeBase(ctx, pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName()) if err != nil { return false, "", fmt.Errorf("GetMergeBase: %w", err) }