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

2 files changed, 45 insertions, 5 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 5a25ccb1dd6..381b1db3758 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -24,14 +24,16 @@ class Repository
     return nil unless path_with_namespace
 
     @raw_repository ||= begin
-      repo = Gitlab::Git::Repository.new(path_to_repo)
-      repo.autocrlf = :input
-      repo
+      Gitlab::Git::Repository.new(path_to_repo)
     rescue Gitlab::Git::Repository::NoRepository
       nil
     end
   end
 
+  def update_autocrlf_option
+    raw_repository.autocrlf = :input if raw_repository.autocrlf != :input
+  end
+
   # Return absolute path to repository
   def path_to_repo
     @path_to_repo ||= File.expand_path(
@@ -693,6 +695,8 @@ class Repository
   end
 
   def commit_with_hooks(current_user, branch)
+    update_autocrlf_option
+
     oldrev = Gitlab::Git::BLANK_SHA
     ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
     was_empty = empty?
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 2cd0606a61d..8ff198f572d 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -200,13 +200,22 @@ describe Repository, models: true do
 
   describe :commit_with_hooks do
     context 'when pre hooks were successful' do
-      it 'should run without errors' do
-        expect_any_instance_of(GitHooksService).to receive(:execute).and_return(true)
+      before do
+        expect_any_instance_of(GitHooksService).to receive(:execute).
+          and_return(true)
+      end
 
+      it 'should run without errors' do
         expect do
           repository.commit_with_hooks(user, 'feature') { sample_commit.id }
         end.not_to raise_error
       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
 
     context 'when pre hooks failed' do
@@ -249,6 +258,33 @@ describe Repository, models: true do
     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
     let(:empty_repository) { create(:project_empty_repo).repository }