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

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

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)

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
 

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?
 

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

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

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")

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