handle merge conflict not as an error (#823)

This commit is contained in:
Marko Gacesa 2023-11-28 11:14:51 +00:00 committed by Harness
parent 047cf7983d
commit 1d06026d05
9 changed files with 251 additions and 199 deletions

View File

@ -29,7 +29,6 @@ import (
"github.com/harness/gitness/errors" "github.com/harness/gitness/errors"
"github.com/harness/gitness/git" "github.com/harness/gitness/git"
gitenum "github.com/harness/gitness/git/enum" gitenum "github.com/harness/gitness/git/enum"
gittypes "github.com/harness/gitness/git/types"
"github.com/harness/gitness/types" "github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum" "github.com/harness/gitness/types/enum"
@ -197,7 +196,49 @@ func (c *Controller) Merge(
return nil, nil, fmt.Errorf("failed to verify protection rules: %w", err) return nil, nil, fmt.Errorf("failed to verify protection rules: %w", err)
} }
//nolint:nestif
if in.DryRun { if in.DryRun {
// As the merge API is always executed under a global lock, we use the opportunity of dry-running the merge
// to check the PR's mergeability status if it's currently "unchecked". This can happen if the target branch
// has advanced. It's possible that the merge base commit is different too.
// So, the next time the API gets called for the same PR the mergeability status will not be unchecked.
// Without dry-run the execution would proceed below and would either merge the PR or set the conflict status.
if pr.MergeCheckStatus == enum.MergeCheckStatusUnchecked {
mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{
WriteParams: targetWriteParams,
BaseBranch: pr.TargetBranch,
HeadRepoUID: sourceRepo.GitUID,
HeadBranch: pr.SourceBranch,
HeadExpectedSHA: in.SourceSHA,
})
if err != nil {
return nil, nil, fmt.Errorf("merge check execution failed: %w", err)
}
pr, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
if pr.SourceSHA != mergeOutput.HeadSHA {
return errors.New("source SHA has changed")
}
if mergeOutput.MergeSHA == "" || len(mergeOutput.ConflictFiles) > 0 {
pr.MergeCheckStatus = enum.MergeCheckStatusConflict
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA
pr.MergeTargetSHA = &mergeOutput.BaseSHA
pr.MergeSHA = nil
pr.MergeConflicts = mergeOutput.ConflictFiles
} else {
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA
pr.MergeTargetSHA = &mergeOutput.BaseSHA
pr.MergeSHA = &mergeOutput.MergeSHA
pr.MergeConflicts = nil
}
return nil
})
if err != nil {
return nil, nil, fmt.Errorf("failed to update unchecked pull request: %w", err)
}
}
// With in.DryRun=true this function never returns types.MergeViolations // With in.DryRun=true this function never returns types.MergeViolations
out := &types.MergeResponse{ out := &types.MergeResponse{
DryRun: true, DryRun: true,
@ -207,26 +248,6 @@ func (c *Controller) Merge(
RuleViolations: violations, RuleViolations: violations,
} }
// TODO: This is a temporary solution. The changes needed for the proper implementation:
// 1) Git: Change the merge method to return SHAs (source/target/merge base) even in case of conflicts.
// 2) Event handler: Update target and merge base SHA in the event handler even in case of merge conflicts.
// 3) Here: Update the pull request target and merge base SHA in the DB if merge check status is unchecked.
// 4) Remove the recheck API.
if pr.MergeCheckStatus == enum.MergeCheckStatusUnchecked {
_, err = c.git.Merge(ctx, &git.MergeParams{
WriteParams: targetWriteParams,
BaseBranch: pr.TargetBranch,
HeadRepoUID: sourceRepo.GitUID,
HeadBranch: pr.SourceBranch,
HeadExpectedSHA: in.SourceSHA,
})
if cferr := gittypes.AsMergeConflictsError(err); cferr != nil {
out.ConflictFiles = cferr.Files
} else if err != nil {
return nil, nil, fmt.Errorf("merge check execution failed: %w", err)
}
}
return out, nil, nil return out, nil, nil
} }
@ -260,15 +281,27 @@ func (c *Controller) Merge(
Method: gitenum.MergeMethod(in.Method), Method: gitenum.MergeMethod(in.Method),
}) })
if err != nil { if err != nil {
if cf := gittypes.AsMergeConflictsError(err); cf != nil { return nil, nil, fmt.Errorf("merge check execution failed: %w", err)
//nolint: nilerr }
if mergeOutput.MergeSHA == "" || len(mergeOutput.ConflictFiles) > 0 {
_, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
// update all Merge specific information
pr.MergeCheckStatus = enum.MergeCheckStatusConflict
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA
pr.MergeTargetSHA = &mergeOutput.BaseSHA
pr.MergeSHA = nil
pr.MergeConflicts = mergeOutput.ConflictFiles
return nil
})
if err != nil {
return nil, nil, fmt.Errorf("failed to update pull request with conflict files: %w", err)
}
return nil, &types.MergeViolations{ return nil, &types.MergeViolations{
ConflictFiles: cf.Files, ConflictFiles: mergeOutput.ConflictFiles,
RuleViolations: violations, RuleViolations: violations,
}, nil }, nil
} }
return nil, nil, fmt.Errorf("merge check execution failed: %w", err)
}
var activitySeqMerge, activitySeqBranchDeleted int64 var activitySeqMerge, activitySeqBranchDeleted int64
pr, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error { pr, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {

View File

@ -16,10 +16,8 @@ package pullreq
import ( import (
"context" "context"
"fmt"
"github.com/harness/gitness/app/auth" "github.com/harness/gitness/app/auth"
"github.com/harness/gitness/types/enum"
) )
// Recheck re-checks all system PR checks (mergeability check, ...). // Recheck re-checks all system PR checks (mergeability check, ...).
@ -29,6 +27,12 @@ func (c *Controller) Recheck(
repoRef string, repoRef string,
prNum int64, prNum int64,
) error { ) error {
// TODO: Remove the API.
_ = ctx
_ = session
_ = repoRef
_ = prNum
/*
repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoPush) repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoPush)
if err != nil { if err != nil {
return fmt.Errorf("failed to acquire access to repo: %w", err) return fmt.Errorf("failed to acquire access to repo: %w", err)
@ -38,6 +42,7 @@ func (c *Controller) Recheck(
if err != nil { if err != nil {
return fmt.Errorf("failed to refresh merge data: %w", err) return fmt.Errorf("failed to refresh merge data: %w", err)
} }
*/
return nil return nil
} }

View File

@ -16,13 +16,11 @@ package repo
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"github.com/harness/gitness/app/api/controller" "github.com/harness/gitness/app/api/controller"
"github.com/harness/gitness/app/auth" "github.com/harness/gitness/app/auth"
"github.com/harness/gitness/git" "github.com/harness/gitness/git"
gittypes "github.com/harness/gitness/git/types"
"github.com/harness/gitness/types/enum" "github.com/harness/gitness/types/enum"
) )
@ -52,22 +50,21 @@ func (c *Controller) MergeCheck(
return MergeCheck{}, fmt.Errorf("failed to create rpc write params: %w", err) return MergeCheck{}, fmt.Errorf("failed to create rpc write params: %w", err)
} }
_, err = c.git.Merge(ctx, &git.MergeParams{ mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{
WriteParams: writeParams, WriteParams: writeParams,
BaseBranch: info.BaseRef, BaseBranch: info.BaseRef,
HeadRepoUID: writeParams.RepoUID, // forks are not supported for now HeadRepoUID: writeParams.RepoUID, // forks are not supported for now
HeadBranch: info.HeadRef, HeadBranch: info.HeadRef,
}) })
if err != nil { if err != nil {
var cferr *gittypes.MergeConflictsError return MergeCheck{}, fmt.Errorf("merge check execution failed: %w", err)
if errors.As(err, &cferr) { }
if len(mergeOutput.ConflictFiles) > 0 {
return MergeCheck{ return MergeCheck{
Mergeable: false, Mergeable: false,
ConflictFiles: cferr.Files, ConflictFiles: mergeOutput.ConflictFiles,
}, nil }, nil
} }
return MergeCheck{}, fmt.Errorf("merge check execution failed: %w", err)
}
return MergeCheck{ return MergeCheck{
Mergeable: true, Mergeable: true,

View File

@ -25,7 +25,6 @@ import (
"github.com/harness/gitness/events" "github.com/harness/gitness/events"
"github.com/harness/gitness/git" "github.com/harness/gitness/git"
gitenum "github.com/harness/gitness/git/enum" gitenum "github.com/harness/gitness/git/enum"
gittypes "github.com/harness/gitness/git/types"
"github.com/harness/gitness/pubsub" "github.com/harness/gitness/pubsub"
"github.com/harness/gitness/types" "github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum" "github.com/harness/gitness/types/enum"
@ -221,8 +220,7 @@ func (s *Service) updateMergeDataInner(
// call merge and store output in pr merge reference. // call merge and store output in pr merge reference.
now := time.Now() now := time.Now()
var output git.MergeOutput mergeOutput, err := s.git.Merge(ctx, &git.MergeParams{
output, err = s.git.Merge(ctx, &git.MergeParams{
WriteParams: writeParams, WriteParams: writeParams,
BaseBranch: pr.TargetBranch, BaseBranch: pr.TargetBranch,
HeadRepoUID: sourceRepo.GitUID, HeadRepoUID: sourceRepo.GitUID,
@ -240,35 +238,26 @@ func (s *Service) updateMergeDataInner(
pr.SourceBranch, newSHA) pr.SourceBranch, newSHA)
} }
var cferr *gittypes.MergeConflictsError
isNotMergeableError := errors.As(err, &cferr)
if err != nil && !isNotMergeableError {
return fmt.Errorf("merge check failed for %d:%s and %d:%s with err: %w",
targetRepo.ID, pr.TargetBranch,
sourceRepo.ID, pr.SourceBranch,
err)
}
// Update DB in both cases (failure or success) // Update DB in both cases (failure or success)
_, err = s.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error { _, err = s.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
if pr.SourceSHA != newSHA { if pr.SourceSHA != newSHA {
return events.NewDiscardEventErrorf("PR SHA %s is newer than %s", pr.SourceSHA, newSHA) return events.NewDiscardEventErrorf("PR SHA %s is newer than %s", pr.SourceSHA, newSHA)
} }
if isNotMergeableError { if mergeOutput.MergeSHA == "" || len(mergeOutput.ConflictFiles) > 0 {
// TODO: should return sha's either way, and also conflicting files!
pr.MergeCheckStatus = enum.MergeCheckStatusConflict pr.MergeCheckStatus = enum.MergeCheckStatusConflict
// pr.MergeTargetSHA = &output.BaseSHA // TODO: Merge API doesn't return this when there are conflicts pr.MergeBaseSHA = mergeOutput.MergeBaseSHA
pr.MergeTargetSHA = &mergeOutput.BaseSHA
pr.MergeSHA = nil pr.MergeSHA = nil
pr.MergeConflicts = cferr.Files pr.MergeConflicts = mergeOutput.ConflictFiles
} else { } else {
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeTargetSHA = &output.BaseSHA pr.MergeBaseSHA = mergeOutput.MergeBaseSHA
pr.MergeBaseSHA = output.MergeBaseSHA // TODO: Merge check should not update the merge base. pr.MergeTargetSHA = &mergeOutput.BaseSHA
pr.MergeSHA = &output.MergeSHA pr.MergeSHA = &mergeOutput.MergeSHA
pr.MergeConflicts = nil pr.MergeConflicts = nil
} }
return nil return nil
}) })
if err != nil { if err != nil {

View File

@ -72,7 +72,7 @@ type Adapter interface {
CreateTemporaryRepoForPR(ctx context.Context, reposTempPath string, pr *types.PullRequest, CreateTemporaryRepoForPR(ctx context.Context, reposTempPath string, pr *types.PullRequest,
baseBranch, trackingBranch string) (types.TempRepository, error) baseBranch, trackingBranch string) (types.TempRepository, error)
Merge(ctx context.Context, pr *types.PullRequest, mergeMethod enum.MergeMethod, baseBranch, trackingBranch string, Merge(ctx context.Context, pr *types.PullRequest, mergeMethod enum.MergeMethod, baseBranch, trackingBranch string,
tmpBasePath string, mergeMsg string, identity *types.Identity, env ...string) error tmpBasePath string, mergeMsg string, identity *types.Identity, env ...string) (types.MergeResult, error)
GetMergeBase(ctx context.Context, repoPath, remote, base, head string) (string, string, error) GetMergeBase(ctx context.Context, repoPath, remote, base, head string) (string, string, error)
Blame(ctx context.Context, repoPath, rev, file string, lineFrom, lineTo int) types.BlameReader Blame(ctx context.Context, repoPath, rev, file string, lineFrom, lineTo int) types.BlameReader
Sync(ctx context.Context, repoPath string, source string, refSpecs []string) error Sync(ctx context.Context, repoPath string, source string, refSpecs []string) error

View File

@ -30,7 +30,6 @@ import (
"github.com/harness/gitness/git/types" "github.com/harness/gitness/git/types"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"github.com/rs/zerolog/log"
) )
// CreateTemporaryRepo creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch // CreateTemporaryRepo creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch
@ -197,6 +196,10 @@ func (a Adapter) CreateTemporaryRepoForPR(
}, nil }, nil
} }
type runMergeResult struct {
conflictFiles []string
}
func runMergeCommand( func runMergeCommand(
ctx context.Context, ctx context.Context,
pr *types.PullRequest, pr *types.PullRequest,
@ -204,7 +207,7 @@ func runMergeCommand(
cmd *git.Command, cmd *git.Command,
tmpBasePath string, tmpBasePath string,
env []string, env []string,
) error { ) (runMergeResult, error) {
var outbuf, errbuf strings.Builder var outbuf, errbuf strings.Builder
if err := cmd.Run(&git.RunOpts{ if err := cmd.Run(&git.RunOpts{
Dir: tmpBasePath, Dir: tmpBasePath,
@ -212,34 +215,33 @@ func runMergeCommand(
Stderr: &errbuf, Stderr: &errbuf,
Env: env, Env: env,
}); err != nil { }); err != nil {
if strings.Contains(errbuf.String(), "refusing to merge unrelated histories") {
return runMergeResult{}, &types.MergeUnrelatedHistoriesError{
Method: mergeMethod,
StdOut: outbuf.String(),
StdErr: errbuf.String(),
Err: err,
}
}
// Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict // 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 { if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil {
// We have a merge conflict error // We have a merge conflict error
files, cferr := conflictFiles(ctx, pr, env, tmpBasePath) files, cferr := conflictFiles(ctx, pr, env, tmpBasePath)
if cferr != nil { if cferr != nil {
return cferr return runMergeResult{}, cferr
}
return &types.MergeConflictsError{
Method: mergeMethod,
StdOut: outbuf.String(),
StdErr: errbuf.String(),
Err: err,
Files: files,
}
} else if strings.Contains(errbuf.String(), "refusing to merge unrelated histories") {
return &types.MergeUnrelatedHistoriesError{
Method: mergeMethod,
StdOut: outbuf.String(),
StdErr: errbuf.String(),
Err: err,
} }
return runMergeResult{
conflictFiles: files,
}, nil
} }
giteaErr := &giteaRunStdError{err: err, stderr: errbuf.String()} giteaErr := &giteaRunStdError{err: err, stderr: errbuf.String()}
return processGiteaErrorf(giteaErr, "git merge [%s -> %s]\n%s\n%s", return runMergeResult{}, processGiteaErrorf(giteaErr, "git merge [%s -> %s]\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, outbuf.String(), errbuf.String()) pr.HeadBranch, pr.BaseBranch, outbuf.String(), errbuf.String())
} }
return nil return runMergeResult{}, nil
} }
func commitAndSignNoAuthor( func commitAndSignNoAuthor(
@ -290,7 +292,7 @@ func (a Adapter) Merge(
mergeMsg string, mergeMsg string,
identity *types.Identity, identity *types.Identity,
env ...string, env ...string,
) error { ) (types.MergeResult, error) {
var ( var (
outbuf, errbuf strings.Builder outbuf, errbuf strings.Builder
) )
@ -306,18 +308,26 @@ func (a Adapter) Merge(
switch mergeMethod { switch mergeMethod {
case enum.MergeMethodMerge: case enum.MergeMethodMerge:
cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit", trackingBranch) cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit", trackingBranch)
if err := runMergeCommand(ctx, pr, mergeMethod, cmd, tmpBasePath, env); err != nil { result, err := runMergeCommand(ctx, pr, mergeMethod, cmd, tmpBasePath, env)
return fmt.Errorf("unable to merge tracking into base: %w", err) if err != nil {
return types.MergeResult{}, fmt.Errorf("unable to merge tracking into base: %w", err)
}
if len(result.conflictFiles) > 0 {
return types.MergeResult{ConflictFiles: result.conflictFiles}, nil
} }
if err := commitAndSignNoAuthor(ctx, pr, mergeMsg, signArg, tmpBasePath, env); err != nil { if err := commitAndSignNoAuthor(ctx, pr, mergeMsg, signArg, tmpBasePath, env); err != nil {
return fmt.Errorf("unable to make final commit: %w", err) return types.MergeResult{}, fmt.Errorf("unable to make final commit: %w", err)
} }
case enum.MergeMethodSquash: case enum.MergeMethodSquash:
// Merge with squash // Merge with squash
cmd := git.NewCommand(ctx, "merge", "--squash", trackingBranch) cmd := git.NewCommand(ctx, "merge", "--squash", trackingBranch)
if err := runMergeCommand(ctx, pr, mergeMethod, cmd, tmpBasePath, env); err != nil { result, err := runMergeCommand(ctx, pr, mergeMethod, cmd, tmpBasePath, env)
return fmt.Errorf("unable to merge --squash tracking into base: %w", err) if err != nil {
return types.MergeResult{}, fmt.Errorf("unable to merge --squash tracking into base: %w", err)
}
if len(result.conflictFiles) > 0 {
return types.MergeResult{ConflictFiles: result.conflictFiles}, nil
} }
if signArg == "" { if signArg == "" {
@ -328,7 +338,7 @@ func (a Adapter) Merge(
Stdout: &outbuf, Stdout: &outbuf,
Stderr: &errbuf, Stderr: &errbuf,
}); err != nil { }); err != nil {
return processGiteaErrorf(err, "git commit [%s -> %s]\n%s\n%s", return types.MergeResult{}, processGiteaErrorf(err, "git commit [%s -> %s]\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, outbuf.String(), errbuf.String()) pr.HeadBranch, pr.BaseBranch, outbuf.String(), errbuf.String())
} }
} else { } else {
@ -339,19 +349,19 @@ func (a Adapter) Merge(
Stdout: &outbuf, Stdout: &outbuf,
Stderr: &errbuf, Stderr: &errbuf,
}); err != nil { }); err != nil {
return processGiteaErrorf(err, "git commit [%s -> %s]\n%s\n%s", return types.MergeResult{}, processGiteaErrorf(err, "git commit [%s -> %s]\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, outbuf.String(), errbuf.String()) pr.HeadBranch, pr.BaseBranch, outbuf.String(), errbuf.String())
} }
} }
case enum.MergeMethodRebase: case enum.MergeMethodRebase:
// Checkout head branch // Create staging branch
if err := git.NewCommand(ctx, "checkout", "-b", stagingBranch, trackingBranch). if err := git.NewCommand(ctx, "checkout", "-b", stagingBranch, trackingBranch).
Run(&git.RunOpts{ Run(&git.RunOpts{
Dir: tmpBasePath, Dir: tmpBasePath,
Stdout: &outbuf, Stdout: &outbuf,
Stderr: &errbuf, Stderr: &errbuf,
}); err != nil { }); err != nil {
return fmt.Errorf( return types.MergeResult{}, fmt.Errorf(
"git checkout base prior to merge post staging rebase [%s -> %s]: %w\n%s\n%s", "git checkout base prior to merge post staging rebase [%s -> %s]: %w\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(), pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(),
) )
@ -359,6 +369,8 @@ func (a Adapter) Merge(
outbuf.Reset() outbuf.Reset()
errbuf.Reset() errbuf.Reset()
var conflicts bool
// Rebase before merging // Rebase before merging
if err := git.NewCommand(ctx, "rebase", baseBranch). if err := git.NewCommand(ctx, "rebase", baseBranch).
Run(&git.RunOpts{ Run(&git.RunOpts{
@ -368,52 +380,67 @@ func (a Adapter) Merge(
}); err != nil { }); err != nil {
// Rebase will leave a REBASE_HEAD file in .git if there is a conflict // 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 { if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil {
var commitSha string // Rebase works by processing commit by commit. To get the full list of conflict files
// all commits would have to be applied. It's simpler to revert the rebase and
// TBD git version we will support // get the list conflict using git merge.
// failingCommitPath := filepath.Join(tmpBasePath, ".git", "rebase-apply", "original-commit") // Git < 2.26 conflicts = true
// if _, cpErr := os.Stat(failingCommitPath); statErr != nil { } else {
// return fmt.Errorf("git rebase staging on to base [%s -> %s]: %v\n%s\n%s", return types.MergeResult{}, fmt.Errorf(
// pr.HeadBranch, pr.BaseBranch, cpErr, outbuf.String(), errbuf.String())
// }
failingCommitPath := filepath.Join(tmpBasePath, ".git", "rebase-merge", "stopped-sha") // Git >= 2.26
if _, cpErr := os.Stat(failingCommitPath); cpErr != nil {
return fmt.Errorf(
"git rebase staging on to base [%s -> %s]: %w\n%s\n%s", "git rebase staging on to base [%s -> %s]: %w\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, cpErr, outbuf.String(), errbuf.String(), pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(),
) )
} }
}
outbuf.Reset()
errbuf.Reset()
commitShaBytes, readErr := os.ReadFile(failingCommitPath) if conflicts {
if readErr != nil { // Rebase failed because there are conflicts. Abort the rebase.
// Abandon this attempt to handle the error if err := git.NewCommand(ctx, "rebase", "--abort").
return fmt.Errorf( Run(&git.RunOpts{
"git rebase staging on to base [%s -> %s]: %w\n%s\n%s", Dir: tmpBasePath,
pr.HeadBranch, pr.BaseBranch, readErr, outbuf.String(), errbuf.String(), Stdout: &outbuf,
) Stderr: &errbuf,
} }); err != nil {
commitSha = strings.TrimSpace(string(commitShaBytes)) return types.MergeResult{}, fmt.Errorf(
"git abort rebase [%s -> %s]: %w\n%s\n%s",
log.Debug().Msgf("RebaseConflict at %s [%s -> %s]: %v\n%s\n%s",
commitSha, pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(),
)
return &types.MergeConflictsError{
Method: mergeMethod,
CommitSHA: commitSha,
StdOut: outbuf.String(),
StdErr: errbuf.String(),
Err: err,
}
}
return fmt.Errorf(
"git rebase staging on to base [%s -> %s]: %w\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(), pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(),
) )
} }
outbuf.Reset() outbuf.Reset()
errbuf.Reset() errbuf.Reset()
// Go back to the base branch.
if err := git.NewCommand(ctx, "checkout", baseBranch).
Run(&git.RunOpts{
Dir: tmpBasePath,
Stdout: &outbuf,
Stderr: &errbuf,
}); err != nil {
return types.MergeResult{}, fmt.Errorf(
"return to the base branch [%s -> %s]: %w\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(),
)
}
outbuf.Reset()
errbuf.Reset()
// Run the ordinary merge to get the list of conflict files.
cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit", trackingBranch)
result, err := runMergeCommand(ctx, pr, mergeMethod, cmd, tmpBasePath, env)
if err != nil {
return types.MergeResult{}, fmt.Errorf(
"git abort rebase [%s -> %s]: %w\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(),
)
}
if len(result.conflictFiles) > 0 {
return types.MergeResult{ConflictFiles: result.conflictFiles}, nil
}
return types.MergeResult{}, errors.New("rebase reported conflicts, but merge gave no conflict files")
}
// Checkout base branch again // Checkout base branch again
if err := git.NewCommand(ctx, "checkout", baseBranch). if err := git.NewCommand(ctx, "checkout", baseBranch).
Run(&git.RunOpts{ Run(&git.RunOpts{
@ -421,7 +448,7 @@ func (a Adapter) Merge(
Stdout: &outbuf, Stdout: &outbuf,
Stderr: &errbuf, Stderr: &errbuf,
}); err != nil { }); err != nil {
return fmt.Errorf( return types.MergeResult{}, fmt.Errorf(
"git checkout base prior to merge post staging rebase [%s -> %s]: %w\n%s\n%s", "git checkout base prior to merge post staging rebase [%s -> %s]: %w\n%s\n%s",
pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(), pr.HeadBranch, pr.BaseBranch, err, outbuf.String(), errbuf.String(),
) )
@ -429,17 +456,20 @@ func (a Adapter) Merge(
outbuf.Reset() outbuf.Reset()
errbuf.Reset() errbuf.Reset()
cmd := git.NewCommand(ctx, "merge", "--ff-only", stagingBranch)
// Prepare merge with commit // Prepare merge with commit
if err := runMergeCommand(ctx, pr, mergeMethod, cmd, tmpBasePath, env); err != nil { cmd := git.NewCommand(ctx, "merge", "--ff-only", stagingBranch)
return err result, err := runMergeCommand(ctx, pr, mergeMethod, cmd, tmpBasePath, env)
if err != nil {
return types.MergeResult{}, fmt.Errorf("unable to ff-olny merge tracking into base: %w", err)
}
if len(result.conflictFiles) > 0 {
return types.MergeResult{ConflictFiles: result.conflictFiles}, nil
} }
default: default:
return fmt.Errorf("wrong merge method provided: %s", mergeMethod) return types.MergeResult{}, fmt.Errorf("wrong merge method provided: %s", mergeMethod)
} }
return nil return types.MergeResult{}, nil
} }
func conflictFiles( func conflictFiles(

View File

@ -98,6 +98,10 @@ type MergeOutput struct {
MergeBaseSHA string MergeBaseSHA string
// MergeSHA is the sha of the commit after merging HeadSHA with BaseSHA. // MergeSHA is the sha of the commit after merging HeadSHA with BaseSHA.
MergeSHA string MergeSHA string
CommitCount int
ChangedFileCount int
ConflictFiles []string
} }
// Merge method executes git merge operation. Refs can be sha, branch or tag. // Merge method executes git merge operation. Refs can be sha, branch or tag.
@ -114,7 +118,7 @@ type MergeOutput struct {
// params.HeadExpectedSHA which will be compared with the latest sha from head branch // params.HeadExpectedSHA which will be compared with the latest sha from head branch
// if they are not the same error will be returned. // if they are not the same error will be returned.
// //
//nolint:gocognit //nolint:gocognit,gocyclo,cyclop
func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput, error) { func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput, error) {
if err := params.Validate(); err != nil { if err := params.Validate(); err != nil {
return MergeOutput{}, fmt.Errorf("Merge: params not valid: %w", err) return MergeOutput{}, fmt.Errorf("Merge: params not valid: %w", err)
@ -179,6 +183,19 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput,
fmt.Errorf("unable to write .git/info/sparse-checkout file in tmpRepo.Path: %w", err) fmt.Errorf("unable to write .git/info/sparse-checkout file in tmpRepo.Path: %w", err)
} }
shortStat, err := s.adapter.DiffShortStat(ctx, tmpRepo.Path, tmpRepo.BaseSHA, tmpRepo.HeadSHA, true)
if err != nil {
return MergeOutput{}, fmt.Errorf("execution of DiffShortStat failed: %w", err)
}
changedFileCount := shortStat.Files
divergences, err := s.adapter.GetCommitDivergences(ctx, tmpRepo.Path,
[]types.CommitDivergenceRequest{{From: tmpRepo.HeadSHA, To: tmpRepo.BaseSHA}}, 0)
if err != nil {
return MergeOutput{}, fmt.Errorf("execution of GetCommitDivergences failed: %w", err)
}
commitCount := int(divergences[0].Ahead)
// Switch off LFS process (set required, clean and smudge here also) // Switch off LFS process (set required, clean and smudge here also)
if err = s.adapter.Config(ctx, tmpRepo.Path, "filter.lfs.process", ""); err != nil { if err = s.adapter.Config(ctx, tmpRepo.Path, "filter.lfs.process", ""); err != nil {
return MergeOutput{}, err return MergeOutput{}, err
@ -245,7 +262,7 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput,
params.Method = enum.MergeMethodMerge params.Method = enum.MergeMethodMerge
} }
if err = s.adapter.Merge( result, err := s.adapter.Merge(
ctx, ctx,
pr, pr,
params.Method, params.Method,
@ -257,10 +274,23 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput,
Name: author.Name, Name: author.Name,
Email: author.Email, Email: author.Email,
}, },
env...); err != nil { env...)
if err != nil {
return MergeOutput{}, fmt.Errorf("merge failed: %w", err) return MergeOutput{}, fmt.Errorf("merge failed: %w", err)
} }
if len(result.ConflictFiles) > 0 {
return MergeOutput{
BaseSHA: tmpRepo.BaseSHA,
HeadSHA: tmpRepo.HeadSHA,
MergeBaseSHA: mergeBaseCommitSHA,
MergeSHA: "",
CommitCount: commitCount,
ChangedFileCount: changedFileCount,
ConflictFiles: result.ConflictFiles,
}, nil
}
mergeCommitSHA, err := s.adapter.GetFullCommitID(ctx, tmpRepo.Path, baseBranch) mergeCommitSHA, err := s.adapter.GetFullCommitID(ctx, tmpRepo.Path, baseBranch)
if err != nil { if err != nil {
return MergeOutput{}, fmt.Errorf("failed to get full commit id for the new merge: %w", err) return MergeOutput{}, fmt.Errorf("failed to get full commit id for the new merge: %w", err)
@ -272,6 +302,9 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput,
HeadSHA: tmpRepo.HeadSHA, HeadSHA: tmpRepo.HeadSHA,
MergeBaseSHA: mergeBaseCommitSHA, MergeBaseSHA: mergeBaseCommitSHA,
MergeSHA: mergeCommitSHA, MergeSHA: mergeCommitSHA,
CommitCount: commitCount,
ChangedFileCount: changedFileCount,
ConflictFiles: nil,
}, nil }, nil
} }
@ -297,6 +330,9 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput,
HeadSHA: tmpRepo.HeadSHA, HeadSHA: tmpRepo.HeadSHA,
MergeBaseSHA: mergeBaseCommitSHA, MergeBaseSHA: mergeBaseCommitSHA,
MergeSHA: mergeCommitSHA, MergeSHA: mergeCommitSHA,
CommitCount: commitCount,
ChangedFileCount: changedFileCount,
ConflictFiles: nil,
}, nil }, nil
} }

View File

@ -57,48 +57,6 @@ func (e *ValidationError) Error() string {
return e.Msg return e.Msg
} }
// MergeConflictsError represents an error if merging fails with a conflict.
type MergeConflictsError struct {
Method enum.MergeMethod
CommitSHA string
StdOut string
StdErr string
Err error
Files []string
}
func IsMergeConflictsError(err error) bool {
return errors.Is(err, &MergeConflictsError{})
}
func (e *MergeConflictsError) Error() string {
return fmt.Sprintf("Merge Conflict Error: %v: %s\n%s", e.Err, e.StdErr, e.StdOut)
}
func (e *MergeConflictsError) Unwrap() error {
return e.Err
}
func (e *MergeConflictsError) Status() errors.Status {
return StatusNotMergeable
}
func AsMergeConflictsError(err error) (e *MergeConflictsError) {
if err == nil {
return nil
}
if errors.As(err, &e) {
return
}
return
}
//nolint:errorlint // the purpose of this method is to check whether the target itself if of this type.
func (e *MergeConflictsError) Is(target error) bool {
_, ok := target.(*MergeConflictsError)
return ok
}
// MergeUnrelatedHistoriesError represents an error if merging fails due to unrelated histories. // MergeUnrelatedHistoriesError represents an error if merging fails due to unrelated histories.
type MergeUnrelatedHistoriesError struct { type MergeUnrelatedHistoriesError struct {
Method enum.MergeMethod Method enum.MergeMethod

View File

@ -365,3 +365,7 @@ type FileContent struct {
Path string Path string
Content []byte Content []byte
} }
type MergeResult struct {
ConflictFiles []string
}