Commit 9ff26fc2 authored by Robert Speicher's avatar Robert Speicher
Browse files

Merge branch 'gitaly-lfs-client-prep' into 'master'

Refactor Gitlab::Git code related to LFS changes for Gitaly migration

Closes gitaly#980

See merge request gitlab-org/gitlab-ce!16835
......@@ -15,8 +15,8 @@ def self.force_push?(project, oldrev, newrev)
.ancestor?(oldrev, newrev)
else
Gitlab::Git::RevList.new(
path_to_repo: project.repository.path_to_repo,
oldrev: oldrev, newrev: newrev).missed_ref.present?
project.repository.raw, oldrev: oldrev, newrev: newrev
).missed_ref.present?
end
end
end
......
......@@ -25,8 +25,7 @@ def all_pointers
private
def rev_list
::Gitlab::Git::RevList.new(path_to_repo: @repository.path_to_repo,
newrev: @newrev)
Gitlab::Git::RevList.new(@repository, newrev: @newrev)
end
end
end
......
......@@ -25,7 +25,7 @@ def popen(cmd, path, vars = {}, lazy_block: nil)
stdin.close
if lazy_block
return lazy_block.call(stdout.lazy)
return [lazy_block.call(stdout.lazy), 0]
else
cmd_output << stdout.read
end
......
......@@ -1427,6 +1427,26 @@ def last_commit_id_for_path(sha, path)
end
end
def rev_list(including: [], excluding: [], objects: false, &block)
args = ['rev-list']
args.push(*rev_list_param(including))
exclude_param = *rev_list_param(excluding)
if exclude_param.any?
args.push('--not')
args.push(*exclude_param)
end
args.push('--objects') if objects
run_git!(args, lazy_block: block)
end
def missed_ref(oldrev, newrev)
run_git!(['rev-list', '--max-count=1', oldrev, "^#{newrev}"])
end
private
def local_write_ref(ref_path, ref, old_ref: nil, shell: true)
......@@ -1475,7 +1495,7 @@ def rugged_write_ref(ref_path, ref)
Rails.logger.error "Unable to create #{ref_path} reference for repository #{path}: #{ex}"
end
def run_git(args, chdir: path, env: {}, nice: false, &block)
def run_git(args, chdir: path, env: {}, nice: false, lazy_block: nil, &block)
cmd = [Gitlab.config.git.bin_path, *args]
cmd.unshift("nice") if nice
......@@ -1485,12 +1505,12 @@ def run_git(args, chdir: path, env: {}, nice: false, &block)
end
circuit_breaker.perform do
popen(cmd, chdir, env, &block)
popen(cmd, chdir, env, lazy_block: lazy_block, &block)
end
end
def run_git!(args, chdir: path, env: {}, nice: false, &block)
output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block)
def run_git!(args, chdir: path, env: {}, nice: false, lazy_block: nil, &block)
output, status = run_git(args, chdir: chdir, env: env, nice: nice, lazy_block: lazy_block, &block)
raise GitError, output unless status.zero?
......@@ -2372,6 +2392,10 @@ def rugged_commit_count(ref)
rescue Rugged::ReferenceError
0
end
def rev_list_param(spec)
spec == :all ? ['--all'] : spec
end
end
end
end
......@@ -5,17 +5,17 @@ module Git
class RevList
include Gitlab::Git::Popen
attr_reader :oldrev, :newrev, :path_to_repo
attr_reader :oldrev, :newrev, :repository
def initialize(path_to_repo:, newrev:, oldrev: nil)
def initialize(repository, newrev:, oldrev: nil)
@oldrev = oldrev
@newrev = newrev
@path_to_repo = path_to_repo
@repository = repository
end
# This method returns an array of new commit references
def new_refs
execute([*base_args, newrev, '--not', '--all'])
repository.rev_list(including: newrev, excluding: :all).split("\n")
end
# Finds newly added objects
......@@ -28,66 +28,39 @@ def new_refs
# When given a block it will yield objects as a lazy enumerator so
# the caller can limit work done instead of processing megabytes of data
def new_objects(require_path: nil, not_in: nil, &lazy_block)
args = [*base_args, newrev, *not_in_refs(not_in), '--objects']
opts = {
including: newrev,
excluding: not_in.nil? ? :all : not_in,
require_path: require_path
}
get_objects(args, require_path: require_path, &lazy_block)
get_objects(opts, &lazy_block)
end
def all_objects(require_path: nil, &lazy_block)
args = [*base_args, '--all', '--objects']
get_objects(args, require_path: require_path, &lazy_block)
get_objects(including: :all, require_path: require_path, &lazy_block)
end
# This methods returns an array of missed references
#
# Should become obsolete after https://gitlab.com/gitlab-org/gitaly/issues/348.
def missed_ref
execute([*base_args, '--max-count=1', oldrev, "^#{newrev}"])
repository.missed_ref(oldrev, newrev).split("\n")
end
private
def not_in_refs(references)
return ['--not', '--all'] unless references
return [] if references.empty?
references.prepend('--not')
end
def execute(args)
output, status = popen(args, nil, Gitlab::Git::Env.to_env_hash)
unless status.zero?
raise "Got a non-zero exit code while calling out `#{args.join(' ')}`: #{output}"
end
output.split("\n")
end
def lazy_execute(args, &lazy_block)
popen(args, nil, Gitlab::Git::Env.to_env_hash, lazy_block: lazy_block)
end
def base_args
[
Gitlab.config.git.bin_path,
"--git-dir=#{path_to_repo}",
'rev-list'
]
repository.rev_list(args).split("\n")
end
def get_objects(args, require_path: nil)
if block_given?
lazy_execute(args) do |lazy_output|
objects = objects_from_output(lazy_output, require_path: require_path)
def get_objects(including: [], excluding: [], require_path: nil)
opts = { including: including, excluding: excluding, objects: true }
yield(objects)
end
else
object_output = execute(args)
repository.rev_list(opts) do |lazy_output|
objects = objects_from_output(lazy_output, require_path: require_path)
objects_from_output(object_output, require_path: require_path)
yield(objects)
end
end
......
......@@ -2,18 +2,20 @@
describe Gitlab::Checks::ForcePush do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository.raw }
context "exit code checking", :skip_gitaly_mock do
it "does not raise a runtime error if the `popen` call to git returns a zero exit code" do
allow_any_instance_of(Gitlab::Git::RevList).to receive(:popen).and_return(['normal output', 0])
allow(repository).to receive(:popen).and_return(['normal output', 0])
expect { described_class.force_push?(project, 'oldrev', 'newrev') }.not_to raise_error
end
it "raises a runtime error if the `popen` call to git returns a non-zero exit code" do
allow_any_instance_of(Gitlab::Git::RevList).to receive(:popen).and_return(['error', 1])
it "raises a GitError error if the `popen` call to git returns a non-zero exit code" do
allow(repository).to receive(:popen).and_return(['error', 1])
expect { described_class.force_push?(project, 'oldrev', 'newrev') }.to raise_error(RuntimeError)
expect { described_class.force_push?(project, 'oldrev', 'newrev') }
.to raise_error(Gitlab::Git::Repository::GitError)
end
end
end
require 'spec_helper'
describe Gitlab::Git::RevList do
let(:project) { create(:project, :repository) }
let(:rev_list) { described_class.new(newrev: 'newrev', path_to_repo: project.repository.path_to_repo) }
let(:repository) { create(:project, :repository).repository.raw }
let(:rev_list) { described_class.new(repository, newrev: 'newrev') }
let(:env_hash) do
{
'GIT_OBJECT_DIRECTORY' => 'foo',
'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar'
}
end
let(:command_env) { { 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'foo:bar' } }
before do
allow(Gitlab::Git::Env).to receive(:all).and_return(env_hash.symbolize_keys)
allow(Gitlab::Git::Env).to receive(:all).and_return(env_hash)
end
def args_for_popen(args_list)
[
Gitlab.config.git.bin_path,
"--git-dir=#{project.repository.path_to_repo}",
'rev-list',
*args_list
]
end
def stub_popen_rev_list(*additional_args, output:)
args = args_for_popen(additional_args)
expect(rev_list).to receive(:popen).with(args, nil, env_hash)
.and_return([output, 0])
[Gitlab.config.git.bin_path, 'rev-list', *args_list]
end
def stub_lazy_popen_rev_list(*additional_args, output:)
def stub_popen_rev_list(*additional_args, with_lazy_block: true, output:)
params = [
args_for_popen(additional_args),
nil,
env_hash,
hash_including(lazy_block: anything)
repository.path,
command_env,
hash_including(lazy_block: with_lazy_block ? anything : nil)
]
expect(rev_list).to receive(:popen).with(*params) do |*_, lazy_block:|
lazy_block.call(output.lines.lazy.map(&:chomp))
expect(repository).to receive(:popen).with(*params) do |*_, lazy_block:|
output = lazy_block.call(output.lines.lazy.map(&:chomp)) if with_lazy_block
[output, 0]
end
end
context "#new_refs" do
it 'calls out to `popen`' do
stub_popen_rev_list('newrev', '--not', '--all', output: "sha1\nsha2")
stub_popen_rev_list('newrev', '--not', '--all', with_lazy_block: false, output: "sha1\nsha2")
expect(rev_list.new_refs).to eq(%w[sha1 sha2])
end
......@@ -55,18 +46,18 @@ def stub_lazy_popen_rev_list(*additional_args, output:)
it 'fetches list of newly pushed objects using rev-list' do
stub_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2")
expect(rev_list.new_objects).to eq(%w[sha1 sha2])
expect { |b| rev_list.new_objects(&b) }.to yield_with_args(%w[sha1 sha2])
end
it 'can skip pathless objects' do
stub_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2 path/to/file")
expect(rev_list.new_objects(require_path: true)).to eq(%w[sha2])
expect { |b| rev_list.new_objects(require_path: true, &b) }.to yield_with_args(%w[sha2])
end
it 'can handle non utf-8 paths' do
non_utf_char = [0x89].pack("c*").force_encoding("UTF-8")
stub_lazy_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha2 πå†h/†ø/ƒîlé#{non_utf_char}\nsha1")
stub_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha2 πå†h/†ø/ƒîlé#{non_utf_char}\nsha1")
rev_list.new_objects(require_path: true) do |object_ids|
expect(object_ids.force).to eq(%w[sha2])
......@@ -74,7 +65,7 @@ def stub_lazy_popen_rev_list(*additional_args, output:)
end
it 'can yield a lazy enumerator' do
stub_lazy_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2")
stub_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2")
rev_list.new_objects do |object_ids|
expect(object_ids).to be_a Enumerator::Lazy
......@@ -82,7 +73,7 @@ def stub_lazy_popen_rev_list(*additional_args, output:)
end
it 'returns the result of the block when given' do
stub_lazy_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2")
stub_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2")
objects = rev_list.new_objects do |object_ids|
object_ids.first
......@@ -94,13 +85,13 @@ def stub_lazy_popen_rev_list(*additional_args, output:)
it 'can accept list of references to exclude' do
stub_popen_rev_list('newrev', '--not', 'master', '--objects', output: "sha1\nsha2")
expect(rev_list.new_objects(not_in: ['master'])).to eq(%w[sha1 sha2])
expect { |b| rev_list.new_objects(not_in: ['master'], &b) }.to yield_with_args(%w[sha1 sha2])
end
it 'handles empty list of references to exclude as listing all known objects' do
stub_popen_rev_list('newrev', '--objects', output: "sha1\nsha2")
expect(rev_list.new_objects(not_in: [])).to eq(%w[sha1 sha2])
expect { |b| rev_list.new_objects(not_in: [], &b) }.to yield_with_args(%w[sha1 sha2])
end
end
......@@ -108,15 +99,15 @@ def stub_lazy_popen_rev_list(*additional_args, output:)
it 'fetches list of all pushed objects using rev-list' do
stub_popen_rev_list('--all', '--objects', output: "sha1\nsha2")
expect(rev_list.all_objects).to eq(%w[sha1 sha2])
expect { |b| rev_list.all_objects(&b) }.to yield_with_args(%w[sha1 sha2])
end
end
context "#missed_ref" do
let(:rev_list) { described_class.new(oldrev: 'oldrev', newrev: 'newrev', path_to_repo: project.repository.path_to_repo) }
let(:rev_list) { described_class.new(repository, oldrev: 'oldrev', newrev: 'newrev') }
it 'calls out to `popen`' do
stub_popen_rev_list('--max-count=1', 'oldrev', '^newrev', output: "sha1\nsha2")
stub_popen_rev_list('--max-count=1', 'oldrev', '^newrev', with_lazy_block: false, output: "sha1\nsha2")
expect(rev_list.missed_ref).to eq(%w[sha1 sha2])
end
......
......@@ -108,7 +108,7 @@
context 'git commands', :disable_gitaly do
it 'sets GL_REPOSITORY env variable when calling git commands' do
expect(repository).to receive(:popen).exactly(3)
.with(anything, anything, hash_including('GL_REPOSITORY'))
.with(anything, anything, hash_including('GL_REPOSITORY'), anything)
.and_return(['', 0])
service.execute(merge_request)
......
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