From e10437de407a0782cd49cf5f341517c5b17fdd5e Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Tue, 5 Sep 2017 17:29:45 +0000 Subject: Migrate Gitlab::Git::Repository#find_branch to Gitaly --- GITALY_SERVER_VERSION | 2 +- Gemfile | 2 +- Gemfile.lock | 4 ++-- lib/gitlab/git/repository.rb | 18 ++++++++++------ lib/gitlab/gitaly_client/ref_service.rb | 14 ++++++++++++ spec/lib/gitlab/git/repository_spec.rb | 38 +++++++++++++++++++++------------ 6 files changed, 53 insertions(+), 25 deletions(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 93d4c1ef06f..0f1a7dfc7c4 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.36.0 +0.37.0 diff --git a/Gemfile b/Gemfile index 61c941ae449..be3f75df7da 100644 --- a/Gemfile +++ b/Gemfile @@ -397,7 +397,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.31.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.32.0', require: 'gitaly' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index cba30e856ed..5761495dddc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -275,7 +275,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly-proto (0.31.0) + gitaly-proto (0.32.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -1020,7 +1020,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly-proto (~> 0.31.0) + gitaly-proto (~> 0.32.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.5.1) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index f7577b02d5d..75d4efc0bc5 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -134,15 +134,19 @@ module Gitlab # This is to work around a bug in libgit2 that causes in-memory refs to # be stale/invalid when packed-refs is changed. # See https://gitlab.com/gitlab-org/gitlab-ce/issues/15392#note_14538333 - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/474 def find_branch(name, force_reload = false) - reload_rugged if force_reload + gitaly_migrate(:find_branch) do |is_enabled| + if is_enabled + gitaly_ref_client.find_branch(name) + else + reload_rugged if force_reload - rugged_ref = rugged.branches[name] - if rugged_ref - target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target) - Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit) + rugged_ref = rugged.branches[name] + if rugged_ref + target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target) + Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit) + end + end end end diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index 8c0008c6971..a1a25cf2079 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -78,6 +78,20 @@ module Gitlab raise ArgumentError, e.message end + def find_branch(branch_name) + request = Gitaly::DeleteBranchRequest.new( + repository: @gitaly_repo, + name: GitalyClient.encode(branch_name) + ) + + response = GitalyClient.call(@repository.storage, :ref_service, :find_branch, request) + branch = response.branch + return unless branch + + target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit) + Gitlab::Git::Branch.new(@repository, encode!(branch.name.dup), branch.target_commit.id, target_commit) + end + private def consume_refs_response(response) diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 4cfb4b7d357..08959e7bc16 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -916,27 +916,37 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#find_branch' do - it 'should return a Branch for master' do - branch = repository.find_branch('master') + shared_examples 'finding a branch' do + it 'should return a Branch for master' do + branch = repository.find_branch('master') - expect(branch).to be_a_kind_of(Gitlab::Git::Branch) - expect(branch.name).to eq('master') - end + expect(branch).to be_a_kind_of(Gitlab::Git::Branch) + expect(branch.name).to eq('master') + end - it 'should handle non-existent branch' do - branch = repository.find_branch('this-is-garbage') + it 'should handle non-existent branch' do + branch = repository.find_branch('this-is-garbage') - expect(branch).to eq(nil) + expect(branch).to eq(nil) + end end - it 'should reload Rugged::Repository and return master' do - expect(Rugged::Repository).to receive(:new).twice.and_call_original + context 'when Gitaly find_branch feature is enabled' do + it_behaves_like 'finding a branch' + end - repository.find_branch('master') - branch = repository.find_branch('master', force_reload: true) + context 'when Gitaly find_branch feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'finding a branch' - expect(branch).to be_a_kind_of(Gitlab::Git::Branch) - expect(branch.name).to eq('master') + it 'should reload Rugged::Repository and return master' do + expect(Rugged::Repository).to receive(:new).twice.and_call_original + + repository.find_branch('master') + branch = repository.find_branch('master', force_reload: true) + + expect(branch).to be_a_kind_of(Gitlab::Git::Branch) + expect(branch.name).to eq('master') + end end end -- cgit v1.2.1