aboutsummaryrefslogtreecommitdiffstats
path: root/services/pull
diff options
context:
space:
mode:
Diffstat (limited to 'services/pull')
-rw-r--r--services/pull/check.go21
-rw-r--r--services/pull/check_test.go52
-rw-r--r--services/pull/commit_status.go110
-rw-r--r--services/pull/commit_status_test.go101
-rw-r--r--services/pull/merge.go118
-rw-r--r--services/pull/merge_prepare.go6
-rw-r--r--services/pull/merge_squash.go14
-rw-r--r--services/pull/merge_test.go25
-rw-r--r--services/pull/patch.go2
-rw-r--r--services/pull/pull.go30
-rw-r--r--services/pull/review.go10
-rw-r--r--services/pull/reviewer.go2
-rw-r--r--services/pull/temp_repo.go2
-rw-r--r--services/pull/update.go45
14 files changed, 312 insertions, 226 deletions
diff --git a/services/pull/check.go b/services/pull/check.go
index 5d8990aa00..7fcec22f49 100644
--- a/services/pull/check.go
+++ b/services/pull/check.go
@@ -1,5 +1,4 @@
-// Copyright 2019 The Gitea Authors.
-// All rights reserved.
+// Copyright 2019 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package pull
@@ -16,6 +15,7 @@ import (
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
access_model "code.gitea.io/gitea/models/perm/access"
+ "code.gitea.io/gitea/models/pull"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
@@ -29,6 +29,7 @@ import (
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil"
asymkey_service "code.gitea.io/gitea/services/asymkey"
+ "code.gitea.io/gitea/services/automergequeue"
notify_service "code.gitea.io/gitea/services/notify"
)
@@ -230,7 +231,7 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer
return true, nil
}
- sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName())
+ sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName())
return sign, err
}
@@ -238,7 +239,7 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer
// markPullRequestAsMergeable checks if pull request is possible to leaving checking status,
// and set to be either conflict or 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 the status has not been changed to conflict by testPullRequestTmpRepoBranchMergeable then we are mergeable
if pr.Status == issues_model.PullRequestStatusChecking {
pr.Status = issues_model.PullRequestStatusMergeable
}
@@ -257,6 +258,16 @@ func markPullRequestAsMergeable(ctx context.Context, pr *issues_model.PullReques
if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files"); err != nil {
log.Error("Update[%-v]: %v", pr, err)
}
+
+ // if there is a scheduled merge for this pull request, start the auto merge check (again)
+ exist, _, err := pull.GetScheduledMergeByPullID(ctx, pr.ID)
+ if err != nil {
+ log.Error("GetScheduledMergeByPullID[%-v]: %v", pr, err)
+ return
+ } else if !exist {
+ return
+ }
+ automergequeue.StartPRCheckAndAutoMerge(ctx, pr)
}
// getMergeCommit checks if a pull request has been merged
@@ -266,7 +277,7 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com
return nil, fmt.Errorf("unable to load base repo for %s: %w", pr, err)
}
- prHeadRef := pr.GetGitRefName()
+ prHeadRef := pr.GetGitHeadRefName()
// Check if the pull request is merged into BaseBranch
if _, _, err := git.NewCommand("merge-base", "--is-ancestor").
diff --git a/services/pull/check_test.go b/services/pull/check_test.go
index fa3a676ef1..eb66615dcf 100644
--- a/services/pull/check_test.go
+++ b/services/pull/check_test.go
@@ -1,5 +1,4 @@
-// Copyright 2019 The Gitea Authors.
-// All rights reserved.
+// Copyright 2019 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package pull
@@ -11,11 +10,18 @@ import (
"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
+ "code.gitea.io/gitea/models/pull"
+ repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
+ user_model "code.gitea.io/gitea/models/user"
+ "code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/queue"
"code.gitea.io/gitea/modules/setting"
+ "code.gitea.io/gitea/modules/test"
+ "code.gitea.io/gitea/services/automergequeue"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func TestPullRequest_AddToTaskQueue(t *testing.T) {
@@ -63,6 +69,46 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
assert.Equal(t, issues_model.PullRequestStatusChecking, pr.Status)
- prPatchCheckerQueue.ShutdownWait(5 * time.Second)
+ prPatchCheckerQueue.ShutdownWait(time.Second)
prPatchCheckerQueue = nil
}
+
+func TestMarkPullRequestAsMergeable(t *testing.T) {
+ assert.NoError(t, unittest.PrepareTestDatabase())
+
+ prPatchCheckerQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_patch_checker", func(items ...string) []string { return nil })
+ go prPatchCheckerQueue.Run()
+ defer func() {
+ prPatchCheckerQueue.ShutdownWait(time.Second)
+ prPatchCheckerQueue = nil
+ }()
+
+ addToQueueShaChan := make(chan string, 1)
+ defer test.MockVariableValue(&automergequeue.AddToQueue, func(pr *issues_model.PullRequest, sha string) {
+ addToQueueShaChan <- sha
+ })()
+ ctx := t.Context()
+ _, _ = db.GetEngine(ctx).ID(2).Update(&issues_model.PullRequest{Status: issues_model.PullRequestStatusChecking})
+ pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
+ require.False(t, pr.HasMerged)
+ require.Equal(t, issues_model.PullRequestStatusChecking, pr.Status)
+
+ err := pull.ScheduleAutoMerge(ctx, &user_model.User{ID: 99999}, pr.ID, repo_model.MergeStyleMerge, "test msg", true)
+ require.NoError(t, err)
+
+ exist, scheduleMerge, err := pull.GetScheduledMergeByPullID(ctx, pr.ID)
+ require.NoError(t, err)
+ assert.True(t, exist)
+ assert.True(t, scheduleMerge.Doer.IsGhost())
+
+ markPullRequestAsMergeable(ctx, pr)
+ pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
+ require.Equal(t, issues_model.PullRequestStatusMergeable, pr.Status)
+
+ select {
+ case sha := <-addToQueueShaChan:
+ assert.Equal(t, "985f0301dba5e7b34be866819cd15ad3d8f508ee", sha) // ref: refs/pull/3/head
+ case <-time.After(1 * time.Second):
+ assert.FailNow(t, "Timeout: nothing was added to automergequeue")
+ }
+}
diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go
index 0bfff21746..d15d318149 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")
@@ -133,7 +99,7 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR
if pr.Flow == issues_model.PullRequestFlowGithub && !gitrepo.IsBranchExist(ctx, pr.HeadRepo, pr.HeadBranch) {
return "", errors.New("Head branch does not exist, can not merge")
}
- if pr.Flow == issues_model.PullRequestFlowAGit && !gitrepo.IsReferenceExist(ctx, pr.HeadRepo, pr.GetGitRefName()) {
+ if pr.Flow == issues_model.PullRequestFlowAGit && !gitrepo.IsReferenceExist(ctx, pr.HeadRepo, pr.GetGitHeadRefName()) {
return "", errors.New("Head branch does not exist, can not merge")
}
@@ -141,7 +107,7 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR
if pr.Flow == issues_model.PullRequestFlowGithub {
sha, err = headGitRepo.GetBranchCommitID(pr.HeadBranch)
} else {
- sha, err = headGitRepo.GetRefCommitID(pr.GetGitRefName())
+ sha, err = headGitRepo.GetRefCommitID(pr.GetGitHeadRefName())
}
if err != nil {
return "", err
@@ -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 256db847ef..a941c20435 100644
--- a/services/pull/merge.go
+++ b/services/pull/merge.go
@@ -8,11 +8,13 @@ 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"
@@ -94,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))
@@ -161,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
@@ -290,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 {
@@ -396,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())
@@ -649,48 +687,40 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID
return false, fmt.Errorf("unable to merge PullRequest[%d], some required fields are empty", pr.Index)
}
- ctx, committer, err := db.TxContext(ctx)
- if err != nil {
- return false, err
- }
- defer committer.Close()
-
- pr.Issue = nil
- if err := pr.LoadIssue(ctx); err != nil {
- return false, err
- }
-
- if err := pr.Issue.LoadRepo(ctx); err != nil {
- return false, err
- }
+ return db.WithTx2(ctx, func(ctx context.Context) (bool, error) {
+ pr.Issue = nil
+ if err := pr.LoadIssue(ctx); err != nil {
+ return false, err
+ }
- if err := pr.Issue.Repo.LoadOwner(ctx); err != nil {
- return false, err
- }
+ if err := pr.Issue.LoadRepo(ctx); err != nil {
+ return false, err
+ }
- // Removing an auto merge pull and ignore if not exist
- if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) {
- return false, fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", pr.ID, err)
- }
+ if err := pr.Issue.Repo.LoadOwner(ctx); err != nil {
+ return false, err
+ }
- // Set issue as closed
- if _, err := issues_model.SetIssueAsClosed(ctx, pr.Issue, pr.Merger, true); err != nil {
- return false, fmt.Errorf("ChangeIssueStatus: %w", err)
- }
+ // Removing an auto merge pull and ignore if not exist
+ if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) {
+ return false, fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", pr.ID, err)
+ }
- // 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").
- Update(pr); err != nil {
- return false, fmt.Errorf("failed to update pr[%d]: %w", pr.ID, err)
- } else if cnt != 1 {
- return false, issues_model.ErrIssueAlreadyChanged
- }
+ // Set issue as closed
+ if _, err := issues_model.SetIssueAsClosed(ctx, pr.Issue, pr.Merger, true); err != nil {
+ return false, fmt.Errorf("ChangeIssueStatus: %w", err)
+ }
- if err := committer.Commit(); err != nil {
- return false, err
- }
+ // 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").
+ Update(pr); err != nil {
+ return false, fmt.Errorf("failed to update pr[%d]: %w", pr.ID, err)
+ } else if cnt != 1 {
+ return false, issues_model.ErrIssueAlreadyChanged
+ }
- return true, nil
+ return true, nil
+ })
}
diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go
index 719cc6b965..31a1e13734 100644
--- a/services/pull/merge_prepare.go
+++ b/services/pull/merge_prepare.go
@@ -27,7 +27,7 @@ type mergeContext struct {
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
}
@@ -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 72660cd3c5..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, "Co-authored-by: "+sig.String()) {
- message += "\nCo-authored-by: " + 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 153e0baf87..9d9b8d0d07 100644
--- a/services/pull/patch.go
+++ b/services/pull/patch.go
@@ -41,7 +41,7 @@ func DownloadDiffOrPatch(ctx context.Context, pr *issues_model.PullRequest, w io
}
defer closer.Close()
- compareArg := pr.MergeBase + "..." + pr.GetGitRefName()
+ compareArg := pr.MergeBase + "..." + pr.GetGitHeadRefName()
switch {
case patch:
err = gitRepo.GetPatch(compareArg, w)
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 81be797832..2829e15441 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -143,7 +143,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
}
compareInfo, err := baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(),
- git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName(), false, false)
+ git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false, false)
if err != nil {
return err
}
@@ -184,7 +184,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
return nil
}); err != nil {
// cleanup: this will only remove the reference, the real commit will be clean up when next GC
- if err1 := baseGitRepo.RemoveReference(pr.GetGitRefName()); err1 != nil {
+ if err1 := baseGitRepo.RemoveReference(pr.GetGitHeadRefName()); err1 != nil {
log.Error("RemoveReference: %v", err1)
}
return err
@@ -567,7 +567,7 @@ func PushToBaseRepo(ctx context.Context, pr *issues_model.PullRequest) (err erro
}
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.GetGitRefName())
+ 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)
@@ -588,7 +588,7 @@ func pushToBaseRepoHelper(ctx context.Context, pr *issues_model.PullRequest, pre
return fmt.Errorf("unable to load poster %d for pr %d: %w", pr.Issue.PosterID, pr.ID, err)
}
- gitRefName := pr.GetGitRefName()
+ gitRefName := pr.GetGitHeadRefName()
if err := git.Push(ctx, headRepoPath, git.PushOptions{
Remote: baseRepoPath,
@@ -642,13 +642,13 @@ func UpdatePullsRefs(ctx context.Context, repo *repo_model.Repository, update *r
// 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.GetGitRefName())
+ log.Trace("UpdateRef[%d]: upgate 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
}
- _, _, err = git.NewCommand("update-ref").AddDynamicArguments(pr.GetGitRefName(), pr.HeadCommitID).RunStdString(ctx, &git.RunOpts{Dir: pr.BaseRepo.RepoPath()})
+ _, _, err = git.NewCommand("update-ref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.HeadCommitID).RunStdString(ctx, &git.RunOpts{Dir: pr.BaseRepo.RepoPath()})
if err != nil {
log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err)
}
@@ -816,9 +816,9 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
if pr.Flow == issues_model.PullRequestFlowGithub {
headCommit, err = gitRepo.GetBranchCommit(pr.HeadBranch)
} else {
- pr.HeadCommitID, err = gitRepo.GetRefCommitID(pr.GetGitRefName())
+ pr.HeadCommitID, err = gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
- log.Error("Unable to get head commit: %s Error: %v", pr.GetGitRefName(), err)
+ log.Error("Unable to get head commit: %s Error: %v", pr.GetGitHeadRefName(), err)
return ""
}
headCommit, err = gitRepo.GetCommit(pr.HeadCommitID)
@@ -945,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 {
@@ -999,12 +993,12 @@ func GetIssuesAllCommitStatus(ctx context.Context, issues issues_model.IssueList
// getAllCommitStatus get pr's commit statuses.
func getAllCommitStatus(ctx context.Context, gitRepo *git.Repository, pr *issues_model.PullRequest) (statuses []*git_model.CommitStatus, lastStatus *git_model.CommitStatus, err error) {
- sha, shaErr := gitRepo.GetRefCommitID(pr.GetGitRefName())
+ sha, shaErr := gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
if shaErr != nil {
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
}
@@ -1049,7 +1043,7 @@ func IsHeadEqualWithBranch(ctx context.Context, pr *issues_model.PullRequest, br
return false, err
}
} else {
- pr.HeadCommitID, err = baseGitRepo.GetRefCommitID(pr.GetGitRefName())
+ pr.HeadCommitID, err = baseGitRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
return false, err
}
@@ -1083,7 +1077,7 @@ func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]Co
if pull.HasMerged {
baseBranch = pull.MergeBase
}
- prInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), baseBranch, pull.GetGitRefName(), true, false)
+ prInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), baseBranch, pull.GetGitHeadRefName(), true, false)
if err != nil {
return nil, "", err
}
diff --git a/services/pull/review.go b/services/pull/review.go
index 5c80e7b338..ee18db3859 100644
--- a/services/pull/review.go
+++ b/services/pull/review.go
@@ -200,7 +200,7 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
defer closer.Close()
invalidated := false
- head := pr.GetGitRefName()
+ head := pr.GetGitHeadRefName()
if line > 0 {
if reviewID != 0 {
first, err := issues_model.FindComments(ctx, &issues_model.FindCommentsOptions{
@@ -237,16 +237,16 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
if err == nil {
commitID = commit.ID.String()
} else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
- return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %w", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
+ return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %w", pr.GetGitHeadRefName(), gitRepo.Path, treePath, line, err)
}
}
}
// Only fetch diff if comment is review comment
if len(patch) == 0 && reviewID != 0 {
- headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
+ headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
- return nil, fmt.Errorf("GetRefCommitID[%s]: %w", pr.GetGitRefName(), err)
+ return nil, fmt.Errorf("GetRefCommitID[%s]: %w", pr.GetGitHeadRefName(), err)
}
if len(commitID) == 0 {
commitID = headCommitID
@@ -301,7 +301,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos
return nil, nil, ErrSubmitReviewOnClosedPR
}
- headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
+ headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
return nil, nil, err
}
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 72406482e0..89f150fd92 100644
--- a/services/pull/temp_repo.go
+++ b/services/pull/temp_repo.go
@@ -174,7 +174,7 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
} else if len(pr.HeadCommitID) == objectFormat.FullLength() { // for not created pull request
headBranch = pr.HeadCommitID
} else {
- headBranch = pr.GetGitRefName()
+ headBranch = pr.GetGitHeadRefName()
}
if err := git.NewCommand("fetch").AddArguments(fetchArgs...).AddDynamicArguments(remoteRepoName, headBranch+":"+trackingBranch).
Run(ctx, prCtx.RunOpts()); err != nil {
diff --git a/services/pull/update.go b/services/pull/update.go
index 5cc5e2b134..b8f84e3d65 100644
--- a/services/pull/update.go
+++ b/services/pull/update.go
@@ -41,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)
@@ -74,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
@@ -90,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
}