Commit 9d53c15c authored by Sean McGivern's avatar Sean McGivern

Merge branch 'ban-rugged-from-repository' into 'master'

Decouple more of Repository from Rugged

Closes gitaly#969

See merge request gitlab-org/gitlab-ce!16763
parents b886a75d 5c2fe2d3
...@@ -491,7 +491,7 @@ class Repository ...@@ -491,7 +491,7 @@ class Repository
raw_repository.root_ref raw_repository.root_ref
else else
# When the repo does not exist we raise this error so no data is cached. # When the repo does not exist we raise this error so no data is cached.
raise Rugged::ReferenceError raise Gitlab::Git::Repository::NoRepository
end end
end end
cache_method :root_ref cache_method :root_ref
...@@ -525,11 +525,7 @@ class Repository ...@@ -525,11 +525,7 @@ class Repository
def commit_count_for_ref(ref) def commit_count_for_ref(ref)
return 0 unless exists? return 0 unless exists?
begin cache.fetch(:"commit_count_#{ref}") { raw_repository.commit_count(ref) }
cache.fetch(:"commit_count_#{ref}") { raw_repository.commit_count(ref) }
rescue Rugged::ReferenceError
0
end
end end
delegate :branch_names, to: :raw_repository delegate :branch_names, to: :raw_repository
...@@ -653,26 +649,14 @@ class Repository ...@@ -653,26 +649,14 @@ class Repository
end end
def last_commit_for_path(sha, path) def last_commit_for_path(sha, path)
raw_repository.gitaly_migrate(:last_commit_for_path) do |is_enabled| commit_by(oid: last_commit_id_for_path(sha, path))
if is_enabled
last_commit_for_path_by_gitaly(sha, path)
else
last_commit_for_path_by_rugged(sha, path)
end
end
end end
def last_commit_id_for_path(sha, path) def last_commit_id_for_path(sha, path)
key = path.blank? ? "last_commit_id_for_path:#{sha}" : "last_commit_id_for_path:#{sha}:#{Digest::SHA1.hexdigest(path)}" key = path.blank? ? "last_commit_id_for_path:#{sha}" : "last_commit_id_for_path:#{sha}:#{Digest::SHA1.hexdigest(path)}"
cache.fetch(key) do cache.fetch(key) do
raw_repository.gitaly_migrate(:last_commit_for_path) do |is_enabled| raw_repository.last_commit_id_for_path(sha, path)
if is_enabled
last_commit_for_path_by_gitaly(sha, path).id
else
last_commit_id_for_path_by_shelling_out(sha, path)
end
end
end end
end end
...@@ -800,16 +784,6 @@ class Repository ...@@ -800,16 +784,6 @@ class Repository
with_cache_hooks { raw.multi_action(user, **options) } with_cache_hooks { raw.multi_action(user, **options) }
end end
def can_be_merged?(source_sha, target_branch)
raw_repository.gitaly_migrate(:can_be_merged) do |is_enabled|
if is_enabled
gitaly_can_be_merged?(source_sha, find_branch(target_branch).target)
else
rugged_can_be_merged?(source_sha, target_branch)
end
end
end
def merge(user, source_sha, merge_request, message) def merge(user, source_sha, merge_request, message)
with_cache_hooks do with_cache_hooks do
raw_repository.merge(user, source_sha, merge_request.target_branch, message) do |commit_id| raw_repository.merge(user, source_sha, merge_request.target_branch, message) do |commit_id|
...@@ -882,20 +856,12 @@ class Repository ...@@ -882,20 +856,12 @@ class Repository
first_commit_id = commit(first_commit_id).try(:id) || first_commit_id first_commit_id = commit(first_commit_id).try(:id) || first_commit_id
second_commit_id = commit(second_commit_id).try(:id) || second_commit_id second_commit_id = commit(second_commit_id).try(:id) || second_commit_id
raw_repository.merge_base(first_commit_id, second_commit_id) raw_repository.merge_base(first_commit_id, second_commit_id)
rescue Rugged::ReferenceError
nil
end end
def ancestor?(ancestor_id, descendant_id) def ancestor?(ancestor_id, descendant_id)
return false if ancestor_id.nil? || descendant_id.nil? return false if ancestor_id.nil? || descendant_id.nil?
Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled| raw_repository.ancestor?(ancestor_id, descendant_id)
if is_enabled
raw_repository.ancestor?(ancestor_id, descendant_id)
else
rugged_is_ancestor?(ancestor_id, descendant_id)
end
end
end end
def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil) def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil)
...@@ -1077,30 +1043,7 @@ class Repository ...@@ -1077,30 +1043,7 @@ class Repository
Gitlab::Metrics.add_event(event, { path: full_path }.merge(tags)) Gitlab::Metrics.add_event(event, { path: full_path }.merge(tags))
end end
def last_commit_for_path_by_gitaly(sha, path)
c = raw_repository.gitaly_commit_client.last_commit_for_path(sha, path)
commit_by(oid: c)
end
def last_commit_for_path_by_rugged(sha, path)
sha = last_commit_id_for_path_by_shelling_out(sha, path)
commit_by(oid: sha)
end
def last_commit_id_for_path_by_shelling_out(sha, path)
args = %W(rev-list --max-count=1 #{sha} -- #{path})
raw_repository.run_git_with_timeout(args, Gitlab::Git::Popen::FAST_GIT_PROCESS_TIMEOUT).first.strip
end
def initialize_raw_repository def initialize_raw_repository
Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, is_wiki)) Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, is_wiki))
end end
def gitaly_can_be_merged?(their_commit, our_commit)
!raw_repository.gitaly_conflicts_client(our_commit, their_commit).conflicts?
end
def rugged_can_be_merged?(their_commit, our_commit)
!rugged.merge_commits(our_commit, their_commit).conflicts?
end
end end
...@@ -44,7 +44,7 @@ module Gitlab ...@@ -44,7 +44,7 @@ module Gitlab
# branch1...branch2) From the git documentation: # branch1...branch2) From the git documentation:
# "git diff A...B" is equivalent to "git diff # "git diff A...B" is equivalent to "git diff
# $(git-merge-base A B) B" # $(git-merge-base A B) B"
repo.merge_base_commit(head, base) repo.merge_base(head, base)
end end
options ||= {} options ||= {}
......
...@@ -131,7 +131,10 @@ module Gitlab ...@@ -131,7 +131,10 @@ module Gitlab
oldrev = branch.target oldrev = branch.target
if oldrev == repository.merge_base(newrev, branch.target) merge_base = repository.merge_base(newrev, branch.target)
raise Gitlab::Git::Repository::InvalidRef unless merge_base
if oldrev == merge_base
oldrev oldrev
else else
raise Gitlab::Git::CommitError.new('Branch diverged') raise Gitlab::Git::CommitError.new('Branch diverged')
......
...@@ -551,29 +551,34 @@ module Gitlab ...@@ -551,29 +551,34 @@ module Gitlab
end end
# Returns the SHA of the most recent common ancestor of +from+ and +to+ # Returns the SHA of the most recent common ancestor of +from+ and +to+
def merge_base_commit(from, to) def merge_base(from, to)
gitaly_migrate(:merge_base) do |is_enabled| gitaly_migrate(:merge_base) do |is_enabled|
if is_enabled if is_enabled
gitaly_repository_client.find_merge_base(from, to) gitaly_repository_client.find_merge_base(from, to)
else else
rugged.merge_base(from, to) rugged_merge_base(from, to)
end end
end end
end end
alias_method :merge_base, :merge_base_commit
# Gitaly note: JV: check gitlab-ee before removing this method. # Gitaly note: JV: check gitlab-ee before removing this method.
def rugged_is_ancestor?(ancestor_id, descendant_id) def rugged_is_ancestor?(ancestor_id, descendant_id)
return false if ancestor_id.nil? || descendant_id.nil? return false if ancestor_id.nil? || descendant_id.nil?
merge_base_commit(ancestor_id, descendant_id) == ancestor_id rugged_merge_base(ancestor_id, descendant_id) == ancestor_id
rescue Rugged::OdbError rescue Rugged::OdbError
false false
end end
# Returns true is +from+ is direct ancestor to +to+, otherwise false # Returns true is +from+ is direct ancestor to +to+, otherwise false
def ancestor?(from, to) def ancestor?(from, to)
gitaly_commit_client.ancestor?(from, to) Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
if is_enabled
gitaly_commit_client.ancestor?(from, to)
else
rugged_is_ancestor?(from, to)
end
end
end end
def merged_branch_names(branch_names = []) def merged_branch_names(branch_names = [])
...@@ -680,11 +685,7 @@ module Gitlab ...@@ -680,11 +685,7 @@ module Gitlab
if is_enabled if is_enabled
gitaly_commit_client.commit_count(ref) gitaly_commit_client.commit_count(ref)
else else
walker = Rugged::Walker.new(rugged) rugged_commit_count(ref)
walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE)
oid = rugged.rev_parse_oid(ref)
walker.push(oid)
walker.count
end end
end end
end end
...@@ -1130,13 +1131,6 @@ module Gitlab ...@@ -1130,13 +1131,6 @@ module Gitlab
target_ref target_ref
end end
# Refactoring aid; allows us to copy code from app/models/repository.rb
def run_git_with_timeout(args, timeout, env: {})
circuit_breaker.perform do
popen_with_timeout([Gitlab.config.git.bin_path, *args], timeout, path, env)
end
end
# Refactoring aid; allows us to copy code from app/models/repository.rb # Refactoring aid; allows us to copy code from app/models/repository.rb
def commit(ref = 'HEAD') def commit(ref = 'HEAD')
Gitlab::Git::Commit.find(self, ref) Gitlab::Git::Commit.find(self, ref)
...@@ -1417,6 +1411,26 @@ module Gitlab ...@@ -1417,6 +1411,26 @@ module Gitlab
output output
end end
def can_be_merged?(source_sha, target_branch)
gitaly_migrate(:can_be_merged) do |is_enabled|
if is_enabled
gitaly_can_be_merged?(source_sha, find_branch(target_branch).target)
else
rugged_can_be_merged?(source_sha, target_branch)
end
end
end
def last_commit_id_for_path(sha, path)
gitaly_migrate(:last_commit_for_path) do |is_enabled|
if is_enabled
last_commit_for_path_by_gitaly(sha, path).id
else
last_commit_id_for_path_by_shelling_out(sha, path)
end
end
end
private private
def shell_write_ref(ref_path, ref, old_ref) def shell_write_ref(ref_path, ref, old_ref)
...@@ -1460,6 +1474,12 @@ module Gitlab ...@@ -1460,6 +1474,12 @@ module Gitlab
output output
end end
def run_git_with_timeout(args, timeout, env: {})
circuit_breaker.perform do
popen_with_timeout([Gitlab.config.git.bin_path, *args], timeout, path, env)
end
end
def fresh_worktree?(path) def fresh_worktree?(path)
File.exist?(path) && !clean_stuck_worktree(path) File.exist?(path) && !clean_stuck_worktree(path)
end end
...@@ -2160,7 +2180,7 @@ module Gitlab ...@@ -2160,7 +2180,7 @@ module Gitlab
source_sha source_sha
end end
rescue Rugged::ReferenceError rescue Rugged::ReferenceError, InvalidRef
raise ArgumentError, 'Invalid merge source' raise ArgumentError, 'Invalid merge source'
end end
...@@ -2257,6 +2277,39 @@ module Gitlab ...@@ -2257,6 +2277,39 @@ module Gitlab
.commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset) .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
.map { |c| commit(c) } .map { |c| commit(c) }
end end
def gitaly_can_be_merged?(their_commit, our_commit)
!gitaly_conflicts_client(our_commit, their_commit).conflicts?
end
def rugged_can_be_merged?(their_commit, our_commit)
!rugged.merge_commits(our_commit, their_commit).conflicts?
end
def last_commit_for_path_by_gitaly(sha, path)
gitaly_commit_client.last_commit_for_path(sha, path)
end
def last_commit_id_for_path_by_shelling_out(sha, path)
args = %W(rev-list --max-count=1 #{sha} -- #{path})
run_git_with_timeout(args, Gitlab::Git::Popen::FAST_GIT_PROCESS_TIMEOUT).first.strip
end
def rugged_merge_base(from, to)
rugged.merge_base(from, to)
rescue Rugged::ReferenceError
nil
end
def rugged_commit_count(ref)
walker = Rugged::Walker.new(rugged)
walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE)
oid = rugged.rev_parse_oid(ref)
walker.push(oid)
walker.count
rescue Rugged::ReferenceError
0
end
end end
end end
end end
...@@ -21,6 +21,7 @@ ALLOWED = [ ...@@ -21,6 +21,7 @@ ALLOWED = [
].freeze ].freeze
rugged_lines = IO.popen(%w[git grep -i -n rugged -- app config lib], &:read).lines rugged_lines = IO.popen(%w[git grep -i -n rugged -- app config lib], &:read).lines
rugged_lines = rugged_lines.select { |l| /^[^:]*\.rb:/ =~ l }
rugged_lines = rugged_lines.reject { |l| l.start_with?(*ALLOWED) } rugged_lines = rugged_lines.reject { |l| l.start_with?(*ALLOWED) }
rugged_lines = rugged_lines.reject do |line| rugged_lines = rugged_lines.reject do |line|
code, _comment = line.split('# ', 2) code, _comment = line.split('# ', 2)
......
...@@ -2,6 +2,7 @@ require "spec_helper" ...@@ -2,6 +2,7 @@ require "spec_helper"
describe Gitlab::Git::Repository, seed_helper: true do describe Gitlab::Git::Repository, seed_helper: true do
include Gitlab::EncodingHelper include Gitlab::EncodingHelper
using RSpec::Parameterized::TableSyntax
shared_examples 'wrapping gRPC errors' do |gitaly_client_class, gitaly_client_method| shared_examples 'wrapping gRPC errors' do |gitaly_client_class, gitaly_client_method|
it 'wraps gRPC not found error' do it 'wraps gRPC not found error' do
...@@ -442,6 +443,7 @@ describe Gitlab::Git::Repository, seed_helper: true do ...@@ -442,6 +443,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
shared_examples 'simple commit counting' do shared_examples 'simple commit counting' do
it { expect(repository.commit_count("master")).to eq(25) } it { expect(repository.commit_count("master")).to eq(25) }
it { expect(repository.commit_count("feature")).to eq(9) } it { expect(repository.commit_count("feature")).to eq(9) }
it { expect(repository.commit_count("does-not-exist")).to eq(0) }
end end
context 'when Gitaly commit_count feature is enabled' do context 'when Gitaly commit_count feature is enabled' do
...@@ -1032,6 +1034,29 @@ describe Gitlab::Git::Repository, seed_helper: true do ...@@ -1032,6 +1034,29 @@ describe Gitlab::Git::Repository, seed_helper: true do
it { is_expected.to eq(17) } it { is_expected.to eq(17) }
end end
describe '#merge_base' do
shared_examples '#merge_base' do
where(:from, :to, :result) do
'570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d'
'40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d'
'40f4a7a617393735a95a0bb67b08385bc1e7c66d' | 'foobar' | nil
'foobar' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | nil
end
with_them do
it { expect(repository.merge_base(from, to)).to eq(result) }
end
end
context 'with gitaly' do
it_behaves_like '#merge_base'
end
context 'without gitaly', :skip_gitaly_mock do
it_behaves_like '#merge_base'
end
end
describe '#count_commits' do describe '#count_commits' do
shared_examples 'extended commit counting' do shared_examples 'extended commit counting' do
context 'with after timestamp' do context 'with after timestamp' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment