summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacob Vosmaer (GitLab) <jacob@gitlab.com>2017-11-10 14:45:23 +0000
committerSean McGivern <sean@mcgivern.me.uk>2017-11-10 14:45:23 +0000
commitde301d13bb4f6d61cdaebfe186be6012c2f2096d (patch)
tree30aa61a630e1b18f139dcdf05c933eb7012adf45
parent65faebb95556809c5858347f1c24422505056827 (diff)
downloadgitlab-ce-de301d13bb4f6d61cdaebfe186be6012c2f2096d.tar.gz
Prepare Repository#fetch_source_branch for migration
-rw-r--r--app/models/repository.rb4
-rw-r--r--lib/gitlab/git/operation_service.rb10
-rw-r--r--lib/gitlab/git/remote_repository.rb82
-rw-r--r--lib/gitlab/git/repository.rb34
-rw-r--r--spec/lib/gitlab/git/remote_repository_spec.rb99
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb70
6 files changed, 243 insertions, 56 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 6bdee538172..3a89fa9264b 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1062,6 +1062,10 @@ class Repository
blob_data_at(sha, path)
end
+ def fetch_ref(source_repository, source_ref:, target_ref:)
+ raw_repository.fetch_ref(source_repository.raw_repository, source_ref: source_ref, target_ref: target_ref)
+ end
+
private
# TODO Generice finder, later split this on finders by Ref or Oid
diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb
index ab94ba8a73a..e36d5410431 100644
--- a/lib/gitlab/git/operation_service.rb
+++ b/lib/gitlab/git/operation_service.rb
@@ -72,7 +72,7 @@ module Gitlab
# Whenever `start_branch_name` is passed, if `branch_name` doesn't exist,
# it would be created from `start_branch_name`.
- # If `start_project` is passed, and the branch doesn't exist,
+ # If `start_repository` is passed, and the branch doesn't exist,
# it would try to find the commits from it instead of current repository.
def with_branch(
branch_name,
@@ -80,15 +80,13 @@ module Gitlab
start_repository: repository,
&block)
- # Refactoring aid
- unless start_repository.is_a?(Gitlab::Git::Repository)
- raise "expected a Gitlab::Git::Repository, got #{start_repository}"
- end
+ Gitlab::Git.check_namespace!(start_repository)
+ start_repository = RemoteRepository.new(start_repository) unless start_repository.is_a?(RemoteRepository)
start_branch_name = nil if start_repository.empty_repo?
if start_branch_name && !start_repository.branch_exists?(start_branch_name)
- raise ArgumentError, "Cannot find branch #{start_branch_name} in #{start_repository.full_path}"
+ raise ArgumentError, "Cannot find branch #{start_branch_name} in #{start_repository.relative_path}"
end
update_branch_with_hooks(branch_name) do
diff --git a/lib/gitlab/git/remote_repository.rb b/lib/gitlab/git/remote_repository.rb
new file mode 100644
index 00000000000..3685aa20669
--- /dev/null
+++ b/lib/gitlab/git/remote_repository.rb
@@ -0,0 +1,82 @@
+module Gitlab
+ module Git
+ #
+ # When a Gitaly call involves two repositories instead of one we cannot
+ # assume that both repositories are on the same Gitaly server. In this
+ # case we need to make a distinction between the repository that the
+ # call is being made on (a Repository instance), and the "other"
+ # repository (a RemoteRepository instance). This is the reason why we
+ # have the RemoteRepository class in Gitlab::Git.
+ #
+ # When you make changes, be aware that gitaly-ruby sub-classes this
+ # class.
+ #
+ class RemoteRepository
+ attr_reader :path, :relative_path, :gitaly_repository
+
+ def initialize(repository)
+ @relative_path = repository.relative_path
+ @gitaly_repository = repository.gitaly_repository
+
+ # These instance variables will not be available in gitaly-ruby, where
+ # we have no disk access to this repository.
+ @repository = repository
+ @path = repository.path
+ end
+
+ def empty_repo?
+ # We will override this implementation in gitaly-ruby because we cannot
+ # use '@repository' there.
+ @repository.empty_repo?
+ end
+
+ def commit_id(revision)
+ # We will override this implementation in gitaly-ruby because we cannot
+ # use '@repository' there.
+ @repository.commit(revision)&.sha
+ end
+
+ def branch_exists?(name)
+ # We will override this implementation in gitaly-ruby because we cannot
+ # use '@repository' there.
+ @repository.branch_exists?(name)
+ end
+
+ # Compares self to a Gitlab::Git::Repository. This implementation uses
+ # 'self.gitaly_repository' so that it will also work in the
+ # GitalyRemoteRepository subclass defined in gitaly-ruby.
+ def same_repository?(other_repository)
+ gitaly_repository.storage_name == other_repository.storage &&
+ gitaly_repository.relative_path == other_repository.relative_path
+ end
+
+ def fetch_env
+ gitaly_ssh = File.absolute_path(File.join(Gitlab.config.gitaly.client_path, 'gitaly-ssh'))
+ gitaly_address = gitaly_client.address(storage)
+ gitaly_token = gitaly_client.token(storage)
+
+ request = Gitaly::SSHUploadPackRequest.new(repository: gitaly_repository)
+ env = {
+ 'GITALY_ADDRESS' => gitaly_address,
+ 'GITALY_PAYLOAD' => request.to_json,
+ 'GITALY_WD' => Dir.pwd,
+ 'GIT_SSH_COMMAND' => "#{gitaly_ssh} upload-pack"
+ }
+ env['GITALY_TOKEN'] = gitaly_token if gitaly_token.present?
+
+ env
+ end
+
+ private
+
+ # Must return an object that responds to 'address' and 'storage'.
+ def gitaly_client
+ Gitlab::GitalyClient
+ end
+
+ def storage
+ gitaly_repository.storage_name
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index d236e1b03e6..14a8d551524 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -58,7 +58,7 @@ module Gitlab
# Rugged repo object
attr_reader :rugged
- attr_reader :storage, :gl_repository, :relative_path, :gitaly_resolver
+ attr_reader :storage, :gl_repository, :relative_path
# This initializer method is only used on the client side (gitlab-ce).
# Gitaly-ruby uses a different initializer.
@@ -66,7 +66,6 @@ module Gitlab
@storage = storage
@relative_path = relative_path
@gl_repository = gl_repository
- @gitaly_resolver = Gitlab::GitalyClient
storage_path = Gitlab.config.repositories.storages[@storage]['path']
@path = File.join(storage_path, @relative_path)
@@ -1014,23 +1013,22 @@ module Gitlab
def with_repo_branch_commit(start_repository, start_branch_name)
Gitlab::Git.check_namespace!(start_repository)
+ start_repository = RemoteRepository.new(start_repository) unless start_repository.is_a?(RemoteRepository)
return yield nil if start_repository.empty_repo?
- if start_repository == self
+ if start_repository.same_repository?(self)
yield commit(start_branch_name)
else
- start_commit = start_repository.commit(start_branch_name)
+ start_commit_id = start_repository.commit_id(start_branch_name)
- return yield nil unless start_commit
+ return yield nil unless start_commit_id
- sha = start_commit.sha
-
- if branch_commit = commit(sha)
+ if branch_commit = commit(start_commit_id)
yield branch_commit
else
with_repo_tmp_commit(
- start_repository, start_branch_name, sha) do |tmp_commit|
+ start_repository, start_branch_name, start_commit_id) do |tmp_commit|
yield tmp_commit
end
end
@@ -1087,6 +1085,9 @@ module Gitlab
end
def fetch_ref(source_repository, source_ref:, target_ref:)
+ Gitlab::Git.check_namespace!(source_repository)
+ source_repository = RemoteRepository.new(source_repository) unless source_repository.is_a?(RemoteRepository)
+
message, status = GitalyClient.migrate(:fetch_ref) do |is_enabled|
if is_enabled
gitaly_fetch_ref(source_repository, source_ref: source_ref, target_ref: target_ref)
@@ -1620,22 +1621,9 @@ module Gitlab
end
def gitaly_fetch_ref(source_repository, source_ref:, target_ref:)
- gitaly_ssh = File.absolute_path(File.join(Gitlab.config.gitaly.client_path, 'gitaly-ssh'))
- gitaly_address = gitaly_resolver.address(source_repository.storage)
- gitaly_token = gitaly_resolver.token(source_repository.storage)
-
- request = Gitaly::SSHUploadPackRequest.new(repository: source_repository.gitaly_repository)
- env = {
- 'GITALY_ADDRESS' => gitaly_address,
- 'GITALY_PAYLOAD' => request.to_json,
- 'GITALY_WD' => Dir.pwd,
- 'GIT_SSH_COMMAND' => "#{gitaly_ssh} upload-pack"
- }
- env['GITALY_TOKEN'] = gitaly_token if gitaly_token.present?
-
args = %W(fetch --no-tags -f ssh://gitaly/internal.git #{source_ref}:#{target_ref})
- run_git(args, env: env)
+ run_git(args, env: source_repository.fetch_env)
end
def gitaly_ff_merge(user, source_sha, target_branch)
diff --git a/spec/lib/gitlab/git/remote_repository_spec.rb b/spec/lib/gitlab/git/remote_repository_spec.rb
new file mode 100644
index 00000000000..0506210887c
--- /dev/null
+++ b/spec/lib/gitlab/git/remote_repository_spec.rb
@@ -0,0 +1,99 @@
+require 'spec_helper'
+
+describe Gitlab::Git::RemoteRepository, seed_helper: true do
+ let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
+ subject { described_class.new(repository) }
+
+ describe '#empty_repo?' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:repository, :result) do
+ Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') | false
+ Gitlab::Git::Repository.new('default', 'does-not-exist.git', '') | true
+ end
+
+ with_them do
+ it { expect(subject.empty_repo?).to eq(result) }
+ end
+ end
+
+ describe '#commit_id' do
+ it 'returns an OID if the revision exists' do
+ expect(subject.commit_id('v1.0.0')).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
+ end
+
+ it 'is nil when the revision does not exist' do
+ expect(subject.commit_id('does-not-exist')).to be_nil
+ end
+ end
+
+ describe '#branch_exists?' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:branch, :result) do
+ 'master' | true
+ 'does-not-exist' | false
+ end
+
+ with_them do
+ it { expect(subject.branch_exists?(branch)).to eq(result) }
+ end
+ end
+
+ describe '#same_repository?' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:other_repository, :result) do
+ repository | true
+ Gitlab::Git::Repository.new(repository.storage, repository.relative_path, '') | true
+ Gitlab::Git::Repository.new('broken', TEST_REPO_PATH, '') | false
+ Gitlab::Git::Repository.new(repository.storage, 'wrong/relative-path.git', '') | false
+ Gitlab::Git::Repository.new('broken', 'wrong/relative-path.git', '') | false
+ end
+
+ with_them do
+ it { expect(subject.same_repository?(other_repository)).to eq(result) }
+ end
+ end
+
+ describe '#fetch_env' do
+ let(:remote_repository) { described_class.new(repository) }
+
+ let(:gitaly_client) { double(:gitaly_client) }
+ let(:address) { 'fake-address' }
+ let(:token) { 'fake-token' }
+
+ subject { remote_repository.fetch_env }
+
+ before do
+ allow(remote_repository).to receive(:gitaly_client).and_return(gitaly_client)
+
+ expect(gitaly_client).to receive(:address).with(repository.storage).and_return(address)
+ expect(gitaly_client).to receive(:token).with(repository.storage).and_return(token)
+ end
+
+ it { expect(subject).to be_a(Hash) }
+ it { expect(subject['GITALY_ADDRESS']).to eq(address) }
+ it { expect(subject['GITALY_TOKEN']).to eq(token) }
+ it { expect(subject['GITALY_WD']).to eq(Dir.pwd) }
+
+ it 'creates a plausible GIT_SSH_COMMAND' do
+ git_ssh_command = subject['GIT_SSH_COMMAND']
+
+ expect(git_ssh_command).to start_with('/')
+ expect(git_ssh_command).to end_with('/gitaly-ssh upload-pack')
+ end
+
+ it 'creates a plausible GITALY_PAYLOAD' do
+ req = Gitaly::SSHUploadPackRequest.decode_json(subject['GITALY_PAYLOAD'])
+
+ expect(remote_repository.gitaly_repository).to eq(req.repository)
+ end
+
+ context 'when the token is blank' do
+ let(:token) { '' }
+
+ it { expect(subject.keys).not_to include('GITALY_TOKEN') }
+ end
+ end
+end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index ee14b528ec2..5d990b42c24 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -449,7 +449,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
after do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
@@ -484,7 +483,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
after do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
@@ -544,7 +542,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
after(:all) do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
end
@@ -570,7 +567,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
after(:all) do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
end
@@ -588,7 +584,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
after(:all) do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
end
@@ -1122,7 +1117,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
after do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
@@ -1169,7 +1163,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
after do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
@@ -1419,7 +1412,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
after(:all) do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
@@ -1537,35 +1529,60 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
describe '#fetch_source_branch!' do
- let(:local_ref) { 'refs/merge-requests/1/head' }
+ shared_examples '#fetch_source_branch!' do
+ let(:local_ref) { 'refs/merge-requests/1/head' }
+ let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
+ let(:source_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
- context 'when the branch exists' do
- let(:source_branch) { 'master' }
+ after do
+ ensure_seeds
+ end
- it 'writes the ref' do
- expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/)
+ context 'when the branch exists' do
+ context 'when the commit does not exist locally' do
+ let(:source_branch) { 'new-branch-for-fetch-source-branch' }
+ let(:source_rugged) { source_repository.rugged }
+ let(:new_oid) { new_commit_edit_old_file(source_rugged).oid }
- repository.fetch_source_branch!(repository, source_branch, local_ref)
- end
+ before do
+ source_rugged.branches.create(source_branch, new_oid)
+ end
- it 'returns true' do
- expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(true)
- end
- end
+ it 'writes the ref' do
+ expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(true)
+ expect(repository.commit(local_ref).sha).to eq(new_oid)
+ end
+ end
- context 'when the branch does not exist' do
- let(:source_branch) { 'definitely-not-master' }
+ context 'when the commit exists locally' do
+ let(:source_branch) { 'master' }
+ let(:expected_oid) { SeedRepo::LastCommit::ID }
- it 'does not write the ref' do
- expect(repository).not_to receive(:write_ref)
+ it 'writes the ref' do
+ # Sanity check: the commit should already exist
+ expect(repository.commit(expected_oid)).not_to be_nil
- repository.fetch_source_branch!(repository, source_branch, local_ref)
+ expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(true)
+ expect(repository.commit(local_ref).sha).to eq(expected_oid)
+ end
+ end
end
- it 'returns false' do
- expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(false)
+ context 'when the branch does not exist' do
+ let(:source_branch) { 'definitely-not-master' }
+
+ it 'does not write the ref' do
+ expect(repository.fetch_source_branch!(source_repository, source_branch, local_ref)).to eq(false)
+ expect(repository.commit(local_ref)).to be_nil
+ end
end
end
+
+ it_behaves_like '#fetch_source_branch!'
+
+ context 'without gitaly', :skip_gitaly_mock do
+ it_behaves_like '#fetch_source_branch!'
+ end
end
describe '#rm_branch' do
@@ -1641,7 +1658,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
after do
- FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end