From 7354c9f75dfbf4c495d2869b5dd4f0dd4f5c9612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 20 Dec 2017 16:54:09 -0300 Subject: Remove unused method `remote_exists?` --- app/models/repository.rb | 4 ---- lib/gitlab/git/repository.rb | 5 ----- spec/lib/gitlab/git/repository_spec.rb | 15 --------------- 3 files changed, 24 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index a34f5e5439b..b1fd981965c 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1010,10 +1010,6 @@ class Repository raw_repository.fetch_source_branch!(source_repository.raw_repository, source_branch, local_ref) end - def remote_exists?(name) - raw_repository.remote_exists?(name) - end - def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:) raw_repository.compare_source_branch(target_branch_name, source_repository.raw_repository, source_branch_name, straight: straight) end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 044c60caa05..4fc37b63085 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -940,11 +940,6 @@ module Gitlab false end - # Returns true if a remote exists. - def remote_exists?(name) - rugged.remotes[name].present? - end - # Update the specified remote using the values in the +options+ hash # # Example diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 03a9cc488ca..ad16f9af0f1 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -701,21 +701,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#remote_exists?' do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - @repo.add_remote("new_remote", SeedHelper::GITLAB_GIT_TEST_REPO_URL) - end - - it 'returns true for an existing remote' do - expect(@repo.remote_exists?('new_remote')).to eq(true) - end - - it 'returns false for a non-existing remote' do - expect(@repo.remote_exists?('foo')).to eq(false) - end - end - describe "#log" do let(:commit_with_old_name) do Gitlab::Git::Commit.decorate(repository, @commit_with_old_name_id) -- cgit v1.2.1 From 2694355bb6f6bf174b42127db35aa2c912501a9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 20 Dec 2017 17:17:28 -0300 Subject: Incorporate Gitaly's RemoteService RPCs --- GITALY_SERVER_VERSION | 2 +- Gemfile | 2 +- Gemfile.lock | 4 +- lib/gitlab/git/repository.rb | 56 +++++++++++++++------- lib/gitlab/gitaly_client/remote_service.rb | 28 +++++++++++ spec/lib/gitlab/git/repository_spec.rb | 56 ++++++++++++++++++++++ .../gitlab/gitaly_client/remote_service_spec.rb | 34 +++++++++++++ 7 files changed, 162 insertions(+), 20 deletions(-) create mode 100644 lib/gitlab/gitaly_client/remote_service.rb create mode 100644 spec/lib/gitlab/gitaly_client/remote_service_spec.rb diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 7e750b4ebf3..d4f16f06004 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.60.0 +0.64.0 diff --git a/Gemfile b/Gemfile index b6ffaf80f24..a9a50ec084c 100644 --- a/Gemfile +++ b/Gemfile @@ -400,7 +400,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.61.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.62.0', require: 'gitaly' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index a6e3c9e27cc..64f8c955391 100644 --- a/Gemfile.lock +++ b/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.61.0) + gitaly-proto (0.62.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.61.0) + gitaly-proto (~> 0.62.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.6.2) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 4fc37b63085..3ddd8fbe873 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -918,26 +918,23 @@ module Gitlab # If `mirror_refmap` is present the remote is set as mirror with that mapping def add_remote(remote_name, url, mirror_refmap: nil) - rugged.remotes.create(remote_name, url) - - set_remote_as_mirror(remote_name, refmap: mirror_refmap) if mirror_refmap - rescue Rugged::ConfigError - remote_update(remote_name, url: url) + gitaly_migrate(:operation_user_add_tag) do |is_enabled| + if is_enabled + gitaly_remote_client.add_remote(remote_name, url, mirror_refmap) + else + rugged_add_remote(remote_name, url, mirror_refmap) + end + end end def remove_remote(remote_name) - # When a remote is deleted all its remote refs are deleted too, but in - # the case of mirrors we map its refs (that would usualy go under - # [remote_name]/) to the top level namespace. We clean the mapping so - # those don't get deleted. - if rugged.config["remote.#{remote_name}.mirror"] - rugged.config.delete("remote.#{remote_name}.fetch") + gitaly_migrate(:operation_user_add_tag) do |is_enabled| + if is_enabled + gitaly_remote_client.remove_remote(remote_name) + else + rugged_remove_remote(remote_name) + end end - - rugged.remotes.delete(remote_name) - true - rescue Rugged::ConfigError - false end # Update the specified remote using the values in the +options+ hash @@ -1292,6 +1289,10 @@ module Gitlab @gitaly_operation_client ||= Gitlab::GitalyClient::OperationService.new(self) end + def gitaly_remote_client + @gitaly_remote_client ||= Gitlab::GitalyClient::RemoteService.new(self) + end + def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block) Gitlab::GitalyClient.migrate(method, status: status, &block) rescue GRPC::NotFound => e @@ -1911,6 +1912,29 @@ module Gitlab raise ArgumentError, 'Invalid merge source' end + def rugged_add_remote(remote_name, url, mirror_refmap) + rugged.remotes.create(remote_name, url) + + set_remote_as_mirror(remote_name, refmap: mirror_refmap) if mirror_refmap + rescue Rugged::ConfigError + remote_update(remote_name, url: url) + end + + def rugged_remove_remote(remote_name) + # When a remote is deleted all its remote refs are deleted too, but in + # the case of mirrors we map its refs (that would usualy go under + # [remote_name]/) to the top level namespace. We clean the mapping so + # those don't get deleted. + if rugged.config["remote.#{remote_name}.mirror"] + rugged.config.delete("remote.#{remote_name}.fetch") + end + + rugged.remotes.delete(remote_name) + true + rescue Rugged::ConfigError + false + end + def fetch_remote(remote_name = 'origin', env: nil) run_git(['fetch', remote_name], env: env).last.zero? end diff --git a/lib/gitlab/gitaly_client/remote_service.rb b/lib/gitlab/gitaly_client/remote_service.rb new file mode 100644 index 00000000000..9218f6cfd68 --- /dev/null +++ b/lib/gitlab/gitaly_client/remote_service.rb @@ -0,0 +1,28 @@ +module Gitlab + module GitalyClient + class RemoteService + def initialize(repository) + @repository = repository + @gitaly_repo = repository.gitaly_repository + @storage = repository.storage + end + + def add_remote(name, url, mirror_refmap) + request = Gitaly::AddRemoteRequest.new( + repository: @gitaly_repo, name: name, url: url, + mirror_refmap: mirror_refmap.to_s + ) + + GitalyClient.call(@storage, :remote_service, :add_remote, request) + end + + def remove_remote(name) + request = Gitaly::RemoveRemoteRequest.new(repository: @gitaly_repo, name: name) + + response = GitalyClient.call(@storage, :remote_service, :remove_remote, request) + + response.result + end + end + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index ad16f9af0f1..cf14f7d2207 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1828,6 +1828,62 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe 'remotes' do + let(:repository) do + Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') + end + let(:remote_name) { 'my-remote' } + + after do + ensure_seeds + end + + describe '#add_remote' do + let(:url) { 'http://my-repo.git' } + let(:mirror_refmap) { '+refs/*:refs/*' } + + it 'creates a new remote via Gitaly' do + expect_any_instance_of(Gitlab::GitalyClient::RemoteService) + .to receive(:add_remote).with(remote_name, url, mirror_refmap) + + repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) + end + + context 'with Gitaly disabled', :skip_gitaly_mock do + it 'creates a new remote via Rugged' do + expect_any_instance_of(Rugged::RemoteCollection).to receive(:create) + .with(remote_name, url) + expect_any_instance_of(Rugged::Config).to receive(:[]=) + .with("remote.#{remote_name}.mirror", true) + expect_any_instance_of(Rugged::Config).to receive(:[]=) + .with("remote.#{remote_name}.prune", true) + expect_any_instance_of(Rugged::Config).to receive(:[]=) + .with("remote.#{remote_name}.fetch", mirror_refmap) + + repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) + end + end + end + + describe '#remove_remote' do + it 'removes the remote via Gitaly' do + expect_any_instance_of(Gitlab::GitalyClient::RemoteService) + .to receive(:remove_remote).with(remote_name) + + repository.remove_remote(remote_name) + end + + context 'with Gitaly disabled', :skip_gitaly_mock do + it 'removes the remote via Rugged' do + expect_any_instance_of(Rugged::RemoteCollection).to receive(:delete) + .with(remote_name) + + repository.remove_remote(remote_name) + end + end + end + end + def create_remote_branch(repository, remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } rugged = repository.rugged diff --git a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb new file mode 100644 index 00000000000..69c6f054016 --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::RemoteService do + let(:project) { create(:project) } + let(:storage_name) { project.repository_storage } + let(:relative_path) { project.disk_path + '.git' } + let(:remote_name) { 'my-remote' } + let(:client) { described_class.new(project.repository) } + + describe '#add_remote' do + let(:url) { 'http://my-repo.git' } + let(:mirror_refmap) { :all_refs } + + it 'sends an add_remote message' do + expect_any_instance_of(Gitaly::RemoteService::Stub) + .to receive(:add_remote) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(:add_remote_response)) + + client.add_remote(remote_name, url, mirror_refmap) + end + end + + describe '#remove_remote' do + it 'sends an remove_remote message and returns the result value' do + expect_any_instance_of(Gitaly::RemoteService::Stub) + .to receive(:remove_remote) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(result: true)) + + expect(client.remove_remote(remote_name)).to be(true) + end + end +end -- cgit v1.2.1