diff --git a/integrations/pull_status_test.go b/integrations/pull_status_test.go index a5247f56ec..33a27cd812 100644 --- a/integrations/pull_status_test.go +++ b/integrations/pull_status_test.go @@ -105,7 +105,11 @@ func doAPICreateCommitStatus(ctx APITestContext, commitID string, status api.Com } } -func TestPullCreate_EmptyChangesWithCommits(t *testing.T) { +func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) { + // Merge must continue if commits SHA are different, even if content is same + // Reason: gitflow and merging master back into develop, where is high possiblity, there are no changes + // but just commit saying "Merge branch". And this meta commit can be also tagged, + // so we need to have this meta commit also in develop branch. onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") @@ -126,6 +130,28 @@ func TestPullCreate_EmptyChangesWithCommits(t *testing.T) { doc := NewHTMLParser(t, resp.Body) text := strings.TrimSpace(doc.doc.Find(".merge-section").Text()) - assert.Contains(t, text, "This branch is equal with the target branch.") + assert.Contains(t, text, "This pull request can be merged automatically.") + }) +} + +func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + session := loginUser(t, "user1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testCreateBranch(t, session, "user1", "repo1", "branch/master", "status1", http.StatusSeeOther) + url := path.Join("user1", "repo1", "compare", "master...status1") + req := NewRequestWithValues(t, "POST", url, + map[string]string{ + "_csrf": GetCSRF(t, session, url), + "title": "pull request from status1", + }, + ) + session.MakeRequest(t, req, http.StatusSeeOther) + req = NewRequest(t, "GET", "/user1/repo1/pulls/1") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + text := strings.TrimSpace(doc.doc.Find(".merge-section").Text()) + assert.Contains(t, text, "This branch is already included in the target branch. There is nothing to merge.") }) } diff --git a/models/issues/pull.go b/models/issues/pull.go index 52b9596889..f96b03445e 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -122,6 +122,7 @@ const ( PullRequestStatusManuallyMerged PullRequestStatusError PullRequestStatusEmpty + PullRequestStatusAncestor ) // PullRequestFlow the flow of pull request @@ -423,6 +424,11 @@ func (pr *PullRequest) IsEmpty() bool { return pr.Status == PullRequestStatusEmpty } +// IsAncestor returns true if the Head Commit of this PR is an ancestor of the Base Commit +func (pr *PullRequest) IsAncestor() bool { + return pr.Status == PullRequestStatusAncestor +} + // SetMerged sets a pull request to merged and closes the corresponding issue func (pr *PullRequest) SetMerged(ctx context.Context) (bool, error) { if pr.HasMerged { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 9b69d54593..9e8a030339 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1532,7 +1532,8 @@ pulls.remove_prefix = Remove %s prefix pulls.data_broken = This pull request is broken due to missing fork information. pulls.files_conflicted = This pull request has changes conflicting with the target branch. pulls.is_checking = "Merge conflict checking is in progress. Try again in few moments." -pulls.is_empty = "This branch is equal with the target branch." +pulls.is_ancestor = "This branch is already included in the target branch. There is nothing to merge." +pulls.is_empty = "The changes on this branch are already on the target branch. This will be an empty commit." pulls.required_status_check_failed = Some required checks were not successful. pulls.required_status_check_missing = Some required checks are missing. pulls.required_status_check_administrator = As an administrator, you may still merge this pull request. diff --git a/services/pull/check.go b/services/pull/check.go index 6621a281fa..288f4dc0b7 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -89,7 +89,7 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce return ErrIsWorkInProgress } - if !pr.CanAutoMerge() { + if !pr.CanAutoMerge() && !pr.IsEmpty() { return ErrNotMergableState } diff --git a/services/pull/patch.go b/services/pull/patch.go index c7a69501c3..bb09acc89f 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -87,6 +87,14 @@ func TestPatch(pr *issues_model.PullRequest) error { } } pr.MergeBase = strings.TrimSpace(pr.MergeBase) + if pr.HeadCommitID, err = gitRepo.GetRefCommitID(git.BranchPrefix + "tracking"); err != nil { + return fmt.Errorf("GetBranchCommitID: can't find commit ID for head: %w", err) + } + + if pr.HeadCommitID == pr.MergeBase { + pr.Status = issues_model.PullRequestStatusAncestor + return nil + } // 2. Check for conflicts if conflicts, err := checkConflicts(ctx, pr, gitRepo, tmpBasePath); err != nil || conflicts || pr.Status == issues_model.PullRequestStatusEmpty { diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 179f857ba6..60fe667a54 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -195,12 +195,12 @@ {{svg "octicon-sync"}} {{$.locale.Tr "repo.pulls.is_checking"}} - {{else if .Issue.PullRequest.IsEmpty}} + {{else if .Issue.PullRequest.IsAncestor}}