diff --git a/models/issues/pull.go b/models/issues/pull.go index fb7dff3cc9e83..872163e11a8f7 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -256,6 +256,9 @@ func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) { return fmt.Errorf("pr[%d].LoadHeadRepo[%d]: %w", pr.ID, pr.HeadRepoID, err) } pr.isHeadRepoLoaded = true + if pr.IsSameRepo() && pr.BaseRepo == nil { + pr.BaseRepo = pr.HeadRepo + } } return nil } @@ -322,6 +325,9 @@ func (pr *PullRequest) LoadBaseRepo(ctx context.Context) (err error) { if err != nil { return fmt.Errorf("pr[%d].LoadBaseRepo[%d]: %w", pr.ID, pr.BaseRepoID, err) } + if pr.IsSameRepo() && pr.HeadRepo == nil { + pr.HeadRepo = pr.BaseRepo + } return nil } diff --git a/modules/gitrepo/fetch.go b/modules/gitrepo/fetch.go new file mode 100644 index 0000000000000..2611a918062a6 --- /dev/null +++ b/modules/gitrepo/fetch.go @@ -0,0 +1,29 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "context" + "fmt" + + "code.gitea.io/gitea/modules/git/gitcmd" +) + +// FetchRemoteBranch fetches a remote branch into a local branch +func FetchRemoteBranch(ctx context.Context, repo Repository, localBranch string, remoteRepo Repository, remoteBranch string, args ...string) error { + _, _, err := gitcmd.NewCommand("fetch", "--no-tags", "--refmap="). + AddDynamicArguments(repoPath(remoteRepo)). + // + means force fetch + AddDynamicArguments(fmt.Sprintf("+refs/heads/%s:%s", remoteBranch, localBranch)). + RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath(repo)}) + return err +} + +func FetchRemoteCommit(ctx context.Context, repo, remoteRepo Repository, commitID string) error { + _, _, err := gitcmd.NewCommand("fetch", "--no-tags"). + AddDynamicArguments(repoPath(remoteRepo)). + AddDynamicArguments(commitID). + RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath(repo)}) + return err +} diff --git a/services/agit/agit.go b/services/agit/agit.go index 4ed867b358323..c5bd78bcd4d7b 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -215,35 +215,30 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. } } - // Store old commit ID for review staleness checking - oldHeadCommitID := pr.HeadCommitID - pr.HeadCommitID = opts.NewCommitIDs[i] - if err = pull_service.UpdateRef(ctx, pr); err != nil { + if err = pull_service.UpdatePullRequestHeadRef(ctx, pr); err != nil { return nil, fmt.Errorf("failed to update pull ref. Error: %w", err) } // Mark existing reviews as stale when PR content changes (same as regular GitHub flow) - if oldHeadCommitID != opts.NewCommitIDs[i] { - if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil { - log.Error("MarkReviewsAsStale: %v", err) - } + if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil { + log.Error("MarkReviewsAsStale: %v", err) + } - // Dismiss all approval reviews if protected branch rule item enabled - pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) - if err != nil { - log.Error("GetFirstMatchProtectedBranchRule: %v", err) - } - if pb != nil && pb.DismissStaleApprovals { - if err := pull_service.DismissApprovalReviews(ctx, pusher, pr); err != nil { - log.Error("DismissApprovalReviews: %v", err) - } + // Dismiss all approval reviews if protected branch rule item enabled + pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) + if err != nil { + log.Error("GetFirstMatchProtectedBranchRule: %v", err) + } + if pb != nil && pb.DismissStaleApprovals { + if err := pull_service.DismissApprovalReviews(ctx, pusher, pr); err != nil { + log.Error("DismissApprovalReviews: %v", err) } + } - // Mark reviews for the new commit as not stale - if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, opts.NewCommitIDs[i]); err != nil { - log.Error("MarkReviewsAsNotStale: %v", err) - } + // Mark reviews for the new commit as not stale + if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, opts.NewCommitIDs[i]); err != nil { + log.Error("MarkReviewsAsNotStale: %v", err) } pull_service.StartPullRequestCheckImmediately(ctx, pr) diff --git a/services/pull/comment.go b/services/pull/comment.go index f12edaf0326b0..23b4725b21139 100644 --- a/services/pull/comment.go +++ b/services/pull/comment.go @@ -5,6 +5,7 @@ package pull import ( "context" + "fmt" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" @@ -52,6 +53,10 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss return nil, nil } + if err := pr.LoadIssue(ctx); err != nil { + return nil, fmt.Errorf("unable to load issue for PR[%d]: %w", pr.ID, err) + } + opts := &issues_model.CreateCommentOptions{ Type: issues_model.CommentTypePullRequestPush, Doer: pusher, diff --git a/services/pull/pull.go b/services/pull/pull.go index b64a846adc091..ad271d2912acd 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -54,6 +54,10 @@ type NewPullRequestOptions struct { // NewPullRequest creates new pull request with labels for repository. func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { + if opts.PullRequest.Flow == issues_model.PullRequestFlowAGit && opts.PullRequest.HeadCommitID == "" { + return errors.New("head commit ID cannot be empty for agit flow") + } + repo, issue, labelIDs, uuids, pr, assigneeIDs := opts.Repo, opts.Issue, opts.LabelIDs, opts.AttachmentUUIDs, opts.PullRequest, opts.AssigneeIDs if err := issue.LoadPoster(ctx); err != nil { return err @@ -118,12 +122,8 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { pr.Issue = issue issue.PullRequest = pr - if pr.Flow == issues_model.PullRequestFlowGithub { - err = PushToBaseRepo(ctx, pr) - } else { - err = UpdateRef(ctx, pr) - } - if err != nil { + // update head commit id into git repository + if err = UpdatePullRequestHeadRef(ctx, pr); err != nil { return err } @@ -383,7 +383,7 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) { log.Trace("Updating PR[%d]: composing new test task", pr.ID) pr.HeadRepo = repo // avoid loading again if pr.Flow == issues_model.PullRequestFlowGithub { - if err := PushToBaseRepo(ctx, pr); err != nil { + if err := UpdatePullRequestHeadRef(ctx, pr); err != nil { log.Error("PushToBaseRepo: %v", err) continue } @@ -533,68 +533,6 @@ func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, return false, nil } -// PushToBaseRepo pushes commits from branches of head repository to -// corresponding branches of base repository. -// FIXME: Only push branches that are actually updates? -func PushToBaseRepo(ctx context.Context, pr *issues_model.PullRequest) (err error) { - return pushToBaseRepoHelper(ctx, pr, "") -} - -func pushToBaseRepoHelper(ctx context.Context, pr *issues_model.PullRequest, prefixHeadBranch string) (err error) { - log.Trace("PushToBaseRepo[%d]: pushing commits to base repo '%s'", pr.BaseRepoID, pr.GetGitHeadRefName()) - - if err := pr.LoadHeadRepo(ctx); err != nil { - log.Error("Unable to load head repository for PR[%d] Error: %v", pr.ID, err) - return err - } - headRepoPath := pr.HeadRepo.RepoPath() - - if err := pr.LoadBaseRepo(ctx); err != nil { - log.Error("Unable to load base repository for PR[%d] Error: %v", pr.ID, err) - return err - } - baseRepoPath := pr.BaseRepo.RepoPath() - - if err = pr.LoadIssue(ctx); err != nil { - return fmt.Errorf("unable to load issue %d for pr %d: %w", pr.IssueID, pr.ID, err) - } - if err = pr.Issue.LoadPoster(ctx); err != nil { - return fmt.Errorf("unable to load poster %d for pr %d: %w", pr.Issue.PosterID, pr.ID, err) - } - - gitRefName := pr.GetGitHeadRefName() - - if err := git.Push(ctx, headRepoPath, git.PushOptions{ - Remote: baseRepoPath, - Branch: prefixHeadBranch + pr.HeadBranch + ":" + gitRefName, - 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 { - if git.IsErrPushOutOfDate(err) { - // This should not happen as we're using force! - log.Error("Unable to push PR head for %s#%d (%-v:%s) due to ErrPushOfDate: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, gitRefName, err) - return err - } else if git.IsErrPushRejected(err) { - rejectErr := err.(*git.ErrPushRejected) - log.Info("Unable to push PR head for %s#%d (%-v:%s) due to rejection:\nStdout: %s\nStderr: %s\nError: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, gitRefName, rejectErr.StdOut, rejectErr.StdErr, rejectErr.Err) - return err - } else if git.IsErrMoreThanOne(err) { - if prefixHeadBranch != "" { - log.Info("Can't push with %s%s", prefixHeadBranch, pr.HeadBranch) - return err - } - log.Info("Retrying to push with %s%s", git.BranchPrefix, pr.HeadBranch) - err = pushToBaseRepoHelper(ctx, pr, git.BranchPrefix) - return err - } - log.Error("Unable to push PR head for %s#%d (%-v:%s) due to Error: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, gitRefName, err) - return fmt.Errorf("Push: %s:%s %s:%s %w", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), gitRefName, err) - } - - return nil -} - // UpdatePullsRefs update all the PRs head file pointers like /refs/pull/1/head so that it will be dependent by other operations func UpdatePullsRefs(ctx context.Context, repo *repo_model.Repository, update *repo_module.PushUpdateOptions) { branch := update.RefFullName.BranchName() @@ -606,27 +544,38 @@ func UpdatePullsRefs(ctx context.Context, repo *repo_model.Repository, update *r for _, pr := range prs { log.Trace("Updating PR[%d]: composing new test task", pr.ID) if pr.Flow == issues_model.PullRequestFlowGithub { - if err := PushToBaseRepo(ctx, pr); err != nil { - log.Error("PushToBaseRepo: %v", err) + if err := UpdatePullRequestHeadRef(ctx, pr); err != nil { + log.Error("UpdatePullRequestHead: %v", err) } } } } } -// UpdateRef update refs/pull/id/head directly for agit flow pull request -func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) { - log.Trace("UpdateRef[%d]: upgate pull request ref in base repo '%s'", pr.ID, pr.GetGitHeadRefName()) +// UpdatePullRequestHeadRef updates the head reference of a pull request +func UpdatePullRequestHeadRef(ctx context.Context, pr *issues_model.PullRequest) error { + log.Trace("UpdatePullRequestHeadRef[%d]: update pull request ref in base repo '%s'", pr.ID, pr.GetGitHeadRefName()) + if err := pr.LoadBaseRepo(ctx); err != nil { - log.Error("Unable to load base repository for PR[%d] Error: %v", pr.ID, err) return err } - if err := gitrepo.UpdateRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName(), pr.HeadCommitID); err != nil { - log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err) + if pr.Flow == issues_model.PullRequestFlowAGit { + if pr.HeadCommitID == "" { + return errors.New("head commit ID cannot be empty for agit flow") + } + return gitrepo.UpdateRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName(), pr.HeadCommitID) + } + + if !pr.IsSameRepo() { // for cross repository pull request + if err := pr.LoadHeadRepo(ctx); err != nil { + return err + } + + return gitrepo.FetchRemoteBranch(ctx, pr.BaseRepo, pr.GetGitHeadRefName(), pr.HeadRepo, pr.HeadBranch) } - return err + return gitrepo.UpdateRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName(), pr.HeadBranch) } // retargetBranchPulls change target branch for all pull requests whose base branch is the branch diff --git a/services/repository/branch.go b/services/repository/branch.go index 1a34121d9906b..a8a0c8b0f076d 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -716,6 +716,14 @@ func GetBranchDivergingInfo(ctx reqctx.RequestContext, baseRepo *repo_model.Repo return info, nil } + // we need fetch the necessary commits from the head repo first if it's not the same repository + if baseRepo.ID != headRepo.ID { + // git default will gc the commit in 2 weeks, so it's safe to do the compare + if err := gitrepo.FetchRemoteCommit(ctx, baseRepo, headRepo, headGitBranch.CommitID); err != nil { + return nil, err + } + } + // if the fork repo has new commits, this call will fail because they are not in the base repo // exit status 128 - fatal: Invalid symmetric difference expression aaaaaaaaaaaa...bbbbbbbbbbbb // so at the moment, we first check the update time, then check whether the fork branch has base's head diff --git a/tests/integration/pull_update_test.go b/tests/integration/pull_update_test.go index eadc61d849668..3c4778ef6bb0a 100644 --- a/tests/integration/pull_update_test.go +++ b/tests/integration/pull_update_test.go @@ -79,6 +79,9 @@ func TestAPIPullUpdateByRebase(t *testing.T) { AddTokenAuth(token) session.MakeRequest(t, req, http.StatusOK) + // reload pr + pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) + // Test GetDiverging after update diffCount, err = gitrepo.GetDivergingCommits(t.Context(), pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName()) assert.NoError(t, err)