mirror of https://github.com/go-gitea/gitea.git
Abort merge if head has been updated before pressing merge (#18032)
* Abort merge if head has been updated before pressing merge It is possible that a PR head may be pushed to between the merge page being shown and the merge button being pressed. Pass the current expected head in as a parameter and cancel the merge if it has changed. Fix #18028 Signed-off-by: Andrew Thornton <art27@cantab.net> * adjust swagger Signed-off-by: Andrew Thornton <art27@cantab.net> * fix test Signed-off-by: Andrew Thornton <art27@cantab.net> * placate lint Signed-off-by: Andrew Thornton <art27@cantab.net>pull/18040/head
parent
b24a965b81
commit
e4e411821d
|
@ -241,11 +241,11 @@ func TestCantMergeConflict(t *testing.T) {
|
||||||
gitRepo, err := git.OpenRepository(repo_model.RepoPath(user1.Name, repo1.Name))
|
gitRepo, err := git.OpenRepository(repo_model.RepoPath(user1.Name, repo1.Name))
|
||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
|
|
||||||
err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "CONFLICT")
|
err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "CONFLICT")
|
||||||
assert.Error(t, err, "Merge should return an error due to conflict")
|
assert.Error(t, err, "Merge should return an error due to conflict")
|
||||||
assert.True(t, models.IsErrMergeConflicts(err), "Merge error is not a conflict error")
|
assert.True(t, models.IsErrMergeConflicts(err), "Merge error is not a conflict error")
|
||||||
|
|
||||||
err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleRebase, "CONFLICT")
|
err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT")
|
||||||
assert.Error(t, err, "Merge should return an error due to conflict")
|
assert.Error(t, err, "Merge should return an error due to conflict")
|
||||||
assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error")
|
assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error")
|
||||||
gitRepo.Close()
|
gitRepo.Close()
|
||||||
|
@ -329,7 +329,7 @@ func TestCantMergeUnrelated(t *testing.T) {
|
||||||
BaseBranch: "base",
|
BaseBranch: "base",
|
||||||
}).(*models.PullRequest)
|
}).(*models.PullRequest)
|
||||||
|
|
||||||
err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "UNRELATED")
|
err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED")
|
||||||
assert.Error(t, err, "Merge should return an error due to unrelated")
|
assert.Error(t, err, "Merge should return an error due to unrelated")
|
||||||
assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error")
|
assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error")
|
||||||
gitRepo.Close()
|
gitRepo.Close()
|
||||||
|
|
|
@ -1506,6 +1506,7 @@ pulls.rebase_conflict_summary = Error Message
|
||||||
; </summary><code>%[2]s<br>%[3]s</code></details>
|
; </summary><code>%[2]s<br>%[3]s</code></details>
|
||||||
pulls.unrelated_histories = Merge Failed: The merge head and base do not share a common history. Hint: Try a different strategy
|
pulls.unrelated_histories = Merge Failed: The merge head and base do not share a common history. Hint: Try a different strategy
|
||||||
pulls.merge_out_of_date = Merge Failed: Whilst generating the merge, the base was updated. Hint: Try again.
|
pulls.merge_out_of_date = Merge Failed: Whilst generating the merge, the base was updated. Hint: Try again.
|
||||||
|
pulls.head_out_of_date = Merge Failed: Whilst generating the merge, the head was updated. Hint: Try again.
|
||||||
pulls.push_rejected = Merge Failed: The push was rejected. Review the githooks for this repository.
|
pulls.push_rejected = Merge Failed: The push was rejected. Review the githooks for this repository.
|
||||||
pulls.push_rejected_summary = Full Rejection Message
|
pulls.push_rejected_summary = Full Rejection Message
|
||||||
pulls.push_rejected_no_message = Merge Failed: The push was rejected but there was no remote message.<br>Review the githooks for this repository
|
pulls.push_rejected_no_message = Merge Failed: The push was rejected but there was no remote message.<br>Review the githooks for this repository
|
||||||
|
|
|
@ -838,7 +838,7 @@ func MergePullRequest(ctx *context.APIContext) {
|
||||||
message += "\n\n" + form.MergeMessageField
|
message += "\n\n" + form.MergeMessageField
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := pull_service.Merge(pr, ctx.User, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), message); err != nil {
|
if err := pull_service.Merge(pr, ctx.User, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil {
|
||||||
if models.IsErrInvalidMergeStyle(err) {
|
if models.IsErrInvalidMergeStyle(err) {
|
||||||
ctx.Error(http.StatusMethodNotAllowed, "Invalid merge style", fmt.Errorf("%s is not allowed an allowed merge style for this repository", repo_model.MergeStyle(form.Do)))
|
ctx.Error(http.StatusMethodNotAllowed, "Invalid merge style", fmt.Errorf("%s is not allowed an allowed merge style for this repository", repo_model.MergeStyle(form.Do)))
|
||||||
return
|
return
|
||||||
|
@ -854,6 +854,9 @@ func MergePullRequest(ctx *context.APIContext) {
|
||||||
} else if git.IsErrPushOutOfDate(err) {
|
} else if git.IsErrPushOutOfDate(err) {
|
||||||
ctx.Error(http.StatusConflict, "Merge", "merge push out of date")
|
ctx.Error(http.StatusConflict, "Merge", "merge push out of date")
|
||||||
return
|
return
|
||||||
|
} else if models.IsErrSHADoesNotMatch(err) {
|
||||||
|
ctx.Error(http.StatusConflict, "Merge", "head out of date")
|
||||||
|
return
|
||||||
} else if git.IsErrPushRejected(err) {
|
} else if git.IsErrPushRejected(err) {
|
||||||
errPushRej := err.(*git.ErrPushRejected)
|
errPushRej := err.(*git.ErrPushRejected)
|
||||||
if len(errPushRej.Message) == 0 {
|
if len(errPushRej.Message) == 0 {
|
||||||
|
|
|
@ -933,7 +933,7 @@ func MergePullRequest(ctx *context.Context) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if err = pull_service.Merge(pr, ctx.User, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), message); err != nil {
|
if err = pull_service.Merge(pr, ctx.User, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil {
|
||||||
if models.IsErrInvalidMergeStyle(err) {
|
if models.IsErrInvalidMergeStyle(err) {
|
||||||
ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option"))
|
ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option"))
|
||||||
ctx.Redirect(issue.Link())
|
ctx.Redirect(issue.Link())
|
||||||
|
@ -976,6 +976,11 @@ func MergePullRequest(ctx *context.Context) {
|
||||||
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_out_of_date"))
|
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_out_of_date"))
|
||||||
ctx.Redirect(issue.Link())
|
ctx.Redirect(issue.Link())
|
||||||
return
|
return
|
||||||
|
} else if models.IsErrSHADoesNotMatch(err) {
|
||||||
|
log.Debug("MergeHeadOutOfDate error: %v", err)
|
||||||
|
ctx.Flash.Error(ctx.Tr("repo.pulls.head_out_of_date"))
|
||||||
|
ctx.Redirect(issue.Link())
|
||||||
|
return
|
||||||
} else if git.IsErrPushRejected(err) {
|
} else if git.IsErrPushRejected(err) {
|
||||||
log.Debug("MergePushRejected error: %v", err)
|
log.Debug("MergePushRejected error: %v", err)
|
||||||
pushrejErr := err.(*git.ErrPushRejected)
|
pushrejErr := err.(*git.ErrPushRejected)
|
||||||
|
|
|
@ -571,6 +571,7 @@ type MergePullRequestForm struct {
|
||||||
MergeTitleField string
|
MergeTitleField string
|
||||||
MergeMessageField string
|
MergeMessageField string
|
||||||
MergeCommitID string // only used for manually-merged
|
MergeCommitID string // only used for manually-merged
|
||||||
|
HeadCommitID string `json:"head_commit_id,omitempty"`
|
||||||
ForceMerge *bool `json:"force_merge,omitempty"`
|
ForceMerge *bool `json:"force_merge,omitempty"`
|
||||||
DeleteBranchAfterMerge bool `json:"delete_branch_after_merge,omitempty"`
|
DeleteBranchAfterMerge bool `json:"delete_branch_after_merge,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
|
@ -34,7 +34,7 @@ import (
|
||||||
// Merge merges pull request to base repository.
|
// Merge merges pull request to base repository.
|
||||||
// Caller should check PR is ready to be merged (review and status checks)
|
// Caller should check PR is ready to be merged (review and status checks)
|
||||||
// FIXME: add repoWorkingPull make sure two merges does not happen at same time.
|
// FIXME: add repoWorkingPull make sure two merges does not happen at same time.
|
||||||
func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, message string) (err error) {
|
func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) (err error) {
|
||||||
if err = pr.LoadHeadRepo(); err != nil {
|
if err = pr.LoadHeadRepo(); err != nil {
|
||||||
log.Error("LoadHeadRepo: %v", err)
|
log.Error("LoadHeadRepo: %v", err)
|
||||||
return fmt.Errorf("LoadHeadRepo: %v", err)
|
return fmt.Errorf("LoadHeadRepo: %v", err)
|
||||||
|
@ -59,7 +59,7 @@ func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repos
|
||||||
go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "")
|
go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "")
|
||||||
}()
|
}()
|
||||||
|
|
||||||
pr.MergedCommitID, err = rawMerge(pr, doer, mergeStyle, message)
|
pr.MergedCommitID, err = rawMerge(pr, doer, mergeStyle, expectedHeadCommitID, message)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
@ -114,7 +114,7 @@ func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repos
|
||||||
}
|
}
|
||||||
|
|
||||||
// rawMerge perform the merge operation without changing any pull information in database
|
// rawMerge perform the merge operation without changing any pull information in database
|
||||||
func rawMerge(pr *models.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, message string) (string, error) {
|
func rawMerge(pr *models.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) (string, error) {
|
||||||
err := git.LoadGitVersion()
|
err := git.LoadGitVersion()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Error("git.LoadGitVersion: %v", err)
|
log.Error("git.LoadGitVersion: %v", err)
|
||||||
|
@ -137,6 +137,20 @@ func rawMerge(pr *models.PullRequest, doer *user_model.User, mergeStyle repo_mod
|
||||||
trackingBranch := "tracking"
|
trackingBranch := "tracking"
|
||||||
stagingBranch := "staging"
|
stagingBranch := "staging"
|
||||||
|
|
||||||
|
if expectedHeadCommitID != "" {
|
||||||
|
trackingCommitID, err := git.NewCommand("show-ref", "--hash", git.BranchPrefix+trackingBranch).RunInDir(tmpBasePath)
|
||||||
|
if err != nil {
|
||||||
|
log.Error("show-ref[%s] --hash refs/heads/trackingn: %v", tmpBasePath, git.BranchPrefix+trackingBranch, err)
|
||||||
|
return "", fmt.Errorf("getDiffTree: %v", err)
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(trackingCommitID) != expectedHeadCommitID {
|
||||||
|
return "", models.ErrSHADoesNotMatch{
|
||||||
|
GivenSHA: expectedHeadCommitID,
|
||||||
|
CurrentSHA: trackingCommitID,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
var outbuf, errbuf strings.Builder
|
var outbuf, errbuf strings.Builder
|
||||||
|
|
||||||
// Enable sparse-checkout
|
// Enable sparse-checkout
|
||||||
|
|
|
@ -55,7 +55,7 @@ func Update(pull *models.PullRequest, doer *user_model.User, message string, reb
|
||||||
return fmt.Errorf("HeadBranch of PR %d is up to date", pull.Index)
|
return fmt.Errorf("HeadBranch of PR %d is up to date", pull.Index)
|
||||||
}
|
}
|
||||||
|
|
||||||
_, err = rawMerge(pr, doer, style, message)
|
_, err = rawMerge(pr, doer, style, "", message)
|
||||||
|
|
||||||
defer func() {
|
defer func() {
|
||||||
if rebase {
|
if rebase {
|
||||||
|
|
|
@ -327,6 +327,7 @@
|
||||||
<div class="ui form merge-fields" style="display: none">
|
<div class="ui form merge-fields" style="display: none">
|
||||||
<form action="{{.Link}}/merge" method="post">
|
<form action="{{.Link}}/merge" method="post">
|
||||||
{{.CsrfTokenHtml}}
|
{{.CsrfTokenHtml}}
|
||||||
|
<input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}">
|
||||||
<div class="field">
|
<div class="field">
|
||||||
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}">
|
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}">
|
||||||
</div>
|
</div>
|
||||||
|
@ -352,6 +353,7 @@
|
||||||
<div class="ui form rebase-fields" style="display: none">
|
<div class="ui form rebase-fields" style="display: none">
|
||||||
<form action="{{.Link}}/merge" method="post">
|
<form action="{{.Link}}/merge" method="post">
|
||||||
{{.CsrfTokenHtml}}
|
{{.CsrfTokenHtml}}
|
||||||
|
<input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}">
|
||||||
<button class="ui green button" type="submit" name="do" value="rebase">
|
<button class="ui green button" type="submit" name="do" value="rebase">
|
||||||
{{$.i18n.Tr "repo.pulls.rebase_merge_pull_request"}}
|
{{$.i18n.Tr "repo.pulls.rebase_merge_pull_request"}}
|
||||||
</button>
|
</button>
|
||||||
|
@ -371,6 +373,7 @@
|
||||||
<div class="ui form rebase-merge-fields" style="display: none">
|
<div class="ui form rebase-merge-fields" style="display: none">
|
||||||
<form action="{{.Link}}/merge" method="post">
|
<form action="{{.Link}}/merge" method="post">
|
||||||
{{.CsrfTokenHtml}}
|
{{.CsrfTokenHtml}}
|
||||||
|
<input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}">
|
||||||
<div class="field">
|
<div class="field">
|
||||||
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}">
|
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}">
|
||||||
</div>
|
</div>
|
||||||
|
@ -396,6 +399,7 @@
|
||||||
<div class="ui form squash-fields" style="display: none">
|
<div class="ui form squash-fields" style="display: none">
|
||||||
<form action="{{.Link}}/merge" method="post">
|
<form action="{{.Link}}/merge" method="post">
|
||||||
{{.CsrfTokenHtml}}
|
{{.CsrfTokenHtml}}
|
||||||
|
<input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}">
|
||||||
<div class="field">
|
<div class="field">
|
||||||
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultSquashMessage}}">
|
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultSquashMessage}}">
|
||||||
</div>
|
</div>
|
||||||
|
@ -421,6 +425,7 @@
|
||||||
<div class="ui form manually-merged-fields" style="display: none">
|
<div class="ui form manually-merged-fields" style="display: none">
|
||||||
<form action="{{.Link}}/merge" method="post">
|
<form action="{{.Link}}/merge" method="post">
|
||||||
{{.CsrfTokenHtml}}
|
{{.CsrfTokenHtml}}
|
||||||
|
<input type="hidden" name="head_commit_id" value="{{.PullHeadCommitID}}">
|
||||||
<div class="field">
|
<div class="field">
|
||||||
<input type="text" name="merge_commit_id" placeholder="{{$.i18n.Tr "repo.pulls.merge_commit_id"}}">
|
<input type="text" name="merge_commit_id" placeholder="{{$.i18n.Tr "repo.pulls.merge_commit_id"}}">
|
||||||
</div>
|
</div>
|
||||||
|
|
|
@ -15674,6 +15674,10 @@
|
||||||
"force_merge": {
|
"force_merge": {
|
||||||
"type": "boolean",
|
"type": "boolean",
|
||||||
"x-go-name": "ForceMerge"
|
"x-go-name": "ForceMerge"
|
||||||
|
},
|
||||||
|
"head_commit_id": {
|
||||||
|
"type": "string",
|
||||||
|
"x-go-name": "HeadCommitID"
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"x-go-name": "MergePullRequestForm",
|
"x-go-name": "MergePullRequestForm",
|
||||||
|
|
Loading…
Reference in New Issue