Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle push rejection message in Merge & Web Editor #10373

Merged
merged 11 commits into from
Feb 22, 2020
48 changes: 48 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package models

import (
"fmt"
"strings"

"code.gitea.io/gitea/modules/git"
)
Expand Down Expand Up @@ -1371,6 +1372,53 @@ func (err ErrMergePushOutOfDate) Error() string {
return fmt.Sprintf("Merge PushOutOfDate Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
}

// ErrPushRejected represents an error if merging fails due to rejection from a hook
type ErrPushRejected struct {
Style MergeStyle
Message string
StdOut string
StdErr string
Err error
}

// IsErrPushRejected checks if an error is a ErrPushRejected.
func IsErrPushRejected(err error) bool {
_, ok := err.(ErrPushRejected)
return ok
}

func (err ErrPushRejected) Error() string {
return fmt.Sprintf("Merge PushRejected Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
}

// GenerateMessage generates the remote message from the stderr
func (err *ErrPushRejected) GenerateMessage() {
messageBuilder := &strings.Builder{}
i := strings.Index(err.StdErr, "remote: ")
if i < 0 {
err.Message = ""
return
}
for {
if len(err.StdErr) <= i+8 {
break
}
if err.StdErr[i:i+8] != "remote: " {
break
guillep2k marked this conversation as resolved.
Show resolved Hide resolved
}
i += 8
nl := strings.IndexByte(err.StdErr[i:], '\n')
guillep2k marked this conversation as resolved.
Show resolved Hide resolved
if nl > 0 {
messageBuilder.WriteString(err.StdErr[i : i+nl+1])
i = i + nl + 1
} else {
messageBuilder.WriteString(err.StdErr[i:])
i = len(err.StdErr)
}
}
err.Message = strings.TrimSpace(messageBuilder.String())
}

// ErrRebaseConflicts represents an error if rebase fails with a conflict
type ErrRebaseConflicts struct {
Style MergeStyle
Expand Down
24 changes: 22 additions & 2 deletions modules/repofiles/temp_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,28 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(author, committer *models
func (t *TemporaryUploadRepository) Push(doer *models.User, commitHash string, branch string) error {
// Because calls hooks we need to pass in the environment
env := models.PushingEnvironment(doer, t.repo)

if stdout, err := git.NewCommand("push", t.repo.RepoPath(), strings.TrimSpace(commitHash)+":refs/heads/"+strings.TrimSpace(branch)).RunInDirWithEnv(t.basePath, env); err != nil {
stdout := &strings.Builder{}
stderr := &strings.Builder{}

if err := git.NewCommand("push", t.repo.RepoPath(), strings.TrimSpace(commitHash)+":refs/heads/"+strings.TrimSpace(branch)).RunInDirTimeoutEnvPipeline(env, -1, t.basePath, stdout, stderr); err != nil {
errString := stderr.String()
if strings.Contains(errString, "non-fast-forward") {
return models.ErrMergePushOutOfDate{
StdOut: stdout.String(),
StdErr: errString,
Err: err,
}
} else if strings.Contains(errString, "! [remote rejected]") {
log.Error("Unable to push back to repo from temporary repo due to rejection: %s (%s)\nStdout: %s\nError: %v",
zeripath marked this conversation as resolved.
Show resolved Hide resolved
t.repo.FullName(), t.basePath, stdout, err)
zeripath marked this conversation as resolved.
Show resolved Hide resolved
err := models.ErrPushRejected{
StdOut: stdout.String(),
StdErr: errString,
Err: err,
}
err.GenerateMessage()
return err
}
log.Error("Unable to push back to repo from temporary repo: %s (%s)\nStdout: %s\nError: %v",
t.repo.FullName(), t.basePath, stdout, err)
return fmt.Errorf("Unable to push back to repo from temporary repo: %s (%s) Error: %v",
Expand Down
4 changes: 4 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,8 @@ editor.commit_empty_file_header = Commit an empty file
editor.commit_empty_file_text = The file you're about commit is empty. Proceed?
editor.no_changes_to_show = There are no changes to show.
editor.fail_to_update_file = Failed to update/create file '%s' with error: %v
editor.push_rejected_no_message = The change was rejected by the server without a message. Please check githooks.
editor.push_rejected = The change was rejected by the server with the following message:<br>%s<br> Please check githooks.
editor.add_subdir = Add a directory…
editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v
editor.upload_file_is_locked = File '%s' is locked by %s.
Expand Down Expand Up @@ -1087,6 +1089,8 @@ pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s<
pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s<br>%[2]s<br>%[3]s<br>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.push_rejected = Merge Failed: The push was rejected with the following message:<br>%s<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
pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
pulls.status_checking = Some checks are pending
pulls.status_checks_success = All checks were successful
Expand Down
8 changes: 8 additions & 0 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,14 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) {
} else if models.IsErrMergePushOutOfDate(err) {
ctx.Error(http.StatusConflict, "Merge", "merge push out of date")
return
} else if models.IsErrPushRejected(err) {
errPushRej := err.(models.ErrPushRejected)
if len(errPushRej.Message) == 0 {
ctx.Error(http.StatusConflict, "Merge", "PushRejected without remote error message")
return
}
ctx.Error(http.StatusConflict, "Merge", "PushRejected with remote message: "+errPushRej.Message)
return
}
ctx.Error(http.StatusInternalServerError, "Merge", err)
return
Expand Down
21 changes: 18 additions & 3 deletions routers/repo/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/upload"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/routers/utils"
)

const (
Expand Down Expand Up @@ -264,10 +265,17 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
} else {
ctx.Error(500, err.Error())
}
} else if models.IsErrCommitIDDoesNotMatch(err) {
} else if models.IsErrCommitIDDoesNotMatch(err) || models.IsErrMergePushOutOfDate(err) {
ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_editing", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplEditFile, &form)
} else if models.IsErrPushRejected(err) {
errPushRej := err.(models.ErrPushRejected)
if len(errPushRej.Message) == 0 {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form)
} else {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplEditFile, &form)
}
} else {
ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, err), tplEditFile, &form)
ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, utils.SanitizeFlashErrorString(err.Error())), tplEditFile, &form)
}
}

Expand Down Expand Up @@ -428,8 +436,15 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
} else {
ctx.Error(500, err.Error())
}
} else if models.IsErrCommitIDDoesNotMatch(err) {
} else if models.IsErrCommitIDDoesNotMatch(err) || models.IsErrMergePushOutOfDate(err) {
ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_deleting", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplDeleteFile, &form)
} else if models.IsErrPushRejected(err) {
errPushRej := err.(models.ErrPushRejected)
if len(errPushRej.Message) == 0 {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form)
} else {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplDeleteFile, &form)
}
} else {
ctx.ServerError("DeleteRepoFile", err)
}
Expand Down
25 changes: 14 additions & 11 deletions routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"code.gitea.io/gitea/modules/repofiles"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/routers/utils"
"code.gitea.io/gitea/services/gitdiff"
pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
Expand Down Expand Up @@ -774,27 +775,18 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
}

if err = pull_service.Merge(pr, ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil {
sanitize := func(x string) string {
runes := []rune(x)

if len(runes) > 512 {
x = "..." + string(runes[len(runes)-512:])
}

return strings.Replace(html.EscapeString(x), "\n", "<br>", -1)
}
if models.IsErrInvalidMergeStyle(err) {
ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option"))
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return
} else if models.IsErrMergeConflicts(err) {
conflictError := err.(models.ErrMergeConflicts)
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", sanitize(conflictError.StdErr), sanitize(conflictError.StdOut)))
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return
} else if models.IsErrRebaseConflicts(err) {
conflictError := err.(models.ErrRebaseConflicts)
ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", sanitize(conflictError.CommitSHA), sanitize(conflictError.StdErr), sanitize(conflictError.StdOut)))
ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA), utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return
} else if models.IsErrMergeUnrelatedHistories(err) {
Expand All @@ -807,6 +799,17 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_out_of_date"))
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return
} else if models.IsErrPushRejected(err) {
log.Debug("MergePushRejected error: %v", err)
pushrejErr := err.(models.ErrPushRejected)
message := pushrejErr.Message
if len(message) == 0 {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
} else {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
}
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return
}
ctx.ServerError("Merge", err)
return
Expand Down
12 changes: 12 additions & 0 deletions routers/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package utils

import (
"html"
"strings"
)

Expand Down Expand Up @@ -34,3 +35,14 @@ func IsValidSlackChannel(channelName string) bool {

return true
}

// SanitizeFlashErrorString will sanitize a flash error string
func SanitizeFlashErrorString(x string) string {
runes := []rune(x)

if len(runes) > 512 {
x = "..." + string(runes[len(runes)-512:])
}

return strings.Replace(html.EscapeString(x), "\n", "<br>", -1)
}
9 changes: 9 additions & 0 deletions services/pull/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,15 @@ func rawMerge(pr *models.PullRequest, doer *models.User, mergeStyle models.Merge
StdErr: errbuf.String(),
Err: err,
}
} else if strings.Contains(errbuf.String(), "! [remote rejected]") {
err := models.ErrPushRejected{
Style: mergeStyle,
StdOut: outbuf.String(),
StdErr: errbuf.String(),
Err: err,
}
err.GenerateMessage()
return "", err
}
return "", fmt.Errorf("git push: %s", errbuf.String())
}
Expand Down