Commit 8bacfbd1 authored by Nick Thomas's avatar Nick Thomas

Merge branch 'zj-calculate-checksum-mandator' into 'master'

Calculating repository checksums executed by Gitaly

Closes gitaly#1105 and #46293

See merge request gitlab-org/gitlab-ce!18907
parents 63831a24 18a8eb96
---
title: Remove shellout implementation for Repository checksums
merge_request:
author:
type: other
......@@ -1572,14 +1572,12 @@ module Gitlab
end
def checksum
gitaly_migrate(:calculate_checksum,
status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
if is_enabled
gitaly_repository_client.calculate_checksum
else
calculate_checksum_by_shelling_out
end
end
# The exists? RPC is much cheaper, so we perform this request first
raise NoRepository, "Repository does not exists" unless exists?
gitaly_repository_client.calculate_checksum
rescue GRPC::NotFound
raise NoRepository # Guard against data races.
end
private
......@@ -2478,36 +2476,6 @@ module Gitlab
rev_parse_target(ref).oid
end
def calculate_checksum_by_shelling_out
raise NoRepository unless exists?
args = %W(--git-dir=#{path} show-ref --heads --tags)
output, status = run_git(args)
if status.nil? || !status.zero?
# Non-valid git repositories return 128 as the status code and an error output
raise InvalidRepository if status == 128 && output.to_s.downcase =~ /not a git repository/
# Empty repositories returns with a non-zero status and an empty output.
raise ChecksumError, output unless output.blank?
return EMPTY_REPOSITORY_CHECKSUM
end
refs = output.split("\n")
result = refs.inject(nil) do |checksum, ref|
value = Digest::SHA1.hexdigest(ref).hex
if checksum.nil?
value
else
checksum ^ value
end
end
result.to_s(16)
end
def build_git_cmd(*args)
object_directories = alternate_object_directories.join(File::PATH_SEPARATOR)
......
......@@ -2247,66 +2247,42 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
describe '#checksum' do
shared_examples 'calculating checksum' do
it 'calculates the checksum for non-empty repo' do
expect(repository.checksum).to eq '54f21be4c32c02f6788d72207fa03ad3bce725e4'
end
it 'returns 0000000000000000000000000000000000000000 for an empty repo' do
FileUtils.rm_rf(File.join(storage_path, 'empty-repo.git'))
system(git_env, *%W(#{Gitlab.config.git.bin_path} init --bare empty-repo.git),
chdir: storage_path,
out: '/dev/null',
err: '/dev/null')
it 'calculates the checksum for non-empty repo' do
expect(repository.checksum).to eq '54f21be4c32c02f6788d72207fa03ad3bce725e4'
end
empty_repo = described_class.new('default', 'empty-repo.git', '')
it 'returns 0000000000000000000000000000000000000000 for an empty repo' do
FileUtils.rm_rf(File.join(storage_path, 'empty-repo.git'))
expect(empty_repo.checksum).to eq '0000000000000000000000000000000000000000'
end
system(git_env, *%W(#{Gitlab.config.git.bin_path} init --bare empty-repo.git),
chdir: storage_path,
out: '/dev/null',
err: '/dev/null')
it 'raises Gitlab::Git::Repository::InvalidRepository error for non-valid git repo' do
FileUtils.rm_rf(File.join(storage_path, 'non-valid.git'))
empty_repo = described_class.new('default', 'empty-repo.git', '')
system(git_env, *%W(#{Gitlab.config.git.bin_path} clone --bare #{TEST_REPO_PATH} non-valid.git),
chdir: SEED_STORAGE_PATH,
out: '/dev/null',
err: '/dev/null')
expect(empty_repo.checksum).to eq '0000000000000000000000000000000000000000'
end
File.truncate(File.join(storage_path, 'non-valid.git/HEAD'), 0)
it 'raises Gitlab::Git::Repository::InvalidRepository error for non-valid git repo' do
FileUtils.rm_rf(File.join(storage_path, 'non-valid.git'))
non_valid = described_class.new('default', 'non-valid.git', '')
system(git_env, *%W(#{Gitlab.config.git.bin_path} clone --bare #{TEST_REPO_PATH} non-valid.git),
chdir: SEED_STORAGE_PATH,
out: '/dev/null',
err: '/dev/null')
expect { non_valid.checksum }.to raise_error(Gitlab::Git::Repository::InvalidRepository)
end
File.truncate(File.join(storage_path, 'non-valid.git/HEAD'), 0)
it 'raises Gitlab::Git::Repository::NoRepository error when there is no repo' do
broken_repo = described_class.new('default', 'a/path.git', '')
non_valid = described_class.new('default', 'non-valid.git', '')
expect { broken_repo.checksum }.to raise_error(Gitlab::Git::Repository::NoRepository)
end
expect { non_valid.checksum }.to raise_error(Gitlab::Git::Repository::InvalidRepository)
end
context 'when calculate_checksum Gitaly feature is enabled' do
it_behaves_like 'calculating checksum'
end
context 'when calculate_checksum Gitaly feature is disabled', :disable_gitaly do
it_behaves_like 'calculating checksum'
describe 'when storage is broken', :broken_storage do
it 'raises a storage exception when storage is not available' do
broken_repo = described_class.new('broken', 'a/path.git', '')
expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Storage::Inaccessible)
end
end
it "raises a Gitlab::Git::Repository::Failure error if the `popen` call to git returns a non-zero exit code" do
allow(repository).to receive(:popen).and_return(['output', nil])
it 'raises Gitlab::Git::Repository::NoRepository error when there is no repo' do
broken_repo = described_class.new('default', 'a/path.git', '')
expect { repository.checksum }.to raise_error Gitlab::Git::Repository::ChecksumError
end
expect { broken_repo.checksum }.to raise_error(Gitlab::Git::Repository::NoRepository)
end
end
......
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