Merge branch 'feature/migrate-merge-base-to-gitaly' into 'master'

Migrate Gitlab::Git::Repository#merge_base_commit to Gitaly

Closes gitaly#808

See merge request gitlab-org/gitlab-ce!15770

GITALY_SERVER_VERSION

-0.59.0
+0.60.0

Gemfile

@@ -400,7 +400,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.59.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.61.0', require: 'gitaly'
 
 gem 'toml-rb', '~> 0.3.15', require: false
 

Gemfile.lock

@@ -284,7 +284,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly-proto (0.59.0)
+    gitaly-proto (0.61.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -1042,7 +1042,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly-proto (~> 0.59.0)
+  gitaly-proto (~> 0.61.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-markup (~> 1.6.2)

app/controllers/projects/merge_requests/creations_controller.rb

@@ -9,7 +9,10 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
   before_action :build_merge_request, except: [:create]
 
   def new
-    define_new_vars
+    # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/40934
+    Gitlab::GitalyClient.allow_n_plus_1_calls do
+      define_new_vars
+    end
   end
 
   def create

app/models/repository.rb

@@ -934,7 +934,7 @@ class Repository
   def merge_base(first_commit_id, second_commit_id)
     first_commit_id = commit(first_commit_id).try(:id) || first_commit_id
     second_commit_id = commit(second_commit_id).try(:id) || second_commit_id
-    rugged.merge_base(first_commit_id, second_commit_id)
+    raw_repository.merge_base(first_commit_id, second_commit_id)
   rescue Rugged::ReferenceError
     nil
   end

lib/gitlab/git/operation_service.rb

@@ -126,7 +126,7 @@ module Gitlab
 
         oldrev = branch.target
 
-        if oldrev == repository.rugged.merge_base(newrev, branch.target)
+        if oldrev == repository.merge_base(newrev, branch.target)
           oldrev
         else
           raise Gitlab::Git::CommitError.new('Branch diverged')

lib/gitlab/git/repository.rb

@@ -517,8 +517,15 @@ module Gitlab
 
       # Returns the SHA of the most recent common ancestor of +from+ and +to+
       def merge_base_commit(from, to)
-        rugged.merge_base(from, to)
+        gitaly_migrate(:merge_base) do |is_enabled|
+          if is_enabled
+            gitaly_repository_client.find_merge_base(from, to)
+          else
+            rugged.merge_base(from, to)
+          end
+        end
       end
+      alias_method :merge_base, :merge_base_commit
 
       # Gitaly note: JV: check gitlab-ee before removing this method.
       def rugged_is_ancestor?(ancestor_id, descendant_id)

lib/gitlab/gitaly_client/repository_service.rb

@@ -69,6 +69,16 @@ module Gitlab
         response.value
       end
 
+      def find_merge_base(*revisions)
+        request = Gitaly::FindMergeBaseRequest.new(
+          repository: @gitaly_repo,
+          revisions: revisions.map { |r| GitalyClient.encode(r) }
+        )
+
+        response = GitalyClient.call(@storage, :repository_service, :find_merge_base, request)
+        response.base.presence
+      end
+
       def fetch_source_branch(source_repository, source_branch, local_ref)
         request = Gitaly::FetchSourceBranchRequest.new(
           repository: @gitaly_repo,

spec/models/repository_spec.rb

@@ -1013,7 +1013,7 @@ describe Repository do
 
       it 'runs without errors' do
         # old_rev is an ancestor of new_rev
-        expect(repository.rugged.merge_base(old_rev, new_rev)).to eq(old_rev)
+        expect(repository.merge_base(old_rev, new_rev)).to eq(old_rev)
 
         # old_rev is not a direct ancestor (parent) of new_rev
         expect(repository.rugged.lookup(new_rev).parent_ids).not_to include(old_rev)
@@ -1035,7 +1035,7 @@ describe Repository do
 
       it 'raises an exception' do
         # The 'master' branch is NOT an ancestor of new_rev.
-        expect(repository.rugged.merge_base(old_rev, new_rev)).not_to eq(old_rev)
+        expect(repository.merge_base(old_rev, new_rev)).not_to eq(old_rev)
 
         # Updating 'master' to new_rev would lose the commits on 'master' that
         # are not contained in new_rev. This should not be allowed.