From 70e3be10d7e3181d342ed3bfa9346b922f8a9570 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 16 Sep 2020 08:08:43 +0200 Subject: git2go: Pass merge arguments via JSON In order to invoke the gitaly-git2go merge subcommand, one currently has to pass each parameter as a separate flag. This is quite easy to get wrong and makes refactoring of the interface much harder to accomplish. This commit refactors the code to instead pass parameters via a serialized JSON structure. The new `internal/git2go` package contains all logic required to serialize or deserialize merge parameters as well as running the actual command. --- internal/gitaly/service/operations/merge.go | 41 ++++++++++------------------- 1 file changed, 14 insertions(+), 27 deletions(-) (limited to 'internal/gitaly/service/operations/merge.go') diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 4ff11fea6..2a19c9171 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -6,13 +6,11 @@ import ( "errors" "fmt" "io" - "os/exec" - "path" "strings" - "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/updateref" + "gitlab.com/gitlab-org/gitaly/internal/git2go" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/gitlabshell" @@ -201,34 +199,23 @@ func (s *server) userMergeBranch(stream gitalypb.OperationService_UserMergeBranc return err } - binary := path.Join(s.cfg.BinDir, "gitaly-git2go") - args := []string{ - "merge", - "-repository", repoPath, - "-author-name", string(firstRequest.User.Name), - "-author-mail", string(firstRequest.User.Email), - "-message", string(firstRequest.Message), - "-ours", firstRequest.CommitId, - "-theirs", revision, - } - - var stderr, stdout bytes.Buffer - mergeCommand, err := command.New(ctx, exec.Command(binary, args...), nil, &stdout, &stderr) + merge, err := git2go.MergeCommand{ + Repository: repoPath, + AuthorName: string(firstRequest.User.Name), + AuthorMail: string(firstRequest.User.Email), + Message: string(firstRequest.Message), + Ours: firstRequest.CommitId, + Theirs: revision, + }.Run(ctx, s.cfg) if err != nil { - return err - } - - if err := mergeCommand.Wait(); err != nil { - if _, ok := err.(*exec.ExitError); ok { - return fmt.Errorf("%w: %s", err, stderr.String()) + if errors.Is(err, git2go.ErrInvalidArgument) { + return helper.ErrInvalidArgument(err) } return err } - mergeCommit := text.ChompBytes(stdout.Bytes()) - if err := stream.Send(&gitalypb.UserMergeBranchResponse{ - CommitId: mergeCommit, + CommitId: merge.CommitID, }); err != nil { return err } @@ -242,7 +229,7 @@ func (s *server) userMergeBranch(stream gitalypb.OperationService_UserMergeBranc } branch := "refs/heads/" + text.ChompBytes(firstRequest.Branch) - if err := s.updateReferenceWithHooks(ctx, firstRequest.Repository, firstRequest.User, branch, mergeCommit, revision); err != nil { + if err := s.updateReferenceWithHooks(ctx, firstRequest.Repository, firstRequest.User, branch, merge.CommitID, revision); err != nil { var preReceiveError preReceiveError var updateRefError updateRefError @@ -262,7 +249,7 @@ func (s *server) userMergeBranch(stream gitalypb.OperationService_UserMergeBranc if err := stream.Send(&gitalypb.UserMergeBranchResponse{ BranchUpdate: &gitalypb.OperationBranchUpdate{ - CommitId: mergeCommit, + CommitId: merge.CommitID, RepoCreated: false, BranchCreated: false, }, -- cgit v1.2.3