Merge branch '30586-fix-gitaly-is-ancestor' into 'master'

Fix Gitaly::Commit#is_ancestor

Closes #30586

See merge request !10634

GITALY_SERVER_VERSION

@@ -1 +1 @@
-0.6.0
+0.8.0

app/models/repository.rb

@@ -961,15 +961,13 @@ class Repository
   end
 
   def is_ancestor?(ancestor_id, descendant_id)
-    # NOTE: This feature is intentionally disabled until
-    # https://gitlab.com/gitlab-org/gitlab-ce/issues/30586 is resolved
-    # Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
-    #   if is_enabled
-    #     raw_repository.is_ancestor?(ancestor_id, descendant_id)
-    #   else
-    merge_base_commit(ancestor_id, descendant_id) == ancestor_id
-    #   end
-    # end
+    Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
+      if is_enabled
+        raw_repository.is_ancestor?(ancestor_id, descendant_id)
+      else
+        merge_base_commit(ancestor_id, descendant_id) == ancestor_id
+      end
+    end
   end
 
   def empty_repo?

lib/gitlab/git/repository.rb

@@ -451,7 +451,7 @@ module Gitlab
 
       # Returns true is +from+ is direct ancestor to +to+, otherwise false
       def is_ancestor?(from, to)
-        Gitlab::GitalyClient::Commit.is_ancestor(self, from, to)
+        gitaly_commit_client.is_ancestor(from, to)
       end
 
       # Return an array of Diff objects that represent the diff
@@ -1273,6 +1273,10 @@ module Gitlab
         @gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(self)
       end
 
+      def gitaly_commit_client
+        @gitaly_commit_client ||= Gitlab::GitalyClient::Commit.new(self)
+      end
+
       # Returns the `Rugged` sorting type constant for a given
       # sort type key. Valid keys are `:none`, `:topo`, and `:date`
       def rugged_sort_type(key)

lib/gitlab/gitaly_client/commit.rb

@@ -5,6 +5,23 @@ module Gitlab
       # See http://stackoverflow.com/a/40884093/1856239 and https://github.com/git/git/blob/3ad8b5bf26362ac67c9020bf8c30eee54a84f56d/cache.h#L1011-L1012
       EMPTY_TREE_ID = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'.freeze
 
+      attr_accessor :stub
+
+      def initialize(repository)
+        @gitaly_repo = repository.gitaly_repository
+        @stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: repository.gitaly_channel)
+      end
+
+      def is_ancestor(ancestor_id, child_id)
+        request = Gitaly::CommitIsAncestorRequest.new(
+          repository: @gitaly_repo,
+          ancestor_id: ancestor_id,
+          child_id: child_id
+        )
+
+        @stub.commit_is_ancestor(request).value
+      end
+
       class << self
         def diff_from_parent(commit, options = {})
           repository = commit.project.repository
@@ -20,18 +37,6 @@ module Gitlab
 
           Gitlab::Git::DiffCollection.new(stub.commit_diff(request), options)
         end
-
-        def is_ancestor(repository, ancestor_id, child_id)
-          gitaly_repo = repository.gitaly_repository
-          stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: repository.gitaly_channel)
-          request = Gitaly::CommitIsAncestorRequest.new(
-            repository: gitaly_repo,
-            ancestor_id: ancestor_id,
-            child_id: child_id
-          )
-
-          stub.commit_is_ancestor(request).value
-        end
       end
     end
   end

spec/models/repository_spec.rb

@@ -1849,17 +1849,15 @@ describe Repository, models: true do
     end
   end
 
-  # TODO: Uncomment when feature is reenabled
-  # describe '#is_ancestor?' do
-  #   context 'Gitaly is_ancestor feature enabled' do
-  #     it 'asks Gitaly server if it\'s an ancestor' do
-  #       commit = repository.commit
-  #       allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true)
-  #       expect(Gitlab::GitalyClient::Commit).to receive(:is_ancestor).
-  #         with(repository.raw_repository, commit.id, commit.id).and_return(true)
-  #
-  #       expect(repository.is_ancestor?(commit.id, commit.id)).to be true
-  #     end
-  #   end
-  # end
+  describe '#is_ancestor?' do
+    context 'Gitaly is_ancestor feature enabled' do
+      it "asks Gitaly server if it's an ancestor" do
+        commit = repository.commit
+        expect(repository.raw_repository).to receive(:is_ancestor?).and_call_original
+        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true)
+
+        expect(repository.is_ancestor?(commit.id, commit.id)).to be true
+      end
+    end
+  end
 end