From 0e2577229d8e12fdb02a156fcc6672ebc6bb3f5d Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 13 Jun 2018 16:36:43 +0200 Subject: Move GC RPCs to mandatory Closes https://gitlab.com/gitlab-org/gitaly/issues/354 --- app/workers/git_garbage_collect_worker.rb | 58 ++---------- spec/workers/git_garbage_collect_worker_spec.rb | 117 +++++++++--------------- 2 files changed, 52 insertions(+), 123 deletions(-) diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb index f3c9e2b1582..ae5c5fac834 100644 --- a/app/workers/git_garbage_collect_worker.rb +++ b/app/workers/git_garbage_collect_worker.rb @@ -6,12 +6,6 @@ class GitGarbageCollectWorker # Timeout set to 24h LEASE_TIMEOUT = 86400 - GITALY_MIGRATED_TASKS = { - gc: :garbage_collect, - full_repack: :repack_full, - incremental_repack: :repack_incremental - }.freeze - def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil) project = Project.find(project_id) active_uuid = get_lease_uuid(lease_key) @@ -27,21 +21,7 @@ class GitGarbageCollectWorker end task = task.to_sym - cmd = command(task) - - gitaly_migrate(GITALY_MIGRATED_TASKS[task], status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_call(task, project.repository.raw_repository) - else - repo_path = project.repository.path_to_repo - description = "'#{cmd.join(' ')}' in #{repo_path}" - Gitlab::GitLogger.info(description) - - output, status = Gitlab::Popen.popen(cmd, repo_path) - - Gitlab::GitLogger.error("#{description} failed:\n#{output}") unless status.zero? - end - end + gitaly_call(task, project.repository.raw_repository) # Refresh the branch cache in case garbage collection caused a ref lookup to fail flush_ref_caches(project) if task == :gc @@ -82,21 +62,12 @@ class GitGarbageCollectWorker when :incremental_repack client.repack_incremental end - end - - def command(task) - case task - when :gc - git(write_bitmaps: bitmaps_enabled?) + %w[gc] - when :full_repack - git(write_bitmaps: bitmaps_enabled?) + %w[repack -A -d --pack-kept-objects] - when :incremental_repack - # Normal git repack fails when bitmaps are enabled. It is impossible to - # create a bitmap here anyway. - git(write_bitmaps: false) + %w[repack -d] - else - raise "Invalid gc task: #{task.inspect}" - end + rescue GRPC::NotFound => e + Gitlab::GitLogger.error("#{method} failed:\nRepository not found") + raise Gitlab::Git::Repository::NoRepository.new(e) + rescue GRPC::BadStatus => e + Gitlab::GitLogger.error("#{method} failed:\n#{e}") + raise Gitlab::Git::CommandError.new(e) end def flush_ref_caches(project) @@ -108,19 +79,4 @@ class GitGarbageCollectWorker def bitmaps_enabled? Gitlab::CurrentSettings.housekeeping_bitmaps_enabled end - - def git(write_bitmaps:) - config_value = write_bitmaps ? 'true' : 'false' - %W[git -c repack.writeBitmaps=#{config_value}] - end - - def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block) - Gitlab::GitalyClient.migrate(method, status: status, &block) - rescue GRPC::NotFound => e - Gitlab::GitLogger.error("#{method} failed:\nRepository not found") - raise Gitlab::Git::Repository::NoRepository.new(e) - rescue GRPC::BadStatus => e - Gitlab::GitLogger.error("#{method} failed:\n#{e}") - raise Gitlab::Git::CommandError.new(e) - end end diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index 807d1b8c084..e39dec556fc 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -11,36 +11,63 @@ describe GitGarbageCollectWorker do subject { described_class.new } describe "#perform" do - shared_examples 'flushing ref caches' do |gitaly| - context 'with active lease_uuid' do + context 'with active lease_uuid' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) + end + + it "flushes ref caches when the task if 'gc'" do + expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) + .and_return(nil) + expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original + expect_any_instance_of(Repository).to receive(:branch_names).and_call_original + expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original + expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original + + subject.perform(project.id, :gc, lease_key, lease_uuid) + end + end + + context 'with different lease than the active one' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid) + end + + it 'returns silently' do + expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original + expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original + expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original + + subject.perform(project.id, :gc, lease_key, lease_uuid) + end + end + + context 'with no active lease' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(false) + end + + context 'when is able to get the lease' do before do - allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) + allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid) end it "flushes ref caches when the task if 'gc'" do - expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original - expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) - - if gitaly - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) - .and_return(nil) - else - expect(Gitlab::Popen).to receive(:popen) - .with([:the, :command], project.repository.path_to_repo).and_return(["", 0]) - end - + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) + .and_return(nil) expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original - subject.perform(project.id, :gc, lease_key, lease_uuid) + subject.perform(project.id) end end - context 'with different lease than the active one' do + context 'when no lease can be obtained' do before do - allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid) + expect(subject).to receive(:try_obtain_lease).and_return(false) end it 'returns silently' do @@ -49,63 +76,9 @@ describe GitGarbageCollectWorker do expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original - subject.perform(project.id, :gc, lease_key, lease_uuid) + subject.perform(project.id) end end - - context 'with no active lease' do - before do - allow(subject).to receive(:get_lease_uuid).and_return(false) - end - - context 'when is able to get the lease' do - before do - allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid) - end - - it "flushes ref caches when the task if 'gc'" do - expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) - - if gitaly - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) - .and_return(nil) - else - expect(Gitlab::Popen).to receive(:popen) - .with([:the, :command], project.repository.path_to_repo).and_return(["", 0]) - end - - expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original - expect_any_instance_of(Repository).to receive(:branch_names).and_call_original - expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original - expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original - - subject.perform(project.id) - end - end - - context 'when no lease can be obtained' do - before do - expect(subject).to receive(:try_obtain_lease).and_return(false) - end - - it 'returns silently' do - expect(subject).not_to receive(:command) - expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original - expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original - expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original - - subject.perform(project.id) - end - end - end - end - - context "with Gitaly turned on" do - it_should_behave_like 'flushing ref caches', true - end - - context "with Gitaly turned off", :disable_gitaly do - it_should_behave_like 'flushing ref caches', false end context "repack_full" do -- cgit v1.2.1 From f195a7436d8848cccc8888f2de047547fa9eb94f Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 13 Jun 2018 16:44:59 +0200 Subject: RawBlame only called through Gitaly Closes https://gitlab.com/gitlab-org/gitaly/issues/376 --- lib/gitlab/git/blame.rb | 19 ++----------------- spec/lib/gitlab/git/blame_spec.rb | 10 +--------- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb index 40b65f6c0da..e25e15f5c80 100644 --- a/lib/gitlab/git/blame.rb +++ b/lib/gitlab/git/blame.rb @@ -22,24 +22,9 @@ module Gitlab private def load_blame - raw_output = @repo.gitaly_migrate(:blame, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - load_blame_by_gitaly - else - load_blame_by_shelling_out - end - end - - output = encode_utf8(raw_output) - process_raw_blame output - end - - def load_blame_by_gitaly - @repo.gitaly_commit_client.raw_blame(@sha, @path) - end + output = encode_utf8(@repo.gitaly_commit_client.raw_blame(@sha, @path)) - def load_blame_by_shelling_out - @repo.shell_blame(@sha, @path) + process_raw_blame(output) end def process_raw_blame(output) diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index 793228701cf..ba790b717ae 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::Git::Blame, seed_helper: true do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end - shared_examples 'blaming a file' do + describe 'blaming a file' do context "each count" do it do data = [] @@ -68,12 +68,4 @@ describe Gitlab::Git::Blame, seed_helper: true do end end end - - context 'when Gitaly blame feature is enabled' do - it_behaves_like 'blaming a file' - end - - context 'when Gitaly blame feature is disabled', :skip_gitaly_mock do - it_behaves_like 'blaming a file' - end end -- cgit v1.2.1 From 4fc46007479784997a90e3b648fd5d4b3ff897ec Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Jun 2018 09:56:16 +0200 Subject: Default branch detection happens through Gitaly Migration: https://gitlab.com/gitlab-org/gitaly/issues/220 --- lib/gitlab/git/repository.rb | 37 ++++-------------------- spec/lib/gitlab/git/repository_spec.rb | 51 ---------------------------------- 2 files changed, 5 insertions(+), 83 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 61ae42a116b..49e7330618d 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -120,13 +120,11 @@ module Gitlab # Default branch in the repository def root_ref - @root_ref ||= gitaly_migrate(:root_ref, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.default_branch_name - else - discover_default_branch - end - end + gitaly_ref_client.default_branch_name + rescue GRPC::NotFound => e + raise NoRepository.new(e.message) + rescue GRPC::Unknown => e + raise Gitlab::Git::CommandError.new(e.message) end def rugged @@ -364,31 +362,6 @@ module Gitlab end.map(&:name) end - # Discovers the default branch based on the repository's available branches - # - # - If no branches are present, returns nil - # - If one branch is present, returns its name - # - If two or more branches are present, returns current HEAD or master or first branch - def discover_default_branch - names = branch_names - - return if names.empty? - - return names[0] if names.length == 1 - - if rugged_head - extracted_name = Ref.extract_branch_name(rugged_head.name) - - return extracted_name if names.include?(extracted_name) - end - - if names.include?('master') - 'master' - else - names[0] - end - end - def rugged_head rugged.head rescue Rugged::ReferenceError diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 1744db1b17e..9cc7a0d79dc 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -77,17 +77,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#root_ref' do - context 'with gitaly disabled' do - before do - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) - end - - it 'calls #discover_default_branch' do - expect(repository).to receive(:discover_default_branch) - repository.root_ref - end - end - it 'returns UTF-8' do expect(repository.root_ref).to be_utf8 end @@ -153,46 +142,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe "#discover_default_branch" do - let(:master) { 'master' } - let(:feature) { 'feature' } - let(:feature2) { 'feature2' } - - around do |example| - # discover_default_branch will be moved to gitaly-ruby - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - it "returns 'master' when master exists" do - expect(repository).to receive(:branch_names).at_least(:once).and_return([feature, master]) - expect(repository.discover_default_branch).to eq('master') - end - - it "returns non-master when master exists but default branch is set to something else" do - File.write(File.join(repository_path, 'HEAD'), 'ref: refs/heads/feature') - expect(repository).to receive(:branch_names).at_least(:once).and_return([feature, master]) - expect(repository.discover_default_branch).to eq('feature') - File.write(File.join(repository_path, 'HEAD'), 'ref: refs/heads/master') - end - - it "returns a non-master branch when only one exists" do - expect(repository).to receive(:branch_names).at_least(:once).and_return([feature]) - expect(repository.discover_default_branch).to eq('feature') - end - - it "returns a non-master branch when more than one exists and master does not" do - expect(repository).to receive(:branch_names).at_least(:once).and_return([feature, feature2]) - expect(repository.discover_default_branch).to eq('feature') - end - - it "returns nil when no branch exists" do - expect(repository).to receive(:branch_names).at_least(:once).and_return([]) - expect(repository.discover_default_branch).to be_nil - end - end - describe '#branch_names' do subject { repository.branch_names } -- cgit v1.2.1 From 89095530bc750a780f1f03bc3c6a31c5491418db Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Jun 2018 10:00:47 +0200 Subject: Tag names returned through Gitaly Migration: https://gitlab.com/gitlab-org/gitaly/issues/220 --- lib/gitlab/git/repository.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 49e7330618d..6e149984a49 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -268,13 +268,7 @@ module Gitlab # Returns an Array of tag names def tag_names - gitaly_migrate(:tag_names, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.tag_names - else - rugged.tags.map { |t| t.name } - end - end + gitaly_ref_client.tag_names end # Returns an Array of Tags -- cgit v1.2.1 From 36d75be393eaa8385a4ba121b11b9d8e02ad52de Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Jun 2018 10:13:15 +0200 Subject: Move TagNames to mandatory through Gitaly Closes: https://gitlab.com/gitlab-org/gitaly/issues/220 --- lib/gitlab/git/repository.rb | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 6e149984a49..37c0af3622e 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -150,12 +150,8 @@ module Gitlab # Returns an Array of branch names # sorted by name ASC def branch_names - gitaly_migrate(:branch_names, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.branch_names - else - branches.map(&:name) - end + wrapped_gitaly_errors do + gitaly_ref_client.branch_names end end @@ -268,7 +264,9 @@ module Gitlab # Returns an Array of tag names def tag_names - gitaly_ref_client.tag_names + wrapped_gitaly_errors do + gitaly_ref_client.tag_names + end end # Returns an Array of Tags @@ -1420,6 +1418,16 @@ module Gitlab raise CommandError.new(e) end + def wrapped_gitaly_errors(&block) + yield block + rescue GRPC::NotFound => e + raise NoRepository.new(e) + rescue GRPC::InvalidArgument => e + raise ArgumentError.new(e) + rescue GRPC::BadStatus => e + raise CommandError.new(e) + end + def clean_stale_repository_files gitaly_migrate(:repository_cleanup, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| gitaly_repository_client.cleanup if is_enabled && exists? -- cgit v1.2.1 From 143a632ebe1772f2c74a892817659a464f3e887c Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Jun 2018 10:18:45 +0200 Subject: Tags are migrated to Gitaly Closes https://gitlab.com/gitlab-org/gitaly/issues/390 --- lib/gitlab/git/repository.rb | 34 ++-------------------------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 37c0af3622e..f6b21692493 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -273,12 +273,8 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/390 def tags - gitaly_migrate(:tags, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - tags_from_gitaly - else - tags_from_rugged - end + wrapped_gitaly_errors do + gitaly_ref_client.tags end end @@ -1931,37 +1927,11 @@ module Gitlab end end - def tags_from_rugged - rugged.references.each("refs/tags/*").map do |ref| - message = nil - - if ref.target.is_a?(Rugged::Tag::Annotation) - tag_message = ref.target.message - - if tag_message.respond_to?(:chomp) - message = tag_message.chomp - end - end - - target_commit = Gitlab::Git::Commit.find(self, ref.target) - Gitlab::Git::Tag.new(self, { - name: ref.name, - target: ref.target, - target_commit: target_commit, - message: message - }) - end.sort_by(&:name) - end - def last_commit_for_path_by_rugged(sha, path) sha = last_commit_id_for_path_by_shelling_out(sha, path) commit(sha) end - def tags_from_gitaly - gitaly_ref_client.tags - end - def size_by_shelling_out popen(%w(du -sk), path).first.strip.to_i end -- cgit v1.2.1 From 89ab32c6b20b9a0e8cde9d7f28759db8f53c7ffb Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Jun 2018 10:25:28 +0200 Subject: Branches are fully migrated to Gitaly Closes: https://gitlab.com/gitlab-org/gitaly/issues/389 --- lib/gitlab/git/repository.rb | 8 ++------ spec/lib/gitlab/git/repository_spec.rb | 18 ------------------ 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index f6b21692493..4410b81ed9a 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -157,12 +157,8 @@ module Gitlab # Returns an Array of Branches def branches - gitaly_migrate(:branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.branches - else - branches_filter - end + wrapped_gitaly_errors do + gitaly_ref_client.branches end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 9cc7a0d79dc..9de7724e34e 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1344,24 +1344,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - # With Gitaly enabled, Gitaly just doesn't return deleted branches. - context 'with deleted branch with Gitaly disabled' do - before do - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) - end - - it 'returns no results' do - ref = double() - allow(ref).to receive(:name) { 'bad-branch' } - allow(ref).to receive(:target) { raise Rugged::ReferenceError } - branches = double() - allow(branches).to receive(:each) { [ref].each } - allow(repository_rugged).to receive(:branches) { branches } - - expect(subject).to be_empty - end - end - it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :branches end -- cgit v1.2.1 From 3ed4a1b3aadd50b47181312b49911d2a770c4c82 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Jun 2018 10:31:24 +0200 Subject: HasLocalBranches check is done by Gitaly only Closes https://gitlab.com/gitlab-org/gitaly/issues/217 --- lib/gitlab/git/repository.rb | 20 ++------------------ spec/lib/gitlab/git/repository_spec.rb | 10 +--------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 4410b81ed9a..dfdd151aaba 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -235,18 +235,6 @@ module Gitlab # This refs by default not visible in project page and not cloned to client side. alias_method :has_visible_content?, :has_local_branches? - def has_local_branches_rugged? - rugged.branches.each(:local).any? do |ref| - begin - ref.name && ref.target # ensures the branch is valid - - true - rescue Rugged::ReferenceError - false - end - end - end - # Returns the number of valid tags def tag_count gitaly_migrate(:tag_names) do |is_enabled| @@ -1573,12 +1561,8 @@ module Gitlab private def uncached_has_local_branches? - gitaly_migrate(:has_local_branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_repository_client.has_local_branches? - else - has_local_branches_rugged? - end + wrapped_gitaly_errors do + gitaly_repository_client.has_local_branches? end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 9de7724e34e..5bae99101e6 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -425,7 +425,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#has_local_branches?' do - shared_examples 'check for local branches' do + context 'check for local branches' do it { expect(repository.has_local_branches?).to eq(true) } context 'mutable' do @@ -459,14 +459,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end end - - context 'with gitaly' do - it_behaves_like 'check for local branches' - end - - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like 'check for local branches' - end end describe "#delete_branch" do -- cgit v1.2.1 From 15f0cef1d026b161ecf6ffe5021e28f95cbd69f3 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 14 Jun 2018 10:44:33 +0200 Subject: Local branches go through Gitaly Closes https://gitlab.com/gitlab-org/gitaly/issues/217 --- lib/gitlab/git/repository.rb | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index dfdd151aaba..eb5d6318dcb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -190,12 +190,8 @@ module Gitlab end def local_branches(sort_by: nil) - gitaly_migrate(:local_branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.local_branches(sort_by: sort_by) - else - branches_filter(filter: :local, sort_by: sort_by) - end + wrapped_gitaly_errors do + gitaly_ref_client.local_branches(sort_by: sort_by) end end @@ -1682,20 +1678,6 @@ module Gitlab } end - # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'. - def branches_filter(filter: nil, sort_by: nil) - branches = rugged.branches.each(filter).map do |rugged_ref| - begin - target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target) - Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit) - rescue Rugged::ReferenceError - # Omit invalid branch - end - end.compact - - sort_branches(branches, sort_by) - end - def git_merged_branch_names(branch_names, root_sha) git_arguments = %W[branch --merged #{root_sha} -- cgit v1.2.1