fix(pull): fetch PR head refs instead of pushing them (#39603)

Creating a PR fetches the head commit into the base repo, then pushes
the same objects into `refs/pull/N/head`. Since git 2.54, background
repacks race that push:

1. The push can be rejected with "unable to migrate objects to permanent
storage", see
https://github.com/go-gitea/gitea/actions/runs/37171442185/job/111345034829.
2. For heads with 100+ new objects, the repack can delete the reused
pack, leaving the PR ref pointing at missing objects.

Fetching the head commit with `FetchRemoteTempCommit` and setting the PR
ref with `UpdateRef` avoids both, as the fetch transfers nothing when
the objects exist. Fork PR refs are now updated like AGit PR refs
already are, without going through receive hooks.

Also syncs the PR ref when a PR is reopened again.
https://github.com/go-gitea/gitea/pull/37077 inverted that condition, so
a reopened PR kept a stale ref.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: bircni <bircni@icloud.com>

routers/web/repo/issue_comment.go

@@ -14,10 +14,8 @@ import (
 	issues_model "gitea.dev/models/issues"
 	"gitea.dev/models/renderhelper"
 	user_model "gitea.dev/models/user"
-	"gitea.dev/modules/git"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/markup/markdown"
-	repo_module "gitea.dev/modules/repository"
 	"gitea.dev/modules/setting"
 	api "gitea.dev/modules/structs"
 	"gitea.dev/modules/util"
@@ -87,6 +85,7 @@ func NewComment(ctx *context.Context) {
 	if (ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) || (ctx.IsSigned && issue.IsPoster(ctx.Doer.ID))) &&
 		(form.Status == "reopen" || form.Status == "close") &&
 		!(issue.IsPull && issue.PullRequest.HasMerged) {
+		// TODO: move the code to services package, don't make route handler logic so complex
 		// Duplication and conflict check should apply to reopen pull request.
 		var branchOtherUnmergedPR *issues_model.PullRequest
 		var err error
@@ -102,59 +101,20 @@ func NewComment(ctx *context.Context) {
 			if branchOtherUnmergedPR != nil {
 				ctx.Flash.Error(ctx.Tr("repo.pulls.open_unmerged_pull_exists", branchOtherUnmergedPR.Index))
 			} else {
-				// Regenerate patch and test conflict.
+				// sync ref of PR <refs/pulls/pr_index/head> in base repo for a reopened PR
-				issue.PullRequest.HeadCommitID = ""
+				if pull.Flow == issues_model.PullRequestFlowGithub {
-				pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest)
+					if exist, _ := git_model.IsBranchExist(ctx, pull.HeadRepoID, pull.HeadBranch); !exist {
-			}
+						ctx.Flash.Error("The origin branch is delete, cannot reopen.")
-
+						return
-			// check whether the ref of PR <refs/pulls/pr_index/head> in base repo is consistent with the head commit of head branch in the head repo
+					}
-			// get head commit of PR
+					if err := pull_service.PushToBaseRepo(ctx, pull); err != nil {
-			if branchOtherUnmergedPR != nil && pull.Flow == issues_model.PullRequestFlowGithub {
+						ctx.ServerError("PushToBaseRepo", err)
-				prHeadRef := pull.GetGitHeadRefName()
-				if err := pull.LoadBaseRepo(ctx); err != nil {
-					ctx.ServerError("Unable to load base repo", err)
-					return
-				}
-				prHeadCommitID, err := git.GetFullCommitID(ctx, pull.BaseRepo, prHeadRef)
-				if err != nil {
-					ctx.ServerError("Get head commit Id of pr fail", err)
-					return
-				}
-
-				// get head commit of branch in the head repo
-				if err := pull.LoadHeadRepo(ctx); err != nil {
-					ctx.ServerError("Unable to load head repo", err)
-					return
-				}
-				if exist, _ := git_model.IsBranchExist(ctx, pull.HeadRepo.ID, pull.BaseBranch); !exist {
-					ctx.Flash.Error("The origin branch is delete, cannot reopen.")
-					return
-				}
-				headBranchRef := git.RefNameFromBranch(pull.HeadBranch)
-				headBranchCommitID, err := git.GetFullCommitID(ctx, pull.HeadRepo, headBranchRef.String())
-				if err != nil {
-					ctx.ServerError("Get head commit Id of head branch fail", err)
-					return
-				}
-
-				err = pull.LoadIssue(ctx)
-				if err != nil {
-					ctx.ServerError("load the issue of pull request error", err)
-					return
-				}
-
-				if prHeadCommitID != headBranchCommitID {
-					// force push to base repo
-					err := git.PushManaged(ctx, pull.HeadRepo, pull.BaseRepo, git.PushOptions{
-						Branch: pull.HeadBranch + ":" + prHeadRef,
-						Force:  true,
-						Env:    repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo),
-					})
-					if err != nil {
-						ctx.ServerError("force push error", err)
 						return
 					}
 				}
+				// Regenerate patch and test conflict.
+				issue.PullRequest.HeadCommitID = ""
+				pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest)
 			}
 		}
 

services/pull/pull.go

@@ -556,11 +556,10 @@ func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest,
 	return false, mergeBase, nil
 }
 
-// PushToBaseRepo pushes commits from branches of head repository to
+// PushToBaseRepo fetches the head branch commit into the base repository and points the PR head ref at it.
-// corresponding branches of base repository.
+// FIXME: Only update refs that actually changed?
-// FIXME: Only push branches that are actually updates?
 func PushToBaseRepo(ctx context.Context, pr *issues_model.PullRequest) error {
-	log.Trace("PushToBaseRepo[%d]: pushing commits to base repo '%s'", pr.BaseRepoID, pr.GetGitHeadRefName())
+	log.Trace("PushToBaseRepo[%d]: updating base repo ref '%s'", pr.BaseRepoID, pr.GetGitHeadRefName())
 
 	if err := pr.LoadHeadRepo(ctx); err != nil {
 		return err
@@ -571,23 +570,17 @@ func PushToBaseRepo(ctx context.Context, pr *issues_model.PullRequest) error {
 	if err := pr.LoadIssue(ctx); err != nil {
 		return err
 	}
-	if err := pr.Issue.LoadPoster(ctx); err != nil {
+
+	headCommitID, err := git.GetFullCommitID(ctx, pr.HeadRepo, git.BranchPrefix+pr.HeadBranch)
+	if err != nil {
 		return err
 	}
-
+	// fetch, not push: pushing objects FetchRemoteTempCommit already fetched races background repacks
-	baseRepoHeadRefName := pr.GetGitHeadRefName()
+	if err := git.FetchRemoteTempCommit(ctx, pr.BaseRepo, pr.HeadRepo, headCommitID); err != nil {
-	if err := git.PushManaged(ctx, pr.HeadRepo, pr.BaseRepo, git.PushOptions{
+		return fmt.Errorf("unable to fetch head branch %s:%s into base repo %s, err: %w",
-		Branch: git.BranchPrefix + pr.HeadBranch + ":" + baseRepoHeadRefName,
+			pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), err)
-		Force:  true,
-		// Use InternalPushingEnvironment here because we know that pre-receive and post-receive do not run on a refs/pulls/...
-		Env: repo_module.InternalPushingEnvironment(pr.Issue.Poster, pr.BaseRepo),
-	}); err != nil {
-		// Since we use internal force-push, there should be no git error.
-		// If any error happens, it must be an internal error (e.g.: broken git hooks) but not user error.
-		return fmt.Errorf("unable to push from head branch %s:%s to base repo %s:%s, err: %w",
-			pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), baseRepoHeadRefName, err)
 	}
-	return nil
+	return git.UpdateRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName(), headCommitID)
 }
 
 // UpdatePullsRefs update all the PRs head file pointers like /refs/pull/1/head so that it will be dependent by other operations