Merge branch '1091-rewrite-findmergebase-in-go' into 'master'

Resolve "Rewrite FindMergeBase in Go"

Closes #1091

See merge request gitlab-org/gitaly!632

3 files changed, 26 insertions, 19 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b064ef18..5c3216c15 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 UNRELEASED
 
+- Rewrite RepositoryService.MergeBase in Go
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/632
 - Encode OperationsService errors in UTF-8 before sending them
   https://gitlab.com/gitlab-org/gitaly/merge_requests/627
 - Add param logging in NamespaceService RPCs
diff --git a/internal/service/repository/merge_base.go b/internal/service/repository/merge_base.go
index 192cc3a25..a9c5692e1 100644
--- a/internal/service/repository/merge_base.go
+++ b/internal/service/repository/merge_base.go
@@ -1,9 +1,11 @@
 package repository
 
 import (
-	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
+	"io/ioutil"
+	"strings"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
@@ -11,19 +13,35 @@ import (
 )
 
 func (s *server) FindMergeBase(ctx context.Context, req *pb.FindMergeBaseRequest) (*pb.FindMergeBaseResponse, error) {
-	if len(req.Revisions) != 2 {
+	revisions := req.GetRevisions()
+	if len(revisions) != 2 {
 		return nil, status.Errorf(codes.InvalidArgument, "FindMergeBase: 2 revisions are required")
 	}
 
-	client, err := s.RepositoryServiceClient(ctx)
+	args := []string{"merge-base"}
+	for _, revision := range revisions {
+		args = append(args, string(revision))
+	}
+
+	cmd, err := git.Command(ctx, req.GetRepository(), args...)
 	if err != nil {
-		return nil, err
+		if _, ok := status.FromError(err); ok {
+			return nil, err
+		}
+		return nil, status.Errorf(codes.Internal, "FindMergeBase: cmd: %v", err)
 	}
 
-	clientCtx, err := rubyserver.SetHeaders(ctx, req.GetRepository())
+	mergeBase, err := ioutil.ReadAll(cmd)
 	if err != nil {
 		return nil, err
 	}
 
-	return client.FindMergeBase(clientCtx, req)
+	mergeBaseStr := strings.TrimSpace(string(mergeBase))
+
+	if err := cmd.Wait(); err != nil {
+		// On error just return an empty merge base
+		return &pb.FindMergeBaseResponse{Base: ""}, nil
+	}
+
+	return &pb.FindMergeBaseResponse{Base: mergeBaseStr}, nil
 }
diff --git a/ruby/lib/gitaly_server/repository_service.rb b/ruby/lib/gitaly_server/repository_service.rb
index 33227becd..cddb4e351 100644
--- a/ruby/lib/gitaly_server/repository_service.rb
+++ b/ruby/lib/gitaly_server/repository_service.rb
@@ -42,19 +42,6 @@ module GitalyServer
       Gitaly::FsckResponse.new(error: ex.message.b)
     end
 
-    def find_merge_base(request, call)
-      bridge_exceptions do
-        begin
-          repo = Gitlab::Git::Repository.from_gitaly(request.repository, call)
-          base = repo.merge_base(*request.revisions)
-
-          Gitaly::FindMergeBaseResponse.new(base: base.to_s)
-        rescue Rugged::ReferenceError
-          Gitaly::FindMergeBaseResponse.new
-        end
-      end
-    end
-
     def fetch_remote(request, call)
       bridge_exceptions do
         gitlab_projects = Gitlab::Git::GitlabProjects.from_gitaly(request.repository, call)