diff options
author | Ahmad Sherif <me@ahmadsherif.com> | 2017-12-28 19:07:21 +0100 |
---|---|---|
committer | Ahmad Sherif <me@ahmadsherif.com> | 2018-01-11 14:52:15 +0100 |
commit | 831019412f32089935755215db609b78794f997a (patch) | |
tree | 3120a616a2b5815960fe61e141be002f87d59583 | |
parent | be623ef3c1a867d23e9625fe372c17fcad3c47ce (diff) | |
download | gitlab-ce-831019412f32089935755215db609b78794f997a.tar.gz |
Migrate merged_branch_names to Gitaly
Closes gitaly#851
-rw-r--r-- | lib/gitlab/git/repository.rb | 33 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/ref_service.rb | 31 | ||||
-rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 66 |
3 files changed, 85 insertions, 45 deletions
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 283134e043e..05fa44b48c7 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -571,7 +571,21 @@ module Gitlab end def merged_branch_names(branch_names = []) - Set.new(git_merged_branch_names(branch_names)) + return [] unless root_ref + + root_sha = find_branch(root_ref)&.target + + return [] unless root_sha + + branches = gitaly_migrate(:merged_branch_names) do |is_enabled| + if is_enabled + gitaly_merged_branch_names(branch_names, root_sha) + else + git_merged_branch_names(branch_names, root_sha) + end + end + + Set.new(branches) end # Return an array of Diff objects that represent the diff @@ -1475,14 +1489,7 @@ module Gitlab sort_branches(branches, sort_by) end - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/695 - def git_merged_branch_names(branch_names = []) - return [] unless root_ref - - root_sha = find_branch(root_ref)&.target - - return [] unless root_sha - + def git_merged_branch_names(branch_names, root_sha) git_arguments = %W[branch --merged #{root_sha} --format=%(refname:short)\ %(objectname)] + branch_names @@ -1496,6 +1503,14 @@ module Gitlab end end + def gitaly_merged_branch_names(branch_names, root_sha) + qualified_branch_names = branch_names.map { |b| "refs/heads/#{b}" } + + gitaly_ref_client.merged_branches(qualified_branch_names) + .reject { |b| b.target == root_sha } + .map(&:name) + end + def process_count_commits_options(options) if options[:from] || options[:to] ref = diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index 5bce1009878..f8e2a27f3fe 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -14,12 +14,18 @@ module Gitlab request = Gitaly::FindAllBranchesRequest.new(repository: @gitaly_repo) response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request) - response.flat_map do |message| - message.branches.map do |branch| - target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target) - Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit) - end - end + consume_find_all_branches_response(response) + end + + def merged_branches(branch_names = []) + request = Gitaly::FindAllBranchesRequest.new( + repository: @gitaly_repo, + merged_only: true, + merged_branches: branch_names.map { |s| encode_binary(s) } + ) + response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request) + + consume_find_all_branches_response(response) end def default_branch_name @@ -62,7 +68,7 @@ module Gitlab request = Gitaly::FindLocalBranchesRequest.new(repository: @gitaly_repo) request.sort_by = sort_by_param(sort_by) if sort_by response = GitalyClient.call(@storage, :ref_service, :find_local_branches, request) - consume_branches_response(response) + consume_find_local_branches_response(response) end def tags @@ -151,7 +157,7 @@ module Gitlab enum_value end - def consume_branches_response(response) + def consume_find_local_branches_response(response) response.flat_map do |message| message.branches.map do |gitaly_branch| Gitlab::Git::Branch.new( @@ -164,6 +170,15 @@ module Gitlab end end + def consume_find_all_branches_response(response) + response.flat_map do |message| + message.branches.map do |branch| + target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target) + Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit) + end + end + end + def consume_tags_response(response) response.flat_map do |message| message.tags.map { |gitaly_tag| Util.gitlab_tag_from_gitaly_tag(@repository, gitaly_tag) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index f346a345f00..f4e781c599e 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1283,48 +1283,58 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#merged_branch_names' do - context 'when branch names are passed' do - it 'only returns the names we are asking' do - names = repository.merged_branch_names(%w[merge-test]) + shared_examples 'finding merged branch names' do + context 'when branch names are passed' do + it 'only returns the names we are asking' do + names = repository.merged_branch_names(%w[merge-test]) - expect(names).to contain_exactly('merge-test') - end + expect(names).to contain_exactly('merge-test') + end - it 'does not return unmerged branch names' do - names = repository.merged_branch_names(%w[feature]) + it 'does not return unmerged branch names' do + names = repository.merged_branch_names(%w[feature]) - expect(names).to be_empty + expect(names).to be_empty + end end - end - context 'when no root ref is available' do - it 'returns empty list' do - project = create(:project, :empty_repo) + context 'when no root ref is available' do + it 'returns empty list' do + project = create(:project, :empty_repo) - names = project.repository.merged_branch_names(%w[feature]) + names = project.repository.merged_branch_names(%w[feature]) - expect(names).to be_empty + expect(names).to be_empty + end end - end - context 'when no branch names are specified' do - before do - repository.create_branch('identical', 'master') - end + context 'when no branch names are specified' do + before do + repository.create_branch('identical', 'master') + end - after do - ensure_seeds - end + after do + ensure_seeds + end - it 'returns all merged branch names except for identical one' do - names = repository.merged_branch_names + it 'returns all merged branch names except for identical one' do + names = repository.merged_branch_names - expect(names).to include('merge-test') - expect(names).to include('fix-mode') - expect(names).not_to include('feature') - expect(names).not_to include('identical') + expect(names).to include('merge-test') + expect(names).to include('fix-mode') + expect(names).not_to include('feature') + expect(names).not_to include('identical') + end end end + + context 'when Gitaly merged_branch_names feature is enabled' do + it_behaves_like 'finding merged branch names' + end + + context 'when Gitaly merged_branch_names feature is disabled', :disable_gitaly do + it_behaves_like 'finding merged branch names' + end end describe "#ls_files" do |