aboutsummaryrefslogtreecommitdiffstats
path: root/services/pull
diff options
context:
space:
mode:
Diffstat (limited to 'services/pull')
-rw-r--r--services/pull/commit_status.go99
-rw-r--r--services/pull/commit_status_test.go92
-rw-r--r--services/pull/merge.go36
-rw-r--r--services/pull/merge_squash.go7
-rw-r--r--services/pull/merge_test.go25
-rw-r--r--services/pull/pull.go6
-rw-r--r--services/pull/update.go45
7 files changed, 165 insertions, 145 deletions
diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go
index 553496713f..d3a0f718a7 100644
--- a/services/pull/commit_status.go
+++ b/services/pull/commit_status.go
@@ -10,93 +10,56 @@ 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))
+ for _, gp := range requiredContextsGlob {
for _, commitStatus := range commitStatuses {
- if commitStatus.Context == ctx {
- if commitStatus.State != structs.CommitStatusSuccess {
- return false
- }
-
- found = true
+ if gp.Match(commitStatus.Context) {
+ requiredCommitStatuses = append(requiredCommitStatuses, commitStatus)
break
}
}
- if !found {
- return false
- }
}
- return true
+ if len(requiredCommitStatuses) == 0 {
+ return commitstatus.CommitStatusPending
+ }
+
+ returnedStatus := git_model.CalcCommitStatus(requiredCommitStatuses).State
+ if len(requiredCommitStatuses) == len(requiredContexts) {
+ return returnedStatus
+ }
+
+ if returnedStatus == commitstatus.CommitStatusFailure {
+ return commitstatus.CommitStatusFailure
+ }
+ // even if part of success, return pending
+ return commitstatus.CommitStatusPending
}
// IsPullCommitStatusPass returns if all required status checks PASS
@@ -117,7 +80,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")
diff --git a/services/pull/commit_status_test.go b/services/pull/commit_status_test.go
index 592acdd55c..b985a9de8e 100644
--- a/services/pull/commit_status_test.go
+++ b/services/pull/commit_status_test.go
@@ -8,58 +8,76 @@ 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.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..829d4b7ee1 100644
--- a/services/pull/merge.go
+++ b/services/pull/merge.go
@@ -13,6 +13,7 @@ import (
"regexp"
"strconv"
"strings"
+ "unicode"
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
@@ -161,6 +162,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
diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go
index 72660cd3c5..119b28736c 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,10 +65,8 @@ 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).
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/pull.go b/services/pull/pull.go
index f879f61136..701c4f4d32 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -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 {
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
}