1 files changed, 100 insertions, 41 deletions

diff --git a/services/pull/check.go b/services/pull/check.go
index b036970fbf..5d8990aa00 100644
--- a/services/pull/check.go
+++ b/services/pull/check.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"time"
 
 	"code.gitea.io/gitea/models/db"
 	git_model "code.gitea.io/gitea/models/git"
@@ -25,6 +26,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/queue"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
 	asymkey_service "code.gitea.io/gitea/services/asymkey"
 	notify_service "code.gitea.io/gitea/services/notify"
@@ -34,27 +36,88 @@ import (
 var prPatchCheckerQueue *queue.WorkerPoolQueue[string]
 
 var (
-	ErrIsClosed              = errors.New("pull is closed")
-	ErrUserNotAllowedToMerge = ErrDisallowedToMerge{}
-	ErrHasMerged             = errors.New("has already been merged")
-	ErrIsWorkInProgress      = errors.New("work in progress PRs cannot be merged")
-	ErrIsChecking            = errors.New("cannot merge while conflict checking is in progress")
-	ErrNotMergeableState     = errors.New("not in mergeable state")
-	ErrDependenciesLeft      = errors.New("is blocked by an open dependency")
+	ErrIsClosed            = errors.New("pull is closed")
+	ErrNoPermissionToMerge = errors.New("no permission to merge")
+	ErrNotReadyToMerge     = errors.New("not ready to merge")
+	ErrHasMerged           = errors.New("has already been merged")
+	ErrIsWorkInProgress    = errors.New("work in progress PRs cannot be merged")
+	ErrIsChecking          = errors.New("cannot merge while conflict checking is in progress")
+	ErrNotMergeableState   = errors.New("not in mergeable state")
+	ErrDependenciesLeft    = errors.New("is blocked by an open dependency")
 )
 
-// AddToTaskQueue adds itself to pull request test task queue.
-func AddToTaskQueue(ctx context.Context, pr *issues_model.PullRequest) {
+func markPullRequestStatusAsChecking(ctx context.Context, pr *issues_model.PullRequest) bool {
 	pr.Status = issues_model.PullRequestStatusChecking
 	err := pr.UpdateColsIfNotMerged(ctx, "status")
 	if err != nil {
-		log.Error("AddToTaskQueue(%-v).UpdateCols.(add to queue): %v", pr, err)
+		log.Error("UpdateColsIfNotMerged failed, pr: %-v, err: %v", pr, err)
+		return false
+	}
+	pr, err = issues_model.GetPullRequestByID(ctx, pr.ID)
+	if err != nil {
+		log.Error("GetPullRequestByID failed, pr: %-v, err: %v", pr, err)
+		return false
+	}
+	return pr.Status == issues_model.PullRequestStatusChecking
+}
+
+var AddPullRequestToCheckQueue = realAddPullRequestToCheckQueue
+
+func realAddPullRequestToCheckQueue(prID int64) {
+	err := prPatchCheckerQueue.Push(strconv.FormatInt(prID, 10))
+	if err != nil && !errors.Is(err, queue.ErrAlreadyInQueue) {
+		log.Error("Error adding %v to the pull requests check queue: %v", prID, err)
+	}
+}
+
+func StartPullRequestCheckImmediately(ctx context.Context, pr *issues_model.PullRequest) {
+	if !markPullRequestStatusAsChecking(ctx, pr) {
 		return
 	}
-	log.Trace("Adding %-v to the test pull requests queue", pr)
-	err = prPatchCheckerQueue.Push(strconv.FormatInt(pr.ID, 10))
-	if err != nil && err != queue.ErrAlreadyInQueue {
-		log.Error("Error adding %-v to the test pull requests queue: %v", pr, err)
+	AddPullRequestToCheckQueue(pr.ID)
+}
+
+// StartPullRequestCheckDelayable will delay the check if the pull request was not updated recently.
+// When the "base" branch gets updated, all PRs targeting that "base" branch need to re-check whether
+// they are mergeable.
+// When there are too many stale PRs, each "base" branch update will consume a lot of system resources.
+// So we can delay the checks for PRs that were not updated recently, only mark their status as
+// "checking", and then next time when these PRs are updated or viewed, the real checks will run.
+func StartPullRequestCheckDelayable(ctx context.Context, pr *issues_model.PullRequest) {
+	if !markPullRequestStatusAsChecking(ctx, pr) {
+		return
+	}
+
+	if setting.Repository.PullRequest.DelayCheckForInactiveDays >= 0 {
+		if err := pr.LoadIssue(ctx); err != nil {
+			return
+		}
+		duration := 24 * time.Hour * time.Duration(setting.Repository.PullRequest.DelayCheckForInactiveDays)
+		if pr.Issue.UpdatedUnix.AddDuration(duration) <= timeutil.TimeStampNow() {
+			return
+		}
+	}
+
+	AddPullRequestToCheckQueue(pr.ID)
+}
+
+func StartPullRequestCheckOnView(ctx context.Context, pr *issues_model.PullRequest) {
+	// TODO: its correctness totally depends on the "unique queue" feature and the global lock.
+	// So duplicate "start" requests will be ignored if there is already a task in the queue or one is running.
+	// Ideally in the future we should decouple the "unique queue" feature from the "start" request.
+	if pr.Status == issues_model.PullRequestStatusChecking {
+		if setting.IsInTesting {
+			// In testing mode, there might be an "immediate" queue, which is not a real queue, everything is executed in the same goroutine
+			// So we can't use the global lock here, otherwise it will cause a deadlock.
+			AddPullRequestToCheckQueue(pr.ID)
+		} else {
+			// When a PR check starts, the task is popped from the queue and the task handler acquires the global lock
+			// So we need to acquire the global lock here to prevent from duplicate tasks
+			_, _ = globallock.TryLockAndDo(ctx, getPullWorkingLockKey(pr.ID), func(ctx context.Context) error {
+				AddPullRequestToCheckQueue(pr.ID) // the queue is a unique queue and won't add the same task again
+				return nil
+			})
+		}
 	}
 }
 
@@ -84,7 +147,7 @@ func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *acc
 			log.Error("Error whilst checking if %-v is allowed to merge %-v: %v", doer, pr, err)
 			return err
 		} else if !allowedMerge {
-			return ErrUserNotAllowedToMerge
+			return ErrNoPermissionToMerge
 		}
 
 		if mergeCheckType == MergeCheckTypeManually {
@@ -105,7 +168,7 @@ func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *acc
 		}
 
 		if err := CheckPullBranchProtections(ctx, pr, false); err != nil {
-			if !IsErrDisallowedToMerge(err) {
+			if !errors.Is(err, ErrNotReadyToMerge) {
 				log.Error("Error whilst checking pull branch protection for %-v: %v", pr, err)
 				return err
 			}
@@ -172,10 +235,10 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer
 	return sign, err
 }
 
-// checkAndUpdateStatus checks if pull request is possible to leaving checking status,
+// markPullRequestAsMergeable checks if pull request is possible to leaving checking status,
 // and set to be either conflict or mergeable.
-func checkAndUpdateStatus(ctx context.Context, pr *issues_model.PullRequest) {
-	// If status has not been changed to conflict by testPatch then we are mergeable
+func markPullRequestAsMergeable(ctx context.Context, pr *issues_model.PullRequest) {
+	// If status has not been changed to conflict by testPullRequestTmpRepoBranchMergeable then we are mergeable
 	if pr.Status == issues_model.PullRequestStatusChecking {
 		pr.Status = issues_model.PullRequestStatusMergeable
 	}
@@ -310,6 +373,10 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool {
 
 // InitializePullRequests checks and tests untested patches of pull requests.
 func InitializePullRequests(ctx context.Context) {
+	// If we prefer to delay the checks, then no need to do any check during startup, there should be not much difference
+	if setting.Repository.PullRequest.DelayCheckForInactiveDays >= 0 {
+		return
+	}
 	prs, err := issues_model.GetPullRequestIDsByCheckStatus(ctx, issues_model.PullRequestStatusChecking)
 	if err != nil {
 		log.Error("Find Checking PRs: %v", err)
@@ -320,24 +387,12 @@ func InitializePullRequests(ctx context.Context) {
 		case <-ctx.Done():
 			return
 		default:
-			log.Trace("Adding PR[%d] to the pull requests patch checking queue", prID)
-			if err := prPatchCheckerQueue.Push(strconv.FormatInt(prID, 10)); err != nil {
-				log.Error("Error adding PR[%d] to the pull requests patch checking queue %v", prID, err)
-			}
+			AddPullRequestToCheckQueue(prID)
 		}
 	}
 }
 
-// handle passed PR IDs and test the PRs
-func handler(items ...string) []string {
-	for _, s := range items {
-		id, _ := strconv.ParseInt(s, 10, 64)
-		testPR(id)
-	}
-	return nil
-}
-
-func testPR(id int64) {
+func checkPullRequestMergeable(id int64) {
 	ctx := graceful.GetManager().HammerContext()
 	releaser, err := globallock.Lock(ctx, getPullWorkingLockKey(id))
 	if err != nil {
@@ -351,7 +406,7 @@ func testPR(id int64) {
 
 	pr, err := issues_model.GetPullRequestByID(ctx, id)
 	if err != nil {
-		log.Error("Unable to GetPullRequestByID[%d] for testPR: %v", id, err)
+		log.Error("Unable to GetPullRequestByID[%d] for checkPullRequestMergeable: %v", id, err)
 		return
 	}
 
@@ -370,15 +425,15 @@ func testPR(id int64) {
 		return
 	}
 
-	if err := TestPatch(pr); err != nil {
-		log.Error("testPatch[%-v]: %v", pr, err)
+	if err := testPullRequestBranchMergeable(pr); err != nil {
+		log.Error("testPullRequestTmpRepoBranchMergeable[%-v]: %v", pr, err)
 		pr.Status = issues_model.PullRequestStatusError
 		if err := pr.UpdateCols(ctx, "status"); err != nil {
 			log.Error("update pr [%-v] status to PullRequestStatusError failed: %v", pr, err)
 		}
 		return
 	}
-	checkAndUpdateStatus(ctx, pr)
+	markPullRequestAsMergeable(ctx, pr)
 }
 
 // CheckPRsForBaseBranch check all pulls with baseBrannch
@@ -387,17 +442,21 @@ func CheckPRsForBaseBranch(ctx context.Context, baseRepo *repo_model.Repository,
 	if err != nil {
 		return err
 	}
-
 	for _, pr := range prs {
-		AddToTaskQueue(ctx, pr)
+		StartPullRequestCheckImmediately(ctx, pr)
 	}
-
 	return nil
 }
 
 // Init runs the task queue to test all the checking status pull requests
 func Init() error {
-	prPatchCheckerQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_patch_checker", handler)
+	prPatchCheckerQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_patch_checker", func(items ...string) []string {
+		for _, s := range items {
+			id, _ := strconv.ParseInt(s, 10, 64)
+			checkPullRequestMergeable(id)
+		}
+		return nil
+	})
 
 	if prPatchCheckerQueue == nil {
 		return errors.New("unable to create pr_patch_checker queue")