aboutsummaryrefslogtreecommitdiffstats
path: root/services/pull
diff options
context:
space:
mode:
Diffstat (limited to 'services/pull')
-rw-r--r--services/pull/check.go143
-rw-r--r--services/pull/check_test.go4
-rw-r--r--services/pull/commit_status.go106
-rw-r--r--services/pull/commit_status_test.go101
-rw-r--r--services/pull/merge.go106
-rw-r--r--services/pull/merge_prepare.go12
-rw-r--r--services/pull/merge_squash.go14
-rw-r--r--services/pull/merge_test.go25
-rw-r--r--services/pull/patch.go23
-rw-r--r--services/pull/pull.go27
-rw-r--r--services/pull/review.go4
-rw-r--r--services/pull/reviewer.go2
-rw-r--r--services/pull/temp_repo.go17
-rw-r--r--services/pull/update.go48
14 files changed, 343 insertions, 289 deletions
diff --git a/services/pull/check.go b/services/pull/check.go
index 9b159891d7..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,20 +442,24 @@ 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 fmt.Errorf("unable to create pr_patch_checker queue")
+ return errors.New("unable to create pr_patch_checker queue")
}
go graceful.GetManager().RunWithCancel(prPatchCheckerQueue)
diff --git a/services/pull/check_test.go b/services/pull/check_test.go
index 5508a70f45..fa3a676ef1 100644
--- a/services/pull/check_test.go
+++ b/services/pull/check_test.go
@@ -36,7 +36,7 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {
assert.NoError(t, err)
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
- AddToTaskQueue(db.DefaultContext, pr)
+ StartPullRequestCheckImmediately(db.DefaultContext, pr)
assert.Eventually(t, func() bool {
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
@@ -51,7 +51,7 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {
select {
case id := <-idChan:
- assert.EqualValues(t, pr.ID, id)
+ assert.Equal(t, pr.ID, id)
case <-time.After(time.Second):
assert.FailNow(t, "Timeout: nothing was added to pullRequestQueue")
}
diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go
index 0bfff21746..7952ca6fe3 100644
--- a/services/pull/commit_status.go
+++ b/services/pull/commit_status.go
@@ -10,93 +10,59 @@ import (
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
+ "code.gitea.io/gitea/modules/commitstatus"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log"
- "code.gitea.io/gitea/modules/structs"
"github.com/gobwas/glob"
"github.com/pkg/errors"
)
// MergeRequiredContextsCommitStatus returns a commit status state for given required contexts
-func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, requiredContexts []string) structs.CommitStatusState {
- // matchedCount is the number of `CommitStatus.Context` that match any context of `requiredContexts`
- matchedCount := 0
- returnedStatus := structs.CommitStatusSuccess
-
- if len(requiredContexts) > 0 {
- requiredContextsGlob := make(map[string]glob.Glob, len(requiredContexts))
- for _, ctx := range requiredContexts {
- if gp, err := glob.Compile(ctx); err != nil {
- log.Error("glob.Compile %s failed. Error: %v", ctx, err)
- } else {
- requiredContextsGlob[ctx] = gp
- }
- }
-
- for _, gp := range requiredContextsGlob {
- var targetStatus structs.CommitStatusState
- for _, commitStatus := range commitStatuses {
- if gp.Match(commitStatus.Context) {
- targetStatus = commitStatus.State
- matchedCount++
- break
- }
- }
-
- // If required rule not match any action, then it is pending
- if targetStatus == "" {
- if structs.CommitStatusPending.NoBetterThan(returnedStatus) {
- returnedStatus = structs.CommitStatusPending
- }
- break
- }
-
- if targetStatus.NoBetterThan(returnedStatus) {
- returnedStatus = targetStatus
- }
- }
+func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, requiredContexts []string) commitstatus.CommitStatusState {
+ if len(commitStatuses) == 0 {
+ return commitstatus.CommitStatusPending
}
- if matchedCount == 0 && returnedStatus == structs.CommitStatusSuccess {
- status := git_model.CalcCommitStatus(commitStatuses)
- if status != nil {
- return status.State
- }
- return structs.CommitStatusSuccess
+ if len(requiredContexts) == 0 {
+ return git_model.CalcCommitStatus(commitStatuses).State
}
- return returnedStatus
-}
-
-// IsCommitStatusContextSuccess returns true if all required status check contexts succeed.
-func IsCommitStatusContextSuccess(commitStatuses []*git_model.CommitStatus, requiredContexts []string) bool {
- // If no specific context is required, require that last commit status is a success
- if len(requiredContexts) == 0 {
- status := git_model.CalcCommitStatus(commitStatuses)
- if status == nil || status.State != structs.CommitStatusSuccess {
- return false
+ requiredContextsGlob := make(map[string]glob.Glob, len(requiredContexts))
+ for _, ctx := range requiredContexts {
+ if gp, err := glob.Compile(ctx); err != nil {
+ log.Error("glob.Compile %s failed. Error: %v", ctx, err)
+ } else {
+ requiredContextsGlob[ctx] = gp
}
- return true
}
- for _, ctx := range requiredContexts {
- var found bool
+ requiredCommitStatuses := make([]*git_model.CommitStatus, 0, len(commitStatuses))
+ allRequiredContextsMatched := true
+ for _, gp := range requiredContextsGlob {
+ requiredContextMatched := false
for _, commitStatus := range commitStatuses {
- if commitStatus.Context == ctx {
- if commitStatus.State != structs.CommitStatusSuccess {
- return false
- }
-
- found = true
- break
+ if gp.Match(commitStatus.Context) {
+ requiredCommitStatuses = append(requiredCommitStatuses, commitStatus)
+ requiredContextMatched = true
}
}
- if !found {
- return false
- }
+ allRequiredContextsMatched = allRequiredContextsMatched && requiredContextMatched
+ }
+ if len(requiredCommitStatuses) == 0 {
+ return commitstatus.CommitStatusPending
+ }
+
+ returnedStatus := git_model.CalcCommitStatus(requiredCommitStatuses).State
+ if allRequiredContextsMatched {
+ return returnedStatus
+ }
+
+ if returnedStatus == commitstatus.CommitStatusFailure {
+ return commitstatus.CommitStatusFailure
}
- return true
+ // even if part of success, return pending
+ return commitstatus.CommitStatusPending
}
// IsPullCommitStatusPass returns if all required status checks PASS
@@ -117,7 +83,7 @@ func IsPullCommitStatusPass(ctx context.Context, pr *issues_model.PullRequest) (
}
// GetPullRequestCommitStatusState returns pull request merged commit status state
-func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullRequest) (structs.CommitStatusState, error) {
+func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullRequest) (commitstatus.CommitStatusState, error) {
// Ensure HeadRepo is loaded
if err := pr.LoadHeadRepo(ctx); err != nil {
return "", errors.Wrap(err, "LoadHeadRepo")
@@ -151,7 +117,7 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR
return "", errors.Wrap(err, "LoadBaseRepo")
}
- commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
+ commitStatuses, err := git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
if err != nil {
return "", errors.Wrap(err, "GetLatestCommitStatus")
}
diff --git a/services/pull/commit_status_test.go b/services/pull/commit_status_test.go
index 592acdd55c..a58e788c04 100644
--- a/services/pull/commit_status_test.go
+++ b/services/pull/commit_status_test.go
@@ -8,58 +8,85 @@ import (
"testing"
git_model "code.gitea.io/gitea/models/git"
- "code.gitea.io/gitea/modules/structs"
+ "code.gitea.io/gitea/modules/commitstatus"
"github.com/stretchr/testify/assert"
)
func TestMergeRequiredContextsCommitStatus(t *testing.T) {
- testCases := [][]*git_model.CommitStatus{
+ cases := []struct {
+ commitStatuses []*git_model.CommitStatus
+ requiredContexts []string
+ expected commitstatus.CommitStatusState
+ }{
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 3", State: structs.CommitStatusSuccess},
+ commitStatuses: []*git_model.CommitStatus{},
+ requiredContexts: []string{},
+ expected: commitstatus.CommitStatusPending,
},
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 2t", State: structs.CommitStatusPending},
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build xxx", State: commitstatus.CommitStatusSkipped},
+ },
+ requiredContexts: []string{"Build*"},
+ expected: commitstatus.CommitStatusSuccess,
},
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 2t", State: structs.CommitStatusFailure},
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: commitstatus.CommitStatusSkipped},
+ {Context: "Build 2", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 3", State: commitstatus.CommitStatusSuccess},
+ },
+ requiredContexts: []string{"Build*"},
+ expected: commitstatus.CommitStatusSuccess,
},
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 2t", State: structs.CommitStatusSuccess},
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2t", State: commitstatus.CommitStatusPending},
+ },
+ requiredContexts: []string{"Build*", "Build 2t*"},
+ expected: commitstatus.CommitStatusPending,
},
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 2t", State: structs.CommitStatusSuccess},
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2t", State: commitstatus.CommitStatusFailure},
+ },
+ requiredContexts: []string{"Build*", "Build 2t*"},
+ expected: commitstatus.CommitStatusFailure,
+ },
+ {
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2t", State: commitstatus.CommitStatusFailure},
+ },
+ requiredContexts: []string{"Build*"},
+ expected: commitstatus.CommitStatusFailure,
+ },
+ {
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2t", State: commitstatus.CommitStatusSuccess},
+ },
+ requiredContexts: []string{"Build*", "Build 2t*", "Build 3*"},
+ expected: commitstatus.CommitStatusPending,
+ },
+ {
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2", State: commitstatus.CommitStatusSuccess},
+ {Context: "Build 2t", State: commitstatus.CommitStatusSuccess},
+ },
+ requiredContexts: []string{"Build*", "Build *", "Build 2t*", "Build 1*"},
+ expected: commitstatus.CommitStatusSuccess,
},
}
- testCasesRequiredContexts := [][]string{
- {"Build*"},
- {"Build*", "Build 2t*"},
- {"Build*", "Build 2t*"},
- {"Build*", "Build 2t*", "Build 3*"},
- {"Build*", "Build *", "Build 2t*", "Build 1*"},
- }
-
- testCasesExpected := []structs.CommitStatusState{
- structs.CommitStatusSuccess,
- structs.CommitStatusPending,
- structs.CommitStatusFailure,
- structs.CommitStatusPending,
- structs.CommitStatusSuccess,
- }
-
- for i, commitStatuses := range testCases {
- if MergeRequiredContextsCommitStatus(commitStatuses, testCasesRequiredContexts[i]) != testCasesExpected[i] {
- assert.Fail(t, "Test case failed", "Test case %d failed", i+1)
- }
+ for i, c := range cases {
+ assert.Equal(t, c.expected, MergeRequiredContextsCommitStatus(c.commitStatuses, c.requiredContexts), "case %d", i)
}
}
diff --git a/services/pull/merge.go b/services/pull/merge.go
index 1e1ca55bc1..cd9aeb2ad1 100644
--- a/services/pull/merge.go
+++ b/services/pull/merge.go
@@ -6,12 +6,15 @@ package pull
import (
"context"
+ "errors"
"fmt"
+ "maps"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
+ "unicode"
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
@@ -59,7 +62,7 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue
issueReference = "!"
}
- reviewedOn := fmt.Sprintf("Reviewed-on: %s", httplib.MakeAbsoluteURL(ctx, pr.Issue.Link()))
+ reviewedOn := "Reviewed-on: " + httplib.MakeAbsoluteURL(ctx, pr.Issue.Link())
reviewedBy := pr.GetApprovers(ctx)
if mergeStyle != "" {
@@ -93,9 +96,7 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue
vars["HeadRepoOwnerName"] = pr.HeadRepo.OwnerName
vars["HeadRepoName"] = pr.HeadRepo.Name
}
- for extraKey, extraValue := range extraVars {
- vars[extraKey] = extraValue
- }
+ maps.Copy(vars, extraVars)
refs, err := pr.ResolveCrossReferences(ctx)
if err == nil {
closeIssueIndexes := make([]string, 0, len(refs))
@@ -160,6 +161,41 @@ func GetDefaultMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr
return getMergeMessage(ctx, baseGitRepo, pr, mergeStyle, nil)
}
+func AddCommitMessageTailer(message, tailerKey, tailerValue string) string {
+ tailerLine := tailerKey + ": " + tailerValue
+ message = strings.ReplaceAll(message, "\r\n", "\n")
+ message = strings.ReplaceAll(message, "\r", "\n")
+ if strings.Contains(message, "\n"+tailerLine+"\n") || strings.HasSuffix(message, "\n"+tailerLine) {
+ return message
+ }
+
+ if !strings.HasSuffix(message, "\n") {
+ message += "\n"
+ }
+ pos1 := strings.LastIndexByte(message[:len(message)-1], '\n')
+ pos2 := -1
+ if pos1 != -1 {
+ pos2 = strings.IndexByte(message[pos1:], ':')
+ if pos2 != -1 {
+ pos2 += pos1
+ }
+ }
+ var lastLineKey string
+ if pos1 != -1 && pos2 != -1 {
+ lastLineKey = message[pos1+1 : pos2]
+ }
+
+ isLikelyTailerLine := lastLineKey != "" && unicode.IsUpper(rune(lastLineKey[0])) && strings.Contains(message, "-")
+ for i := 0; isLikelyTailerLine && i < len(lastLineKey); i++ {
+ r := rune(lastLineKey[i])
+ isLikelyTailerLine = unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-'
+ }
+ if !strings.HasSuffix(message, "\n\n") && !isLikelyTailerLine {
+ message += "\n"
+ }
+ return message + tailerLine
+}
+
// ErrInvalidMergeStyle represents an error if merging with disabled merge strategy
type ErrInvalidMergeStyle struct {
ID int64
@@ -289,7 +325,7 @@ func handleCloseCrossReferences(ctx context.Context, pr *issues_model.PullReques
}
// doMergeAndPush performs the merge operation without changing any pull information in database and pushes it up to the base repository
-func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, pushTrigger repo_module.PushTrigger) (string, error) { //nolint:unparam
+func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, pushTrigger repo_module.PushTrigger) (string, error) { //nolint:unparam // non-error result is never used
// Clone base repo.
mergeCtx, cancel, err := createTemporaryRepoForMerge(ctx, pr, doer, expectedHeadCommitID)
if err != nil {
@@ -395,10 +431,13 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use
func commitAndSignNoAuthor(ctx *mergeContext, message string) error {
cmdCommit := git.NewCommand("commit").AddOptionFormat("--message=%s", message)
- if ctx.signKeyID == "" {
+ if ctx.signKey == nil {
cmdCommit.AddArguments("--no-gpg-sign")
} else {
- cmdCommit.AddOptionFormat("-S%s", ctx.signKeyID)
+ if ctx.signKey.Format != "" {
+ cmdCommit.AddConfig("gpg.format", ctx.signKey.Format)
+ }
+ cmdCommit.AddOptionFormat("-S%s", ctx.signKey.KeyID)
}
if err := cmdCommit.Run(ctx, ctx.RunOpts()); err != nil {
log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String())
@@ -517,25 +556,6 @@ func IsUserAllowedToMerge(ctx context.Context, pr *issues_model.PullRequest, p a
return false, nil
}
-// ErrDisallowedToMerge represents an error that a branch is protected and the current user is not allowed to modify it.
-type ErrDisallowedToMerge struct {
- Reason string
-}
-
-// IsErrDisallowedToMerge checks if an error is an ErrDisallowedToMerge.
-func IsErrDisallowedToMerge(err error) bool {
- _, ok := err.(ErrDisallowedToMerge)
- return ok
-}
-
-func (err ErrDisallowedToMerge) Error() string {
- return fmt.Sprintf("not allowed to merge [reason: %s]", err.Reason)
-}
-
-func (err ErrDisallowedToMerge) Unwrap() error {
- return util.ErrPermissionDenied
-}
-
// CheckPullBranchProtections checks whether the PR is ready to be merged (reviews and status checks)
func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullRequest, skipProtectedFilesCheck bool) (err error) {
if err = pr.LoadBaseRepo(ctx); err != nil {
@@ -555,31 +575,21 @@ func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullReques
return err
}
if !isPass {
- return ErrDisallowedToMerge{
- Reason: "Not all required status checks successful",
- }
+ return util.ErrorWrap(ErrNotReadyToMerge, "Not all required status checks successful")
}
if !issues_model.HasEnoughApprovals(ctx, pb, pr) {
- return ErrDisallowedToMerge{
- Reason: "Does not have enough approvals",
- }
+ return util.ErrorWrap(ErrNotReadyToMerge, "Does not have enough approvals")
}
if issues_model.MergeBlockedByRejectedReview(ctx, pb, pr) {
- return ErrDisallowedToMerge{
- Reason: "There are requested changes",
- }
+ return util.ErrorWrap(ErrNotReadyToMerge, "There are requested changes")
}
if issues_model.MergeBlockedByOfficialReviewRequests(ctx, pb, pr) {
- return ErrDisallowedToMerge{
- Reason: "There are official review requests",
- }
+ return util.ErrorWrap(ErrNotReadyToMerge, "There are official review requests")
}
if issues_model.MergeBlockedByOutdatedBranch(pb, pr) {
- return ErrDisallowedToMerge{
- Reason: "The head branch is behind the base branch",
- }
+ return util.ErrorWrap(ErrNotReadyToMerge, "The head branch is behind the base branch")
}
if skipProtectedFilesCheck {
@@ -587,9 +597,7 @@ func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullReques
}
if pb.MergeBlockedByProtectedFiles(pr.ChangedProtectedFiles) {
- return ErrDisallowedToMerge{
- Reason: "Changed protected files",
- }
+ return util.ErrorWrap(ErrNotReadyToMerge, "Changed protected files")
}
return nil
@@ -621,13 +629,13 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use
objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
if len(commitID) != objectFormat.FullLength() {
- return fmt.Errorf("Wrong commit ID")
+ return errors.New("Wrong commit ID")
}
commit, err := baseGitRepo.GetCommit(commitID)
if err != nil {
if git.IsErrNotExist(err) {
- return fmt.Errorf("Wrong commit ID")
+ return errors.New("Wrong commit ID")
}
return err
}
@@ -638,14 +646,14 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use
return err
}
if !ok {
- return fmt.Errorf("Wrong commit ID")
+ return errors.New("Wrong commit ID")
}
var merged bool
if merged, err = SetMerged(ctx, pr, commitID, timeutil.TimeStamp(commit.Author.When.Unix()), doer, issues_model.PullRequestStatusManuallyMerged); err != nil {
return err
} else if !merged {
- return fmt.Errorf("SetMerged failed")
+ return errors.New("SetMerged failed")
}
return nil
})
@@ -708,7 +716,7 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID
return false, fmt.Errorf("ChangeIssueStatus: %w", err)
}
- // We need to save all of the data used to compute this merge as it may have already been changed by TestPatch. FIXME: need to set some state to prevent TestPatch from running whilst we are merging.
+ // We need to save all of the data used to compute this merge as it may have already been changed by testPullRequestBranchMergeable. FIXME: need to set some state to prevent testPullRequestBranchMergeable from running whilst we are merging.
if cnt, err := db.GetEngine(ctx).Where("id = ?", pr.ID).
And("has_merged = ?", false).
Cols("has_merged, status, merge_base, merged_commit_id, merger_id, merged_unix, conflicted_files").
diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go
index 593cba550a..31a1e13734 100644
--- a/services/pull/merge_prepare.go
+++ b/services/pull/merge_prepare.go
@@ -23,11 +23,11 @@ import (
)
type mergeContext struct {
- *prContext
+ *prTmpRepoContext
doer *user_model.User
sig *git.Signature
committer *git.Signature
- signKeyID string // empty for no-sign, non-empty to sign
+ signKey *git.SigningKey
env []string
}
@@ -68,8 +68,8 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque
}
mergeCtx = &mergeContext{
- prContext: prCtx,
- doer: doer,
+ prTmpRepoContext: prCtx,
+ doer: doer,
}
if expectedHeadCommitID != "" {
@@ -99,9 +99,9 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque
mergeCtx.committer = mergeCtx.sig
// Determine if we should sign
- sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch)
+ sign, key, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch)
if sign {
- mergeCtx.signKeyID = keyID
+ mergeCtx.signKey = key
if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel {
mergeCtx.committer = signer
}
diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go
index 076189fd7a..0049c0b117 100644
--- a/services/pull/merge_squash.go
+++ b/services/pull/merge_squash.go
@@ -5,7 +5,6 @@ package pull
import (
"fmt"
- "strings"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
@@ -66,18 +65,19 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error {
if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() {
// add trailer
- if !strings.Contains(message, fmt.Sprintf("Co-authored-by: %s", sig.String())) {
- message += fmt.Sprintf("\nCo-authored-by: %s", sig.String())
- }
- message += fmt.Sprintf("\nCo-committed-by: %s\n", sig.String())
+ message = AddCommitMessageTailer(message, "Co-authored-by", sig.String())
+ message = AddCommitMessageTailer(message, "Co-committed-by", sig.String()) // FIXME: this one should be removed, it is not really used or widely used
}
cmdCommit := git.NewCommand("commit").
AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email).
AddOptionFormat("--message=%s", message)
- if ctx.signKeyID == "" {
+ if ctx.signKey == nil {
cmdCommit.AddArguments("--no-gpg-sign")
} else {
- cmdCommit.AddOptionFormat("-S%s", ctx.signKeyID)
+ if ctx.signKey.Format != "" {
+ cmdCommit.AddConfig("gpg.format", ctx.signKey.Format)
+ }
+ cmdCommit.AddOptionFormat("-S%s", ctx.signKey.KeyID)
}
if err := cmdCommit.Run(ctx, ctx.RunOpts()); err != nil {
log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String())
diff --git a/services/pull/merge_test.go b/services/pull/merge_test.go
index 6df6f55d46..91abeb9d9c 100644
--- a/services/pull/merge_test.go
+++ b/services/pull/merge_test.go
@@ -65,3 +65,28 @@ func Test_expandDefaultMergeMessage(t *testing.T) {
})
}
}
+
+func TestAddCommitMessageTailer(t *testing.T) {
+ // add tailer for empty message
+ assert.Equal(t, "\n\nTest-tailer: TestValue", AddCommitMessageTailer("", "Test-tailer", "TestValue"))
+
+ // add tailer for message without newlines
+ assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title", "Test-tailer", "TestValue"))
+ assert.Equal(t, "title\n\nNot tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nNot tailer: xxx", "Test-tailer", "TestValue"))
+ assert.Equal(t, "title\n\nNotTailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nNotTailer: xxx", "Test-tailer", "TestValue"))
+ assert.Equal(t, "title\n\nnot-tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nnot-tailer: xxx", "Test-tailer", "TestValue"))
+
+ // add tailer for message with one EOL
+ assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n", "Test-tailer", "TestValue"))
+
+ // add tailer for message with two EOLs
+ assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\n", "Test-tailer", "TestValue"))
+
+ // add tailer for message with existing tailer (won't duplicate)
+ assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTailer("title\n\nTest-tailer: TestValue", "Test-tailer", "TestValue"))
+ assert.Equal(t, "title\n\nTest-tailer: TestValue\n", AddCommitMessageTailer("title\n\nTest-tailer: TestValue\n", "Test-tailer", "TestValue"))
+
+ // add tailer for message with existing tailer and different value (will append)
+ assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTailer("title\n\nTest-tailer: v1", "Test-tailer", "v2"))
+ assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTailer("title\n\nTest-tailer: v1\n", "Test-tailer", "v2"))
+}
diff --git a/services/pull/patch.go b/services/pull/patch.go
index 29f2f992ab..153e0baf87 100644
--- a/services/pull/patch.go
+++ b/services/pull/patch.go
@@ -67,9 +67,8 @@ var patchErrorSuffices = []string{
": does not exist in index",
}
-// TestPatch will test whether a simple patch will apply
-func TestPatch(pr *issues_model.PullRequest) error {
- ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("TestPatch: %s", pr))
+func testPullRequestBranchMergeable(pr *issues_model.PullRequest) error {
+ ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("testPullRequestBranchMergeable: %s", pr))
defer finished()
prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr)
@@ -81,10 +80,10 @@ func TestPatch(pr *issues_model.PullRequest) error {
}
defer cancel()
- return testPatch(ctx, prCtx, pr)
+ return testPullRequestTmpRepoBranchMergeable(ctx, prCtx, pr)
}
-func testPatch(ctx context.Context, prCtx *prContext, pr *issues_model.PullRequest) error {
+func testPullRequestTmpRepoBranchMergeable(ctx context.Context, prCtx *prTmpRepoContext, pr *issues_model.PullRequest) error {
gitRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath)
if err != nil {
return fmt.Errorf("OpenRepository: %w", err)
@@ -134,7 +133,7 @@ type errMergeConflict struct {
}
func (e *errMergeConflict) Error() string {
- return fmt.Sprintf("conflict detected at: %s", e.filename)
+ return "conflict detected at: " + e.filename
}
func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, filesToRemove *[]string, filesToAdd *[]git.IndexObjectInfo) error {
@@ -355,23 +354,19 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
}
// 3b. Create a plain patch from head to base
- tmpPatchFile, err := os.CreateTemp("", "patch")
+ tmpPatchFile, cleanup, err := setting.AppDataTempDir("git-repo-content").CreateTempFileRandom("patch")
if err != nil {
log.Error("Unable to create temporary patch file! Error: %v", err)
return false, fmt.Errorf("unable to create temporary patch file! Error: %w", err)
}
- defer func() {
- _ = util.Remove(tmpPatchFile.Name())
- }()
+ defer cleanup()
if err := gitRepo.GetDiffBinary(pr.MergeBase+"...tracking", tmpPatchFile); err != nil {
- tmpPatchFile.Close()
log.Error("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err)
return false, fmt.Errorf("unable to get patch file from %s to %s in %s Error: %w", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err)
}
stat, err := tmpPatchFile.Stat()
if err != nil {
- tmpPatchFile.Close()
return false, fmt.Errorf("unable to stat patch file: %w", err)
}
patchPath := tmpPatchFile.Name()
@@ -384,7 +379,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
return false, nil
}
- log.Trace("PullRequest[%d].testPatch (patchPath): %s", pr.ID, patchPath)
+ log.Trace("PullRequest[%d].testPullRequestTmpRepoBranchMergeable (patchPath): %s", pr.ID, patchPath)
// 4. Read the base branch in to the index of the temporary repository
_, _, err = git.NewCommand("read-tree", "base").RunStdString(gitRepo.Ctx, &git.RunOpts{Dir: tmpBasePath})
@@ -454,7 +449,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
scanner := bufio.NewScanner(stderrReader)
for scanner.Scan() {
line := scanner.Text()
- log.Trace("PullRequest[%d].testPatch: stderr: %s", pr.ID, line)
+ log.Trace("PullRequest[%d].testPullRequestTmpRepoBranchMergeable: stderr: %s", pr.ID, line)
if strings.HasPrefix(line, prefix) {
conflict = true
filepath := strings.TrimSpace(strings.Split(line[len(prefix):], ":")[0])
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 4641d4ac40..701c4f4d32 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -96,7 +96,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
}
defer cancel()
- if err := testPatch(ctx, prCtx, pr); err != nil {
+ if err := testPullRequestTmpRepoBranchMergeable(ctx, prCtx, pr); err != nil {
return err
}
@@ -314,12 +314,12 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer
pr.BaseBranch = targetBranch
// Refresh patch
- if err := TestPatch(pr); err != nil {
+ if err := testPullRequestBranchMergeable(pr); err != nil {
return err
}
// Update target branch, PR diff and status
- // This is the same as checkAndUpdateStatus in check service, but also updates base_branch
+ // This is the same as markPullRequestAsMergeable in check service, but also updates base_branch
if pr.Status == issues_model.PullRequestStatusChecking {
pr.Status = issues_model.PullRequestStatusMergeable
}
@@ -409,7 +409,7 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
continue
}
- AddToTaskQueue(ctx, pr)
+ StartPullRequestCheckImmediately(ctx, pr)
comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID)
if err == nil && comment != nil {
notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment)
@@ -463,12 +463,7 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
}
if !pr.IsWorkInProgress(ctx) {
- var reviewNotifiers []*issue_service.ReviewRequestNotifier
- if opts.IsForcePush {
- reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, pr)
- } else {
- reviewNotifiers, err = issue_service.PullRequestCodeOwnersReviewSpecialCommits(ctx, pr, opts.OldCommitID, opts.NewCommitID)
- }
+ reviewNotifiers, err := issue_service.PullRequestCodeOwnersReview(ctx, pr)
if err != nil {
log.Error("PullRequestCodeOwnersReview: %v", err)
}
@@ -502,7 +497,7 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
log.Error("UpdateCommitDivergence: %v", err)
}
}
- AddToTaskQueue(ctx, pr)
+ StartPullRequestCheckDelayable(ctx, pr)
}
})
}
@@ -763,7 +758,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
var errs []error
for _, branch := range branches {
- prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repo.ID, branch.Name)
+ prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repo.ID, branch)
if err != nil {
return err
}
@@ -950,12 +945,6 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
return stringBuilder.String()
}
-// GetIssuesLastCommitStatus returns a map of issue ID to the most recent commit's latest status
-func GetIssuesLastCommitStatus(ctx context.Context, issues issues_model.IssueList) (map[int64]*git_model.CommitStatus, error) {
- _, lastStatus, err := GetIssuesAllCommitStatus(ctx, issues)
- return lastStatus, err
-}
-
// GetIssuesAllCommitStatus returns a map of issue ID to a list of all statuses for the most recent commit as well as a map of issue ID to only the commit's latest status
func GetIssuesAllCommitStatus(ctx context.Context, issues issues_model.IssueList) (map[int64][]*git_model.CommitStatus, map[int64]*git_model.CommitStatus, error) {
if err := issues.LoadPullRequests(ctx); err != nil {
@@ -1009,7 +998,7 @@ func getAllCommitStatus(ctx context.Context, gitRepo *git.Repository, pr *issues
return nil, nil, shaErr
}
- statuses, _, err = git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
+ statuses, err = git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
lastStatus = git_model.CalcCommitStatus(statuses)
return statuses, lastStatus, err
}
diff --git a/services/pull/review.go b/services/pull/review.go
index 78723a58ae..5c80e7b338 100644
--- a/services/pull/review.go
+++ b/services/pull/review.go
@@ -395,7 +395,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string,
}
if review.Type != issues_model.ReviewTypeApprove && review.Type != issues_model.ReviewTypeReject {
- return nil, fmt.Errorf("not need to dismiss this review because it's type is not Approve or change request")
+ return nil, errors.New("not need to dismiss this review because it's type is not Approve or change request")
}
// load data for notify
@@ -405,7 +405,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string,
// Check if the review's repoID is the one we're currently expecting.
if review.Issue.RepoID != repoID {
- return nil, fmt.Errorf("reviews's repository is not the same as the one we expect")
+ return nil, errors.New("reviews's repository is not the same as the one we expect")
}
issue := review.Issue
diff --git a/services/pull/reviewer.go b/services/pull/reviewer.go
index bf0d8cb298..52f2f3401c 100644
--- a/services/pull/reviewer.go
+++ b/services/pull/reviewer.go
@@ -85,5 +85,5 @@ func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*orga
return nil, nil
}
- return organization.GetTeamsWithAccessToRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
+ return organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
}
diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go
index 3f33370798..72406482e0 100644
--- a/services/pull/temp_repo.go
+++ b/services/pull/temp_repo.go
@@ -28,7 +28,7 @@ const (
stagingBranch = "staging" // this is used for a working branch
)
-type prContext struct {
+type prTmpRepoContext struct {
context.Context
tmpBasePath string
pr *issues_model.PullRequest
@@ -36,7 +36,7 @@ type prContext struct {
errbuf *strings.Builder // any use should be preceded by a Reset and preferably after use
}
-func (ctx *prContext) RunOpts() *git.RunOpts {
+func (ctx *prTmpRepoContext) RunOpts() *git.RunOpts {
ctx.outbuf.Reset()
ctx.errbuf.Reset()
return &git.RunOpts{
@@ -48,7 +48,7 @@ func (ctx *prContext) RunOpts() *git.RunOpts {
// createTemporaryRepoForPR creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch
// it also create a second base branch called "original_base"
-func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) (prCtx *prContext, cancel context.CancelFunc, err error) {
+func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) (prCtx *prTmpRepoContext, cancel context.CancelFunc, err error) {
if err := pr.LoadHeadRepo(ctx); err != nil {
log.Error("%-v LoadHeadRepo: %v", pr, err)
return nil, nil, fmt.Errorf("%v LoadHeadRepo: %w", pr, err)
@@ -74,23 +74,20 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
}
// Clone base repo.
- tmpBasePath, err := repo_module.CreateTemporaryPath("pull")
+ tmpBasePath, cleanup, err := repo_module.CreateTemporaryPath("pull")
if err != nil {
log.Error("CreateTemporaryPath[%-v]: %v", pr, err)
return nil, nil, err
}
- prCtx = &prContext{
+ cancel = cleanup
+
+ prCtx = &prTmpRepoContext{
Context: ctx,
tmpBasePath: tmpBasePath,
pr: pr,
outbuf: &strings.Builder{},
errbuf: &strings.Builder{},
}
- cancel = func() {
- if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil {
- log.Error("Error whilst removing removing temporary repo for %-v: %v", pr, err)
- }
- }
baseRepoPath := pr.BaseRepo.RepoPath()
headRepoPath := pr.HeadRepo.RepoPath()
diff --git a/services/pull/update.go b/services/pull/update.go
index 3e00dd4e65..b8f84e3d65 100644
--- a/services/pull/update.go
+++ b/services/pull/update.go
@@ -5,6 +5,7 @@ package pull
import (
"context"
+ "errors"
"fmt"
git_model "code.gitea.io/gitea/models/git"
@@ -23,7 +24,7 @@ import (
func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, message string, rebase bool) error {
if pr.Flow == issues_model.PullRequestFlowAGit {
// TODO: update of agit flow pull request's head branch is unsupported
- return fmt.Errorf("update of agit flow pull request's head branch is unsupported")
+ return errors.New("update of agit flow pull request's head branch is unsupported")
}
releaser, err := globallock.Lock(ctx, getPullWorkingLockKey(pr.ID))
@@ -40,22 +41,6 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
return fmt.Errorf("HeadBranch of PR %d is up to date", pr.Index)
}
- if rebase {
- defer func() {
- go AddTestPullRequestTask(TestPullRequestOptions{
- RepoID: pr.BaseRepo.ID,
- Doer: doer,
- Branch: pr.BaseBranch,
- IsSync: false,
- IsForcePush: false,
- OldCommitID: "",
- NewCommitID: "",
- })
- }()
-
- return updateHeadByRebaseOnToBase(ctx, pr, doer)
- }
-
if err := pr.LoadBaseRepo(ctx); err != nil {
log.Error("unable to load BaseRepo for %-v during update-by-merge: %v", pr, err)
return fmt.Errorf("unable to load BaseRepo for PR[%d] during update-by-merge: %w", pr.ID, err)
@@ -73,6 +58,22 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
return fmt.Errorf("unable to load HeadRepo for PR[%d] during update-by-merge: %w", pr.ID, err)
}
+ defer func() {
+ go AddTestPullRequestTask(TestPullRequestOptions{
+ RepoID: pr.BaseRepo.ID,
+ Doer: doer,
+ Branch: pr.BaseBranch,
+ IsSync: false,
+ IsForcePush: false,
+ OldCommitID: "",
+ NewCommitID: "",
+ })
+ }()
+
+ if rebase {
+ return updateHeadByRebaseOnToBase(ctx, pr, doer)
+ }
+
// TODO: FakePR: it is somewhat hacky, but it is the only way to "merge" at the moment
// ideally in the future the "merge" functions should be refactored to decouple from the PullRequest
// now use a fake reverse PR to switch head&base repos/branches
@@ -89,19 +90,6 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
}
_, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase)
-
- defer func() {
- go AddTestPullRequestTask(TestPullRequestOptions{
- RepoID: reversePR.HeadRepo.ID,
- Doer: doer,
- Branch: reversePR.HeadBranch,
- IsSync: false,
- IsForcePush: false,
- OldCommitID: "",
- NewCommitID: "",
- })
- }()
-
return err
}
generated by cgit v1.2.3 (git 2.39.1) at 2025-07-04 08:37:09 +0000