Read about our upcoming Code of Conduct on this issue

Commit bfc6a0e3 authored by Rémy Coutable's avatar Rémy Coutable
Browse files

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
......@@ -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
......
......@@ -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)
......
......@@ -901,6 +901,7 @@ def change_head(branch)
repository.rugged.references.create('HEAD',
"refs/heads/#{branch}",
force: true)
repository.copy_gitattributes(branch)
reload_default_branch
end
......
......@@ -938,6 +938,16 @@ def ls_files(ref)
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?
......
......@@ -42,7 +42,12 @@ def execute
# 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 @@ def execute
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
......
......@@ -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
......@@ -795,6 +795,16 @@
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")
......
......@@ -201,6 +201,36 @@ def execute
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
......
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