Merge branch 'gitattributes' into 'master'

Support supressing text file diffs on the default branch with .gitattributes

This change allows users to suppress diffs for text files by adding an entry to the `.gitattributes` file. To take effect the file present at the HEAD of the default branch.

When rendering a diff, if the file is text according to the charlock holmes gem (via the `text?` method) but the file is not diffable according to the project repository, then a message is displayed stating that the diff was suppressed.

![image](/uploads/2e119b725875a301e30d9ad482e283b3/image.png)

I looked into ways to do this using a `binary` flag as suggested by @stanhu in [this comment](https://gitlab.com/gitlab-org/gitlab-ce/issues/2315#note_4435454), however, there was no good way to seperate what was a real binary file from one that had been marked as not diffable in `.gitattributes`.

Fixes and closes gitlab-org/gitlab-ce#2315.

See merge request !3806

8 files changed, 77 insertions, 16 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 9dcb1c1557e..ed151df9e25 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -13,6 +13,7 @@ v 8.8.0 (unreleased)
   - Added button to toggle whitespaces changes on diff view
   - Backport GitLab Enterprise support from EE
   - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718
+  - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
 
 v 8.7.2
   - The "New Branch" button is now loaded asynchronously
diff --git a/Gemfile.lock b/Gemfile.lock
index 91d89b4875a..1dcda0daff6 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -184,7 +184,7 @@ GEM
     encryptor (1.3.0)
     equalizer (0.0.11)
     erubis (2.7.0)
-    escape_utils (1.1.0)
+    escape_utils (1.1.1)
     eventmachine (1.0.8)
     excon (0.45.4)
     execjs (2.6.0)
@@ -334,7 +334,7 @@ GEM
       json
     get_process_mem (0.2.0)
     gherkin-ruby (0.3.2)
-    github-linguist (4.7.5)
+    github-linguist (4.7.6)
       charlock_holmes (~> 0.7.3)
       escape_utils (~> 1.1.0)
       mime-types (>= 1.19)
@@ -351,7 +351,7 @@ GEM
       posix-spawn (~> 0.3)
     gitlab_emoji (0.3.1)
       gemojione (~> 2.2, >= 2.2.1)
-    gitlab_git (10.0.0)
+    gitlab_git (10.0.1)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)
diff --git a/app/models/project.rb b/app/models/project.rb
index 0420c6a61ae..5c6c36e6b31 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -901,6 +901,7 @@ class Project < ActiveRecord::Base
     repository.rugged.references.create('HEAD',
                                         "refs/heads/#{branch}",
                                         force: true)
+    repository.copy_gitattributes(branch)
     reload_default_branch
   end
 
diff --git a/app/models/repository.rb b/app/models/repository.rb
index d495c8d18f5..b4319297e43 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -938,6 +938,16 @@ class Repository
     raw_repository.ls_files(actual_ref)
   end
 
+  def copy_gitattributes(ref)
+    actual_ref = ref || root_ref
+    begin
+      raw_repository.copy_gitattributes(actual_ref)
+      true
+    rescue Gitlab::Git::Repository::InvalidRef
+      false
+    end
+  end
+
   def main_language
     return if empty? || rugged.head_unborn?
 
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 1e1be8cd04b..b7af80055bf 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -42,7 +42,12 @@ class GitPushService < BaseService
       # Collect data for this git push
       @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev])
       process_commit_messages
+
+      # Update the bare repositories info/attributes file using the contents of the default branches
+      # .gitattributes file
+      update_gitattributes if is_default_branch?
     end
+
     # Update merge requests that may be affected by this push. A new branch
     # could cause the last commit of a merge request to change.
     update_merge_requests
@@ -54,6 +59,10 @@ class GitPushService < BaseService
     perform_housekeeping
   end
 
+  def update_gitattributes
+    @project.repository.copy_gitattributes(params[:ref])
+  end
+
   def update_main_language
     # Performance can be bad so for now only check main_language once
     # See https://gitlab.com/gitlab-org/gitlab-ce/issues/14937
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index 83a8d7ae9bf..0f04fc5d33c 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -40,19 +40,19 @@
         = view_file_btn(diff_commit.id, diff_file, project)
 
   .diff-content.diff-wrap-lines
-    -# Skipp all non non-supported blobs
+    - # Skip all non non-supported blobs
     - return unless blob.respond_to?('text?')
     - if diff_file.too_large?
-      .nothing-here-block
-        This diff could not be displayed because it is too large.
-    - else
-      - if blob_text_viewable?(blob)
-        - if diff_view == 'parallel'
-          = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
-        - else
-          = render "projects/diffs/text_file", diff_file: diff_file, index: i
-      - elsif blob.image?
-        - old_file = project.repository.prev_blob_for_diff(diff_commit, diff_file)
-        = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i, diff_refs: diff_refs
+      .nothing-here-block This diff could not be displayed because it is too large.
+    - elsif blob_text_viewable?(blob) && !project.repository.diffable?(blob)
+      .nothing-here-block This diff was suppressed by a .gitattributes entry.
+    - elsif blob_text_viewable?(blob)
+      - if diff_view == 'parallel'
+        = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
       - else
-        .nothing-here-block No preview for this file type
+        = render "projects/diffs/text_file", diff_file: diff_file, index: i
+    - elsif blob.image?
+      - old_file = project.repository.prev_blob_for_diff(diff_commit, diff_file)
+      = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i, diff_refs: diff_refs
+    - else
+      .nothing-here-block No preview for this file type
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index a306cc4aef8..397bb5a8028 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -795,6 +795,16 @@ describe Repository, models: true do
 
   end
 
+  describe "#copy_gitattributes" do
+    it 'returns true with a valid ref' do
+      expect(repository.copy_gitattributes('master')).to be_truthy
+    end
+
+    it 'returns false with an invalid ref' do
+      expect(repository.copy_gitattributes('invalid')).to be_falsey
+    end
+  end
+
   describe "#main_language" do
     it 'shows the main language of the project' do
       expect(repository.main_language).to eq("Ruby")
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index b40a5c1c818..eeab540c2fd 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -201,6 +201,36 @@ describe GitPushService, services: true do
   end
 
 
+  describe "Updates git attributes" do
+    context "for default branch" do
+      it "calls the copy attributes method for the first push to the default branch" do
+        expect(project.repository).to receive(:copy_gitattributes).with('master')
+
+        execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master')
+      end
+
+      it "calls the copy attributes method for changes to the default branch" do
+        expect(project.repository).to receive(:copy_gitattributes).with('refs/heads/master')
+
+        execute_service(project, user, 'oldrev', 'newrev', 'refs/heads/master')
+      end
+    end
+
+    context "for non-default branch" do
+      before do
+        # Make sure the "default" branch is different
+        allow(project).to receive(:default_branch).and_return('not-master')
+      end
+
+      it "does not call copy attributes method" do
+        expect(project.repository).not_to receive(:copy_gitattributes)
+
+        execute_service(project, user, @oldrev, @newrev, @ref)
+      end
+    end
+  end
+
+
   describe "Webhooks" do
     context "execute webhooks" do
       it "when pushing a branch for the first time" do