From 4e2c3e43f28096feaa2bbdb380b20aa3ca262956 Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 5 Oct 2026 20:31:21 +0200 Subject: [PATCH] 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 Co-authored-by: bircni --- routers/web/repo/issue_comment.go | 64 ++++++------------------------- services/pull/pull.go | 29 ++++++-------- 2 files changed, 23 insertions(+), 70 deletions(-) diff --git a/routers/web/repo/issue_comment.go b/routers/web/repo/issue_comment.go index df908277546..478c03f28ef 100644 --- a/routers/web/repo/issue_comment.go +++ b/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. - issue.PullRequest.HeadCommitID = "" - pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest) - } - - // check whether the ref of PR in base repo is consistent with the head commit of head branch in the head repo - // get head commit of PR - if branchOtherUnmergedPR != nil && pull.Flow == issues_model.PullRequestFlowGithub { - 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) + // sync ref of PR in base repo for a reopened PR + if pull.Flow == issues_model.PullRequestFlowGithub { + if exist, _ := git_model.IsBranchExist(ctx, pull.HeadRepoID, pull.HeadBranch); !exist { + ctx.Flash.Error("The origin branch is delete, cannot reopen.") + return + } + if err := pull_service.PushToBaseRepo(ctx, pull); err != nil { + ctx.ServerError("PushToBaseRepo", err) return } } + // Regenerate patch and test conflict. + issue.PullRequest.HeadCommitID = "" + pull_service.StartPullRequestCheckImmediately(ctx, issue.PullRequest) } } diff --git a/services/pull/pull.go b/services/pull/pull.go index 1cf18f1acff..bdaeb8dc0fd 100644 --- a/services/pull/pull.go +++ b/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 -// corresponding branches of base repository. -// FIXME: Only push branches that are actually updates? +// PushToBaseRepo fetches the head branch commit into the base repository and points the PR head ref at it. +// FIXME: Only update refs that actually changed? 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 } - - baseRepoHeadRefName := pr.GetGitHeadRefName() - if err := git.PushManaged(ctx, pr.HeadRepo, pr.BaseRepo, git.PushOptions{ - Branch: git.BranchPrefix + pr.HeadBranch + ":" + baseRepoHeadRefName, - 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) + // fetch, not push: pushing objects FetchRemoteTempCommit already fetched races background repacks + if err := git.FetchRemoteTempCommit(ctx, pr.BaseRepo, pr.HeadRepo, headCommitID); err != nil { + return fmt.Errorf("unable to fetch head branch %s:%s into base repo %s, err: %w", + pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), 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