diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-07-15 17:28:03 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-07-15 17:28:03 +0000 |
commit | 2d6c4d2bb05e52731877ad17a0a78d0d3609ebcc (patch) | |
tree | cb999279f07839c0b6c65244b1704101ba6f4957 | |
parent | d1001e3d98ec294db60f08762c5006e8a8f8c98f (diff) | |
parent | 1d5965267b83f84d00efcddc624813b010e6510a (diff) | |
download | gitlab-ce-2d6c4d2bb05e52731877ad17a0a78d0d3609ebcc.tar.gz |
Merge branch 'fix-huge-mr' into 'master'
Fix huge merge requests
* Refactored suppress logic for diffs
* Increase grit memory limit (should mix most of 502 errors when create MR)
* show first 100 files for huge diffs
Fixes #1424
See merge request !970
23 files changed, 337 insertions, 302 deletions
diff --git a/app/assets/stylesheets/sections/commits.scss b/app/assets/stylesheets/sections/commits.scss index 9b148390115..684e8377a7b 100644 --- a/app/assets/stylesheets/sections/commits.scss +++ b/app/assets/stylesheets/sections/commits.scss @@ -112,7 +112,9 @@ .commit-stat-summary { color: #666; - line-height: 2; + font-size: 14px; + font-weight: normal; + padding: 10px 0; } .commit-info-row { diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 860ab408299..c344297ba8a 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -20,11 +20,10 @@ class Projects::CommitController < Projects::ApplicationController end begin - @suppress_diff = true if commit.diff_suppress? && !params[:force_show_diff] - @force_suppress_diff = commit.diff_force_suppress? + @diffs = @commit.diffs rescue Grit::Git::GitTimeout - @suppress_diff = true - @status = :huge_commit + @diffs = [] + @diff_timeout = true end @note = project.build_commit_note(commit) @@ -38,12 +37,7 @@ class Projects::CommitController < Projects::ApplicationController } respond_to do |format| - format.html do - if @status == :huge_commit - render "huge_commit" and return - end - end - + format.html format.diff { render text: @commit.to_diff } format.patch { render text: @commit.to_patch } end diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 234b6058ff0..eae96396574 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -15,11 +15,7 @@ class Projects::CompareController < Projects::ApplicationController @diffs = compare.diffs @refs_are_same = compare.same @line_notes = [] - @timeout = compare.timeout - - diff_line_count = Commit::diff_line_count(@diffs) - @suppress_diff = Commit::diff_suppress?(@diffs, diff_line_count) && !params[:force_show_diff] - @force_suppress_diff = Commit::diff_force_suppress?(@diffs, diff_line_count) + @diff_timeout = compare.timeout end def create diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index bdd614973b7..fcc6384e27c 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -34,6 +34,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def show @note_counts = Note.where(commit_id: @merge_request.commits.map(&:id)). group(:commit_id).count + respond_to do |format| format.html format.diff { render text: @merge_request.to_diff(current_user) } @@ -43,16 +44,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController def diffs @commit = @merge_request.last_commit - @comments_allowed = @reply_allowed = true - @comments_target = {noteable_type: 'MergeRequest', - noteable_id: @merge_request.id} + @comments_target = { + noteable_type: 'MergeRequest', + noteable_id: @merge_request.id + } @line_notes = @merge_request.notes.where("line_code is not null") - diff_line_count = Commit::diff_line_count(@merge_request.diffs) - @suppress_diff = Commit::diff_suppress?(@merge_request.diffs, diff_line_count) && !params[:force_show_diff] - @force_suppress_diff = Commit::diff_force_suppress?(@merge_request.diffs, diff_line_count) - respond_to do |format| format.html format.json { render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } } @@ -60,54 +58,22 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def new - params[:merge_request] ||= ActionController::Parameters.new( - source_project: @project - ) - - @merge_request = MergeRequest.new(merge_request_params) - @merge_request.source_project = @project unless @merge_request.source_project - @merge_request.target_project ||= (@project.forked_from_project || @project) - @target_branches = @merge_request.target_project.nil? ? [] : @merge_request.target_project.repository.branch_names - @merge_request.target_branch ||= @merge_request.target_project.default_branch - @source_project = @merge_request.source_project - - if @merge_request.target_branch && @merge_request.source_branch - compare_action = Gitlab::Satellite::CompareAction.new( - current_user, - @merge_request.target_project, - @merge_request.target_branch, - @merge_request.source_project, - @merge_request.source_branch - ) - - @compare_failed = false - @commits = compare_action.commits - - if @commits - @commits.map! { |commit| Commit.new(commit) } - @commit = @commits.first - else - # false value because failed to get commits from satellite - @commits = [] - @compare_failed = true - end - - @note_counts = Note.where(commit_id: @commits.map(&:id)). - group(:commit_id).count - - begin - @diffs = compare_action.diffs - @merge_request.title = @merge_request.source_branch.titleize.humanize - @target_project = @merge_request.target_project - @target_repo = @target_project.repository - - diff_line_count = Commit::diff_line_count(@diffs) - @suppress_diff = Commit::diff_suppress?(@diffs, diff_line_count) - @force_suppress_diff = @suppress_diff - rescue Gitlab::Satellite::BranchesWithoutParent - @error = "Selected branches have no common commit so they cannot be merged." - end - end + params[:merge_request] ||= ActionController::Parameters.new(source_project: @project) + @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params).execute + + @target_branches = if @merge_request.target_project + @merge_request.target_project.repository.branch_names + else + [] + end + + @target_project = merge_request.target_project + @source_project = merge_request.source_project + @commits = @merge_request.compare_commits + @commit = @merge_request.compare_base_commit + @diffs = @merge_request.compare_diffs + @note_counts = Note.where(commit_id: @commits.map(&:id)). + group(:commit_id).count end def edit diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb new file mode 100644 index 00000000000..ee4d4fbdff5 --- /dev/null +++ b/app/helpers/diff_helper.rb @@ -0,0 +1,22 @@ +module DiffHelper + def safe_diff_files(diffs) + if diff_hard_limit_enabled? + diffs.first(Commit::DIFF_HARD_LIMIT_FILES) + else + diffs.first(Commit::DIFF_SAFE_FILES) + end + end + + def show_diff_size_warninig?(diffs) + safe_diff_files(diffs).size < diffs.size + end + + def diff_hard_limit_enabled? + # Enabling hard limit allows user to see more diff information + if params[:force_show_diff].present? + true + else + false + end + end +end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index ba4c7068e90..bc2ec84302d 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -221,4 +221,10 @@ module ProjectsHelper "Never" end end + + def contribution_guide_url(project) + if project && project.repository.contribution_guide + project_blob_path(project, tree_join(project.default_branch, project.repository.contribution_guide.name)) + end + end end diff --git a/app/models/commit.rb b/app/models/commit.rb index 82876e10446..ff5392957ce 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -12,6 +12,7 @@ class Commit # User can force display of diff above this size DIFF_SAFE_FILES = 100 DIFF_SAFE_LINES = 5000 + # Commits above this size will not be rendered in HTML DIFF_HARD_LIMIT_FILES = 1000 DIFF_HARD_LIMIT_LINES = 50000 @@ -23,23 +24,7 @@ class Commit # Calculate number of lines to render for diffs def diff_line_count(diffs) - diffs.reduce(0){|sum, d| sum + d.diff.lines.count} - end - - def diff_suppress?(diffs, line_count = nil) - # optimize - check file count first - return true if diffs.size > DIFF_SAFE_FILES - - line_count ||= Commit::diff_line_count(diffs) - line_count > DIFF_SAFE_LINES - end - - def diff_force_suppress?(diffs, line_count = nil) - # optimize - check file count first - return true if diffs.size > DIFF_HARD_LIMIT_FILES - - line_count ||= Commit::diff_line_count(diffs) - line_count > DIFF_HARD_LIMIT_LINES + diffs.reduce(0) { |sum, d| sum + d.diff.lines.count } end end @@ -60,14 +45,6 @@ class Commit @diff_line_count end - def diff_suppress? - Commit::diff_suppress?(self.diffs, diff_line_count) - end - - def diff_force_suppress? - Commit::diff_force_suppress?(self.diffs, diff_line_count) - end - # Returns a string describing the commit for use in a link title # # Example diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 597e02d498b..28486fb41c6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -42,6 +42,11 @@ class MergeRequest < ActiveRecord::Base # It allows us to close or modify broken merge requests attr_accessor :allow_broken + # Temporary fields to store compare vars + # when creating new merge request + attr_accessor :can_be_created, :compare_failed, :compare_base_commit, + :compare_commits, :compare_diffs + ActsAsTaggableOn.strict_case_match = true acts_as_taggable_on :labels diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb new file mode 100644 index 00000000000..fb487539687 --- /dev/null +++ b/app/services/merge_requests/build_service.rb @@ -0,0 +1,70 @@ +module MergeRequests + class BuildService < MergeRequests::BaseService + def execute + merge_request = MergeRequest.new(params) + + # Set MR attributes + merge_request.can_be_created = false + merge_request.compare_failed = false + merge_request.compare_commits = [] + merge_request.compare_diffs = [] + merge_request.source_project = project unless merge_request.source_project + merge_request.target_project ||= (project.forked_from_project || project) + merge_request.target_branch ||= merge_request.target_project.default_branch + + unless merge_request.target_branch && merge_request.source_branch + return build_failed(merge_request, "You must select source and target branches") + end + + # Generate suggested MR title based on source branch name + merge_request.title = merge_request.source_branch.titleize.humanize + + # Try to compare branches to get commits list and diffs + compare_action = Gitlab::Satellite::CompareAction.new( + current_user, + merge_request.target_project, + merge_request.target_branch, + merge_request.source_project, + merge_request.source_branch + ) + + commits = compare_action.commits + + # At this point we decide if merge request can be created + # If we have at least one commit to merge -> creation allowed + if commits.present? + merge_request.compare_commits = Commit.decorate(commits) + merge_request.compare_base_commit = Commit.new(commits.first) + merge_request.can_be_created = true + merge_request.compare_failed = false + + # Try to collect diff for merge request. + diffs = compare_action.diffs + + if diffs.present? + merge_request.compare_diffs = diffs + + elsif diffs == false + # satellite timeout return false + merge_request.can_be_created = false + merge_request.compare_failed = true + end + else + merge_request.can_be_created = false + merge_request.compare_failed = true + end + + merge_request + + rescue Gitlab::Satellite::BranchesWithoutParent + return build_failed(merge_request, "Selected branches have no common commit so they cannot be merged.") + end + + def build_failed(merge_request, message) + merge_request.errors.add(:base, message) + merge_request.compare_commits = [] + merge_request.can_be_created = false + merge_request + end + end +end diff --git a/app/views/projects/commit/huge_commit.html.haml b/app/views/projects/commit/huge_commit.html.haml deleted file mode 100644 index 398ce771426..00000000000 --- a/app/views/projects/commit/huge_commit.html.haml +++ /dev/null @@ -1,3 +0,0 @@ -= render "projects/commit/commit_box" -.alert.alert-danger - %h4 Commit diffs are too big to be displayed diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index da1b4c10f87..0a15aef6cb7 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -1,3 +1,3 @@ = render "commit_box" -= render "projects/commits/diffs", diffs: @commit.diffs, project: @project += render "projects/commits/diffs", diffs: @diffs, project: @project = render "projects/notes/notes_with_form" diff --git a/app/views/projects/commits/_diff_file.html.haml b/app/views/projects/commits/_diff_file.html.haml new file mode 100644 index 00000000000..45d1cd9c9a0 --- /dev/null +++ b/app/views/projects/commits/_diff_file.html.haml @@ -0,0 +1,48 @@ +- file = project.repository.blob_at(@commit.id, diff.new_path) +- file = project.repository.blob_at(@commit.parent_id, diff.old_path) unless file +- return unless file +.diff-file{id: "diff-#{i}"} + .diff-header{id: "file-path-#{hexdigest(diff.new_path || diff.old_path)}"} + - if diff.deleted_file + %span= diff.old_path + + .diff-btn-group + - if @commit.parent_ids.present? + = link_to project_blob_path(project, tree_join(@commit.parent_id, diff.new_path)), { class: 'btn btn-small view-file' } do + View file @ + %span.commit-short-id= @commit.short_id(6) + - else + %span= diff.new_path + - if diff_file_mode_changed?(diff) + %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}" + + .diff-btn-group + = link_to "#", class: "js-toggle-diff-comments btn btn-small" do + %i.icon-chevron-down + Diff comments + + + - if @merge_request && @merge_request.source_project + = link_to project_edit_tree_path(@merge_request.source_project, tree_join(@merge_request.source_branch, diff.new_path), from_merge_request_id: @merge_request.id), { class: 'btn btn-small' } do + Edit + + + = link_to project_blob_path(project, tree_join(@commit.id, diff.new_path)), { class: 'btn btn-small view-file' } do + View file @ + %span.commit-short-id= @commit.short_id(6) + + + .diff-content + -# Skipp all non non-supported blobs + - return unless file.respond_to?('text?') + - if file.text? + - if params[:view] == 'parallel' + = render "projects/commits/parallel_view", diff: diff, project: project, file: file, index: i + - else + = render "projects/commits/text_file", diff: diff, index: i + - elsif file.image? + - old_file = project.repository.blob_at(@commit.parent_id, diff.old_path) if @commit.parent_id + = render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i + - else + .nothing-here-block No preview for this file type + diff --git a/app/views/projects/commits/_diff_head.html.haml b/app/views/projects/commits/_diff_head.html.haml deleted file mode 100644 index 5aa542287fe..00000000000 --- a/app/views/projects/commits/_diff_head.html.haml +++ /dev/null @@ -1,26 +0,0 @@ -%ul.bordered-list - - diffs.each_with_index do |diff, i| - %li - - if diff.deleted_file - %span.deleted-file - %a{href: "#diff-#{i}"} - %i.icon-minus - = diff.old_path - - elsif diff.renamed_file - %span.renamed-file - %a{href: "#diff-#{i}"} - %i.icon-minus - = diff.old_path - = "->" - = diff.new_path - - elsif diff.new_file - %span.new-file - %a{href: "#diff-#{i}"} - %i.icon-plus - = diff.new_path - - else - %span.edit-file - %a{href: "#diff-#{i}"} - %i.icon-adjust - = diff.new_path - diff --git a/app/views/projects/commits/_diff_stats.html.haml b/app/views/projects/commits/_diff_stats.html.haml new file mode 100644 index 00000000000..846a1ee10e6 --- /dev/null +++ b/app/views/projects/commits/_diff_stats.html.haml @@ -0,0 +1,41 @@ +.js-toggle-container + .commit-stat-summary + Showing + %strong.cdark #{pluralize(diffs.count, "changed file")} + - if current_controller?(:commit) + - unless @commit.has_zero_stats? + with + %strong.cgreen #{@commit.stats.additions} additions + and + %strong.cred #{@commit.stats.deletions} deletions + + = link_to '#', class: 'btn btn-small js-toggle-button' do + Show diff stats + %i.icon-chevron-down + .file-stats.js-toggle-content.hide + %ul.bordered-list + - diffs.each_with_index do |diff, i| + %li + - if diff.deleted_file + %span.deleted-file + %a{href: "#diff-#{i}"} + %i.icon-minus + = diff.old_path + - elsif diff.renamed_file + %span.renamed-file + %a{href: "#diff-#{i}"} + %i.icon-minus + = diff.old_path + = "->" + = diff.new_path + - elsif diff.new_file + %span.new-file + %a{href: "#diff-#{i}"} + %i.icon-plus + = diff.new_path + - else + %span.edit-file + %a{href: "#diff-#{i}"} + %i.icon-adjust + = diff.new_path + diff --git a/app/views/projects/commits/_diff_warning.html.haml b/app/views/projects/commits/_diff_warning.html.haml new file mode 100644 index 00000000000..05d516efa11 --- /dev/null +++ b/app/views/projects/commits/_diff_warning.html.haml @@ -0,0 +1,19 @@ +.bs-callout.bs-callout-warning + %h4 + Too many changes. + .pull-right + - unless diff_hard_limit_enabled? + = link_to "Reload with full diff", url_for(params.merge(force_show_diff: true)), class: "btn btn-small btn-warning" + + - if current_controller?(:commit) or current_controller?(:merge_requests) + - if current_controller?(:commit) + = link_to "Plain diff", project_commit_path(@project, @commit, format: :diff), class: "btn btn-warning btn-small" + = link_to "Email patch", project_commit_path(@project, @commit, format: :patch), class: "btn btn-warning btn-small" + - elsif @merge_request && @merge_request.persisted? + = link_to "Plain diff", project_merge_request_path(@project, @merge_request, format: :diff), class: "btn btn-warning btn-small" + = link_to "Email patch", project_merge_request_path(@project, @merge_request, format: :patch), class: "btn btn-warning btn-small" + %p + To preserve performance only + %strong #{safe_diff_files(diffs).size} of #{diffs.size} + files displayed. + diff --git a/app/views/projects/commits/_diffs.html.haml b/app/views/projects/commits/_diffs.html.haml index fcdb40468d9..64d6a2f09cf 100644 --- a/app/views/projects/commits/_diffs.html.haml +++ b/app/views/projects/commits/_diffs.html.haml @@ -1,91 +1,23 @@ -- @suppress_diff ||= @suppress_diff || @force_suppress_diff -- if @suppress_diff - .alert.alert-warning - %p - %strong Warning! This is a large diff. - %p - To preserve performance the diff is not shown. - - if current_controller?(:commit) or current_controller?(:merge_requests) - - if current_controller?(:commit) - Please, download the diff as - = link_to "plain diff", project_commit_path(@project, @commit, format: :diff), class: "underlined-link" - or - = link_to "email patch", project_commit_path(@project, @commit, format: :patch), class: "underlined-link" - instead. - - elsif @merge_request && @merge_request.persisted? - Please, download the diff as - = link_to "plain diff", project_merge_request_path(@project, @merge_request, format: :diff), class: "underlined-link" - or - = link_to "email patch", project_merge_request_path(@project, @merge_request, format: :patch), class: "underlined-link" - instead. - - unless @force_suppress_diff - %p - If you still want to see the diff - = link_to "click this link", url_for(force_show_diff: true), class: "underlined-link" - -%p.commit-stat-summary - Showing - %strong.cdark #{pluralize(diffs.count, "changed file")} - - if current_controller?(:commit) - - unless @commit.has_zero_stats? - with - %strong.cgreen #{@commit.stats.additions} additions - and - %strong.cred #{@commit.stats.deletions} deletions - - if params[:view] == 'parallel' - = link_to "Inline Diff", url_for(view: 'inline'), {id: "commit-diff-viewtype", class: 'btn btn-tiny pull-right'} - - else - = link_to "Side-by-side Diff", url_for(view: 'parallel'), {id: "commit-diff-viewtype", class: 'btn btn-tiny pull-right'} -.file-stats - = render "projects/commits/diff_head", diffs: diffs +.row + .col-md-8 + = render 'projects/commits/diff_stats', diffs: diffs + .col-md-4 + %ul.nav.nav-tabs + %li.pull-right{class: params[:view] == 'parallel' ? 'active' : ''} + = link_to "Side-by-side Diff", url_for(view: 'parallel'), {id: "commit-diff-viewtype"} + %li.pull-right{class: params[:view] != 'parallel' ? 'active' : ''} + = link_to "Inline Diff", url_for(view: 'inline'), {id: "commit-diff-viewtype"} + +- if show_diff_size_warninig?(diffs) + = render 'projects/commits/diff_warning', diffs: diffs .files - - unless @suppress_diff - - diffs.each_with_index do |diff, i| - - file = project.repository.blob_at(@commit.id, diff.new_path) - - file = project.repository.blob_at(@commit.parent_id, diff.old_path) unless file - - next unless file - .diff-file{id: "diff-#{i}"} - .diff-header{id: "file-path-#{hexdigest(diff.new_path || diff.old_path)}"} - - if diff.deleted_file - %span= diff.old_path - - .diff-btn-group - - if @commit.parent_ids.present? - = link_to project_blob_path(project, tree_join(@commit.parent_id, diff.new_path)), { class: 'btn btn-small view-file' } do - View file @ - %span.commit-short-id= @commit.short_id(6) - - else - %span= diff.new_path - - if diff_file_mode_changed?(diff) - %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}" - - .diff-btn-group - = link_to "#", class: "js-toggle-diff-comments btn btn-small" do - %i.icon-chevron-down - Diff comments - + - safe_diff_files(diffs).each_with_index do |diff, i| + = render 'projects/commits/diff_file', diff: diff, i: i, project: project - - if @merge_request && @merge_request.source_project - = link_to project_edit_tree_path(@merge_request.source_project, tree_join(@merge_request.source_branch, diff.new_path), from_merge_request_id: @merge_request.id), { class: 'btn btn-small' } do - Edit - - - = link_to project_blob_path(project, tree_join(@commit.id, diff.new_path)), { class: 'btn btn-small view-file' } do - View file @ - %span.commit-short-id= @commit.short_id(6) - - - .diff-content - -# Skipp all non non-supported blobs - - next unless file.respond_to?('text?') - - if file.text? - - if params[:view] == 'parallel' - = render "projects/commits/parallel_view", diff: diff, project: project, file: file, index: i - - else - = render "projects/commits/text_file", diff: diff, index: i - - elsif file.image? - - old_file = project.repository.blob_at(@commit.parent_id, diff.old_path) if @commit.parent_id - = render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i - - else - .nothing-here-block No preview for this file type +- if @diff_timeout + .alert.alert-danger + %h4 + Failed to collect changes + %p + Maybe diff is really big and operation failed with timeout. Try to get diff localy diff --git a/app/views/projects/commits/_text_file.html.haml b/app/views/projects/commits/_text_file.html.haml index 8ced4133294..f5b0d711416 100644 --- a/app/views/projects/commits/_text_file.html.haml +++ b/app/views/projects/commits/_text_file.html.haml @@ -1,4 +1,4 @@ -- too_big = diff.diff.lines.count > 1000 +- too_big = diff.diff.lines.count > Commit::DIFF_SAFE_LINES - if too_big %a.supp_diff_link Changes suppressed. Click to show diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml index b232d2a6b26..240bfe7484e 100644 --- a/app/views/projects/compare/show.html.haml +++ b/app/views/projects/compare/show.html.haml @@ -18,18 +18,7 @@ - else %ul.well-list= render Commit.decorate(@commits), project: @project - %h4 Changes - - if @diffs.present? - = render "projects/commits/diffs", diffs: @diffs, project: @project - - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE - .bs-callout.bs-callout-danger - %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits. - %p To preserve performance the line changes are not shown. - - elsif @timeout - .bs-callout.bs-callout-danger - %h4 Number of changed files for this comparison is extremely large. - %p Use command line to browse through changes for this comparison. - + = render "projects/commits/diffs", diffs: @diffs, project: @project - else .light-well diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml index 18e3f419c72..99726172154 100644 --- a/app/views/projects/merge_requests/_new_compare.html.haml +++ b/app/views/projects/merge_requests/_new_compare.html.haml @@ -27,13 +27,13 @@ .panel-footer .mr_target_commit - -if @merge_request.errors.any? + - if @merge_request.errors.any? .alert.alert-danger - @merge_request.errors.full_messages.each do |msg| %div= msg - - if @merge_request.source_branch.present? && @merge_request.target_branch.present? - - if @compare_failed + - elsif @merge_request.source_branch.present? && @merge_request.target_branch.present? + - if @merge_request.compare_failed .alert.alert-danger %h4 Compare failed %p We can't compare selected branches. It may be because of huge diff or satellite timeout. Please try again or select different branches. diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index 6050f20d763..73d364b4f93 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -10,75 +10,75 @@ %span.pull-right = link_to 'Change branches', new_project_merge_request_path(@project) -- if @error.present? - .centered-light-block - %h4 #{@error} -- else - = form_for [@project, @merge_request], html: { class: "merge-request-form" } do |f| - .panel.panel-default - .panel-body - .form-group - .light - = f.label :title do - = "Title *" - = f.text_field :title, class: "form-control input-lg js-gfm-input", maxlength: 255, rows: 5, required: true - .form-group - .light - = f.label :description, "Description" - = f.text_area :description, class: "form-control js-gfm-input markdown-area", rows: 10 - .clearfix.hint - .pull-left Description is parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"), target: '_blank'}. - .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }. - .error-alert - .form-group - .issue-assignee - = f.label :assignee_id do - %i.icon-user - Assign to - %div - = project_users_select_tag('merge_request[assignee_id]', placeholder: 'Select a user', class: 'custom-form-control', selected: @merge_request.assignee_id, project_id: @merge_request.target_project_id) - - = link_to 'Assign to me', '#', class: 'btn assign-to-me-link' - .form-group - .issue-milestone - = f.label :milestone_id do - %i.icon-time - Milestone - %div= f.select(:milestone_id, milestone_options(@merge_request), { include_blank: "Select milestone" }, {class: 'select2'}) - .panel-footer - - if @target_repo.contribution_guide - - contribution_guide_url = project_blob_path(@target_project, tree_join(@target_repo.root_ref, @target_repo.contribution_guide.name)) - %p - Please review the - %strong #{link_to "guidelines for contribution", contribution_guide_url} - to this repository. - = f.hidden_field :source_project_id - = f.hidden_field :target_project_id - = f.hidden_field :target_branch - = f.hidden_field :source_branch - = f.submit 'Submit merge request', class: "btn btn-create" += form_for [@project, @merge_request], html: { class: "merge-request-form" } do |f| + .panel.panel-default - .mr-compare - %div.panel.panel-default - .panel-heading - Commits (#{@commits.count}) - - if @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE - %ul.well-list - - Commit.decorate(@commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE)).each do |commit| - = render "projects/commits/inline_commit", commit: commit, project: @project - %li.warning-row.unstyled - other #{@commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE} commits hidden to prevent performance issues. - - else - %ul.well-list= render Commit.decorate(@commits), project: @project + .panel-body + .form-group + .light + = f.label :title do + = "Title *" + = f.text_field :title, class: "form-control input-lg js-gfm-input", maxlength: 255, rows: 5, required: true + .form-group + .light + = f.label :description, "Description" + = f.text_area :description, class: "form-control js-gfm-input markdown-area", rows: 10 + .clearfix.hint + .pull-left Description is parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"), target: '_blank'}. + .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }. + .error-alert + .form-group + .issue-assignee + = f.label :assignee_id do + %i.icon-user + Assign to + %div + = project_users_select_tag('merge_request[assignee_id]', placeholder: 'Select a user', class: 'custom-form-control', selected: @merge_request.assignee_id, project_id: @merge_request.target_project_id) + + = link_to 'Assign to me', '#', class: 'btn assign-to-me-link' + .form-group + .issue-milestone + = f.label :milestone_id do + %i.icon-time + Milestone + %div= f.select(:milestone_id, milestone_options(@merge_request), { include_blank: "Select milestone" }, {class: 'select2'}) + .panel-footer + - if contribution_guide_url(@target_project) + %p + Please review the + %strong #{link_to "guidelines for contribution", contribution_guide_url(@target_project)} + to this repository. + = f.hidden_field :source_project_id + = f.hidden_field :target_project_id + = f.hidden_field :target_branch + = f.hidden_field :source_branch + = f.submit 'Submit merge request', class: "btn btn-create" - %h4 Changes - - if @diffs.present? - = render "projects/commits/diffs", diffs: @diffs, project: @project - - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE - .bs-callout.bs-callout-danger - %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits. - %p To preserve performance the line changes are not shown. +.mr-compare + %div.panel.panel-default + .panel-heading + Commits (#{@commits.count}) + - if @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE + %ul.well-list + - Commit.decorate(@commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE)).each do |commit| + = render "projects/commits/inline_commit", commit: commit, project: @project + %li.warning-row.unstyled + other #{@commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE} commits hidden to prevent performance issues. + - else + %ul.well-list= render Commit.decorate(@commits), project: @project + + %h4 Changes + - if @diffs.present? + = render "projects/commits/diffs", diffs: @diffs, project: @project + - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE + .bs-callout.bs-callout-danger + %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits. + %p To preserve performance the line changes are not shown. + - else + .bs-callout.bs-callout-danger + %h4 This comparison includes huge diff. + %p To preserve performance the line changes are not shown. :javascript diff --git a/app/views/projects/merge_requests/new.html.haml b/app/views/projects/merge_requests/new.html.haml index c24e5916721..4756903d0e0 100644 --- a/app/views/projects/merge_requests/new.html.haml +++ b/app/views/projects/merge_requests/new.html.haml @@ -1,4 +1,4 @@ -- if @commits.present? +- if @merge_request.can_be_created = render 'new_submit' - else = render 'new_compare' diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index e0afdc0400e..b044aef9157 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -226,7 +226,7 @@ production: &base # The next value is the maximum memory size grit can use # Given in number of bytes per git object (e.g. a commit) # This value can be increased if you have very large commits - max_size: 5242880 # 5.megabytes + max_size: 15242880 # 15.megabytes # Git timeout to read a commit, in seconds timeout: 10 diff --git a/features/steps/project/browse_commits.rb b/features/steps/project/browse_commits.rb index bd944dee610..fe47a731915 100644 --- a/features/steps/project/browse_commits.rb +++ b/features/steps/project/browse_commits.rb @@ -61,8 +61,7 @@ class ProjectBrowseCommits < Spinach::FeatureSteps Then 'I see big commit warning' do page.should have_content BigCommits::BIG_COMMIT_MESSAGE - page.should have_content "Warning! This is a large diff" - page.should have_content "If you still want to see the diff" + page.should have_content "Too many changes" end Given 'I visit huge commit page' do @@ -71,8 +70,6 @@ class ProjectBrowseCommits < Spinach::FeatureSteps Then 'I see huge commit message' do page.should have_content BigCommits::HUGE_COMMIT_MESSAGE - page.should have_content "Warning! This is a large diff" - page.should_not have_content "If you still want to see the diff" end Given 'I visit a commit with an image that changed' do |