Skip to content

Commit e931b62

Browse files
lunnywxiaoguang
lunny
and
wxiaoguang
authored
Start automerge check again after the conflict check and the schedule (#35002)
Fix #34988 Backport #34989 Co-authored-by: posativ Co-authored-by: wxiaoguang <[email protected]>
1 parent 81ee93e commit e931b62

File tree

8 files changed

+187
-68
lines changed

8 files changed

+187
-68
lines changed

‎models/pull/automerge.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@ package pull
55

66
import (
77
"context"
8+
"errors"
89
"fmt"
910

1011
"code.gitea.io/gitea/models/db"
1112
repo_model "code.gitea.io/gitea/models/repo"
1213
user_model "code.gitea.io/gitea/models/user"
1314
"code.gitea.io/gitea/modules/timeutil"
15+
"code.gitea.io/gitea/modules/util"
1416
)
1517

1618
// AutoMerge represents a pull request scheduled for merging when checks succeed
@@ -76,7 +78,10 @@ func GetScheduledMergeByPullID(ctx context.Context, pullID int64) (bool, *AutoMe
7678
return false, nil, err
7779
}
7880

79-
doer, err := user_model.GetUserByID(ctx, scheduledPRM.DoerID)
81+
doer, err := user_model.GetPossibleUserByID(ctx, scheduledPRM.DoerID)
82+
if errors.Is(err, util.ErrNotExist) {
83+
doer, err = user_model.NewGhostUser(), nil
84+
}
8085
if err != nil {
8186
return false, nil, err
8287
}

‎services/automerge/automerge.go

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,21 @@ import (
2222
"code.gitea.io/gitea/modules/log"
2323
"code.gitea.io/gitea/modules/process"
2424
"code.gitea.io/gitea/modules/queue"
25+
"code.gitea.io/gitea/services/automergequeue"
2526
notify_service "code.gitea.io/gitea/services/notify"
2627
pull_service "code.gitea.io/gitea/services/pull"
2728
repo_service "code.gitea.io/gitea/services/repository"
2829
)
2930

30-
// prAutoMergeQueue represents a queue to handle update pull request tests
31-
var prAutoMergeQueue *queue.WorkerPoolQueue[string]
32-
3331
// Init runs the task queue to that handles auto merges
3432
func Init() error {
3533
notify_service.RegisterNotifier(NewNotifier())
3634

37-
prAutoMergeQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_auto_merge", handler)
38-
if prAutoMergeQueue == nil {
35+
automergequeue.AutoMergeQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_auto_merge", handler)
36+
if automergequeue.AutoMergeQueue == nil {
3937
return errors.New("unable to create pr_auto_merge queue")
4038
}
41-
go graceful.GetManager().RunWithCancel(prAutoMergeQueue)
39+
go graceful.GetManager().RunWithCancel(automergequeue.AutoMergeQueue)
4240
return nil
4341
}
4442

@@ -56,24 +54,23 @@ func handler(items ...string) []string {
5654
return nil
5755
}
5856

59-
func addToQueue(pr *issues_model.PullRequest, sha string) {
60-
log.Trace("Adding pullID: %d to the pull requests patch checking queue with sha %s", pr.ID, sha)
61-
if err := prAutoMergeQueue.Push(fmt.Sprintf("%d_%s", pr.ID, sha)); err != nil {
62-
log.Error("Error adding pullID: %d to the pull requests patch checking queue %v", pr.ID, err)
63-
}
64-
}
65-
6657
// ScheduleAutoMerge if schedule is false and no error, pull can be merged directly
6758
func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string, deleteBranchAfterMerge bool) (scheduled bool, err error) {
6859
err = db.WithTx(ctx, func(ctx context.Context) error {
6960
if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message, deleteBranchAfterMerge); err != nil {
7061
return err
7162
}
72-
scheduled = true
73-
7463
_, err = issues_model.CreateAutoMergeComment(ctx, issues_model.CommentTypePRScheduledToAutoMerge, pull, doer)
7564
return err
7665
})
66+
// Old code made "scheduled" to be true after "ScheduleAutoMerge", but it's not right:
67+
// If the transaction rolls back, then the pull request is not scheduled to auto merge.
68+
// So we should only set "scheduled" to true if there is no error.
69+
scheduled = err == nil
70+
if scheduled {
71+
log.Trace("Pull request [%d] scheduled for auto merge with style [%s] and message [%s]", pull.ID, style, message)
72+
automergequeue.StartPRCheckAndAutoMerge(ctx, pull)
73+
}
7774
return scheduled, err
7875
}
7976

@@ -99,38 +96,12 @@ func StartPRCheckAndAutoMergeBySHA(ctx context.Context, sha string, repo *repo_m
9996
}
10097

10198
for _, pr := range pulls {
102-
addToQueue(pr, sha)
99+
automergequeue.AddToQueue(pr, sha)
103100
}
104101

105102
return nil
106103
}
107104

108-
// StartPRCheckAndAutoMerge start an automerge check and auto merge task for a pull request
109-
func StartPRCheckAndAutoMerge(ctx context.Context, pull *issues_model.PullRequest) {
110-
if pull == nil || pull.HasMerged || !pull.CanAutoMerge() {
111-
return
112-
}
113-
114-
if err := pull.LoadBaseRepo(ctx); err != nil {
115-
log.Error("LoadBaseRepo: %v", err)
116-
return
117-
}
118-
119-
gitRepo, err := gitrepo.OpenRepository(ctx, pull.BaseRepo)
120-
if err != nil {
121-
log.Error("OpenRepository: %v", err)
122-
return
123-
}
124-
defer gitRepo.Close()
125-
commitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName())
126-
if err != nil {
127-
log.Error("GetRefCommitID: %v", err)
128-
return
129-
}
130-
131-
addToQueue(pull, commitID)
132-
}
133-
134105
func getPullRequestsByHeadSHA(ctx context.Context, sha string, repo *repo_model.Repository, filter func(*issues_model.PullRequest) bool) (map[int64]*issues_model.PullRequest, error) {
135106
gitRepo, err := gitrepo.OpenRepository(ctx, repo)
136107
if err != nil {

‎services/automerge/notify.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
user_model "code.gitea.io/gitea/models/user"
1313
"code.gitea.io/gitea/modules/log"
1414
"code.gitea.io/gitea/modules/repository"
15+
"code.gitea.io/gitea/services/automergequeue"
1516
notify_service "code.gitea.io/gitea/services/notify"
1617
)
1718

@@ -45,7 +46,7 @@ func (n *automergeNotifier) PullReviewDismiss(ctx context.Context, doer *user_mo
4546
return
4647
}
4748
// as reviews could have blocked a pending automerge let's recheck
48-
StartPRCheckAndAutoMerge(ctx, review.Issue.PullRequest)
49+
automergequeue.StartPRCheckAndAutoMerge(ctx, review.Issue.PullRequest)
4950
}
5051

5152
func (n *automergeNotifier) CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, commit *repository.PushCommit, sender *user_model.User, status *git_model.CommitStatus) {

‎services/automergequeue/automergequeue.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package automergequeue
5+
6+
import (
7+
"context"
8+
"fmt"
9+
10+
issues_model "code.gitea.io/gitea/models/issues"
11+
"code.gitea.io/gitea/modules/gitrepo"
12+
"code.gitea.io/gitea/modules/log"
13+
"code.gitea.io/gitea/modules/queue"
14+
)
15+
16+
var AutoMergeQueue *queue.WorkerPoolQueue[string]
17+
18+
var AddToQueue = func(pr *issues_model.PullRequest, sha string) {
19+
log.Trace("Adding pullID: %d to the pull requests patch checking queue with sha %s", pr.ID, sha)
20+
if err := AutoMergeQueue.Push(fmt.Sprintf("%d_%s", pr.ID, sha)); err != nil {
21+
log.Error("Error adding pullID: %d to the pull requests patch checking queue %v", pr.ID, err)
22+
}
23+
}
24+
25+
// StartPRCheckAndAutoMerge start an automerge check and auto merge task for a pull request
26+
func StartPRCheckAndAutoMerge(ctx context.Context, pull *issues_model.PullRequest) {
27+
if pull == nil || pull.HasMerged || !pull.CanAutoMerge() {
28+
return
29+
}
30+
31+
if err := pull.LoadBaseRepo(ctx); err != nil {
32+
log.Error("LoadBaseRepo: %v", err)
33+
return
34+
}
35+
36+
gitRepo, err := gitrepo.OpenRepository(ctx, pull.BaseRepo)
37+
if err != nil {
38+
log.Error("OpenRepository: %v", err)
39+
return
40+
}
41+
defer gitRepo.Close()
42+
commitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName())
43+
if err != nil {
44+
log.Error("GetRefCommitID: %v", err)
45+
return
46+
}
47+
48+
AddToQueue(pull, commitID)
49+
}

‎services/pull/check.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
// Copyright 2019 The Gitea Authors.
2-
// All rights reserved.
1+
// Copyright 2019 The Gitea Authors. All rights reserved.
32
// SPDX-License-Identifier: MIT
43

54
package pull
@@ -16,6 +15,7 @@ import (
1615
git_model "code.gitea.io/gitea/models/git"
1716
issues_model "code.gitea.io/gitea/models/issues"
1817
access_model "code.gitea.io/gitea/models/perm/access"
18+
"code.gitea.io/gitea/models/pull"
1919
repo_model "code.gitea.io/gitea/models/repo"
2020
"code.gitea.io/gitea/models/unit"
2121
user_model "code.gitea.io/gitea/models/user"
@@ -29,6 +29,7 @@ import (
2929
"code.gitea.io/gitea/modules/setting"
3030
"code.gitea.io/gitea/modules/timeutil"
3131
asymkey_service "code.gitea.io/gitea/services/asymkey"
32+
"code.gitea.io/gitea/services/automergequeue"
3233
notify_service "code.gitea.io/gitea/services/notify"
3334
)
3435

@@ -238,7 +239,7 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer
238239
// markPullRequestAsMergeable checks if pull request is possible to leaving checking status,
239240
// and set to be either conflict or mergeable.
240241
func markPullRequestAsMergeable(ctx context.Context, pr *issues_model.PullRequest) {
241-
// If status has not been changed to conflict by testPullRequestTmpRepoBranchMergeable then we are mergeable
242+
// If the status has not been changed to conflict by testPullRequestTmpRepoBranchMergeable then we are mergeable
242243
if pr.Status == issues_model.PullRequestStatusChecking {
243244
pr.Status = issues_model.PullRequestStatusMergeable
244245
}
@@ -257,6 +258,16 @@ func markPullRequestAsMergeable(ctx context.Context, pr *issues_model.PullReques
257258
if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files"); err != nil {
258259
log.Error("Update[%-v]: %v", pr, err)
259260
}
261+
262+
// if there is a scheduled merge for this pull request, start the auto merge check (again)
263+
exist, _, err := pull.GetScheduledMergeByPullID(ctx, pr.ID)
264+
if err != nil {
265+
log.Error("GetScheduledMergeByPullID[%-v]: %v", pr, err)
266+
return
267+
} else if !exist {
268+
return
269+
}
270+
automergequeue.StartPRCheckAndAutoMerge(ctx, pr)
260271
}
261272

262273
// getMergeCommit checks if a pull request has been merged

‎services/pull/check_test.go

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
// Copyright 2019 The Gitea Authors.
2-
// All rights reserved.
1+
// Copyright 2019 The Gitea Authors. All rights reserved.
32
// SPDX-License-Identifier: MIT
43

54
package pull
@@ -11,11 +10,18 @@ import (
1110

1211
"code.gitea.io/gitea/models/db"
1312
issues_model "code.gitea.io/gitea/models/issues"
13+
"code.gitea.io/gitea/models/pull"
14+
repo_model "code.gitea.io/gitea/models/repo"
1415
"code.gitea.io/gitea/models/unittest"
16+
user_model "code.gitea.io/gitea/models/user"
17+
"code.gitea.io/gitea/modules/graceful"
1518
"code.gitea.io/gitea/modules/queue"
1619
"code.gitea.io/gitea/modules/setting"
20+
"code.gitea.io/gitea/modules/test"
21+
"code.gitea.io/gitea/services/automergequeue"
1722

1823
"github.com/stretchr/testify/assert"
24+
"github.com/stretchr/testify/require"
1925
)
2026

2127
func TestPullRequest_AddToTaskQueue(t *testing.T) {
@@ -63,6 +69,46 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {
6369
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
6470
assert.Equal(t, issues_model.PullRequestStatusChecking, pr.Status)
6571

66-
prPatchCheckerQueue.ShutdownWait(5 * time.Second)
72+
prPatchCheckerQueue.ShutdownWait(time.Second)
6773
prPatchCheckerQueue = nil
6874
}
75+
76+
func TestMarkPullRequestAsMergeable(t *testing.T) {
77+
assert.NoError(t, unittest.PrepareTestDatabase())
78+
79+
prPatchCheckerQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_patch_checker", func(items ...string) []string { return nil })
80+
go prPatchCheckerQueue.Run()
81+
defer func() {
82+
prPatchCheckerQueue.ShutdownWait(time.Second)
83+
prPatchCheckerQueue = nil
84+
}()
85+
86+
addToQueueShaChan := make(chan string, 1)
87+
defer test.MockVariableValue(&automergequeue.AddToQueue, func(pr *issues_model.PullRequest, sha string) {
88+
addToQueueShaChan <- sha
89+
})()
90+
ctx := t.Context()
91+
_, _ = db.GetEngine(ctx).ID(2).Update(&issues_model.PullRequest{Status: issues_model.PullRequestStatusChecking})
92+
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
93+
require.False(t, pr.HasMerged)
94+
require.Equal(t, issues_model.PullRequestStatusChecking, pr.Status)
95+
96+
err := pull.ScheduleAutoMerge(ctx, &user_model.User{ID: 99999}, pr.ID, repo_model.MergeStyleMerge, "test msg", true)
97+
require.NoError(t, err)
98+
99+
exist, scheduleMerge, err := pull.GetScheduledMergeByPullID(ctx, pr.ID)
100+
require.NoError(t, err)
101+
assert.True(t, exist)
102+
assert.True(t, scheduleMerge.Doer.IsGhost())
103+
104+
markPullRequestAsMergeable(ctx, pr)
105+
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
106+
require.Equal(t, issues_model.PullRequestStatusMergeable, pr.Status)
107+
108+
select {
109+
case sha := <-addToQueueShaChan:
110+
assert.Equal(t, "985f0301dba5e7b34be866819cd15ad3d8f508ee", sha) // ref: refs/pull/3/head
111+
case <-time.After(1 * time.Second):
112+
assert.FailNow(t, "Timeout: nothing was added to automergequeue")
113+
}
114+
}

0 commit comments

Comments
 (0)