diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 121e45502fd..45c7fe35f03 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -354,7 +354,11 @@ export default { v-if="!diffFile.submodule && addMergeRequestButtons" class="file-actions d-flex align-items-center gl-ml-auto gl-align-self-start" > - + null, + }, addedLines: { type: Number, required: true, @@ -33,6 +39,12 @@ export default { hasDiffFiles() { return isNumber(this.diffFilesLength) && this.diffFilesLength >= 0; }, + notDiffable() { + return isNotDiffable(this.diffFile); + }, + fileStats() { + return stats(this.diffFile); + }, }, }; @@ -45,23 +57,28 @@ export default { 'd-none d-sm-inline-flex': !isCompareVersionsHeader, }" > -
- - {{ diffFilesCountText }} {{ filesText }} +
+ {{ fileStats.text }}
-
- + - {{ addedLines }} -
-
- - - {{ removedLines }} +
+
+ + {{ diffFilesCountText }} {{ filesText }} +
+
+ + + {{ addedLines }} +
+
+ - + {{ removedLines }} +
diff --git a/app/assets/javascripts/diffs/utils/diff_file.js b/app/assets/javascripts/diffs/utils/diff_file.js index a96c1207a04..363ace2b7c6 100644 --- a/app/assets/javascripts/diffs/utils/diff_file.js +++ b/app/assets/javascripts/diffs/utils/diff_file.js @@ -1,3 +1,5 @@ +import { diffViewerModes as viewerModes } from '~/ide/constants'; +import { changeInPercent, numberToHumanSize } from '~/lib/utils/number_utils'; import { truncateSha } from '~/lib/utils/text_utility'; import { uuids } from '~/lib/utils/uuids'; @@ -46,6 +48,8 @@ function identifier(file) { })[0]; } +export const isNotDiffable = (file) => file?.viewer?.name === viewerModes.not_diffable; + export function prepareRawDiffFile({ file, allFiles, meta = false }) { const additionalProperties = { brokenSymlink: fileSymlinkInformation(file, allFiles), @@ -84,3 +88,35 @@ export function isCollapsed(file) { export function getShortShaFromFile(file) { return file.content_sha ? truncateSha(String(file.content_sha)) : null; } + +export function stats(file) { + let valid = false; + let classes = ''; + let sign = ''; + let text = ''; + let percent = 0; + let diff = 0; + + if (file) { + percent = changeInPercent(file.old_size, file.new_size); + diff = file.new_size - file.old_size; + sign = diff >= 0 ? '+' : ''; + text = `${sign}${numberToHumanSize(diff)} (${sign}${percent}%)`; + valid = true; + + if (diff > 0) { + classes = 'cgreen'; + } else if (diff < 0) { + classes = 'cred'; + } + } + + return { + changed: diff, + text, + percent, + classes, + sign, + valid, + }; +} diff --git a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue index 55fa24fb51a..07821b01dd5 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue @@ -32,9 +32,9 @@ export default { :href="helpPath" :title="__('About this feature')" target="_blank" - class="d-flex-center pl-1" + class="d-flex-center" > - +
diff --git a/app/assets/javascripts/vue_shared/security_reports/components/help_icon.vue b/app/assets/javascripts/vue_shared/security_reports/components/help_icon.vue index 26bc9b5d60e..eed1c86c318 100644 --- a/app/assets/javascripts/vue_shared/security_reports/components/help_icon.vue +++ b/app/assets/javascripts/vue_shared/security_reports/components/help_icon.vue @@ -53,6 +53,6 @@ export default { - + diff --git a/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue b/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue index b7f283b8fd9..d7a3d4e611e 100644 --- a/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue +++ b/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue @@ -191,6 +191,7 @@ export default { @@ -219,6 +220,7 @@ export default { {{ $options.i18n.scansHaveRun }} diff --git a/app/assets/stylesheets/framework/diffs.scss b/app/assets/stylesheets/framework/diffs.scss index 222bfa97b17..c0e9289309a 100644 --- a/app/assets/stylesheets/framework/diffs.scss +++ b/app/assets/stylesheets/framework/diffs.scss @@ -646,6 +646,10 @@ table.code { align-items: center; padding: 0 1rem; + .diff-stats-contents { + display: contents; + } + .diff-stats-group { padding: 0 0.25rem; } diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 136b674f0a9..66816d4c587 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -183,7 +183,12 @@ class GroupsController < Groups::ApplicationController def download_export if @group.export_file_exists? - send_upload(@group.export_file, attachment: @group.export_file.filename) + if @group.export_archive_exists? + send_upload(@group.export_file, attachment: @group.export_file.filename) + else + redirect_to edit_group_path(@group), + alert: _('The file containing the export is not available yet; it may still be transferring. Please try again later.') + end else redirect_to edit_group_path(@group), alert: _('Group export link has expired. Please generate a new export from your group settings.') diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index b37d5466757..53d80b8be58 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -226,7 +226,14 @@ class ProjectsController < Projects::ApplicationController def download_export if @project.export_file_exists? - send_upload(@project.export_file, attachment: @project.export_file.filename) + if @project.export_archive_exists? + send_upload(@project.export_file, attachment: @project.export_file.filename) + else + redirect_to( + edit_project_path(@project, anchor: 'js-export-project'), + alert: _("The file containing the export is not available yet; it may still be transferring. Please try again later.") + ) + end else redirect_to( edit_project_path(@project, anchor: 'js-export-project'), diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 46414b49f37..efdad22fa54 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -338,6 +338,8 @@ module ApplicationSettingsHelper :version_check_enabled, :web_ide_clientside_preview_enabled, :diff_max_patch_bytes, + :diff_max_files, + :diff_max_lines, :commit_email_hostname, :protected_ci_variables, :local_markdown_version, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 65800e40d6c..f8047ed9b78 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -273,6 +273,18 @@ class ApplicationSetting < ApplicationRecord greater_than_or_equal_to: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, less_than_or_equal_to: Gitlab::Git::Diff::MAX_PATCH_BYTES_UPPER_BOUND } + validates :diff_max_files, + presence: true, + numericality: { only_integer: true, + greater_than_or_equal_to: Commit::DEFAULT_MAX_DIFF_FILES_SETTING, + less_than_or_equal_to: Commit::MAX_DIFF_FILES_SETTING_UPPER_BOUND } + + validates :diff_max_lines, + presence: true, + numericality: { only_integer: true, + greater_than_or_equal_to: Commit::DEFAULT_MAX_DIFF_LINES_SETTING, + less_than_or_equal_to: Commit::MAX_DIFF_LINES_SETTING_UPPER_BOUND } + validates :user_default_internal_regex, js_regex: true, allow_nil: true validates :personal_access_token_prefix, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index bf9df3b9efc..b613e698471 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -60,6 +60,8 @@ module ApplicationSettingImplementation default_projects_limit: Settings.gitlab['default_projects_limit'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, + diff_max_files: Commit::DEFAULT_MAX_DIFF_FILES_SETTING, + diff_max_lines: Commit::DEFAULT_MAX_DIFF_LINES_SETTING, disable_feed_token: false, disabled_oauth_sign_in_sources: [], dns_rebinding_protection_enabled: true, diff --git a/app/models/commit.rb b/app/models/commit.rb index 23c1dffcc63..a1ed5eb9ab9 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -33,6 +33,12 @@ class Commit # Used by GFM to match and present link extensions on node texts and hrefs. LINK_EXTENSION_PATTERN = /(patch)/.freeze + DEFAULT_MAX_DIFF_LINES_SETTING = 50_000 + DEFAULT_MAX_DIFF_FILES_SETTING = 1_000 + MAX_DIFF_LINES_SETTING_UPPER_BOUND = 100_000 + MAX_DIFF_FILES_SETTING_UPPER_BOUND = 3_000 + DIFF_SAFE_LIMIT_FACTOR = 10 + cache_markdown_field :title, pipeline: :single_line cache_markdown_field :full_title, pipeline: :single_line, limit: 1.kilobyte cache_markdown_field :description, pipeline: :commit_description, limit: 1.megabyte @@ -78,20 +84,24 @@ class Commit end def diff_safe_lines(project: nil) - Gitlab::Git::DiffCollection.default_limits(project: project)[:max_lines] + diff_safe_max_lines(project: project) end - def diff_hard_limit_files(project: nil) + def diff_max_files(project: nil) if Feature.enabled?(:increased_diff_limits, project) 3000 + elsif Feature.enabled?(:configurable_diff_limits, project) + Gitlab::CurrentSettings.diff_max_files else 1000 end end - def diff_hard_limit_lines(project: nil) + def diff_max_lines(project: nil) if Feature.enabled?(:increased_diff_limits, project) 100000 + elsif Feature.enabled?(:configurable_diff_limits, project) + Gitlab::CurrentSettings.diff_max_lines else 50000 end @@ -99,11 +109,19 @@ class Commit def max_diff_options(project: nil) { - max_files: diff_hard_limit_files(project: project), - max_lines: diff_hard_limit_lines(project: project) + max_files: diff_max_files(project: project), + max_lines: diff_max_lines(project: project) } end + def diff_safe_max_files(project: nil) + diff_max_files(project: project) / DIFF_SAFE_LIMIT_FACTOR + end + + def diff_safe_max_lines(project: nil) + diff_max_lines(project: project) / DIFF_SAFE_LIMIT_FACTOR + end + def from_hash(hash, container) raw_commit = Gitlab::Git::Commit.new(container.repository.raw, hash) new(raw_commit, container) diff --git a/app/models/group.rb b/app/models/group.rb index 6c04bbdb032..1ce2fc18979 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -647,13 +647,17 @@ class Group < Namespace end def export_file_exists? - export_file&.file + import_export_upload&.export_file_exists? end def export_file import_export_upload&.export_file end + def export_archive_exists? + import_export_upload&.export_archive_exists? + end + def adjourned_deletion? false end diff --git a/app/models/import_export_upload.rb b/app/models/import_export_upload.rb index fce4ef40902..bc363cce8dd 100644 --- a/app/models/import_export_upload.rb +++ b/app/models/import_export_upload.rb @@ -11,10 +11,42 @@ class ImportExportUpload < ApplicationRecord mount_uploader :import_file, ImportExportUploader mount_uploader :export_file, ImportExportUploader + # This causes CarrierWave v1 and v3 (but not v2) to upload the file to + # object storage *after* the database entry has been committed to the + # database. This avoids idling in a transaction. + if Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_STORE_EXPORT_FILE_AFTER_COMMIT', true)) + skip_callback :save, :after, :store_export_file! + set_callback :commit, :after, :store_export_file! + end + scope :updated_before, ->(date) { where('updated_at < ?', date) } scope :with_export_file, -> { where.not(export_file: nil) } def retrieve_upload(_identifier, paths) Upload.find_by(model: self, path: paths) end + + def export_file_exists? + !!carrierwave_export_file + end + + # This checks if the export archive is actually stored on disk. It + # requires a HEAD request if object storage is used. + def export_archive_exists? + !!carrierwave_export_file&.exists? + # Handle any HTTP unexpected error + # https://github.com/excon/excon/blob/bbb5bd791d0bb2251593b80e3bce98dbec6e8f24/lib/excon/error.rb#L129-L169 + rescue Excon::Error => e + # The HEAD request will fail with a 403 Forbidden if the file does not + # exist, and the user does not have permission to list the object + # storage bucket. + Gitlab::ErrorTracking.track_exception(e) + false + end + + private + + def carrierwave_export_file + export_file&.file + end end diff --git a/app/models/project.rb b/app/models/project.rb index 3a89a85d65d..a28389c359f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1987,7 +1987,11 @@ class Project < ApplicationRecord end def export_file_exists? - export_file&.file + import_export_upload&.export_file_exists? + end + + def export_archive_exists? + import_export_upload&.export_archive_exists? end def export_file diff --git a/app/views/admin/application_settings/_diff_limits.html.haml b/app/views/admin/application_settings/_diff_limits.html.haml index 5351ac5abd1..6a51d2e39d4 100644 --- a/app/views/admin/application_settings/_diff_limits.html.haml +++ b/app/views/admin/application_settings/_diff_limits.html.haml @@ -3,11 +3,30 @@ %fieldset .form-group - = f.label :diff_max_patch_bytes, _('Maximum diff patch size in bytes'), class: 'label-light' + = f.label :diff_max_patch_bytes, _('Maximum diff patch size (Bytes)'), class: 'label-light' = f.number_field :diff_max_patch_bytes, class: 'form-control gl-form-input' %span.form-text.text-muted - = _("Collapse diffs larger than this size, and show a 'too large' message instead.") + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") = link_to sprite_icon('question-o'), - help_page_path('user/admin_area/diff_limits') + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') + + = f.label :diff_max_files, _('Maximum files in a diff'), class: 'label-light' + = f.number_field :diff_max_files, class: 'form-control gl-form-input' + %span.form-text.text-muted + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") + + = link_to sprite_icon('question-o'), + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') + + = f.label :diff_max_lines, _('Maximum lines in a diff'), class: 'label-light' + = f.number_field :diff_max_lines, class: 'form-control gl-form-input' + %span.form-text.text-muted + = _("Diff files surpassing this limit will be presented as 'too large' and won't be expandable.") + + = link_to sprite_icon('question-o'), + help_page_path('user/admin_area/diff_limits', + anchor: 'diff-limits-administration') = f.submit _('Save changes'), class: 'gl-button btn btn-confirm' diff --git a/app/views/admin/services/_service_templates_deprecated_alert.html.haml b/app/views/admin/services/_service_templates_deprecated_alert.html.haml index 0cc44099049..eac2f9c7f4e 100644 --- a/app/views/admin/services/_service_templates_deprecated_alert.html.haml +++ b/app/views/admin/services/_service_templates_deprecated_alert.html.haml @@ -2,7 +2,9 @@ - settings_link_start = "".html_safe .gl-alert.gl-alert-danger.gl-mt-5{ role: 'alert' } - = sprite_icon('error', css_class: 'gl-alert-icon gl-alert-icon-no-title') - %h4.gl-alert-title= s_('AdminSettings|Service templates are deprecated and will be removed in GitLab 14.0.') - .gl-alert-body - = html_escape_once(s_("AdminSettings|You can't add new templates. To migrate or remove a Service template, create a new integration at %{settings_link_start}Settings > Integrations%{link_end}. Learn more about %{doc_link_start}Project integration management%{link_end}.")).html_safe % { settings_link_start: settings_link_start, doc_link_start: doc_link_start, link_end: ''.html_safe } + .gl-alert-container + = sprite_icon('error', css_class: 'gl-alert-icon gl-alert-icon-no-title') + .gl-alert-content + %h4.gl-alert-title= s_('AdminSettings|Service templates are deprecated and will be removed in GitLab 14.0.') + .gl-alert-body + = html_escape_once(s_("AdminSettings|You can't add new templates. To migrate or remove a Service template, create a new integration at %{settings_link_start}Settings > Integrations%{link_end}. Learn more about %{doc_link_start}Project integration management%{link_end}.")).html_safe % { settings_link_start: settings_link_start, doc_link_start: doc_link_start, link_end: ''.html_safe } diff --git a/app/views/layouts/nav/projects_dropdown/_show.html.haml b/app/views/layouts/nav/projects_dropdown/_show.html.haml index 85a5486ccdc..46070975566 100644 --- a/app/views/layouts/nav/projects_dropdown/_show.html.haml +++ b/app/views/layouts/nav/projects_dropdown/_show.html.haml @@ -18,10 +18,10 @@ - experiment(:new_repo, user: current_user) do |e| - e.use do = nav_link(path: 'projects/new#blank_project', html_options: { class: 'gl-border-0 gl-border-t-1 gl-border-solid gl-border-gray-100' }) do - = link_to new_project_path(anchor: 'blank_project'), data: { track_label: "projects_dropdown_blank_project", track_event: "click_link", track_experiment: "new_repo" } do + = link_to new_project_path(anchor: 'blank_project'), data: { track_label: "projects_dropdown_blank_project", track_event: "click_link", track_experiment: "new_repo", qa_selector: "create_project_link" } do = _('Create blank project') = nav_link(path: 'projects/new#import_project') do - = link_to new_project_path(anchor: 'import_project'), data: { track_label: "projects_dropdown_import_project", track_event: "click_link", track_experiment: "new_repo" } do + = link_to new_project_path(anchor: 'import_project'), data: { track_label: "projects_dropdown_import_project", track_event: "click_link", track_experiment: "new_repo", qa_selector: "import_project_link" } do = _('Import project') - e.try do = nav_link(path: 'projects/new#blank_project', html_options: { class: 'gl-border-0 gl-border-t-1 gl-border-solid gl-border-gray-100' }) do diff --git a/config/feature_flags/development/configurable_diff_limits.yml b/config/feature_flags/development/configurable_diff_limits.yml new file mode 100644 index 00000000000..e73d45fac65 --- /dev/null +++ b/config/feature_flags/development/configurable_diff_limits.yml @@ -0,0 +1,8 @@ +--- +name: configurable_diff_limits +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56722 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332194 +milestone: '14.0' +type: development +group: group::code review +default_enabled: false diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index 06d2929cd54..c8ea494bc41 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -2,15 +2,21 @@ require 'digest/md5' -MESSAGE = <= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)), diff --git a/doc/api/settings.md b/doc/api/settings.md index 76ae0174496..8225713fc00 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -251,7 +251,9 @@ listed in the descriptions of the relevant settings. | `default_projects_limit` | integer | no | Project limit per user. Default is `100000`. | | `default_snippet_visibility` | string | no | What visibility level new snippets receive. Can take `private`, `internal` and `public` as a parameter. Default is `private`. | | `deletion_adjourned_period` | integer | no | **(PREMIUM SELF)** The number of days to wait before deleting a project or group that is marked for deletion. Value must be between 0 and 90. -| `diff_max_patch_bytes` | integer | no | Maximum diff patch size (Bytes). | +| `diff_max_patch_bytes` | integer | no | Maximum [diff patch size](../user/admin_area/diff_limits.md), in bytes. | +| `diff_max_files` | integer | no | Maximum [files in a diff](../user/admin_area/diff_limits.md). | +| `diff_max_lines` | integer | no | Maximum [lines in a diff](../user/admin_area/diff_limits.md). | | `disable_feed_token` | boolean | no | Disable display of RSS/Atom and calendar feed tokens ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/231493) in GitLab 13.7) | | `disabled_oauth_sign_in_sources` | array of strings | no | Disabled OAuth sign-in sources. | | `dns_rebinding_protection_enabled` | boolean | no | Enforce DNS rebinding attack protection. | diff --git a/doc/user/admin_area/diff_limits.md b/doc/user/admin_area/diff_limits.md index c6c2190b7c7..37fdb3ae195 100644 --- a/doc/user/admin_area/diff_limits.md +++ b/doc/user/admin_area/diff_limits.md @@ -12,17 +12,25 @@ You can set a maximum size for display of diff files (patches). For details about diff files, [view changes between files](../project/merge_requests/changes.md). Read more about the [built-in limits for merge requests and diffs](../../administration/instance_limits.md#merge-requests). -## Maximum diff patch size +## Configure diff limits -Diff files which exceed this value are presented as 'too large' and cannot -be expandable. Instead of an expandable view, a link to the blob view is -shown. +WARNING: +These settings are experimental. An increased maximum increases resource +consumption of your instance. Keep this in mind when adjusting the maximum. -Patches greater than 10% of this size are automatically collapsed, and a -link to expand the diff is presented. -This affects merge requests and branch comparison views. +To speed the loading time of merge request views and branch comparison views +on your instance, you can configure three instance-level maximum values for diffs: -To set the maximum diff patch size: +- **Maximum diff patch size**: The total size, in bytes, of the entire diff. +- **Maximum diff files**: The total number of files changed in a diff. +- **Maximum diff files**: The total number of files changed in a diff. The default value is 1000. +- **Maximum diff lines**: The total number of lines changed in a diff. The default value is 50,000. + +When a diff reaches 10% of any of these values, the files are shown in a +collapsed view, with a link to expand the diff. Diffs that exceed any of the +set values are presented as **Too large** are cannot be expanded in the UI. + +To configure these values: 1. On the top bar, select **Menu >** **{admin}** **Admin**. 1. In the left sidebar, select **Settings > General**. @@ -30,10 +38,6 @@ To set the maximum diff patch size: 1. Enter a value for **Maximum diff patch size**, measured in bytes. 1. Select **Save changes**. -WARNING: -This setting is experimental. An increased maximum increases resource -consumption of your instance. Keep this in mind when adjusting the maximum. -