Prepare GitOperationService for move to Gitlab::Git

This commit is contained in:
Jacob Vosmaer 2017-08-23 17:14:21 +02:00
parent 3e092caa91
commit 8ad690b0d4
8 changed files with 179 additions and 152 deletions

View file

@ -949,13 +949,6 @@ class MergeRequest < ActiveRecord::Base
private
def write_ref
target_project.repository.with_repo_branch_commit(
source_project.repository, source_branch) do |commit|
if commit
target_project.repository.write_ref(ref_path, commit.sha)
else
raise Rugged::ReferenceError, 'source repository is empty'
end
end
target_project.repository.fetch_source_branch(source_project.repository, source_branch, ref_path)
end
end

View file

@ -95,19 +95,6 @@ class Repository
"#<#{self.class.name}:#{@disk_path}>"
end
#
# Git repository can contains some hidden refs like:
# /refs/notes/*
# /refs/git-as-svn/*
# /refs/pulls/*
# This refs by default not visible in project page and not cloned to client side.
#
# This method return true if repository contains some content visible in project page.
#
def has_visible_content?
branch_count > 0
end
def commit(ref = 'HEAD')
return nil unless exists?
@ -184,7 +171,7 @@ class Repository
return false unless newrev
GitOperationService.new(user, self).add_branch(branch_name, newrev)
GitOperationService.new(user, raw_repository).add_branch(branch_name, newrev)
after_create_branch
find_branch(branch_name)
@ -196,7 +183,7 @@ class Repository
return false unless newrev
GitOperationService.new(user, self).add_tag(tag_name, newrev, options)
GitOperationService.new(user, raw_repository).add_tag(tag_name, newrev, options)
find_tag(tag_name)
end
@ -205,7 +192,7 @@ class Repository
before_remove_branch
branch = find_branch(branch_name)
GitOperationService.new(user, self).rm_branch(branch)
GitOperationService.new(user, raw_repository).rm_branch(branch)
after_remove_branch
true
@ -215,7 +202,7 @@ class Repository
before_remove_tag
tag = find_tag(tag_name)
GitOperationService.new(user, self).rm_tag(tag)
GitOperationService.new(user, raw_repository).rm_tag(tag)
after_remove_tag
true
@ -784,16 +771,30 @@ class Repository
multi_action(**options)
end
def with_branch(user, *args)
result = GitOperationService.new(user, raw_repository).with_branch(*args) do |start_commit|
yield start_commit
end
newrev, should_run_after_create, should_run_after_create_branch = result
after_create if should_run_after_create
after_create_branch if should_run_after_create_branch
newrev
end
# rubocop:disable Metrics/ParameterLists
def multi_action(
user:, branch_name:, message:, actions:,
author_email: nil, author_name: nil,
start_branch_name: nil, start_project: project)
GitOperationService.new(user, self).with_branch(
with_branch(
user,
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
start_repository: start_project.repository.raw_repository) do |start_commit|
index = Gitlab::Git::Index.new(raw_repository)
@ -846,7 +847,8 @@ class Repository
end
def merge(user, source, merge_request, options = {})
GitOperationService.new(user, self).with_branch(
with_branch(
user,
merge_request.target_branch) do |start_commit|
our_commit = start_commit.sha
their_commit = source
@ -873,10 +875,11 @@ class Repository
def revert(
user, commit, branch_name,
start_branch_name: nil, start_project: project)
GitOperationService.new(user, self).with_branch(
with_branch(
user,
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
start_repository: start_project.repository.raw_repository) do |start_commit|
revert_tree_id = check_revert_content(commit, start_commit.sha)
unless revert_tree_id
@ -896,10 +899,11 @@ class Repository
def cherry_pick(
user, commit, branch_name,
start_branch_name: nil, start_project: project)
GitOperationService.new(user, self).with_branch(
with_branch(
user,
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
start_repository: start_project.repository.raw_repository) do |start_commit|
cherry_pick_tree_id = check_cherry_pick_content(commit, start_commit.sha)
unless cherry_pick_tree_id
@ -921,7 +925,7 @@ class Repository
end
def resolve_conflicts(user, branch_name, params)
GitOperationService.new(user, self).with_branch(branch_name) do
with_branch(user, branch_name) do
committer = user_to_committer(user)
create_commit(params.merge(author: committer, committer: committer))
@ -1011,25 +1015,6 @@ class Repository
run_git(args).first.lines.map(&:strip)
end
def with_repo_branch_commit(start_repository, start_branch_name)
return yield nil if start_repository.empty_repo?
if start_repository == self
yield commit(start_branch_name)
else
sha = start_repository.commit(start_branch_name).sha
if branch_commit = commit(sha)
yield branch_commit
else
with_repo_tmp_commit(
start_repository, start_branch_name, sha) do |tmp_commit|
yield tmp_commit
end
end
end
end
def add_remote(name, url)
raw_repository.remote_add(name, url)
rescue Rugged::ConfigError
@ -1047,14 +1032,12 @@ class Repository
gitlab_shell.fetch_remote(repository_storage_path, disk_path, remote, forced: forced, no_tags: no_tags)
end
def fetch_ref(source_path, source_ref, target_ref)
args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
message, status = run_git(args)
def fetch_source_branch(source_repository, source_branch, local_ref)
raw_repository.fetch_source_branch(source_repository.raw_repository, source_branch, local_ref)
end
# Make sure ref was created, and raise Rugged::ReferenceError when not
raise Rugged::ReferenceError, message if status != 0
target_ref
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
def create_ref(ref, ref_path)
@ -1135,12 +1118,6 @@ class Repository
private
def run_git(args)
circuit_breaker.perform do
Gitlab::Popen.popen([Gitlab.config.git.bin_path, *args], path_to_repo)
end
end
def blob_data_at(sha, path)
blob = blob_at(sha, path)
return unless blob
@ -1236,16 +1213,4 @@ class Repository
.commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
.map { |c| commit(c) }
end
def with_repo_tmp_commit(start_repository, start_branch_name, sha)
tmp_ref = fetch_ref(
start_repository.path_to_repo,
"#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}",
"refs/tmp/#{SecureRandom.hex}/head"
)
yield commit(sha)
ensure
delete_refs(tmp_ref) if tmp_ref
end
end

View file

@ -11,26 +11,8 @@ class CompareService
end
def execute(target_project, target_branch, straight: false)
# If compare with other project we need to fetch ref first
target_project.repository.with_repo_branch_commit(
start_project.repository,
start_branch_name) do |commit|
break unless commit
raw_compare = target_project.repository.compare_source_branch(target_branch, start_project.repository, start_branch_name, straight: straight)
compare(commit.sha, target_project, target_branch, straight: straight)
end
end
private
def compare(source_sha, target_project, target_branch, straight:)
raw_compare = Gitlab::Git::Compare.new(
target_project.repository.raw_repository,
target_branch,
source_sha,
straight: straight
)
Compare.new(raw_compare, target_project, straight: straight)
Compare.new(raw_compare, target_project, straight: straight) if raw_compare
end
end

View file

@ -5,6 +5,11 @@ class GitOperationService
committer = Gitlab::Git::Committer.from_user(committer) if committer.is_a?(User)
@committer = committer
# Refactoring aid
unless new_repository.is_a?(Gitlab::Git::Repository)
raise "expected a Gitlab::Git::Repository, got #{new_repository}"
end
@repository = new_repository
end
@ -55,10 +60,14 @@ class GitOperationService
def with_branch(
branch_name,
start_branch_name: nil,
start_project: repository.project,
start_repository: repository,
&block)
start_repository = start_project.repository
# Refactoring aid
unless start_repository.is_a?(Gitlab::Git::Repository)
raise "expected a Gitlab::Git::Repository, got #{start_repository}"
end
start_branch_name = nil if start_repository.empty_repo?
if start_branch_name && !start_repository.branch_exists?(start_branch_name)
@ -75,6 +84,7 @@ class GitOperationService
private
# Returns [newrev, should_run_after_create, should_run_after_create_branch]
def update_branch_with_hooks(branch_name)
update_autocrlf_option
@ -93,12 +103,7 @@ class GitOperationService
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
update_ref_in_hooks(ref, newrev, oldrev)
# If repo was empty expire cache
repository.after_create if was_empty
repository.after_create_branch if
was_empty || Gitlab::Git.blank_ref?(oldrev)
newrev
[newrev, was_empty, was_empty || Gitlab::Git.blank_ref?(oldrev)]
end
def find_oldrev_from_branch(newrev, branch)
@ -140,7 +145,7 @@ class GitOperationService
command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
_, status = Gitlab::Popen.popen(
command,
repository.path_to_repo) do |stdin|
repository.path) do |stdin|
stdin.write("update #{ref}\x00#{newrev}\x00#{oldrev}\x00")
end
@ -152,8 +157,8 @@ class GitOperationService
end
def update_autocrlf_option
if repository.raw_repository.autocrlf != :input
repository.raw_repository.autocrlf = :input
if repository.autocrlf != :input
repository.autocrlf = :input
end
end
end

View file

@ -18,7 +18,7 @@ module Gitlab
new(merge_request, project).tap do |file_collection|
project
.repository
.with_repo_branch_commit(merge_request.target_project.repository, merge_request.target_branch) do
.with_repo_branch_commit(merge_request.target_project.repository.raw_repository, merge_request.target_branch) do
yield file_collection
end

View file

@ -731,6 +731,106 @@ module Gitlab
end
end
def with_repo_branch_commit(start_repository, start_branch_name)
raise "expected Gitlab::Git::Repository, got #{start_repository}" unless start_repository.is_a?(Gitlab::Git::Repository)
return yield nil if start_repository.empty_repo?
if start_repository == self
yield commit(start_branch_name)
else
sha = start_repository.commit(start_branch_name).sha
if branch_commit = commit(sha)
yield branch_commit
else
with_repo_tmp_commit(
start_repository, start_branch_name, sha) do |tmp_commit|
yield tmp_commit
end
end
end
end
def with_repo_tmp_commit(start_repository, start_branch_name, sha)
tmp_ref = fetch_ref(
start_repository.path,
"#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}",
"refs/tmp/#{SecureRandom.hex}/head"
)
yield commit(sha)
ensure
delete_refs(tmp_ref) if tmp_ref
end
def fetch_source_branch(source_repository, source_branch, local_ref)
with_repo_branch_commit(source_repository, source_branch) do |commit|
if commit
write_ref(local_ref, commit.sha)
else
raise Rugged::ReferenceError, 'source repository is empty'
end
end
end
def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
with_repo_branch_commit(source_repository, source_branch_name) do |commit|
break unless commit
Gitlab::Git::Compare.new(
self,
target_branch_name,
commit.sha,
straight: straight
)
end
end
def write_ref(ref_path, sha)
rugged.references.create(ref_path, sha, force: true)
end
def fetch_ref(source_path, source_ref, target_ref)
args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
message, status = run_git(args)
# Make sure ref was created, and raise Rugged::ReferenceError when not
raise Rugged::ReferenceError, message if status != 0
target_ref
end
# Refactoring aid; allows us to copy code from app/models/repository.rb
def run_git(args)
circuit_breaker.perform do
popen([Gitlab.config.git.bin_path, *args], path)
end
end
# Refactoring aid; allows us to copy code from app/models/repository.rb
def commit(ref = 'HEAD')
Gitlab::Git::Commit.find(self, ref)
end
# Refactoring aid; allows us to copy code from app/models/repository.rb
def empty_repo?
!exists? || !has_visible_content?
end
#
# Git repository can contains some hidden refs like:
# /refs/notes/*
# /refs/git-as-svn/*
# /refs/pulls/*
# This refs by default not visible in project page and not cloned to client side.
#
# This method return true if repository contains some content visible in project page.
#
def has_visible_content?
branch_count > 0
end
def gitaly_repository
Gitlab::GitalyClient::Util.repository(@storage, @relative_path)
end

View file

@ -886,7 +886,7 @@ describe Repository, models: true do
context 'when pre hooks were successful' do
it 'runs without errors' do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:execute)
.with(committer, repository, old_rev, blank_sha, 'refs/heads/feature')
.with(committer, repository.raw_repository, old_rev, blank_sha, 'refs/heads/feature')
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
end
@ -932,20 +932,20 @@ describe Repository, models: true do
service = Gitlab::Git::HooksService.new
expect(Gitlab::Git::HooksService).to receive(:new).and_return(service)
expect(service).to receive(:execute)
.with(committer, target_repository, old_rev, new_rev, updating_ref)
.with(committer, target_repository.raw_repository, old_rev, new_rev, updating_ref)
.and_yield(service).and_return(true)
end
it 'runs without errors' do
expect do
GitOperationService.new(committer, repository).with_branch('feature') do
GitOperationService.new(committer, repository.raw_repository).with_branch('feature') do
new_rev
end
end.not_to raise_error
end
it 'ensures the autocrlf Git option is set to :input' do
service = GitOperationService.new(committer, repository)
service = GitOperationService.new(committer, repository.raw_repository)
expect(service).to receive(:update_autocrlf_option)
@ -956,7 +956,7 @@ describe Repository, models: true do
it 'updates the head' do
expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev)
GitOperationService.new(committer, repository).with_branch('feature') do
GitOperationService.new(committer, repository.raw_repository).with_branch('feature') do
new_rev
end
@ -971,13 +971,13 @@ describe Repository, models: true do
let(:updating_ref) { 'refs/heads/master' }
it 'fetch_ref and create the branch' do
expect(target_project.repository).to receive(:fetch_ref)
expect(target_project.repository.raw_repository).to receive(:fetch_ref)
.and_call_original
GitOperationService.new(committer, target_repository)
GitOperationService.new(committer, target_repository.raw_repository)
.with_branch(
'master',
start_project: project,
start_repository: project.repository.raw_repository,
start_branch_name: 'feature') { new_rev }
expect(target_repository.branch_names).to contain_exactly('master')
@ -990,8 +990,8 @@ describe Repository, models: true do
it 'does not fetch_ref and just pass the commit' do
expect(target_repository).not_to receive(:fetch_ref)
GitOperationService.new(committer, target_repository)
.with_branch('feature', start_project: project) { new_rev }
GitOperationService.new(committer, target_repository.raw_repository)
.with_branch('feature', start_repository: project.repository.raw_repository) { new_rev }
end
end
end
@ -1000,7 +1000,7 @@ describe Repository, models: true do
let(:target_project) { create(:project, :empty_repo) }
before do
expect(target_project.repository).to receive(:run_git)
expect(target_project.repository.raw_repository).to receive(:run_git)
end
it 'raises Rugged::ReferenceError' do
@ -1009,9 +1009,9 @@ describe Repository, models: true do
end
expect do
GitOperationService.new(committer, target_project.repository)
GitOperationService.new(committer, target_project.repository.raw_repository)
.with_branch('feature',
start_project: project,
start_repository: project.repository.raw_repository,
&:itself)
end.to raise_reference_error
end
@ -1031,7 +1031,7 @@ describe Repository, models: true do
repository.add_branch(user, branch, old_rev)
expect do
GitOperationService.new(committer, repository).with_branch(branch) do
GitOperationService.new(committer, repository.raw_repository).with_branch(branch) do
new_rev
end
end.not_to raise_error
@ -1049,7 +1049,7 @@ describe Repository, models: true do
# Updating 'master' to new_rev would lose the commits on 'master' that
# are not contained in new_rev. This should not be allowed.
expect do
GitOperationService.new(committer, repository).with_branch(branch) do
GitOperationService.new(committer, repository.raw_repository).with_branch(branch) do
new_rev
end
end.to raise_error(Repository::CommitError)
@ -1061,31 +1061,13 @@ describe Repository, models: true do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
GitOperationService.new(committer, repository).with_branch('feature') do
GitOperationService.new(committer, repository.raw_repository).with_branch('feature') do
new_rev
end
end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
end
end
context 'when target branch is different from source branch' do
before do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, ''])
end
it 'expires branch cache' do
expect(repository).not_to receive(:expire_exists_cache)
expect(repository).not_to receive(:expire_root_ref_cache)
expect(repository).not_to receive(:expire_emptiness_caches)
expect(repository).to receive(:expire_branches_cache)
GitOperationService.new(committer, repository)
.with_branch('new-feature') do
new_rev
end
end
end
context 'when repository is empty' do
before do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, ''])
@ -1139,7 +1121,7 @@ describe Repository, models: true do
describe 'when there are no branches' do
before do
allow(repository).to receive(:branch_count).and_return(0)
allow(repository.raw_repository).to receive(:branch_count).and_return(0)
end
it { is_expected.to eq(false) }
@ -1147,7 +1129,7 @@ describe Repository, models: true do
describe 'when there are branches' do
it 'returns true' do
expect(repository).to receive(:branch_count).and_return(3)
expect(repository.raw_repository).to receive(:branch_count).and_return(3)
expect(subject).to eq(true)
end
@ -1161,7 +1143,7 @@ describe Repository, models: true do
end
it 'sets autocrlf to :input' do
GitOperationService.new(nil, repository).send(:update_autocrlf_option)
GitOperationService.new(nil, repository.raw_repository).send(:update_autocrlf_option)
expect(repository.raw_repository.autocrlf).to eq(:input)
end
@ -1176,7 +1158,7 @@ describe Repository, models: true do
expect(repository.raw_repository).not_to receive(:autocrlf=)
.with(:input)
GitOperationService.new(nil, repository).send(:update_autocrlf_option)
GitOperationService.new(nil, repository.raw_repository).send(:update_autocrlf_option)
end
end
end
@ -1759,14 +1741,14 @@ describe Repository, models: true do
describe '#update_ref' do
it 'can create a ref' do
GitOperationService.new(nil, repository).send(:update_ref, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
GitOperationService.new(nil, repository.raw_repository).send(:update_ref, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
expect(repository.find_branch('foobar')).not_to be_nil
end
it 'raises CommitError when the ref update fails' do
expect do
GitOperationService.new(nil, repository).send(:update_ref, 'refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
GitOperationService.new(nil, repository.raw_repository).send(:update_ref, 'refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
end.to raise_error(Repository::CommitError)
end
end

View file

@ -23,8 +23,8 @@ describe GitGarbageCollectWorker do
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(:branch_count).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(:branch_count).and_call_original
expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
subject.perform(project.id)
end
@ -143,7 +143,7 @@ describe GitGarbageCollectWorker do
tree: old_commit.tree,
parents: [old_commit]
)
GitOperationService.new(nil, project.repository).send(
GitOperationService.new(nil, project.repository.raw_repository).send(
:update_ref,
"refs/heads/#{SecureRandom.hex(6)}",
new_commit_sha,