From 97b534f6c33dc5225d2e35bc8964cadcd7f5e062 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 15 Oct 2019 12:37:38 +0100 Subject: [PATCH 01/25] Adjust error reporting from merge failures --- models/error.go | 37 ++++++++++++++ options/locale/locale_en-US.ini | 2 + routers/repo/pull.go | 8 +++ services/pull/merge.go | 89 +++++++++++++++++++++++++++++++-- 4 files changed, 131 insertions(+), 5 deletions(-) diff --git a/models/error.go b/models/error.go index 995617e83b8ae..ba792c7474adb 100644 --- a/models/error.go +++ b/models/error.go @@ -1189,6 +1189,43 @@ func (err ErrInvalidMergeStyle) Error() string { err.ID, err.Style) } +// ErrMergeConflicts represents an error if merging fails with a conflict +type ErrMergeConflicts struct { + Style MergeStyle + StdOut string + StdErr string + Err error +} + +// IsErrMergeConflicts checks if an error is a ErrMergeConflicts. +func IsErrMergeConflicts(err error) bool { + _, ok := err.(ErrMergeConflicts) + return ok +} + +func (err ErrMergeConflicts) Error() string { + return fmt.Sprintf("Merge Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut) +} + +// ErrRebaseConflicts represents an error if rebase fails with a conflict +type ErrRebaseConflicts struct { + Style MergeStyle + CommitSHA string + StdOut string + StdErr string + Err error +} + +// IsErrRebaseConflicts checks if an error is a ErrRebaseConflicts. +func IsErrRebaseConflicts(err error) bool { + _, ok := err.(ErrRebaseConflicts) + return ok +} + +func (err ErrRebaseConflicts) Error() string { + return fmt.Sprintf("Rebase Error: %v: Whilst Rebasing: %s\n%s\n%s", err.Err, err.CommitSHA, err.StdErr, err.StdOut) +} + // _________ __ // \_ ___ \ ____ _____ _____ ____ _____/ |_ // / \ \/ / _ \ / \ / \_/ __ \ / \ __\ diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4d73d91aa26d9..a9bc91abb45cf 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1007,6 +1007,8 @@ pulls.rebase_merge_pull_request = Rebase and Merge pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff) pulls.squash_merge_pull_request = Squash and Merge pulls.invalid_merge_option = You cannot use this merge option for this pull request. +pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s
%[2]s
Try a different strategy +pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %s[1]
%[1]s
%[2]s
Try a different strategy 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 diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 8b97e55670987..3a6a946f7994b 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -659,6 +659,14 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { 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", conflictError.StdErr, conflictError.StdOut)) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) + } else if models.IsErrRebaseConflicts(err) { + conflictError := err.(models.ErrRebaseConflicts) + ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", conflictError.CommitSHA, conflictError.StdErr, conflictError.StdOut)) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) } ctx.ServerError("Merge", err) return diff --git a/services/pull/merge.go b/services/pull/merge.go index 0d762dbc30d02..fdd01ed9da3da 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -99,7 +99,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) } - var errbuf strings.Builder + var outbuf, errbuf strings.Builder if err := git.NewCommand("remote", "add", "-t", pr.BaseBranch, "-m", pr.BaseBranch, "origin", baseGitRepo.Path).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git remote add [%s -> %s]: %s", baseGitRepo.Path, tmpBasePath, errbuf.String()) } @@ -209,9 +209,20 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Merge commits. switch mergeStyle { case models.MergeStyleMerge: - if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict + if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { + // We have a merge conflict error + return models.ErrMergeConflicts{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) } + outbuf.Reset() if signArg == "" { if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { @@ -229,33 +240,89 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor } // Rebase before merging if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + // Rebase will leave a REBASE_HEAD file in .git if there is a conflict + if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { + // The original commit SHA1 that is failing will be in .git/rebase-apply/original-commit + commitShaBytes, readErr := ioutil.ReadFile(filepath.Join(tmpBasePath, ".git", "rebase-apply", "original-commit")) + if readErr != nil { + // Abandon this attempt to handle the error + return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + } + return models.ErrRebaseConflicts{ + Style: mergeStyle, + CommitSHA: strings.TrimSpace(string(commitShaBytes)), + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + outbuf.Reset() // Checkout base branch again if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } // Merge fast forward - if err := git.NewCommand("merge", "--ff-only", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("merge", "--ff-only", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict + if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { + // We have a merge conflict error + return models.ErrMergeConflicts{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } return fmt.Errorf("git merge --ff-only [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + outbuf.Reset() case models.MergeStyleRebaseMerge: // Checkout head branch if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } // Rebase before merging - if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + // Rebase will leave a REBASE_HEAD file in .git if there is a conflict + if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { + // The original commit SHA1 that is failing will be in .git/rebase-apply/original-commit + commitShaBytes, readErr := ioutil.ReadFile(filepath.Join(tmpBasePath, ".git", "rebase-apply", "original-commit")) + if readErr != nil { + // Abandon this attempt to handle the error + return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + } + return models.ErrRebaseConflicts{ + Style: mergeStyle, + CommitSHA: strings.TrimSpace(string(commitShaBytes)), + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + outbuf.Reset() // Checkout base branch again if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } // Prepare merge with commit - if err := git.NewCommand("merge", "--no-ff", "--no-commit", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("merge", "--no-ff", "--no-commit", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict + if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { + // We have a merge conflict error + return models.ErrMergeConflicts{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } return fmt.Errorf("git merge --no-ff [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + outbuf.Reset() // Set custom message and author and create merge commit if signArg == "" { @@ -271,8 +338,20 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor case models.MergeStyleSquash: // Merge with squash if err := git.NewCommand("merge", "-q", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + // Merge will leave a MERGE_MSG file in the .git folder if there is a conflict + if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_MSG")); statErr == nil { + // We have a merge conflict error + return models.ErrMergeConflicts{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } return fmt.Errorf("git merge --squash [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + outbuf.Reset() + sig := pr.Issue.Poster.NewGitSig() if signArg == "" { if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { From 33d1d283f60ac2f01501126185a96dcfff0f0868 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 22 Oct 2019 20:29:16 +0100 Subject: [PATCH 02/25] Add errbuf resets --- services/pull/merge.go | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index fdd01ed9da3da..c246c97f7e4c1 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -103,28 +103,34 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor if err := git.NewCommand("remote", "add", "-t", pr.BaseBranch, "-m", pr.BaseBranch, "origin", baseGitRepo.Path).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git remote add [%s -> %s]: %s", baseGitRepo.Path, tmpBasePath, errbuf.String()) } + errbuf.Reset() if err := git.NewCommand("fetch", "origin", "--no-tags", pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + errbuf.Reset() if err := git.NewCommand("symbolic-ref", "HEAD", git.BranchPrefix+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git symbolic-ref HEAD base [%s]: %s", tmpBasePath, errbuf.String()) } + errbuf.Reset() if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) } + errbuf.Reset() if err := git.NewCommand("remote", "add", remoteRepoName, headRepoPath).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + errbuf.Reset() trackingBranch := "tracking" // Fetch head branch if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + errbuf.Reset() stagingBranch := "staging" @@ -163,24 +169,30 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor if err := gitConfigCommand().AddArguments("filter.lfs.process", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [filter.lfs.process -> <> ]: %v", errbuf.String()) } + errbuf.Reset() if err := gitConfigCommand().AddArguments("filter.lfs.required", "false").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [filter.lfs.required -> ]: %v", errbuf.String()) } + errbuf.Reset() if err := gitConfigCommand().AddArguments("filter.lfs.clean", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [filter.lfs.clean -> <> ]: %v", errbuf.String()) } + errbuf.Reset() if err := gitConfigCommand().AddArguments("filter.lfs.smudge", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %v", errbuf.String()) } + errbuf.Reset() if err := gitConfigCommand().AddArguments("core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", errbuf.String()) } + errbuf.Reset() // Read base branch index if err := git.NewCommand("read-tree", "HEAD").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git read-tree HEAD: %s", errbuf.String()) } + errbuf.Reset() // Determine if we should sign signArg := "" @@ -223,6 +235,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) } outbuf.Reset() + errbuf.Reset() if signArg == "" { if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { @@ -233,11 +246,14 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) } } + errbuf.Reset() case models.MergeStyleRebase: // Checkout head branch if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } + errbuf.Reset() + // Rebase before merging if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { // Rebase will leave a REBASE_HEAD file in .git if there is a conflict @@ -259,10 +275,14 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } outbuf.Reset() + errbuf.Reset() + // Checkout base branch again if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } + errbuf.Reset() + // Merge fast forward if err := git.NewCommand("merge", "--ff-only", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict @@ -278,11 +298,14 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git merge --ff-only [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } outbuf.Reset() + errbuf.Reset() case models.MergeStyleRebaseMerge: // Checkout head branch if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } + errbuf.Reset() + // Rebase before merging if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Rebase will leave a REBASE_HEAD file in .git if there is a conflict @@ -304,10 +327,14 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } outbuf.Reset() + errbuf.Reset() + // Checkout base branch again if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } + errbuf.Reset() + // Prepare merge with commit if err := git.NewCommand("merge", "--no-ff", "--no-commit", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict @@ -323,6 +350,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git merge --no-ff [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } outbuf.Reset() + errbuf.Reset() // Set custom message and author and create merge commit if signArg == "" { @@ -334,7 +362,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) } } - + errbuf.Reset() case models.MergeStyleSquash: // Merge with squash if err := git.NewCommand("merge", "-q", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { @@ -351,6 +379,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git merge --squash [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } outbuf.Reset() + errbuf.Reset() sig := pr.Issue.Poster.NewGitSig() if signArg == "" { @@ -362,6 +391,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) } } + errbuf.Reset() default: return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: mergeStyle} } @@ -407,6 +437,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor if err := git.NewCommand("push", "origin", baseBranch+":"+pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git push: %s", errbuf.String()) } + errbuf.Reset() pr.MergedCommitID, err = baseGitRepo.GetBranchCommitID(pr.BaseBranch) if err != nil { From c1f49d85a4c501c79bb020b064e8554344f1b8b1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 22 Oct 2019 21:08:07 +0100 Subject: [PATCH 03/25] Remove the -q as we want the message if there is a failure --- services/pull/merge.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index c246c97f7e4c1..0fe92d064b9c2 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -255,7 +255,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor errbuf.Reset() // Rebase before merging - if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("rebase", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { // Rebase will leave a REBASE_HEAD file in .git if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { // The original commit SHA1 that is failing will be in .git/rebase-apply/original-commit @@ -284,7 +284,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor errbuf.Reset() // Merge fast forward - if err := git.NewCommand("merge", "--ff-only", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + if err := git.NewCommand("merge", "--ff-only", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error @@ -307,7 +307,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor errbuf.Reset() // Rebase before merging - if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + if err := git.NewCommand("rebase", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Rebase will leave a REBASE_HEAD file in .git if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { // The original commit SHA1 that is failing will be in .git/rebase-apply/original-commit @@ -336,7 +336,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor errbuf.Reset() // Prepare merge with commit - if err := git.NewCommand("merge", "--no-ff", "--no-commit", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + if err := git.NewCommand("merge", "--no-ff", "--no-commit", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error @@ -365,7 +365,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor errbuf.Reset() case models.MergeStyleSquash: // Merge with squash - if err := git.NewCommand("merge", "-q", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("merge", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { // Merge will leave a MERGE_MSG file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_MSG")); statErr == nil { // We have a merge conflict error From 137d7003aefa1e987fb3bcb10d7b6f2a6fc7c7b9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 22 Oct 2019 21:17:50 +0100 Subject: [PATCH 04/25] save the stdout --- services/pull/merge.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 0fe92d064b9c2..89f04b393df30 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -255,7 +255,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor errbuf.Reset() // Rebase before merging - if err := git.NewCommand("rebase", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("rebase", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Rebase will leave a REBASE_HEAD file in .git if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { // The original commit SHA1 that is failing will be in .git/rebase-apply/original-commit @@ -365,7 +365,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor errbuf.Reset() case models.MergeStyleSquash: // Merge with squash - if err := git.NewCommand("merge", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("merge", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Merge will leave a MERGE_MSG file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_MSG")); statErr == nil { // We have a merge conflict error From a439b5723aa05bc260a62461e998fb7d93209ad2 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 22 Oct 2019 22:00:40 +0100 Subject: [PATCH 05/25] sanitize the error out --- routers/repo/pull.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 3a6a946f7994b..f587f11e240a3 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -10,6 +10,7 @@ import ( "container/list" "crypto/subtle" "fmt" + "html" "io" "path" "strings" @@ -655,17 +656,20 @@ 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 { + return strings.ReplaceAll(html.EscapeString(x), "\n", "
") + } 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", conflictError.StdErr, conflictError.StdOut)) + ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", sanitize(conflictError.StdErr), sanitize(conflictError.StdOut))) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) } else if models.IsErrRebaseConflicts(err) { conflictError := err.(models.ErrRebaseConflicts) - ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", conflictError.CommitSHA, conflictError.StdErr, conflictError.StdOut)) + ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", sanitize(conflictError.CommitSHA), sanitize(conflictError.StdErr), sanitize(conflictError.StdOut))) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) } ctx.ServerError("Merge", err) From 28d0f8494df50daec4d2c0c4ff5770e6d832beb4 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 22 Oct 2019 22:11:34 +0100 Subject: [PATCH 06/25] truncate message slightly and adjust message --- options/locale/locale_en-US.ini | 4 ++-- routers/repo/pull.go | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a9bc91abb45cf..c63dd7900d35c 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1007,8 +1007,8 @@ pulls.rebase_merge_pull_request = Rebase and Merge pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff) pulls.squash_merge_pull_request = Squash and Merge pulls.invalid_merge_option = You cannot use this merge option for this pull request. -pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s
%[2]s
Try a different strategy -pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %s[1]
%[1]s
%[2]s
Try a different strategy +pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s
%[2]s
Hint: Try a different strategy +pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %s[1]
%[1]s
%[2]s
Hint:Try a different strategy 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 diff --git a/routers/repo/pull.go b/routers/repo/pull.go index f587f11e240a3..e3e944fd4b6a8 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -657,6 +657,10 @@ 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 { + if len(x) > 512 { + x = "..." + x[len(x)-512:] + } + return strings.ReplaceAll(html.EscapeString(x), "\n", "
") } if models.IsErrInvalidMergeStyle(err) { From e4eb801901225645f94d1988a965312795d3345a Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 22 Oct 2019 22:13:46 +0100 Subject: [PATCH 07/25] in runes... --- routers/repo/pull.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index e3e944fd4b6a8..a2daa4fe8f1e8 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -657,8 +657,10 @@ 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 { - if len(x) > 512 { - x = "..." + x[len(x)-512:] + runes := []rune(x) + + if len(runes) > 512 { + x = "..." + string(runes[len(runes)-512:]) } return strings.ReplaceAll(html.EscapeString(x), "\n", "
") From ad111ce2d162a7e0eefa768241055d44755609a8 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 22 Oct 2019 22:26:38 +0100 Subject: [PATCH 08/25] ReplaceAll is too new - use Replace(... -1) --- routers/repo/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index a2daa4fe8f1e8..000f6d2c8e7fb 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -663,7 +663,7 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { x = "..." + string(runes[len(runes)-512:]) } - return strings.ReplaceAll(html.EscapeString(x), "\n", "
") + return strings.Replace(html.EscapeString(x), "\n", "
", -1) } if models.IsErrInvalidMergeStyle(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) From a1e23e92e127bf32f1cbf795b5c338daf780aa8f Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 23 Oct 2019 22:32:05 +0100 Subject: [PATCH 09/25] Apply suggestions from code review --- routers/repo/pull.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index b4f4ef20a0775..dbb8395adc74e 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -677,10 +677,12 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { conflictError := err.(models.ErrMergeConflicts) ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", sanitize(conflictError.StdErr), sanitize(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.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) + return } ctx.ServerError("Merge", err) return From d3eaac2e542d984f3cb94ec44102917d33e12656 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 30 Oct 2019 14:42:45 +0000 Subject: [PATCH 10/25] Set locale --- services/pull/merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 288a4b2a5987d..e23cff69d80a6 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -221,7 +221,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Merge commits. switch mergeStyle { case models.MergeStyleMerge: - if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirTimeoutEnvPipeline(append(os.Environ(), "LC_ALL=C"), -1, tmpBasePath, &outbuf, &errbuf); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error From b6077bbff556fe6c6170b2f5b1d1842823e7d128 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 30 Oct 2019 22:23:32 +0000 Subject: [PATCH 11/25] Handle lots of types of error better --- models/error.go | 38 ++- models/pull.go | 2 + models/repo_unit.go | 5 +- modules/auth/repo_form.go | 2 +- modules/git/command.go | 20 +- options/locale/locale_en-US.ini | 3 + routers/api/v1/repo/pull.go | 14 +- routers/repo/issue.go | 2 + routers/repo/pull.go | 12 +- services/pull/merge.go | 243 ++++++++++++++++---- templates/repo/issue/view_content/pull.tmpl | 6 + 11 files changed, 294 insertions(+), 53 deletions(-) diff --git a/models/error.go b/models/error.go index ba792c7474adb..73675a0860e25 100644 --- a/models/error.go +++ b/models/error.go @@ -1204,7 +1204,43 @@ func IsErrMergeConflicts(err error) bool { } func (err ErrMergeConflicts) Error() string { - return fmt.Sprintf("Merge Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut) + return fmt.Sprintf("Merge Conflict Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut) +} + +// ErrMergeUnrelatedHistories represents an error if merging fails due to unrelated histories +type ErrMergeUnrelatedHistories struct { + Style MergeStyle + StdOut string + StdErr string + Err error +} + +// IsErrMergeUnrelatedHistories checks if an error is a ErrMergeUnrelatedHistories. +func IsErrMergeUnrelatedHistories(err error) bool { + _, ok := err.(ErrMergeUnrelatedHistories) + return ok +} + +func (err ErrMergeUnrelatedHistories) Error() string { + return fmt.Sprintf("Merge UnrelatedHistories Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut) +} + +// ErrMergePushOutOfDate represents an error if merging fails due to unrelated histories +type ErrMergePushOutOfDate struct { + Style MergeStyle + StdOut string + StdErr string + Err error +} + +// IsErrMergePushOutOfDate checks if an error is a ErrMergePushOutOfDate. +func IsErrMergePushOutOfDate(err error) bool { + _, ok := err.(ErrMergePushOutOfDate) + return ok +} + +func (err ErrMergePushOutOfDate) Error() string { + return fmt.Sprintf("Merge PushOutOfDate Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut) } // ErrRebaseConflicts represents an error if rebase fails with a conflict diff --git a/models/pull.go b/models/pull.go index c6da63ec55795..9f5065e7a8061 100644 --- a/models/pull.go +++ b/models/pull.go @@ -412,6 +412,8 @@ const ( MergeStyleRebaseMerge MergeStyle = "rebase-merge" // MergeStyleSquash squash commits into single commit before merging MergeStyleSquash MergeStyle = "squash" + // MergeStyleMergeUnrelated create merge commit allow unrelated + MergeStyleMergeUnrelated MergeStyle = "merge-unrelated" ) // CheckUserAllowedToMerge checks whether the user is allowed to merge diff --git a/models/repo_unit.go b/models/repo_unit.go index a6162a65e516b..eee15bb8065ff 100644 --- a/models/repo_unit.go +++ b/models/repo_unit.go @@ -93,6 +93,7 @@ type PullRequestsConfig struct { AllowRebase bool AllowRebaseMerge bool AllowSquash bool + AllowMergeUnrelated bool } // FromDB fills up a PullRequestsConfig from serialized format. @@ -110,7 +111,9 @@ func (cfg *PullRequestsConfig) IsMergeStyleAllowed(mergeStyle MergeStyle) bool { return mergeStyle == MergeStyleMerge && cfg.AllowMerge || mergeStyle == MergeStyleRebase && cfg.AllowRebase || mergeStyle == MergeStyleRebaseMerge && cfg.AllowRebaseMerge || - mergeStyle == MergeStyleSquash && cfg.AllowSquash + mergeStyle == MergeStyleSquash && cfg.AllowSquash || + mergeStyle == MergeStyleMergeUnrelated && cfg.AllowMergeUnrelated + } // BeforeSet is invoked from XORM before setting the value of a field of this object. diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 2280666114d52..a6dd1b7c79f64 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -433,7 +433,7 @@ func (f *InitializeLabelsForm) Validate(ctx *macaron.Context, errs binding.Error type MergePullRequestForm struct { // required: true // enum: merge,rebase,rebase-merge,squash - Do string `binding:"Required;In(merge,rebase,rebase-merge,squash)"` + Do string `binding:"Required;In(merge,rebase,rebase-merge,squash,merge-unrelated)"` MergeTitleField string MergeMessageField string } diff --git a/modules/git/command.go b/modules/git/command.go index 347dcfe39f884..36ec6d143654d 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "io" + "os" "os/exec" "strings" "time" @@ -24,10 +25,14 @@ var ( DefaultCommandExecutionTimeout = 60 * time.Second ) +// DefaultLCALL is the default LC_ALL to run git commands in. +const DefaultLCALL = "C" + // Command represents a command with its subcommands or arguments. type Command struct { - name string - args []string + name string + args []string + LCALL string } func (c *Command) String() string { @@ -43,8 +48,9 @@ func NewCommand(args ...string) *Command { cargs := make([]string, len(GlobalCommandArgs)) copy(cargs, GlobalCommandArgs) return &Command{ - name: GitExecutable, - args: append(cargs, args...), + name: GitExecutable, + args: append(cargs, args...), + LCALL: DefaultLCALL, } } @@ -77,7 +83,11 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura defer cancel() cmd := exec.CommandContext(ctx, c.name, c.args...) - cmd.Env = env + if env == nil { + cmd.Env = append(os.Environ(), fmt.Sprintf("LC_ALL=%s", DefaultLCALL)) + } else { + cmd.Env = append(env, fmt.Sprintf("LC_ALL=%s", DefaultLCALL)) + } cmd.Dir = dir cmd.Stdout = stdout cmd.Stderr = stderr diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index f3cc375cb72ca..1b4cb23b9baf5 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1019,9 +1019,12 @@ pulls.merge_pull_request = Merge Pull Request pulls.rebase_merge_pull_request = Rebase and Merge pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff) pulls.squash_merge_pull_request = Squash and Merge +pulls.unrelated_merge_pull_request = Merge Pull Request (--allow-unrelated-histories) pulls.invalid_merge_option = You cannot use this merge option for this pull request. pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s
%[2]s
Hint: Try a different strategy pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %s[1]
%[1]s
%[2]s
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.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 diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 0180aebf89ddd..7dee680f6a5d2 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -603,7 +603,7 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { message := strings.TrimSpace(form.MergeTitleField) if len(message) == 0 { - if models.MergeStyle(form.Do) == models.MergeStyleMerge { + if models.MergeStyle(form.Do) == models.MergeStyleMerge || models.MergeStyle(form.Do) == models.MergeStyleMergeUnrelated { message = pr.GetDefaultMergeMessage() } if models.MergeStyle(form.Do) == models.MergeStyleSquash { @@ -620,6 +620,18 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { if models.IsErrInvalidMergeStyle(err) { ctx.Status(405) return + } else if models.IsErrMergeConflicts(err) { + conflictError := err.(models.ErrMergeConflicts) + ctx.JSON(http.StatusConflict, conflictError) + } else if models.IsErrRebaseConflicts(err) { + conflictError := err.(models.ErrRebaseConflicts) + ctx.JSON(http.StatusConflict, conflictError) + } else if models.IsErrMergeUnrelatedHistories(err) { + conflictError := err.(models.ErrMergeUnrelatedHistories) + ctx.JSON(http.StatusConflict, conflictError) + } else if models.IsErrMergePushOutOfDate(err) { + ctx.Status(http.StatusConflict) + return } ctx.Error(500, "Merge", err) return diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 12ff0a054c13d..8c6261c43455f 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -917,6 +917,8 @@ func ViewIssue(ctx *context.Context) { ctx.Data["MergeStyle"] = models.MergeStyleRebaseMerge } else if prConfig.AllowSquash { ctx.Data["MergeStyle"] = models.MergeStyleSquash + } else if prConfig.AllowMergeUnrelated { + ctx.Data["MergeStyle"] = models.MergeStyleMergeUnrelated } else { ctx.Data["MergeStyle"] = "" } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 9c47ed263eb6f..1de1dffaf485e 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -630,7 +630,7 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { message := strings.TrimSpace(form.MergeTitleField) if len(message) == 0 { - if models.MergeStyle(form.Do) == models.MergeStyleMerge { + if models.MergeStyle(form.Do) == models.MergeStyleMerge || models.MergeStyle(form.Do) == models.MergeStyleMergeUnrelated { message = pr.GetDefaultMergeMessage() } if models.MergeStyle(form.Do) == models.MergeStyleRebaseMerge { @@ -684,6 +684,16 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", sanitize(conflictError.CommitSHA), sanitize(conflictError.StdErr), sanitize(conflictError.StdOut))) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) return + } else if models.IsErrMergeUnrelatedHistories(err) { + log.Debug("MergeUnrelatedHistories error: %v", err) + ctx.Flash.Error(ctx.Tr("repo.pulls.unrelated_histories")) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) + return + } else if models.IsErrMergePushOutOfDate(err) { + log.Debug("MergePushOutOfDate error: %v", err) + ctx.Flash.Error(ctx.Tr("repo.pulls.merge_out_of_date")) + ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) + return } ctx.ServerError("Merge", err) return diff --git a/services/pull/merge.go b/services/pull/merge.go index e23cff69d80a6..d0562efbfc1d6 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -31,22 +31,27 @@ import ( func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repository, mergeStyle models.MergeStyle, message string) (err error) { binVersion, err := git.BinVersion() if err != nil { + log.Error("git.BinVersion: %v", err) return fmt.Errorf("Unable to get git version: %v", err) } if err = pr.GetHeadRepo(); err != nil { + log.Error("GetHeadRepo: %v", err) return fmt.Errorf("GetHeadRepo: %v", err) } else if err = pr.GetBaseRepo(); err != nil { + log.Error("GetBaseRepo: %v", err) return fmt.Errorf("GetBaseRepo: %v", err) } prUnit, err := pr.BaseRepo.GetUnit(models.UnitTypePullRequests) if err != nil { + log.Error("pr.BaseRepo.GetUnit: %v", err) return err } prConfig := prUnit.PullRequestsConfig() if err := pr.CheckUserAllowedToMerge(doer); err != nil { + log.Error("CheckUserAllowedToMerge(%v): %v", doer, err) return fmt.Errorf("CheckUserAllowedToMerge: %v", err) } @@ -62,6 +67,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Clone base repo. tmpBasePath, err := models.CreateTemporaryPath("merge") if err != nil { + log.Error("CreateTemporaryPath: %v", err) return err } @@ -74,6 +80,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor headRepoPath := pr.HeadRepo.RepoPath() if err := git.InitRepository(tmpBasePath, false); err != nil { + log.Error("git init: %v", err) return fmt.Errorf("git init: %v", err) } @@ -85,51 +92,64 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor p := filepath.Join(staging, ".git", "objects", "info", "alternates") f, err := os.OpenFile(p, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) if err != nil { + log.Error("Creating alternates file: %v", err) return err } defer f.Close() data := filepath.Join(cache, "objects") if _, err := fmt.Fprintln(f, data); err != nil { + log.Error("Unable to print to alternates file in temporary repository: %v", err) return err } return nil } if err := addCacheRepo(tmpBasePath, baseGitRepo.Path); err != nil { + log.Error("addCacheRepo to temporary repo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) } var outbuf, errbuf strings.Builder - if err := git.NewCommand("remote", "add", "-t", pr.BaseBranch, "-m", pr.BaseBranch, "origin", baseGitRepo.Path).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("remote", "add", "-t", pr.BaseBranch, "-m", pr.BaseBranch, "origin", baseGitRepo.Path).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git remote add [%s -> %s]: %v\n%s\n%s", baseGitRepo.Path, tmpBasePath, err, outbuf.String(), errbuf.String()) return fmt.Errorf("git remote add [%s -> %s]: %s", baseGitRepo.Path, tmpBasePath, errbuf.String()) } + outbuf.Reset() errbuf.Reset() - if err := git.NewCommand("fetch", "origin", "--no-tags", pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("fetch", "origin", "--no-tags", pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git fetch [%s -> %s]: %v:\n%s\n%s", headRepoPath, tmpBasePath, err, outbuf.String(), errbuf.String()) return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + outbuf.Reset() errbuf.Reset() - if err := git.NewCommand("symbolic-ref", "HEAD", git.BranchPrefix+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("symbolic-ref", "HEAD", git.BranchPrefix+baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git symbolic-ref HEAD base [%s]: %v\n%s\n%s", tmpBasePath, err, outbuf.String(), errbuf.String()) return fmt.Errorf("git symbolic-ref HEAD base [%s]: %s", tmpBasePath, errbuf.String()) } + outbuf.Reset() errbuf.Reset() if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { + log.Error("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) } - errbuf.Reset() - if err := git.NewCommand("remote", "add", remoteRepoName, headRepoPath).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("remote", "add", remoteRepoName, headRepoPath).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git remote add [%s -> %s]: %v\n%s\n%s", headRepoPath, tmpBasePath, err, outbuf.String(), errbuf.String()) return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + outbuf.Reset() errbuf.Reset() trackingBranch := "tracking" // Fetch head branch - if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git fetch [%s -> %s]: %v\n%s\n%s", headRepoPath, tmpBasePath, err, outbuf.String(), errbuf.String()) return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + outbuf.Reset() errbuf.Reset() stagingBranch := "staging" @@ -137,22 +157,25 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Enable sparse-checkout sparseCheckoutList, err := getDiffTree(tmpBasePath, baseBranch, trackingBranch) if err != nil { + log.Error("getDiffTree(%s, %s, %s): %v", tmpBasePath, baseBranch, trackingBranch, err) return fmt.Errorf("getDiffTree: %v", err) } infoPath := filepath.Join(tmpBasePath, ".git", "info") if err := os.MkdirAll(infoPath, 0700); err != nil { + log.Error("creating directory failed [%s]: %v", infoPath, err) return fmt.Errorf("creating directory failed [%s]: %v", infoPath, err) } sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") if err := ioutil.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0600); err != nil { + log.Error("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) return fmt.Errorf("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) } gitConfigCommand := func() func() *git.Command { binVersion, err := git.BinVersion() if err != nil { - log.Fatal("Error retrieving git version: %v", err) + log.Error("Error retrieving git version: %v", err) } if version.Compare(binVersion, "1.8.0", ">=") { @@ -166,32 +189,47 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor }() // Switch off LFS process (set required, clean and smudge here also) - if err := gitConfigCommand().AddArguments("filter.lfs.process", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := gitConfigCommand().AddArguments("filter.lfs.process", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git config [filter.lfs.process -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) return fmt.Errorf("git config [filter.lfs.process -> <> ]: %v", errbuf.String()) } + outbuf.Reset() errbuf.Reset() - if err := gitConfigCommand().AddArguments("filter.lfs.required", "false").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + + if err := gitConfigCommand().AddArguments("filter.lfs.required", "false").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git config [filter.lfs.required -> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) return fmt.Errorf("git config [filter.lfs.required -> ]: %v", errbuf.String()) } + outbuf.Reset() errbuf.Reset() - if err := gitConfigCommand().AddArguments("filter.lfs.clean", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + + if err := gitConfigCommand().AddArguments("filter.lfs.clean", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git config [filter.lfs.clean -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) return fmt.Errorf("git config [filter.lfs.clean -> <> ]: %v", errbuf.String()) } + outbuf.Reset() errbuf.Reset() - if err := gitConfigCommand().AddArguments("filter.lfs.smudge", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + + if err := gitConfigCommand().AddArguments("filter.lfs.smudge", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git config [filter.lfs.smudge -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) return fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %v", errbuf.String()) } + outbuf.Reset() errbuf.Reset() - if err := gitConfigCommand().AddArguments("core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := gitConfigCommand().AddArguments("core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git config [core.sparseCheckout -> true ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", errbuf.String()) } + outbuf.Reset() errbuf.Reset() // Read base branch index - if err := git.NewCommand("read-tree", "HEAD").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("read-tree", "HEAD").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git read-tree HEAD: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) return fmt.Errorf("git read-tree HEAD: %s", errbuf.String()) } + outbuf.Reset() errbuf.Reset() // Determine if we should sign @@ -221,37 +259,97 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Merge commits. switch mergeStyle { case models.MergeStyleMerge: - if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirTimeoutEnvPipeline(append(os.Environ(), "LC_ALL=C"), -1, tmpBasePath, &outbuf, &errbuf); err != nil { + if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict + if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { + // We have a merge conflict error + log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return models.ErrMergeConflicts{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { + log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return models.ErrMergeUnrelatedHistories{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } + log.Error("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git merge --no-ff --no-commit[%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + } + outbuf.Reset() + errbuf.Reset() + + if signArg == "" { + if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + } + } else { + if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + } + } + outbuf.Reset() + errbuf.Reset() + case models.MergeStyleMergeUnrelated: + cmd := git.NewCommand("merge", "--no-ff", "--no-commit") + if version.Compare(binVersion, "2.9.0", ">=") { + cmd.AddArguments("--allow-unrelated-histories") + } + cmd.AddArguments(trackingBranch) + if err := cmd.RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error + log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) return models.ErrMergeConflicts{ Style: mergeStyle, StdOut: outbuf.String(), StdErr: errbuf.String(), Err: err, } + } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { + log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return models.ErrMergeUnrelatedHistories{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } } - return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + log.Error("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() if signArg == "" { - if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } } else { - if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } } + outbuf.Reset() errbuf.Reset() case models.MergeStyleRebase: // Checkout head branch - if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git checkout: %s", errbuf.String()) + if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git checkout -b staging tracking [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git checkout -b staging tracking [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } + outbuf.Reset() errbuf.Reset() // Rebase before merging @@ -262,8 +360,10 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor commitShaBytes, readErr := ioutil.ReadFile(filepath.Join(tmpBasePath, ".git", "rebase-apply", "original-commit")) if readErr != nil { // Abandon this attempt to handle the error - return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } + log.Debug("RebaseConflict at %s [%s:%s -> %s:%s]: %v\n%s\n%s", strings.TrimSpace(string(commitShaBytes)), pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) return models.ErrRebaseConflicts{ Style: mergeStyle, CommitSHA: strings.TrimSpace(string(commitShaBytes)), @@ -272,15 +372,18 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor Err: err, } } - return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() // Checkout base branch again - if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git checkout: %s", errbuf.String()) + if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } + outbuf.Reset() errbuf.Reset() // Merge fast forward @@ -288,22 +391,35 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error + log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) return models.ErrMergeConflicts{ Style: mergeStyle, StdOut: outbuf.String(), StdErr: errbuf.String(), Err: err, } + } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { + // This shouldn't happen but may aswell check. + log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return models.ErrMergeUnrelatedHistories{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } } - return fmt.Errorf("git merge --ff-only [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("git merge --no-ff [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git merge --no-ff [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() case models.MergeStyleRebaseMerge: // Checkout head branch - if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git checkout: %s", errbuf.String()) + if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } + outbuf.Reset() errbuf.Reset() // Rebase before merging @@ -314,8 +430,10 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor commitShaBytes, readErr := ioutil.ReadFile(filepath.Join(tmpBasePath, ".git", "rebase-apply", "original-commit")) if readErr != nil { // Abandon this attempt to handle the error - return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } + log.Debug("RebaseConflict at %s [%s:%s -> %s:%s]: %v\n%s\n%s", strings.TrimSpace(string(commitShaBytes)), pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) return models.ErrRebaseConflicts{ Style: mergeStyle, CommitSHA: strings.TrimSpace(string(commitShaBytes)), @@ -324,15 +442,18 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor Err: err, } } - return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() // Checkout base branch again - if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git checkout: %s", errbuf.String()) + if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } + outbuf.Reset() errbuf.Reset() // Prepare merge with commit @@ -340,28 +461,42 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error + log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) return models.ErrMergeConflicts{ Style: mergeStyle, StdOut: outbuf.String(), StdErr: errbuf.String(), Err: err, } + } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { + // This shouldn't happen but may aswell check. + log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return models.ErrMergeUnrelatedHistories{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } } - return fmt.Errorf("git merge --no-ff [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() // Set custom message and author and create merge commit if signArg == "" { - if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } } else { - if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } } + outbuf.Reset() errbuf.Reset() case models.MergeStyleSquash: // Merge with squash @@ -369,28 +504,41 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Merge will leave a MERGE_MSG file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_MSG")); statErr == nil { // We have a merge conflict error + log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) return models.ErrMergeConflicts{ Style: mergeStyle, StdOut: outbuf.String(), StdErr: errbuf.String(), Err: err, } + } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { + log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return models.ErrMergeUnrelatedHistories{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } } - return fmt.Errorf("git merge --squash [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("git merge --squash [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git merge --squash [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() sig := pr.Issue.Poster.NewGitSig() if signArg == "" { - if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } } else { - if err := git.NewCommand("commit", signArg, fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + if err := git.NewCommand("commit", signArg, fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } } + outbuf.Reset() errbuf.Reset() default: return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: mergeStyle} @@ -437,9 +585,18 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor ) // Push back to upstream. - if err := git.NewCommand("push", "origin", baseBranch+":"+pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("push", "origin", baseBranch+":"+pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + if strings.Contains(err.Error(), "non-fast-forward") { + return models.ErrMergePushOutOfDate{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } return fmt.Errorf("git push: %s", errbuf.String()) } + outbuf.Reset() errbuf.Reset() pr.MergedCommitID, err = baseGitRepo.GetBranchCommitID(pr.BaseBranch) diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index f5ce8e0886c39..aaf7714f62388 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -219,6 +219,9 @@ {{if eq .MergeStyle "squash"}} {{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}} {{end}} + {{if eq .MergeStyle "merge-unrelated"}} + {{$.i18n.Tr "repo.pulls.merge_pull_request"}} + {{end}}
@@ -236,6 +239,9 @@ {{if $prUnit.PullRequestsConfig.AllowSquash}}
{{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}}
{{end}} + {{if $prUnit.PullRequestsConfig.AllowMergeUnrelated}} +
{{$.i18n.Tr "repo.pulls.unrelated_merge_pull_request"}}
+ {{end}}
From 08cffb36578c86228f3bdce53ea4242e6385d623 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 30 Oct 2019 22:36:10 +0000 Subject: [PATCH 12/25] use fallthrough to reduce replication --- services/pull/merge.go | 42 ++---------------------------------------- 1 file changed, 2 insertions(+), 40 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index d0562efbfc1d6..a6a9d0b4fbdb0 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -259,48 +259,10 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Merge commits. switch mergeStyle { case models.MergeStyleMerge: - if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { - // We have a merge conflict error - log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeConflicts{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { - log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeUnrelatedHistories{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } - log.Error("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git merge --no-ff --no-commit[%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if signArg == "" { - if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - } else { - if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - } - outbuf.Reset() - errbuf.Reset() + fallthrough case models.MergeStyleMergeUnrelated: cmd := git.NewCommand("merge", "--no-ff", "--no-commit") - if version.Compare(binVersion, "2.9.0", ">=") { + if mergeStyle == models.MergeStyleMergeUnrelated && version.Compare(binVersion, "2.9.0", ">=") { cmd.AddArguments("--allow-unrelated-histories") } cmd.AddArguments(trackingBranch) From 252b605c29b44b4b5b8acd0b533fbf204b45bd2d Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 31 Oct 2019 21:32:49 +0000 Subject: [PATCH 13/25] More golangci-lint fixes --- modules/git/command.go | 3 +- services/pull/merge.go | 210 ++++++++++++++--------------------------- 2 files changed, 74 insertions(+), 139 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index 36ec6d143654d..4dd0a2cc17b43 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -86,7 +86,8 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura if env == nil { cmd.Env = append(os.Environ(), fmt.Sprintf("LC_ALL=%s", DefaultLCALL)) } else { - cmd.Env = append(env, fmt.Sprintf("LC_ALL=%s", DefaultLCALL)) + env = append(env, fmt.Sprintf("LC_ALL=%s", DefaultLCALL)) + cmd.Env = env } cmd.Dir = dir cmd.Stdout = stdout diff --git a/services/pull/merge.go b/services/pull/merge.go index a6a9d0b4fbdb0..831fb652f7daa 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -45,7 +45,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor prUnit, err := pr.BaseRepo.GetUnit(models.UnitTypePullRequests) if err != nil { - log.Error("pr.BaseRepo.GetUnit: %v", err) + log.Error("pr.BaseRepo.GetUnit(models.UnitTypePullRequests): %v", err) return err } prConfig := prUnit.PullRequestsConfig() @@ -80,8 +80,8 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor headRepoPath := pr.HeadRepo.RepoPath() if err := git.InitRepository(tmpBasePath, false); err != nil { - log.Error("git init: %v", err) - return fmt.Errorf("git init: %v", err) + log.Error("git init tmpBasePath: %v", err) + return fmt.Errorf("git init tmpBasePath: %v", err) } remoteRepoName := "head_repo" @@ -92,53 +92,53 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor p := filepath.Join(staging, ".git", "objects", "info", "alternates") f, err := os.OpenFile(p, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) if err != nil { - log.Error("Creating alternates file: %v", err) + log.Error("Could not create .git/objects/info/alternates file in %s: %v", staging, err) return err } defer f.Close() data := filepath.Join(cache, "objects") if _, err := fmt.Fprintln(f, data); err != nil { - log.Error("Unable to print to alternates file in temporary repository: %v", err) + log.Error("Could not write to .git/objects/info/alternates file in %s: %v", staging, err) return err } return nil } if err := addCacheRepo(tmpBasePath, baseGitRepo.Path); err != nil { - log.Error("addCacheRepo to temporary repo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) - return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) + log.Error("Unable to add base repository to temporary repo [%s -> %s]: %v", pr.BaseRepo.FullName(), tmpBasePath, err) + return fmt.Errorf("Unable to add base repository to temporary repo [%s -> tmpBasePath]: %v", pr.BaseRepo.FullName(), err) } var outbuf, errbuf strings.Builder if err := git.NewCommand("remote", "add", "-t", pr.BaseBranch, "-m", pr.BaseBranch, "origin", baseGitRepo.Path).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git remote add [%s -> %s]: %v\n%s\n%s", baseGitRepo.Path, tmpBasePath, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git remote add [%s -> %s]: %s", baseGitRepo.Path, tmpBasePath, errbuf.String()) + log.Error("Unable to add base repository as origin [%s -> %s]: %v\n%s\n%s", pr.BaseRepo.FullName(), tmpBasePath, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("Unable to add base repository as origin [%s -> tmpBasePath]: %v\n%s\n%s", pr.BaseRepo.FullName(), err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() if err := git.NewCommand("fetch", "origin", "--no-tags", pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git fetch [%s -> %s]: %v:\n%s\n%s", headRepoPath, tmpBasePath, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("Unable to fetch origin base branch [%s:%s -> base, original_base in %s]: %v:\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, tmpBasePath, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("Unable to fetch origin base branch [%s:%s -> base, original_base in tmpBasePath]: %v\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() if err := git.NewCommand("symbolic-ref", "HEAD", git.BranchPrefix+baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git symbolic-ref HEAD base [%s]: %v\n%s\n%s", tmpBasePath, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git symbolic-ref HEAD base [%s]: %s", tmpBasePath, errbuf.String()) + log.Error("Unable to set HEAD as base branch [%s]: %v\n%s\n%s", tmpBasePath, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("Unable to set HEAD as base branch [tmpBasePath]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { - log.Error("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) - return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) + log.Error("Unable to add head repository to temporary repo [%s -> %s]: %v", pr.HeadRepo.FullName(), tmpBasePath, err) + return fmt.Errorf("Unable to head base repository to temporary repo [%s -> tmpBasePath]: %v", pr.HeadRepo.FullName(), err) } if err := git.NewCommand("remote", "add", remoteRepoName, headRepoPath).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git remote add [%s -> %s]: %v\n%s\n%s", headRepoPath, tmpBasePath, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("Unable to add head repository as head_repo [%s -> %s]: %v\n%s\n%s", pr.HeadRepo.FullName(), tmpBasePath, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("Unable to add head repository as head_repo [%s -> tmpBasePath]: %v\n%s\n%s", pr.HeadRepo.FullName(), err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() @@ -146,8 +146,8 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor trackingBranch := "tracking" // Fetch head branch if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git fetch [%s -> %s]: %v\n%s\n%s", headRepoPath, tmpBasePath, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() @@ -163,63 +163,59 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor infoPath := filepath.Join(tmpBasePath, ".git", "info") if err := os.MkdirAll(infoPath, 0700); err != nil { - log.Error("creating directory failed [%s]: %v", infoPath, err) - return fmt.Errorf("creating directory failed [%s]: %v", infoPath, err) + log.Error("Unable to create .git/info in %s: %v", tmpBasePath, err) + return fmt.Errorf("Unable to create .git/info in tmpBasePath: %v", err) } + sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") if err := ioutil.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0600); err != nil { - log.Error("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) - return fmt.Errorf("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) + log.Error("Unable to write .git/info/sparse-checkout file in %s: %v", tmpBasePath, err) + return fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %v", err) } - gitConfigCommand := func() func() *git.Command { - binVersion, err := git.BinVersion() - if err != nil { - log.Error("Error retrieving git version: %v", err) + var gitConfigCommand func() *git.Command + if version.Compare(binVersion, "1.8.0", ">=") { + gitConfigCommand = func() *git.Command { + return git.NewCommand("config", "--local") } - - if version.Compare(binVersion, "1.8.0", ">=") { - return func() *git.Command { - return git.NewCommand("config", "--local") - } - } - return func() *git.Command { + } else { + gitConfigCommand = func() *git.Command { return git.NewCommand("config") } - }() + } // Switch off LFS process (set required, clean and smudge here also) if err := gitConfigCommand().AddArguments("filter.lfs.process", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { log.Error("git config [filter.lfs.process -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git config [filter.lfs.process -> <> ]: %v", errbuf.String()) + return fmt.Errorf("git config [filter.lfs.process -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() if err := gitConfigCommand().AddArguments("filter.lfs.required", "false").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { log.Error("git config [filter.lfs.required -> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git config [filter.lfs.required -> ]: %v", errbuf.String()) + return fmt.Errorf("git config [filter.lfs.required -> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() if err := gitConfigCommand().AddArguments("filter.lfs.clean", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { log.Error("git config [filter.lfs.clean -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git config [filter.lfs.clean -> <> ]: %v", errbuf.String()) + return fmt.Errorf("git config [filter.lfs.clean -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() if err := gitConfigCommand().AddArguments("filter.lfs.smudge", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { log.Error("git config [filter.lfs.smudge -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %v", errbuf.String()) + return fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() if err := gitConfigCommand().AddArguments("core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { log.Error("git config [core.sparseCheckout -> true ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", errbuf.String()) + return fmt.Errorf("git config [core.sparsecheckout -> true]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() @@ -227,7 +223,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Read base branch index if err := git.NewCommand("read-tree", "HEAD").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { log.Error("git read-tree HEAD: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git read-tree HEAD: %s", errbuf.String()) + return fmt.Errorf("Unable to read base branch in to the index: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() @@ -291,90 +287,12 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor } outbuf.Reset() errbuf.Reset() - - if signArg == "" { - if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - } else { - if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } + if err := commitAndSignNoAuthor(pr, message, signArg, tmpBasePath, env); err != nil { + log.Error("Unable to make final commit: %v", err) + return err } - outbuf.Reset() - errbuf.Reset() case models.MergeStyleRebase: - // Checkout head branch - if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git checkout -b staging tracking [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git checkout -b staging tracking [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // Rebase before merging - if err := git.NewCommand("rebase", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - // Rebase will leave a REBASE_HEAD file in .git if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { - // The original commit SHA1 that is failing will be in .git/rebase-apply/original-commit - commitShaBytes, readErr := ioutil.ReadFile(filepath.Join(tmpBasePath, ".git", "rebase-apply", "original-commit")) - if readErr != nil { - // Abandon this attempt to handle the error - log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - log.Debug("RebaseConflict at %s [%s:%s -> %s:%s]: %v\n%s\n%s", strings.TrimSpace(string(commitShaBytes)), pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrRebaseConflicts{ - Style: mergeStyle, - CommitSHA: strings.TrimSpace(string(commitShaBytes)), - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } - log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // Checkout base branch again - if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // Merge fast forward - if err := git.NewCommand("merge", "--ff-only", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { - // We have a merge conflict error - log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeConflicts{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { - // This shouldn't happen but may aswell check. - log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeUnrelatedHistories{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } - log.Error("git merge --no-ff [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git merge --no-ff [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() + fallthrough case models.MergeStyleRebaseMerge: // Checkout head branch if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { @@ -418,8 +336,16 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor outbuf.Reset() errbuf.Reset() + mergeCommand := git.NewCommand("merge") + if mergeStyle == models.MergeStyleRebase { + mergeCommand.AddArguments("--ff-only") + } else { + mergeCommand.AddArguments("--no-ff", "--no-commit") + } + mergeCommand.AddArguments(stagingBranch) + // Prepare merge with commit - if err := git.NewCommand("merge", "--no-ff", "--no-commit", stagingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + if err := mergeCommand.RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error @@ -440,26 +366,18 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor Err: err, } } - log.Error("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + log.Error("git merge [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git merge [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() - // Set custom message and author and create merge commit - if signArg == "" { - if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - } else { - if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + if mergeStyle == models.MergeStyleRebaseMerge { + if err := commitAndSignNoAuthor(pr, message, signArg, tmpBasePath, env); err != nil { + log.Error("Unable to make final commit: %v", err) + return err } } - outbuf.Reset() - errbuf.Reset() case models.MergeStyleSquash: // Merge with squash if err := git.NewCommand("merge", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { @@ -603,6 +521,22 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return nil } +func commitAndSignNoAuthor(pr *models.PullRequest, message, signArg, tmpBasePath string, env []string) error { + var outbuf, errbuf strings.Builder + if signArg == "" { + if err := git.NewCommand("commit", "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + } + } else { + if err := git.NewCommand("commit", signArg, "-m", message).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + } + } + return nil +} + func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { var outbuf, errbuf strings.Builder From 6b7855f2ad1ccb8d1ce9bb8e45ce975913967fa6 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 31 Oct 2019 21:52:19 +0000 Subject: [PATCH 14/25] finally fixed golangci-lint --- services/pull/merge.go | 122 +++++++++++++++-------------------------- 1 file changed, 44 insertions(+), 78 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 831fb652f7daa..138ebb933848f 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -262,31 +262,11 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor cmd.AddArguments("--allow-unrelated-histories") } cmd.AddArguments(trackingBranch) - if err := cmd.RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { - // We have a merge conflict error - log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeConflicts{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { - log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeUnrelatedHistories{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } - log.Error("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git merge --no-ff --no-commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { + log.Error("Unable to merge tracking into base: %v", err) + return err } - outbuf.Reset() - errbuf.Reset() + if err := commitAndSignNoAuthor(pr, message, signArg, tmpBasePath, env); err != nil { log.Error("Unable to make final commit: %v", err) return err @@ -336,42 +316,19 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor outbuf.Reset() errbuf.Reset() - mergeCommand := git.NewCommand("merge") + cmd := git.NewCommand("merge") if mergeStyle == models.MergeStyleRebase { - mergeCommand.AddArguments("--ff-only") + cmd.AddArguments("--ff-only") } else { - mergeCommand.AddArguments("--no-ff", "--no-commit") + cmd.AddArguments("--no-ff", "--no-commit") } - mergeCommand.AddArguments(stagingBranch) + cmd.AddArguments(stagingBranch) // Prepare merge with commit - if err := mergeCommand.RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { - // We have a merge conflict error - log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeConflicts{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { - // This shouldn't happen but may aswell check. - log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeUnrelatedHistories{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } - log.Error("git merge [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git merge [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { + log.Error("Unable to merge staging into base: %v", err) + return err } - outbuf.Reset() - errbuf.Reset() - if mergeStyle == models.MergeStyleRebaseMerge { if err := commitAndSignNoAuthor(pr, message, signArg, tmpBasePath, env); err != nil { log.Error("Unable to make final commit: %v", err) @@ -380,31 +337,11 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor } case models.MergeStyleSquash: // Merge with squash - if err := git.NewCommand("merge", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { - // Merge will leave a MERGE_MSG file in the .git folder if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_MSG")); statErr == nil { - // We have a merge conflict error - log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeConflicts{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { - log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return models.ErrMergeUnrelatedHistories{ - Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } - log.Error("git merge --squash [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git merge --squash [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + cmd := git.NewCommand("merge", "--squash", trackingBranch) + if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { + log.Error("Unable to merge --squash tracking into base: %v", err) + return err } - outbuf.Reset() - errbuf.Reset() sig := pr.Issue.Poster.NewGitSig() if signArg == "" { @@ -537,6 +474,35 @@ func commitAndSignNoAuthor(pr *models.PullRequest, message, signArg, tmpBasePath return nil } +func runMergeCommand(pr *models.PullRequest, mergeStyle models.MergeStyle, cmd *git.Command, tmpBasePath string) error { + var outbuf, errbuf strings.Builder + if err := cmd.RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { + // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict + if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { + // We have a merge conflict error + log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return models.ErrMergeConflicts{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { + log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return models.ErrMergeUnrelatedHistories{ + Style: mergeStyle, + StdOut: outbuf.String(), + StdErr: errbuf.String(), + Err: err, + } + } + log.Error("git merge [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + return fmt.Errorf("git merge [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + } + + return nil +} + func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { var outbuf, errbuf strings.Builder From 6511e1814628f7573ca046495d2f6a1eb0c1b356 Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 31 Oct 2019 21:58:36 +0000 Subject: [PATCH 15/25] Update modules/auth/repo_form.go --- modules/auth/repo_form.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index a6dd1b7c79f64..a3d8d1ac52b10 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -432,7 +432,7 @@ func (f *InitializeLabelsForm) Validate(ctx *macaron.Context, errs binding.Error // swagger:model MergePullRequestOption type MergePullRequestForm struct { // required: true - // enum: merge,rebase,rebase-merge,squash + // enum: merge,rebase,rebase-merge,squash,merge-unrelated Do string `binding:"Required;In(merge,rebase,rebase-merge,squash,merge-unrelated)"` MergeTitleField string MergeMessageField string From 18300d04c46a3d55cfa6333995cfe88ef12c63f4 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 31 Oct 2019 22:04:35 +0000 Subject: [PATCH 16/25] update swagger --- templates/swagger/v1_json.tmpl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 5be36d23be94c..4bdc6f7158f48 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -9314,7 +9314,8 @@ "merge", "rebase", "rebase-merge", - "squash" + "squash", + "merge-unrelated" ] }, "MergeMessageField": { From 130f61aeb7d38a4ab0a044fc3c82d44bfb26ebaa Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 31 Oct 2019 23:43:38 +0000 Subject: [PATCH 17/25] Update command.go --- modules/git/command.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index 4dd0a2cc17b43..8a0004881d846 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -84,10 +84,10 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura cmd := exec.CommandContext(ctx, c.name, c.args...) if env == nil { - cmd.Env = append(os.Environ(), fmt.Sprintf("LC_ALL=%s", DefaultLCALL)) + cmd.Env = append(os.Environ(), fmt.Sprintf("LC_ALL=%s", c.LCALL)) } else { - env = append(env, fmt.Sprintf("LC_ALL=%s", DefaultLCALL)) cmd.Env = env + cmd.Env = append(cmd.Env, fmt.Sprintf("LC_ALL=%s", c.LCALL)) } cmd.Dir = dir cmd.Stdout = stdout From 6546958d47f08ff9f488542beba446eb452e299b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 3 Nov 2019 17:21:20 +0000 Subject: [PATCH 18/25] Remove MergeStyleUnrelatedMerge --- models/pull.go | 2 -- models/repo_unit.go | 5 +---- modules/auth/repo_form.go | 4 ++-- routers/api/v1/repo/pull.go | 2 +- routers/repo/issue.go | 2 -- routers/repo/pull.go | 2 +- services/pull/merge.go | 8 +------- templates/repo/issue/view_content/pull.tmpl | 6 ------ templates/swagger/v1_json.tmpl | 3 +-- 9 files changed, 7 insertions(+), 27 deletions(-) diff --git a/models/pull.go b/models/pull.go index 17c2f2ddc9d1c..45a1daac467f9 100644 --- a/models/pull.go +++ b/models/pull.go @@ -410,8 +410,6 @@ const ( MergeStyleRebaseMerge MergeStyle = "rebase-merge" // MergeStyleSquash squash commits into single commit before merging MergeStyleSquash MergeStyle = "squash" - // MergeStyleMergeUnrelated create merge commit allow unrelated - MergeStyleMergeUnrelated MergeStyle = "merge-unrelated" ) // CheckUserAllowedToMerge checks whether the user is allowed to merge diff --git a/models/repo_unit.go b/models/repo_unit.go index eee15bb8065ff..a6162a65e516b 100644 --- a/models/repo_unit.go +++ b/models/repo_unit.go @@ -93,7 +93,6 @@ type PullRequestsConfig struct { AllowRebase bool AllowRebaseMerge bool AllowSquash bool - AllowMergeUnrelated bool } // FromDB fills up a PullRequestsConfig from serialized format. @@ -111,9 +110,7 @@ func (cfg *PullRequestsConfig) IsMergeStyleAllowed(mergeStyle MergeStyle) bool { return mergeStyle == MergeStyleMerge && cfg.AllowMerge || mergeStyle == MergeStyleRebase && cfg.AllowRebase || mergeStyle == MergeStyleRebaseMerge && cfg.AllowRebaseMerge || - mergeStyle == MergeStyleSquash && cfg.AllowSquash || - mergeStyle == MergeStyleMergeUnrelated && cfg.AllowMergeUnrelated - + mergeStyle == MergeStyleSquash && cfg.AllowSquash } // BeforeSet is invoked from XORM before setting the value of a field of this object. diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index a3d8d1ac52b10..2280666114d52 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -432,8 +432,8 @@ func (f *InitializeLabelsForm) Validate(ctx *macaron.Context, errs binding.Error // swagger:model MergePullRequestOption type MergePullRequestForm struct { // required: true - // enum: merge,rebase,rebase-merge,squash,merge-unrelated - Do string `binding:"Required;In(merge,rebase,rebase-merge,squash,merge-unrelated)"` + // enum: merge,rebase,rebase-merge,squash + Do string `binding:"Required;In(merge,rebase,rebase-merge,squash)"` MergeTitleField string MergeMessageField string } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 7dee680f6a5d2..28c19565c8d9e 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -603,7 +603,7 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { message := strings.TrimSpace(form.MergeTitleField) if len(message) == 0 { - if models.MergeStyle(form.Do) == models.MergeStyleMerge || models.MergeStyle(form.Do) == models.MergeStyleMergeUnrelated { + if models.MergeStyle(form.Do) == models.MergeStyleMerge { message = pr.GetDefaultMergeMessage() } if models.MergeStyle(form.Do) == models.MergeStyleSquash { diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 1243b322243e1..9a691471d54d6 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -920,8 +920,6 @@ func ViewIssue(ctx *context.Context) { ctx.Data["MergeStyle"] = models.MergeStyleRebaseMerge } else if prConfig.AllowSquash { ctx.Data["MergeStyle"] = models.MergeStyleSquash - } else if prConfig.AllowMergeUnrelated { - ctx.Data["MergeStyle"] = models.MergeStyleMergeUnrelated } else { ctx.Data["MergeStyle"] = "" } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 1de1dffaf485e..f825264fc3f8b 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -630,7 +630,7 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { message := strings.TrimSpace(form.MergeTitleField) if len(message) == 0 { - if models.MergeStyle(form.Do) == models.MergeStyleMerge || models.MergeStyle(form.Do) == models.MergeStyleMergeUnrelated { + if models.MergeStyle(form.Do) == models.MergeStyleMerge { message = pr.GetDefaultMergeMessage() } if models.MergeStyle(form.Do) == models.MergeStyleRebaseMerge { diff --git a/services/pull/merge.go b/services/pull/merge.go index 6cca96009c417..b1133a246daec 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -256,13 +256,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Merge commits. switch mergeStyle { case models.MergeStyleMerge: - fallthrough - case models.MergeStyleMergeUnrelated: - cmd := git.NewCommand("merge", "--no-ff", "--no-commit") - if mergeStyle == models.MergeStyleMergeUnrelated && version.Compare(binVersion, "2.9.0", ">=") { - cmd.AddArguments("--allow-unrelated-histories") - } - cmd.AddArguments(trackingBranch) + cmd := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch) if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { log.Error("Unable to merge tracking into base: %v", err) return err diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index aaf7714f62388..f5ce8e0886c39 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -219,9 +219,6 @@ {{if eq .MergeStyle "squash"}} {{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}} {{end}} - {{if eq .MergeStyle "merge-unrelated"}} - {{$.i18n.Tr "repo.pulls.merge_pull_request"}} - {{end}}
@@ -239,9 +236,6 @@ {{if $prUnit.PullRequestsConfig.AllowSquash}}
{{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}}
{{end}} - {{if $prUnit.PullRequestsConfig.AllowMergeUnrelated}} -
{{$.i18n.Tr "repo.pulls.unrelated_merge_pull_request"}}
- {{end}}
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 63e7446954e5f..da7ebda8523ac 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -9367,8 +9367,7 @@ "merge", "rebase", "rebase-merge", - "squash", - "merge-unrelated" + "squash" ] }, "MergeMessageField": { From cef24890e5a5836757a587e17adeaf6c10480dec Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 3 Nov 2019 17:58:07 +0000 Subject: [PATCH 19/25] remove mergeunrelated message in locale_en-US.ini --- options/locale/locale_en-US.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 42d2723b12b44..5a53e4f12bb94 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1021,7 +1021,6 @@ pulls.merge_pull_request = Merge Pull Request pulls.rebase_merge_pull_request = Rebase and Merge pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff) pulls.squash_merge_pull_request = Squash and Merge -pulls.unrelated_merge_pull_request = Merge Pull Request (--allow-unrelated-histories) pulls.invalid_merge_option = You cannot use this merge option for this pull request. pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s
%[2]s
Hint: Try a different strategy pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %s[1]
%[1]s
%[2]s
Hint:Try a different strategy From e56d6cebafc0676c4c1106dabdf27ca795a648c3 Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 5 Nov 2019 23:44:03 +0000 Subject: [PATCH 20/25] Apply suggestions from code review Co-Authored-By: Lauris BH --- services/pull/merge.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 0e27c9522a586..a4988f51a6f5d 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -83,7 +83,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor if err := git.InitRepository(tmpBasePath, false); err != nil { log.Error("git init tmpBasePath: %v", err) - return fmt.Errorf("git init tmpBasePath: %v", err) + return err } remoteRepoName := "head_repo" @@ -399,7 +399,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Push back to upstream. if err := git.NewCommand("push", "origin", baseBranch+":"+pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil { - if strings.Contains(err.Error(), "non-fast-forward") { + if strings.Contains(errbuf.String(), "non-fast-forward") { return models.ErrMergePushOutOfDate{ Style: mergeStyle, StdOut: outbuf.String(), @@ -481,7 +481,7 @@ func runMergeCommand(pr *models.PullRequest, mergeStyle models.MergeStyle, cmd * StdErr: errbuf.String(), Err: err, } - } else if strings.Contains(err.Error(), "refusing to merge unrelated histories") { + } else if strings.Contains(errbuf.String(), "refusing to merge unrelated histories") { log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) return models.ErrMergeUnrelatedHistories{ Style: mergeStyle, From 2954f0ff8bd5cd51f2808e572a1bb3bb86872684 Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 5 Nov 2019 23:49:26 +0000 Subject: [PATCH 21/25] Switch to use Locale instead of LCALL name --- modules/git/command.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index 8a0004881d846..af1ee71f2060e 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -25,14 +25,14 @@ var ( DefaultCommandExecutionTimeout = 60 * time.Second ) -// DefaultLCALL is the default LC_ALL to run git commands in. -const DefaultLCALL = "C" +// DefaultLocale is the default LC_ALL to run git commands in. +const DefaultLocale = "C" // Command represents a command with its subcommands or arguments. type Command struct { name string args []string - LCALL string + Locale string } func (c *Command) String() string { @@ -50,7 +50,7 @@ func NewCommand(args ...string) *Command { return &Command{ name: GitExecutable, args: append(cargs, args...), - LCALL: DefaultLCALL, + Locale: DefaultLocale, } } @@ -84,10 +84,10 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura cmd := exec.CommandContext(ctx, c.name, c.args...) if env == nil { - cmd.Env = append(os.Environ(), fmt.Sprintf("LC_ALL=%s", c.LCALL)) + cmd.Env = append(os.Environ(), fmt.Sprintf("LC_ALL=%s", c.Locale)) } else { cmd.Env = env - cmd.Env = append(cmd.Env, fmt.Sprintf("LC_ALL=%s", c.LCALL)) + cmd.Env = append(cmd.Env, fmt.Sprintf("LC_ALL=%s", c.Locale)) } cmd.Dir = dir cmd.Stdout = stdout From ac66b00ecda4b8d873e8bd7bdf72b159c3bab451 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 6 Nov 2019 01:45:19 +0000 Subject: [PATCH 22/25] Fix fmt --- modules/git/command.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index af1ee71f2060e..6900a7a866d45 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -30,8 +30,8 @@ const DefaultLocale = "C" // Command represents a command with its subcommands or arguments. type Command struct { - name string - args []string + name string + args []string Locale string } From 4f35d66c91590833acebcffb62542278af231207 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 6 Nov 2019 01:59:52 +0000 Subject: [PATCH 23/25] Fix fmt 2 (the revenge) --- modules/git/command.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index 6900a7a866d45..e1b94beaacd37 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -48,8 +48,8 @@ func NewCommand(args ...string) *Command { cargs := make([]string, len(GlobalCommandArgs)) copy(cargs, GlobalCommandArgs) return &Command{ - name: GitExecutable, - args: append(cargs, args...), + name: GitExecutable, + args: append(cargs, args...), Locale: DefaultLocale, } } From 5a30d6fc43042509ae244ab221fff6fd946fa1ca Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 6 Nov 2019 16:21:30 +0000 Subject: [PATCH 24/25] Remove unused configurability from git.Command --- modules/git/command.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index e1b94beaacd37..2b5288aeab72d 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -30,9 +30,8 @@ const DefaultLocale = "C" // Command represents a command with its subcommands or arguments. type Command struct { - name string - args []string - Locale string + name string + args []string } func (c *Command) String() string { @@ -48,9 +47,8 @@ func NewCommand(args ...string) *Command { cargs := make([]string, len(GlobalCommandArgs)) copy(cargs, GlobalCommandArgs) return &Command{ - name: GitExecutable, - args: append(cargs, args...), - Locale: DefaultLocale, + name: GitExecutable, + args: append(cargs, args...), } } @@ -84,10 +82,10 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura cmd := exec.CommandContext(ctx, c.name, c.args...) if env == nil { - cmd.Env = append(os.Environ(), fmt.Sprintf("LC_ALL=%s", c.Locale)) + cmd.Env = append(os.Environ(), fmt.Sprintf("LC_ALL=%s", DefaultLocale)) } else { cmd.Env = env - cmd.Env = append(cmd.Env, fmt.Sprintf("LC_ALL=%s", c.Locale)) + cmd.Env = append(cmd.Env, fmt.Sprintf("LC_ALL=%s", DefaultLocale)) } cmd.Dir = dir cmd.Stdout = stdout From c115d79127428e1a2c8f899caf4bd084fdc871b1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 9 Nov 2019 10:14:28 +0000 Subject: [PATCH 25/25] Add tests --- integrations/pull_merge_test.go | 137 ++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go index 27f9fc6bb9a56..a63e27e1494d9 100644 --- a/integrations/pull_merge_test.go +++ b/integrations/pull_merge_test.go @@ -5,15 +5,22 @@ package integrations import ( + "bytes" + "fmt" "net/http" "net/http/httptest" "net/url" + "os" "path" "strings" "testing" + "time" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/services/pull" "github.com/stretchr/testify/assert" "github.com/unknwon/i18n" @@ -202,3 +209,133 @@ func TestCantMergeWorkInProgress(t *testing.T) { assert.Equal(t, replacer.Replace(expected), text, "Unable to find WIP text") }) } + +func TestCantMergeConflict(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + prepareTestEnv(t) + session := loginUser(t, "user1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "conflict", "README.md", "Hello, World (Edited Once)\n") + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base", "README.md", "Hello, World (Edited Twice)\n") + + // Use API to create a conflicting pr + token := getTokenForLoggedInUser(t, session) + req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls?token=%s", "user1", "repo1", token), &api.CreatePullRequestOption{ + Head: "conflict", + Base: "base", + Title: "create a conflicting pr", + }) + session.MakeRequest(t, req, 201) + + // Now this PR will be marked conflict - or at least a race will do - so drop down to pure code at this point... + user1 := models.AssertExistsAndLoadBean(t, &models.User{ + Name: "user1", + }).(*models.User) + repo1 := models.AssertExistsAndLoadBean(t, &models.Repository{ + OwnerID: user1.ID, + Name: "repo1", + }).(*models.Repository) + + pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{ + HeadRepoID: repo1.ID, + BaseRepoID: repo1.ID, + HeadBranch: "conflict", + BaseBranch: "base", + }).(*models.PullRequest) + + gitRepo, err := git.OpenRepository(models.RepoPath(user1.Name, repo1.Name)) + assert.NoError(t, err) + + err = pull.Merge(pr, user1, gitRepo, models.MergeStyleMerge, "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") + + err = pull.Merge(pr, user1, gitRepo, models.MergeStyleRebase, "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") + }) +} + +func TestCantMergeUnrelated(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + prepareTestEnv(t) + session := loginUser(t, "user1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base", "README.md", "Hello, World (Edited Twice)\n") + + // Now we want to create a commit on a branch that is totally unrelated to our current head + // Drop down to pure code at this point + user1 := models.AssertExistsAndLoadBean(t, &models.User{ + Name: "user1", + }).(*models.User) + repo1 := models.AssertExistsAndLoadBean(t, &models.Repository{ + OwnerID: user1.ID, + Name: "repo1", + }).(*models.Repository) + path := models.RepoPath(user1.Name, repo1.Name) + + _, err := git.NewCommand("read-tree", "--empty").RunInDir(path) + assert.NoError(t, err) + + stdin := bytes.NewBufferString("Unrelated File") + var stdout strings.Builder + err = git.NewCommand("hash-object", "-w", "--stdin").RunInDirFullPipeline(path, &stdout, nil, stdin) + assert.NoError(t, err) + sha := strings.TrimSpace(stdout.String()) + + _, err = git.NewCommand("update-index", "--add", "--replace", "--cacheinfo", "100644", sha, "somewher-over-the-rainbow").RunInDir(path) + assert.NoError(t, err) + + treeSha, err := git.NewCommand("write-tree").RunInDir(path) + assert.NoError(t, err) + treeSha = strings.TrimSpace(treeSha) + + commitTimeStr := time.Now().Format(time.RFC3339) + doerSig := user1.NewGitSig() + env := append(os.Environ(), + "GIT_AUTHOR_NAME="+doerSig.Name, + "GIT_AUTHOR_EMAIL="+doerSig.Email, + "GIT_AUTHOR_DATE="+commitTimeStr, + "GIT_COMMITTER_NAME="+doerSig.Name, + "GIT_COMMITTER_EMAIL="+doerSig.Email, + "GIT_COMMITTER_DATE="+commitTimeStr, + ) + + messageBytes := new(bytes.Buffer) + _, _ = messageBytes.WriteString("Unrelated") + _, _ = messageBytes.WriteString("\n") + + stdout.Reset() + err = git.NewCommand("commit-tree", treeSha).RunInDirTimeoutEnvFullPipeline(env, -1, path, &stdout, nil, messageBytes) + assert.NoError(t, err) + commitSha := strings.TrimSpace(stdout.String()) + + _, err = git.NewCommand("branch", "unrelated", commitSha).RunInDir(path) + assert.NoError(t, err) + + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "conflict", "README.md", "Hello, World (Edited Once)\n") + + // Use API to create a conflicting pr + token := getTokenForLoggedInUser(t, session) + req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls?token=%s", "user1", "repo1", token), &api.CreatePullRequestOption{ + Head: "unrelated", + Base: "base", + Title: "create an unrelated pr", + }) + session.MakeRequest(t, req, 201) + + // Now this PR could be marked conflict - or at least a race may occur - so drop down to pure code at this point... + gitRepo, err := git.OpenRepository(path) + assert.NoError(t, err) + pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{ + HeadRepoID: repo1.ID, + BaseRepoID: repo1.ID, + HeadBranch: "unrelated", + BaseBranch: "base", + }).(*models.PullRequest) + + err = pull.Merge(pr, user1, gitRepo, models.MergeStyleMerge, "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") + }) +}