Commit 80ea3710 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'gitlab-git-dont-memoize-path' into 'master'

Remove Repository#path memoization

See merge request gitlab-org/gitlab-ce!20491
parents cfe21219 62ffad08
......@@ -86,9 +86,6 @@ module Gitlab
# Relative path of repo
attr_reader :relative_path
# Rugged repo object
attr_reader :rugged
attr_reader :gitlab_projects, :storage, :gl_repository, :relative_path
# This initializer method is only used on the client side (gitlab-ce).
......@@ -112,8 +109,9 @@ module Gitlab
[storage, relative_path] == [other.storage, other.relative_path]
end
# This method will be removed when Gitaly reaches v1.1.
def path
@path ||= File.join(
File.join(
Gitlab.config.repositories.storages[@storage].legacy_disk_path, @relative_path
)
end
......@@ -127,8 +125,9 @@ module Gitlab
raise Gitlab::Git::CommandError.new(e.message)
end
# This method will be removed when Gitaly reaches v1.1.
def rugged
@rugged ||= circuit_breaker.perform do
circuit_breaker.perform do
Rugged::Repository.new(path, alternates: alternate_object_directories)
end
rescue Rugged::RepositoryError, Rugged::OSError
......@@ -713,12 +712,6 @@ module Gitlab
Gitlab::Git.committer_hash(email: user.email, name: user.name)
end
def create_commit(params = {})
params[:message].delete!("\r")
Rugged::Commit.create(rugged, params)
end
# Delete the specified branch from the repository
def delete_branch(branch_name)
gitaly_migrate(:delete_branch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
......@@ -1758,6 +1751,12 @@ module Gitlab
def sha_from_ref(ref)
rev_parse_target(ref).oid
end
def create_commit(params = {})
params[:message].delete!("\r")
Rugged::Commit.create(rugged, params)
end
end
end
end
......@@ -12,35 +12,12 @@ module Gitlab
end
# This method returns an array of new commit references
def new_refs
repository.rev_list(including: newrev, excluding: :all).split("\n")
end
# Finds newly added objects
# Returns an array of shas
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1233
#
# Can skip objects which do not have a path using required_path: true
# This skips commit objects and root trees, which might not be needed when
# looking for blobs
#
# 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(options: [], require_path: nil, not_in: nil, &lazy_block)
opts = {
including: newrev,
options: options,
excluding: not_in.nil? ? :all : not_in,
require_path: require_path
}
get_objects(opts, &lazy_block)
end
def all_objects(options: [], require_path: nil, &lazy_block)
get_objects(including: :all,
options: options,
require_path: require_path,
&lazy_block)
def new_refs
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
repository.rev_list(including: newrev, excluding: :all).split("\n")
end
end
private
......
......@@ -2,6 +2,11 @@ require "spec_helper"
describe Gitlab::Git::Branch, seed_helper: true do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
let(:rugged) do
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
repository.rugged
end
end
subject { repository.branches }
......@@ -124,6 +129,7 @@ describe Gitlab::Git::Branch, seed_helper: true do
it { expect(repository.branches.size).to eq(SeedRepo::Repo::BRANCHES.size) }
def create_commit
repository.create_commit(params.merge(committer: committer.merge(time: Time.now)))
params[:message].delete!("\r")
Rugged::Commit.create(rugged, params.merge(committer: committer.merge(time: Time.now)))
end
end
......@@ -27,6 +27,7 @@ EOT
too_large: false
}
# TODO use a Gitaly diff object instead
@rugged_diff = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
repository.rugged.diff("5937ac0a7beb003549fc5fd26fc247adbce4a52e^", "5937ac0a7beb003549fc5fd26fc247adbce4a52e", paths:
[".gitmodules"]).patches.first
......@@ -266,8 +267,12 @@ EOT
describe '#submodule?' do
before do
commit = repository.lookup('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
@diffs = commit.parents[0].diff(commit).patches
# TODO use a Gitaly diff object instead
rugged_commit = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
repository.lookup('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
end
@diffs = rugged_commit.parents[0].diff(rugged_commit).patches
end
it { expect(described_class.new(@diffs[0]).submodule?).to eq(false) }
......
......@@ -611,21 +611,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
end
describe "#remove_remote" do
before(:all) do
@repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
@repo.remove_remote("expendable")
end
it "should remove the remote" do
expect(@repo.rugged.remotes).not_to include("expendable")
end
after(:all) do
ensure_seeds
end
end
describe "#remote_update" do
before(:all) do
@repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
......@@ -633,7 +618,9 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
it "should add the remote" do
expect(@repo.rugged.remotes["expendable"].url).to(
rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access { @repo.rugged }
expect(rugged.remotes["expendable"].url).to(
eq(TEST_NORMAL_REPO_PATH)
)
end
......@@ -1157,6 +1144,13 @@ describe Gitlab::Git::Repository, seed_helper: true do
@repo.rugged.config['core.autocrlf'] = true
end
around do |example|
# OK because autocrlf is only used in gitaly-ruby
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
example.run
end
end
it 'return the value of the autocrlf option' do
expect(@repo.autocrlf).to be(true)
end
......@@ -1172,6 +1166,13 @@ describe Gitlab::Git::Repository, seed_helper: true do
@repo.rugged.config['core.autocrlf'] = false
end
around do |example|
# OK because autocrlf= is only used in gitaly-ruby
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
example.run
end
end
it 'should set the autocrlf option to the provided option' do
@repo.autocrlf = :input
......@@ -2042,54 +2043,61 @@ describe Gitlab::Git::Repository, seed_helper: true do
let(:repository) do
Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
end
let(:rugged) do
Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.rugged }
end
let(:remote_name) { 'my-remote' }
let(:url) { 'http://my-repo.git' }
after do
ensure_seeds
end
describe '#add_remote' do
let(:url) { 'http://my-repo.git' }
let(:mirror_refmap) { '+refs/*:refs/*' }
it 'creates a new remote via Gitaly' do
expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
.to receive(:add_remote).with(remote_name, url, mirror_refmap)
shared_examples 'add_remote' do
it 'added the remote' do
begin
rugged.remotes.delete(remote_name)
rescue Rugged::ConfigError
end
repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap)
repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap)
expect(rugged.remotes[remote_name]).not_to be_nil
expect(rugged.config["remote.#{remote_name}.mirror"]).to eq('true')
expect(rugged.config["remote.#{remote_name}.prune"]).to eq('true')
expect(rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap)
end
end
context 'with Gitaly disabled', :skip_gitaly_mock do
it 'creates a new remote via Rugged' do
expect_any_instance_of(Rugged::RemoteCollection).to receive(:create)
.with(remote_name, url)
expect_any_instance_of(Rugged::Config).to receive(:[]=)
.with("remote.#{remote_name}.mirror", true)
expect_any_instance_of(Rugged::Config).to receive(:[]=)
.with("remote.#{remote_name}.prune", true)
expect_any_instance_of(Rugged::Config).to receive(:[]=)
.with("remote.#{remote_name}.fetch", mirror_refmap)
context 'using Gitaly' do
it_behaves_like 'add_remote'
end
repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap)
end
context 'with Gitaly disabled', :disable_gitaly do
it_behaves_like 'add_remote'
end
end
describe '#remove_remote' do
it 'removes the remote via Gitaly' do
expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
.to receive(:remove_remote).with(remote_name)
shared_examples 'remove_remote' do
it 'removes the remote' do
rugged.remotes.create(remote_name, url)
repository.remove_remote(remote_name)
repository.remove_remote(remote_name)
expect(rugged.remotes[remote_name]).to be_nil
end
end
context 'with Gitaly disabled', :skip_gitaly_mock do
it 'removes the remote via Rugged' do
expect_any_instance_of(Rugged::RemoteCollection).to receive(:delete)
.with(remote_name)
context 'using Gitaly' do
it_behaves_like 'remove_remote'
end
repository.remove_remote(remote_name)
end
context 'with Gitaly disabled', :disable_gitaly do
it_behaves_like 'remove_remote'
end
end
end
......@@ -2281,20 +2289,25 @@ describe Gitlab::Git::Repository, seed_helper: true do
let(:worktree_path) { File.join(repository_path, 'worktrees', 'delete-me') }
it 'cleans up the files' do
repository.with_worktree(worktree_path, 'master', env: ENV) do
FileUtils.touch(worktree_path, mtime: Time.now - 8.hours)
# git rev-list --all will fail in git 2.16 if HEAD is pointing to a non-existent object,
# but the HEAD must be 40 characters long or git will ignore it.
File.write(File.join(worktree_path, 'HEAD'), Gitlab::Git::BLANK_SHA)
create_worktree = %W[git -C #{repository_path} worktree add --detach #{worktree_path} master]
raise 'preparation failed' unless system(*create_worktree, err: '/dev/null')
# git 2.16 fails with "fatal: bad object HEAD"
expect { repository.rev_list(including: :all) }.to raise_error(Gitlab::Git::Repository::GitError)
FileUtils.touch(worktree_path, mtime: Time.now - 8.hours)
# git rev-list --all will fail in git 2.16 if HEAD is pointing to a non-existent object,
# but the HEAD must be 40 characters long or git will ignore it.
File.write(File.join(worktree_path, 'HEAD'), Gitlab::Git::BLANK_SHA)
repository.clean_stale_repository_files
# git 2.16 fails with "fatal: bad object HEAD"
expect(rev_list_all).to be false
expect { repository.rev_list(including: :all) }.not_to raise_error
expect(File.exist?(worktree_path)).to be_falsey
end
repository.clean_stale_repository_files
expect(rev_list_all).to be true
expect(File.exist?(worktree_path)).to be_falsey
end
def rev_list_all
system(*%W[git -C #{repository_path} rev-list --all], out: '/dev/null', err: '/dev/null')
end
it 'increments a counter upon an error' do
......
......@@ -32,65 +32,4 @@ describe Gitlab::Git::RevList do
expect(rev_list.new_refs).to eq(%w[sha1 sha2])
end
end
context '#new_objects' do
it 'fetches list of newly pushed objects using rev-list' do
stub_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2")
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 { |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_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])
end
end
it 'can yield a lazy enumerator' do
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
end
end
it 'returns the result of the block when given' do
stub_popen_rev_list('newrev', '--not', '--all', '--objects', output: "sha1\nsha2")
objects = rev_list.new_objects do |object_ids|
object_ids.first
end
expect(objects).to eq 'sha1'
end
it 'can accept list of references to exclude' do
stub_popen_rev_list('newrev', '--not', 'master', '--objects', output: "sha1\nsha2")
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 { |b| rev_list.new_objects(not_in: [], &b) }.to yield_with_args(%w[sha1 sha2])
end
end
context '#all_objects' do
it 'fetches list of all pushed objects using rev-list' do
stub_popen_rev_list('--all', '--objects', output: "sha1\nsha2")
expect { |b| rev_list.all_objects(&b) }.to yield_with_args(%w[sha1 sha2])
end
end
end
......@@ -734,26 +734,18 @@ describe Gitlab::GitAccess do
merge_into_protected_branch: "0b4bc9a #{merge_into_protected_branch} refs/heads/feature" }
end
def stub_git_hooks
# Running the `pre-receive` hook is expensive, and not necessary for this test.
allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) do |service, &block|
block.call(service)
end
end
def merge_into_protected_branch
@protected_branch_merge_commit ||= begin
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
stub_git_hooks
project.repository.add_branch(user, unprotected_branch, 'feature')
target_branch = project.repository.lookup('feature')
rugged = project.repository.rugged
target_branch = rugged.rev_parse('feature')
source_branch = project.repository.create_file(
user,
'filename',
'This is the file content',
message: 'This is a good commit message',
branch_name: unprotected_branch)
rugged = project.repository.rugged
author = { email: "email@example.com", time: Time.now, name: "Example Git User" }
merge_index = rugged.merge_commits(target_branch, source_branch)
......
......@@ -151,7 +151,9 @@ describe Repository do
it { is_expected.to eq(['v1.1.0', 'v1.0.0', annotated_tag_name]) }
after do
repository.rugged.tags.delete(annotated_tag_name)
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
repository.rugged.tags.delete(annotated_tag_name)
end
end
end
end
......@@ -2231,8 +2233,11 @@ describe Repository do
create_remote_branch('joe', 'remote_branch', masterrev)
repository.add_branch(user, 'local_branch', masterrev.id)
expect(repository.remote_branches('joe').any? { |branch| branch.name == 'local_branch' }).to eq(false)
expect(repository.remote_branches('joe').any? { |branch| branch.name == 'remote_branch' }).to eq(true)
# TODO: move this test to gitaly https://gitlab.com/gitlab-org/gitaly/issues/1243
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
expect(repository.remote_branches('joe').any? { |branch| branch.name == 'local_branch' }).to eq(false)
expect(repository.remote_branches('joe').any? { |branch| branch.name == 'remote_branch' }).to eq(true)
end
end
end
......
......@@ -209,12 +209,7 @@ describe GitGarbageCollectWorker do
tree: old_commit.tree,
parents: [old_commit]
)
Gitlab::Git::OperationService.new(nil, project.repository.raw_repository).send(
:update_ref,
"refs/heads/#{SecureRandom.hex(6)}",
new_commit_sha,
Gitlab::Git::BLANK_SHA
)
rugged.references.create("refs/heads/#{SecureRandom.hex(6)}", new_commit_sha)
end
def packs(project)
......
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