Skip to content

Commit d686e09

Browse files
committed
Fix checks in PR for empty commits #19603
* Fixes issue #19603 * fill HeadCommitID in PullRequest * compare real commits ID as check for merging * basd on zeripath patch
1 parent c273dea commit d686e09

File tree

6 files changed

+48
-8
lines changed

6 files changed

+48
-8
lines changed

Diff for: integrations/pull_status_test.go

+27-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,11 @@ func doAPICreateCommitStatus(ctx APITestContext, commitID string, status api.Com
105105
}
106106
}
107107

108-
func TestPullCreate_EmptyChangesWithCommits(t *testing.T) {
108+
func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) {
109+
// Merge must continue if commits SHA are different, even if content is same
110+
// Reason: gitflow and merging master back into develop, where is high possiblity, there are no changes
111+
// but just commit saying "Merge branch". And this meta commit can be also tagged,
112+
// so we need to have this meta commit also in develop branch.
109113
onGiteaRun(t, func(t *testing.T, u *url.URL) {
110114
session := loginUser(t, "user1")
111115
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
@@ -125,6 +129,28 @@ func TestPullCreate_EmptyChangesWithCommits(t *testing.T) {
125129
resp := session.MakeRequest(t, req, http.StatusOK)
126130
doc := NewHTMLParser(t, resp.Body)
127131

132+
text := strings.TrimSpace(doc.doc.Find(".merge-section").Text())
133+
assert.Contains(t, text, "This pull request can be merged automatically.")
134+
})
135+
}
136+
137+
func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) {
138+
onGiteaRun(t, func(t *testing.T, u *url.URL) {
139+
session := loginUser(t, "user1")
140+
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
141+
testCreateBranch(t, session, "user1", "repo1", "branch/master", "status1", http.StatusSeeOther)
142+
url := path.Join("user1", "repo1", "compare", "master...status1")
143+
req := NewRequestWithValues(t, "POST", url,
144+
map[string]string{
145+
"_csrf": GetCSRF(t, session, url),
146+
"title": "pull request from status1",
147+
},
148+
)
149+
session.MakeRequest(t, req, http.StatusSeeOther)
150+
req = NewRequest(t, "GET", "/user1/repo1/pulls/1")
151+
resp := session.MakeRequest(t, req, http.StatusOK)
152+
doc := NewHTMLParser(t, resp.Body)
153+
128154
text := strings.TrimSpace(doc.doc.Find(".merge-section").Text())
129155
assert.Contains(t, text, "This branch is equal with the target branch.")
130156
})

Diff for: models/issues/pull.go

+5
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,11 @@ func (pr *PullRequest) IsEmpty() bool {
423423
return pr.Status == PullRequestStatusEmpty
424424
}
425425

426+
// IsAncestor returns true if the Head Commit of this PR is an ancestor of the Base Commit
427+
func (pr *PullRequest) IsAncestor() bool {
428+
return pr.HeadCommitID == pr.MergeBase
429+
}
430+
426431
// SetMerged sets a pull request to merged and closes the corresponding issue
427432
func (pr *PullRequest) SetMerged(ctx context.Context) (bool, error) {
428433
if pr.HasMerged {

Diff for: options/locale/locale_en-US.ini

+2-1
Original file line numberDiff line numberDiff line change
@@ -1530,7 +1530,8 @@ pulls.remove_prefix = Remove <strong>%s</strong> prefix
15301530
pulls.data_broken = This pull request is broken due to missing fork information.
15311531
pulls.files_conflicted = This pull request has changes conflicting with the target branch.
15321532
pulls.is_checking = "Merge conflict checking is in progress. Try again in few moments."
1533-
pulls.is_empty = "This branch is equal with the target branch."
1533+
pulls.is_ancestor = "This branch is already included in the target branch. There is nothing to merge."
1534+
pulls.is_empty = "The changes on this branch are already on the target branch. This will be an empty commit."
15341535
pulls.required_status_check_failed = Some required checks were not successful.
15351536
pulls.required_status_check_missing = Some required checks are missing.
15361537
pulls.required_status_check_administrator = As an administrator, you may still merge this pull request.

Diff for: services/pull/check.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce
8989
return ErrIsWorkInProgress
9090
}
9191

92-
if !pr.CanAutoMerge() {
92+
if !pr.CanAutoMerge() && !pr.IsEmpty() || pr.IsAncestor() {
9393
return ErrNotMergableState
9494
}
9595

Diff for: templates/repo/issue/view_content/pull.tmpl

+12-4
Original file line numberDiff line numberDiff line change
@@ -195,12 +195,12 @@
195195
<i class="icon icon-octicon">{{svg "octicon-sync"}}</i>
196196
{{$.locale.Tr "repo.pulls.is_checking"}}
197197
</div>
198-
{{else if .Issue.PullRequest.IsEmpty}}
198+
{{else if .Issue.PullRequest.IsAncestor}}
199199
<div class="item">
200200
<i class="icon icon-octicon">{{svg "octicon-alert" 16}}</i>
201-
{{$.locale.Tr "repo.pulls.is_empty"}}
201+
{{$.locale.Tr "repo.pulls.is_ancestor"}}
202202
</div>
203-
{{else if .Issue.PullRequest.CanAutoMerge}}
203+
{{else if or .Issue.PullRequest.CanAutoMerge .Issue.PullRequest.IsEmpty}}
204204
{{if .IsBlockedByApprovals}}
205205
<div class="item">
206206
<i class="icon icon-octicon">{{svg "octicon-x"}}</i>
@@ -282,7 +282,6 @@
282282
</div>
283283
{{end}}
284284
{{end}}
285-
286285
{{if and (gt .Issue.PullRequest.CommitsBehind 0) (not .Issue.IsClosed) (not .Issue.PullRequest.IsChecking) (not .IsPullFilesConflicted) (not .IsPullRequestBroken) (not $canAutoMerge)}}
287286
<div class="ui divider"></div>
288287
<div class="item item-section">
@@ -321,6 +320,14 @@
321320
</div>
322321
</div>
323322
{{end}}
323+
{{if .Issue.PullRequest.IsEmpty}}
324+
<div class="ui divider"></div>
325+
326+
<div class="item">
327+
<i class="icon icon-octicon">{{svg "octicon-alert" 16}}</i>
328+
{{$.locale.Tr "repo.pulls.is_empty"}}
329+
</div>
330+
{{end}}
324331

325332
{{if .AllowMerge}} {{/* user is allowed to merge */}}
326333
{{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}}
@@ -348,6 +355,7 @@
348355

349356
'canMergeNow': {{$canMergeNow}},
350357
'allOverridableChecksOk': {{not $notAllOverridableChecksOk}},
358+
'emptyCommit': {{.Issue.PullRequest.IsEmpty}},
351359
'pullHeadCommitID': {{.PullHeadCommitID}},
352360
'isPullBranchDeletable': {{.IsPullBranchDeletable}},
353361
'defaultDeleteBranchAfterMerge': {{$prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}},

Diff for: web_src/js/components/PullRequestMergeForm.vue

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848

4949
<div v-if="!showActionForm" class="df">
5050
<!-- the merge button -->
51-
<div class="ui buttons merge-button" :class="mergeButtonStyleClass" @click="toggleActionForm(true)" >
51+
<div class="ui buttons merge-button" :class="[mergeForm.emptyCommit?'grey':mergeForm.allOverridableChecksOk?'green':'red']" @click="toggleActionForm(true)" >
5252
<button class="ui button">
5353
<svg-icon name="octicon-git-merge"/>
5454
<span class="button-text">

0 commit comments

Comments
 (0)