Only set autocrlf when creating/updating files
Setting the "autocrlf" Git option is an overkill since it's rarely actually needed. More importantly, it has quite the impact on performance (see gitlab-org/gitlab-ce#13457 for more information). By setting "autocrlf" when creating or updating files we guarantee the option is always set properly when we actually need it _without_ introducing overhead for requests that have nothing to do with this option. Fixes gitlab-org/gitlab-ce#13457
This commit is contained in:
parent
8b918267c7
commit
c475b17111
|
@ -24,14 +24,16 @@ class Repository
|
||||||
return nil unless path_with_namespace
|
return nil unless path_with_namespace
|
||||||
|
|
||||||
@raw_repository ||= begin
|
@raw_repository ||= begin
|
||||||
repo = Gitlab::Git::Repository.new(path_to_repo)
|
Gitlab::Git::Repository.new(path_to_repo)
|
||||||
repo.autocrlf = :input
|
|
||||||
repo
|
|
||||||
rescue Gitlab::Git::Repository::NoRepository
|
rescue Gitlab::Git::Repository::NoRepository
|
||||||
nil
|
nil
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def update_autocrlf_option
|
||||||
|
raw_repository.autocrlf = :input if raw_repository.autocrlf != :input
|
||||||
|
end
|
||||||
|
|
||||||
# Return absolute path to repository
|
# Return absolute path to repository
|
||||||
def path_to_repo
|
def path_to_repo
|
||||||
@path_to_repo ||= File.expand_path(
|
@path_to_repo ||= File.expand_path(
|
||||||
|
@ -693,6 +695,8 @@ class Repository
|
||||||
end
|
end
|
||||||
|
|
||||||
def commit_with_hooks(current_user, branch)
|
def commit_with_hooks(current_user, branch)
|
||||||
|
update_autocrlf_option
|
||||||
|
|
||||||
oldrev = Gitlab::Git::BLANK_SHA
|
oldrev = Gitlab::Git::BLANK_SHA
|
||||||
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
|
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
|
||||||
was_empty = empty?
|
was_empty = empty?
|
||||||
|
|
|
@ -200,13 +200,22 @@ describe Repository, models: true do
|
||||||
|
|
||||||
describe :commit_with_hooks do
|
describe :commit_with_hooks do
|
||||||
context 'when pre hooks were successful' do
|
context 'when pre hooks were successful' do
|
||||||
it 'should run without errors' do
|
before do
|
||||||
expect_any_instance_of(GitHooksService).to receive(:execute).and_return(true)
|
expect_any_instance_of(GitHooksService).to receive(:execute).
|
||||||
|
and_return(true)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'should run without errors' do
|
||||||
expect do
|
expect do
|
||||||
repository.commit_with_hooks(user, 'feature') { sample_commit.id }
|
repository.commit_with_hooks(user, 'feature') { sample_commit.id }
|
||||||
end.not_to raise_error
|
end.not_to raise_error
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'should ensure the autocrlf Git option is set to :input' do
|
||||||
|
expect(repository).to receive(:update_autocrlf_option)
|
||||||
|
|
||||||
|
repository.commit_with_hooks(user, 'feature') { sample_commit.id }
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when pre hooks failed' do
|
context 'when pre hooks failed' do
|
||||||
|
@ -249,6 +258,33 @@ describe Repository, models: true do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#update_autocrlf_option' do
|
||||||
|
describe 'when autocrlf is not already set to :input' do
|
||||||
|
before do
|
||||||
|
repository.raw_repository.autocrlf = true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'sets autocrlf to :input' do
|
||||||
|
repository.update_autocrlf_option
|
||||||
|
|
||||||
|
expect(repository.raw_repository.autocrlf).to eq(:input)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'when autocrlf is already set to :input' do
|
||||||
|
before do
|
||||||
|
repository.raw_repository.autocrlf = :input
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does nothing' do
|
||||||
|
expect(repository.raw_repository).to_not receive(:autocrlf=).
|
||||||
|
with(:input)
|
||||||
|
|
||||||
|
repository.update_autocrlf_option
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '#empty?' do
|
describe '#empty?' do
|
||||||
let(:empty_repository) { create(:project_empty_repo).repository }
|
let(:empty_repository) { create(:project_empty_repo).repository }
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue