From 3380949aff2cf2ea99fa09230120b2581382b557 Mon Sep 17 00:00:00 2001 From: Igor Drozdov Date: Thu, 17 Dec 2020 19:29:59 +0300 Subject: [PATCH] Allow to resolve conflicts on UserMergeToRef When AllowConflicts option is passed, let's set the "ours" strategy to git-merge command In this case we won't have the merge failed due to the conflicts --- .../id-go-allow-conflicts-on-merge.yml | 5 +++ cmd/gitaly-git2go/merge.go | 4 +++ internal/git2go/merge.go | 3 ++ internal/gitaly/service/operations/merge.go | 31 +++++-------------- .../gitaly/service/operations/merge_test.go | 18 ++++------- 5 files changed, 26 insertions(+), 35 deletions(-) create mode 100644 changelogs/unreleased/id-go-allow-conflicts-on-merge.yml diff --git a/changelogs/unreleased/id-go-allow-conflicts-on-merge.yml b/changelogs/unreleased/id-go-allow-conflicts-on-merge.yml new file mode 100644 index 00000000000..6bf95f66801 --- /dev/null +++ b/changelogs/unreleased/id-go-allow-conflicts-on-merge.yml @@ -0,0 +1,5 @@ +--- +title: Allow to resolve conflicts on UserMergeToRef +merge_request: 2642 +author: +type: performance diff --git a/cmd/gitaly-git2go/merge.go b/cmd/gitaly-git2go/merge.go index 907e717c0a4..565d7921e8b 100644 --- a/cmd/gitaly-git2go/merge.go +++ b/cmd/gitaly-git2go/merge.go @@ -57,6 +57,10 @@ func (cmd *mergeSubcommand) Run(context.Context, io.Reader, io.Writer) error { } mergeOpts.RecursionLimit = git2go.MergeRecursionLimit + if request.AllowConflicts { + mergeOpts.FileFavor = git.MergeFileFavorOurs + } + index, err := repo.MergeCommits(ours, theirs, &mergeOpts) if err != nil { return fmt.Errorf("could not merge commits: %w", err) diff --git a/internal/git2go/merge.go b/internal/git2go/merge.go index 39b2efe155b..3bc0337bb84 100644 --- a/internal/git2go/merge.go +++ b/internal/git2go/merge.go @@ -37,6 +37,9 @@ type MergeCommand struct { Ours string `json:"ours"` // Theirs is the commit into which ours is to be merged. Theirs string `json:"theirs"` + // AllowConflicts resolves conflicts that occurred on merge commit and + // overrides the conflicts with our changes. + AllowConflicts bool `json:"allow_conflicts"` } // MergeResult contains results from a merge. diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 6dd16f82a2d..5e27e3ec8d1 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -318,12 +318,13 @@ func (s *Server) userMergeToRef(ctx context.Context, request *gitalypb.UserMerge // Now, we create the merge commit... merge, err := git2go.MergeCommand{ - Repository: repoPath, - AuthorName: string(request.User.Name), - AuthorMail: string(request.User.Email), - Message: string(request.Message), - Ours: ref, - Theirs: sourceRef, + Repository: repoPath, + AuthorName: string(request.User.Name), + AuthorMail: string(request.User.Email), + Message: string(request.Message), + Ours: ref, + Theirs: sourceRef, + AllowConflicts: request.AllowConflicts, }.Run(ctx, s.cfg) if err != nil { if errors.Is(err, git2go.ErrInvalidArgument) { @@ -350,23 +351,7 @@ func (s *Server) UserMergeToRef(ctx context.Context, in *gitalypb.UserMergeToRef return nil, helper.ErrInvalidArgument(err) } - // Ruby has grown a new feature since being ported to Go, and we don't - // handle that yet. - if !in.AllowConflicts { - return s.userMergeToRef(ctx, in) - } - - client, err := s.ruby.OperationServiceClient(ctx) - if err != nil { - return nil, helper.ErrInternal(err) - } - - clientCtx, err := rubyserver.SetHeaders(ctx, s.locator, in.GetRepository()) - if err != nil { - return nil, err - } - - return client.UserMergeToRef(clientCtx, in) + return s.userMergeToRef(ctx, in) } func isAncestor(ctx context.Context, repo repository.GitRepo, ancestor, descendant string) (bool, error) { diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index 37ad293a5e5..d1874b026ac 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -1,7 +1,6 @@ package operations import ( - "bytes" "context" "fmt" "io/ioutil" @@ -718,25 +717,20 @@ func TestConflictsOnUserMergeToRefRequest(t *testing.T) { FirstParentRef: []byte("refs/heads/" + mergeBranchName), } - t.Run("allow conflicts to be merged with markers", func(t *testing.T) { + t.Run("resolves conflicts and overrides them with our changes", func(t *testing.T) { request.AllowConflicts = true resp, err := client.UserMergeToRef(ctx, request) require.NoError(t, err) - var buf bytes.Buffer - cmd := exec.Command(config.Config.Git.BinPath, "-C", testRepoPath, "show", resp.CommitId) - cmd.Stdout = &buf - require.NoError(t, cmd.Run()) + output := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "show", resp.CommitId)) - bufStr := buf.String() - require.Contains(t, bufStr, "+<<<<<<< files/ruby/popen.rb") - require.Contains(t, bufStr, "+>>>>>>> files/ruby/popen.rb") - require.Contains(t, bufStr, "+<<<<<<< files/ruby/regex.rb") - require.Contains(t, bufStr, "+>>>>>>> files/ruby/regex.rb") + // Overrides conflicts with our changes + require.Contains(t, output, `/(zip|tar|7z|tar\.gz|tgz|gz|tar\.bz2|tbz|tbz2|tb2|bz2)/`) + require.Contains(t, output, "options = { chdir: path }") }) - t.Run("disallow conflicts to be merged", func(t *testing.T) { + t.Run("disallow to be merged due to the conflicts", func(t *testing.T) { request.AllowConflicts = false _, err := client.UserMergeToRef(ctx, request) -- GitLab