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