Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce

CHANGELOG

@@ -3,8 +3,9 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 8.11.0 (unreleased)
   - Fix of 'Commits being passed to custom hooks are already reachable when using the UI'
   - Limit git rev-list output count to one in forced push check
+  - Retrieve rendered HTML from cache in one request
 
-v 8.10.0 (unreleased)
+v 8.10.0
   - Fix profile activity heatmap to show correct day name (eanplatter)
   - Speed up ExternalWikiHelper#get_project_wiki_path
   - Expose {should,force}_remove_source_branch (Ben Boeckel)
@@ -21,6 +22,7 @@ v 8.10.0 (unreleased)
   - Delete award emoji when deleting a user
   - Remove pinTo from Flash and make inline flash messages look nicer. !4854 (winniehell)
   - Add an API for downloading latest successful build from a particular branch or tag. !5347
+  - Avoid data-integrity issue when cleaning up repository archive cache.
   - Add link to profile to commit avatar. !5163 (winniehell)
   - Wrap code blocks on Activies and Todos page. !4783 (winniehell)
   - Align flash messages with left side of page content. !4959 (winniehell)
@@ -80,6 +82,7 @@ v 8.10.0 (unreleased)
   - API: Todos. !3188 (Robert Schilling)
   - API: Expose shared groups for projects and shared projects for groups. !5050 (Robert Schilling)
   - API: Expose `developers_can_push` and `developers_can_merge` for branches. !5208 (Robert Schilling)
+  - Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects
   - Add "Enabled Git access protocols" to Application Settings
   - Diffs will create button/diff form on demand no on server side
   - Reduce size of HTML used by diff comment forms

Gemfile

@@ -52,7 +52,7 @@ gem 'browser', '~> 2.2'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem 'gitlab_git', '~> 10.3.2'
+gem 'gitlab_git', '~> 10.4.1'
 
 # LDAP Auth
 # GitLab fork with several improvements to original library. For full list of changes
@@ -347,5 +347,5 @@ gem 'paranoia', '~> 2.0'
 gem 'health_check', '~> 2.1.0'
 
 # System information
-gem 'vmstat', '~> 2.1.0'
+gem 'vmstat', '~> 2.1.1'
 gem 'sys-filesystem', '~> 1.1.6'

Gemfile.lock

@@ -274,7 +274,7 @@ GEM
       diff-lcs (~> 1.1)
       mime-types (>= 1.16, < 3)
       posix-spawn (~> 0.3)
-    gitlab_git (10.3.2)
+    gitlab_git (10.4.1)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)
@@ -775,7 +775,7 @@ GEM
       coercible (~> 1.0)
       descendants_tracker (~> 0.0, >= 0.0.3)
       equalizer (~> 0.0, >= 0.0.9)
-    vmstat (2.1.0)
+    vmstat (2.1.1)
     warden (1.2.6)
       rack (>= 1.0)
     web-console (2.3.0)
@@ -861,7 +861,7 @@ DEPENDENCIES
   github-linguist (~> 4.7.0)
   github-markup (~> 1.4)
   gitlab-flowdock-git-hook (~> 1.0.1)
-  gitlab_git (~> 10.3.2)
+  gitlab_git (~> 10.4.1)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.2)
@@ -980,7 +980,7 @@ DEPENDENCIES
   unicorn-worker-killer (~> 0.4.2)
   version_sorter (~> 2.0.0)
   virtus (~> 1.0.1)
-  vmstat (~> 2.1.0)
+  vmstat (~> 2.1.1)
   web-console (~> 2.0)
   webmock (~> 1.21.0)
   wikicloth (= 0.8.1)

VERSION

-8.10.0-pre
+8.11.0-pre

app/assets/javascripts/notes.js.coffee

@@ -162,7 +162,7 @@ class @Notes
         @last_fetched_at = data.last_fetched_at
         @setPollingInterval(data.notes.length)
         $.each notes, (i, note) =>
-          if note.discussion_with_diff_html?
+          if note.discussion_html?
             @renderDiscussionNote(note)
           else
             @renderNote(note)
@@ -251,7 +251,7 @@ class @Notes
       discussionContainer = $(".notes[data-discussion-id='" + note.original_discussion_id + "']")
     if discussionContainer.length is 0
       # insert the note and the reply button after the temp row
-      row.after note.discussion_html
+      row.after note.diff_discussion_html
 
       # remove the note (will be added again below)
       row.next().find(".note").remove()
@@ -265,7 +265,7 @@ class @Notes
       # Init discussion on 'Discussion' page if it is merge request page
       if $('body').attr('data-page').indexOf('projects:merge_request') is 0
         $('ul.main-notes-list')
-          .append(note.discussion_with_diff_html)
+          .append(note.discussion_html)
           .syntaxHighlight()
     else
       # append new note to all matching discussions

app/assets/stylesheets/pages/builds.scss

@@ -53,6 +53,14 @@
       left: 70px;
     }
   }
+
+  .nav-links {
+    svg {
+      position: relative;
+      top: 2px;
+      margin-right: 3px;
+    }
+  }
 }
 
 .build-header {

app/assets/stylesheets/pages/commit.scss

@@ -68,6 +68,12 @@
   }
 }
 
+.ci-status-link {
+  svg {
+    overflow: visible;
+  }
+}
+
 .commit-box {
   border-top: 1px solid $border-color;
 

app/assets/stylesheets/pages/merge_requests.scss

@@ -64,6 +64,7 @@
       margin-right: 4px;
       position: relative;
       top: 1px;
+      overflow: visible;
     }
 
     &.ci-success {

app/assets/stylesheets/pages/pipelines.scss

@@ -29,9 +29,18 @@
   }
 }
 
+.pipeline-holder {
+  width: 100%;
+  overflow: auto;
+}
+
 .table.builds {
   min-width: 1200px;
 
+  &.pipeline {
+    min-width: 650px;
+  }
+
   tr {
     th {
       padding: 16px 8px;
@@ -76,7 +85,7 @@
 
     svg {
       height: 14px;
-      width: auto;
+      width: 14px;
       vertical-align: middle;
       fill: $table-text-gray;
     }
@@ -93,7 +102,7 @@
 
     .commit-title {
       margin-top: 4px;
-      max-width: 320px;
+      max-width: 300px;
       overflow: hidden;
       white-space: nowrap;
       text-overflow: ellipsis;
@@ -138,6 +147,11 @@
       height: 18px;
       width: 18px;
       vertical-align: middle;
+      overflow: visible;
+    }
+
+    .light {
+      width: 3px;
     }
   }
 
@@ -153,7 +167,7 @@
 
     svg {
       width: 12px;
-      height: auto;
+      height: 12px;
       vertical-align: middle;
       margin-right: 4px;
     }

app/assets/stylesheets/pages/status.scss

@@ -49,6 +49,7 @@
       position: relative;
       top: 1px;
       margin: 0 3px;
+      overflow: visible;
     }
   }
 
@@ -74,3 +75,11 @@
     color: $gl-gray;
   }
 }
+
+.visible-xs-inline {
+  .ci-status-link {
+    position: relative;
+    top: 2px;
+    left: 5px;
+  }
+}

app/controllers/projects/commit_controller.rb

@@ -115,11 +115,11 @@ class Projects::CommitController < Projects::ApplicationController
   end
 
   def define_note_vars
-    @grouped_diff_notes = commit.notes.grouped_diff_notes
+    @grouped_diff_discussions = commit.notes.grouped_diff_discussions
     @notes = commit.notes.non_diff_notes.fresh
 
     Banzai::NoteRenderer.render(
-      @grouped_diff_notes.values.flatten + @notes,
+      @grouped_diff_discussions.values.flat_map(&:notes) + @notes,
       @project,
       current_user,
     )

app/controllers/projects/compare_controller.rb

@@ -54,7 +54,7 @@ class Projects::CompareController < Projects::ApplicationController
       )
 
       @diff_notes_disabled = true
-      @grouped_diff_notes = {}
+      @grouped_diff_discussions = {}
     end
   end
 

app/controllers/projects/merge_requests_controller.rb

@@ -97,7 +97,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     else
       build_merge_request
       @diff_notes_disabled = true
-      @grouped_diff_notes = {}
+      @grouped_diff_discussions = {}
     end
 
     define_commit_vars
@@ -378,7 +378,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
     # This is not executed lazily
     @notes = Banzai::NoteRenderer.render(
-      @discussions.flatten,
+      @discussions.flat_map(&:notes),
       @project,
       current_user,
       @path,
@@ -404,10 +404,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     }
 
     @use_legacy_diff_notes = !@merge_request.support_new_diff_notes?
-    @grouped_diff_notes = @merge_request.notes.grouped_diff_notes
+    @grouped_diff_discussions = @merge_request.notes.grouped_diff_discussions
 
     Banzai::NoteRenderer.render(
-      @grouped_diff_notes.values.flatten,
+      @grouped_diff_discussions.values.flat_map(&:notes),
       @project,
       current_user,
       @path,

app/controllers/projects/notes_controller.rb

@@ -73,7 +73,7 @@ class Projects::NotesController < Projects::ApplicationController
   end
   alias_method :awardable, :note
 
-  def note_to_html(note)
+  def note_html(note)
     render_to_string(
       "projects/notes/_note",
       layout: false,
@@ -82,20 +82,20 @@ class Projects::NotesController < Projects::ApplicationController
     )
   end
 
-  def note_to_discussion_html(note)
-    return unless note.diff_note?
+  def diff_discussion_html(discussion)
+    return unless discussion.diff_discussion?
 
     if params[:view] == 'parallel'
-      template = "projects/notes/_diff_notes_with_reply_parallel"
+      template = "discussions/_parallel_diff_discussion"
       locals =
         if params[:line_type] == 'old'
-          { notes_left: [note], notes_right: [] }
+          { discussion_left: discussion, discussion_right: nil }
         else
-          { notes_left: [], notes_right: [note] }
+          { discussion_left: nil, discussion_right: discussion }
         end
     else
-      template = "projects/notes/_diff_notes_with_reply"
-      locals = { notes: [note] }
+      template = "discussions/_diff_discussion"
+      locals = { discussion: discussion }
     end
 
     render_to_string(
@@ -106,14 +106,14 @@ class Projects::NotesController < Projects::ApplicationController
     )
   end
 
-  def note_to_discussion_with_diff_html(note)
-    return unless note.diff_note?
+  def discussion_html(discussion)
+    return unless discussion.diff_discussion?
 
     render_to_string(
-      "projects/notes/_discussion",
+      "discussions/_discussion",
       layout: false,
       formats: [:html],
-      locals: { discussion_notes: [note] }
+      locals: { discussion: discussion }
     )
   end
 
@@ -132,13 +132,19 @@ class Projects::NotesController < Projects::ApplicationController
         valid: true,
         id: note.id,
         discussion_id: note.discussion_id,
-        html: note_to_html(note),
+        html: note_html(note),
         award: false,
-        note: note.note,
-        discussion_html: note_to_discussion_html(note),
-        discussion_with_diff_html: note_to_discussion_with_diff_html(note)
+        note: note.note
       }
 
+      if note.diff_note?
+        discussion = Discussion.new([note])
+
+        attrs.merge!(
+          diff_discussion_html: diff_discussion_html(discussion),
+          discussion_html: discussion_html(discussion)
+        )
+
         # The discussion_id is used to add the comment to the correct discussion
         # element on the merge request page. Among other things, the discussion_id
         # contains the sha of head commit of the merge request.
@@ -153,6 +159,7 @@ class Projects::NotesController < Projects::ApplicationController
         if note.new_diff_note? && note.position != note.original_position
           attrs[:original_discussion_id] = note.original_discussion_id
         end
+      end
 
       attrs
     else

app/helpers/ci_status_helper.rb

@@ -45,10 +45,10 @@ module CiStatusHelper
     custom_icon(icon_name)
   end
 
-  def render_commit_status(commit, tooltip_placement: 'auto left', cssclass: '')
+  def render_commit_status(commit, tooltip_placement: 'auto left')
     project = commit.project
     path = builds_namespace_project_commit_path(project.namespace, project, commit)
-    render_status_with_link('commit', commit.status, path, tooltip_placement, cssclass: cssclass)
+    render_status_with_link('commit', commit.status, path, tooltip_placement)
   end
 
   def render_pipeline_status(pipeline, tooltip_placement: 'auto left')

app/helpers/diff_helper.rb

@@ -54,18 +54,20 @@ module DiffHelper
     end
   end
 
-  def organize_comments(left, right)
-    notes_left = notes_right = nil
+  def parallel_diff_discussions(left, right, diff_file)
+    discussion_left = discussion_right = nil
 
-    unless left[:type].nil? && right[:type] == 'new'
-      notes_left = @grouped_diff_notes[left[:line_code]]
+    if left && (left.unchanged? || left.removed?)
+      line_code = diff_file.line_code(left)
+      discussion_left = @grouped_diff_discussions[line_code]
     end
 
-    unless left[:type].nil? && right[:type].nil?
-      notes_right = @grouped_diff_notes[right[:line_code]]
+    if right && right.added?
+      line_code = diff_file.line_code(right)
+      discussion_right = @grouped_diff_discussions[line_code]
     end
 
-    [notes_left, notes_right]
+    [discussion_left, discussion_right]
   end
 
   def inline_diff_btn

app/helpers/notes_helper.rb

 module NotesHelper
-  # Helps to distinguish e.g. commit notes in mr notes list
-  def note_for_main_target?(note)
-    @noteable.class.name == note.noteable_type && !note.diff_note?
-  end
-
   def note_target_fields(note)
     if note.noteable
       hidden_field_tag(:target_type, note.noteable.class.name.underscore) +
@@ -44,8 +39,8 @@ module NotesHelper
     # If we didn't, diff notes that would show for the same line on the changes
     # tab, would show in different discussions on the discussion tab.
     use_legacy_diff_note ||= begin
-      line_diff_notes = @grouped_diff_notes[line_code]
-      line_diff_notes && line_diff_notes.any?(&:legacy_diff_note?)
+      discussion = @grouped_diff_discussions[line_code]
+      discussion && discussion.legacy_diff_discussion?
     end
 
     data = {
@@ -81,22 +76,10 @@ module NotesHelper
     data
   end
 
-  def link_to_reply_discussion(note, line_type = nil)
+  def link_to_reply_discussion(discussion, line_type = nil)
     return unless current_user
 
-    data = {
-      noteable_type: note.noteable_type,
-      noteable_id:   note.noteable_id,
-      commit_id:     note.commit_id,
-      discussion_id: note.discussion_id,
-      line_type:     line_type
-    }
-
-    if note.diff_note?
-      data[:note_type] = note.type
-
-      data.merge!(note.diff_attributes)
-    end
+    data = discussion.reply_attributes.merge(line_type: line_type)
 
     content_tag(:div, class: "discussion-reply-holder") do
       button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
@@ -114,13 +97,13 @@ module NotesHelper
     @max_access_by_user_id[full_key]
   end
 
-  def diff_note_path(note)
-    return unless note.diff_note?
+  def discussion_diff_path(discussion)
+    return unless discussion.diff_discussion?
 
-    if note.for_merge_request? && note.active?
-      diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code)
-    elsif note.for_commit?
-      namespace_project_commit_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code)
+    if discussion.for_merge_request? && discussion.active?
+      diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
+    elsif discussion.for_commit?
+      namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
     end
   end
 end

app/models/concerns/note_on_diff.rb

 module NoteOnDiff
   extend ActiveSupport::Concern
 
-  NUMBER_OF_TRUNCATED_DIFF_LINES = 16
-
-  included do
-    delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true
-  end
-
   def diff_note?
     true
   end
@@ -30,23 +24,4 @@ module NoteOnDiff
   def can_be_award_emoji?
     false
   end
-
-  # Returns an array of at most 16 highlighted lines above a diff note
-  def truncated_diff_lines
-    prev_lines = []
-
-    highlighted_diff_lines.each do |line|
-      if line.meta?
-        prev_lines.clear
-      else
-        prev_lines << line
-
-        break if for_line?(line)
-
-        prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES
-      end
-    end
-
-    prev_lines
-  end
 end

app/models/discussion.rb

+class Discussion
+  NUMBER_OF_TRUNCATED_DIFF_LINES = 16
+
+  attr_reader :first_note, :notes
+
+  delegate  :created_at,
+            :project,
+            :author,
+
+            :noteable,
+            :for_commit?,
+            :for_merge_request?,
+
+            :line_code,
+            :diff_file,
+            :for_line?,
+            :active?,
+
+            to: :first_note
+
+  delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true
+
+  def self.for_notes(notes)
+    notes.group_by(&:discussion_id).values.map { |notes| new(notes) }
+  end
+
+  def self.for_diff_notes(notes)
+    notes.group_by(&:line_code).values.map { |notes| new(notes) }
+  end
+
+  def initialize(notes)
+    @first_note = notes.first
+    @notes = notes
+  end
+
+  def id
+    first_note.discussion_id
+  end
+
+  def diff_discussion?
+    first_note.diff_note?
+  end
+
+  def legacy_diff_discussion?
+    notes.any?(&:legacy_diff_note?)
+  end
+
+  def for_target?(target)
+    self.noteable == target && !diff_discussion?
+  end
+
+  def expanded?
+    !diff_discussion? || active?
+  end
+
+  def reply_attributes
+    data = {
+      noteable_type: first_note.noteable_type,
+      noteable_id:   first_note.noteable_id,
+      commit_id:     first_note.commit_id,
+      discussion_id: self.id,
+    }
+
+    if diff_discussion?
+      data[:note_type] = first_note.type
+
+      data.merge!(first_note.diff_attributes)
+    end
+
+    data
+  end
+
+  # Returns an array of at most 16 highlighted lines above a diff note
+  def truncated_diff_lines
+    prev_lines = []
+
+    highlighted_diff_lines.each do |line|
+      if line.meta?
+        prev_lines.clear
+      else
+        prev_lines << line
+
+        break if for_line?(line)
+
+        prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES
+      end
+    end
+
+    prev_lines
+  end
+end

app/models/note.rb

@@ -82,11 +82,12 @@ class Note < ActiveRecord::Base
     end
 
     def discussions
-      all.group_by(&:discussion_id).values
+      Discussion.for_notes(all)
     end
 
-    def grouped_diff_notes
-      diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code)
+    def grouped_diff_discussions
+      notes = diff_notes.fresh.select(&:active?)
+      Discussion.for_diff_notes(notes).map { |d| [d.line_code, d] }.to_h
     end
 
     # Searches for notes matching the given query.

app/models/project.rb

@@ -431,13 +431,13 @@ class Project < ActiveRecord::Base
 
   # ref can't be HEAD, can only be branch/tag name or SHA
   def latest_successful_builds_for(ref = default_branch)
-    pipeline = pipelines.latest_successful_for(ref).to_sql
-    join_sql = "INNER JOIN (#{pipeline}) pipelines" +
-               " ON pipelines.id = #{Ci::Build.quoted_table_name}.commit_id"
-    builds.joins(join_sql).latest.with_artifacts
-    # TODO: Whenever we dropped support for MySQL, we could change to:
-    # pipeline = pipelines.latest_successful_for(ref)
-    # builds.where(pipeline: pipeline).latest.with_artifacts
+    latest_pipeline = pipelines.latest_successful_for(ref).first
+
+    if latest_pipeline
+      latest_pipeline.builds.latest.with_artifacts
+    else
+      builds.none
+    end
   end
 
   def merge_base_commit(first_commit_id, second_commit_id)

app/models/repository.rb

@@ -11,16 +11,6 @@ class Repository
 
   attr_accessor :path_with_namespace, :project
 
-  def self.clean_old_archives
-    Gitlab::Metrics.measure(:clean_old_archives) do
-      repository_downloads_path = Gitlab.config.gitlab.repository_downloads_path
-
-      return unless File.directory?(repository_downloads_path)
-
-      Gitlab::Popen.popen(%W(find #{repository_downloads_path} -not -path #{repository_downloads_path} -mmin +120 -delete))
-    end
-  end
-
   def initialize(path_with_namespace, project)
     @path_with_namespace = path_with_namespace
     @project = project
@@ -80,7 +70,12 @@ class Repository
 
   def commit(ref = 'HEAD')
     return nil unless exists?
-    commit = Gitlab::Git::Commit.find(raw_repository, ref)
+    commit =
+      if ref.is_a?(Gitlab::Git::Commit)
+        ref
+      else
+        Gitlab::Git::Commit.find(raw_repository, ref)
+      end
     commit = ::Commit.new(commit, @project) if commit
     commit
   rescue Rugged::OdbError
@@ -257,10 +252,10 @@ class Repository
       # Rugged seems to throw a `ReferenceError` when given branch_names rather
       # than SHA-1 hashes
       number_commits_behind = raw_repository.
-        count_commits_between(branch.target, root_ref_hash)
+        count_commits_between(branch.target.sha, root_ref_hash)
 
       number_commits_ahead = raw_repository.
-        count_commits_between(root_ref_hash, branch.target)
+        count_commits_between(root_ref_hash, branch.target.sha)
 
       { behind: number_commits_behind, ahead: number_commits_ahead }
     end
@@ -684,9 +679,7 @@ class Repository
   end
 
   def local_branches
-    @local_branches ||= rugged.branches.each(:local).map do |branch|
-      Gitlab::Git::Branch.new(branch.name, branch.target)
-    end
+    @local_branches ||= raw_repository.local_branches
   end
 
   alias_method :branches, :local_branches
@@ -827,7 +820,7 @@ class Repository
   end
 
   def revert(user, commit, base_branch, revert_tree_id = nil)
-    source_sha = find_branch(base_branch).target
+    source_sha = find_branch(base_branch).target.sha
     revert_tree_id ||= check_revert_content(commit, base_branch)
 
     return false unless revert_tree_id
@@ -844,7 +837,7 @@ class Repository
   end
 
   def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil)
-    source_sha = find_branch(base_branch).target
+    source_sha = find_branch(base_branch).target.sha
     cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch)
 
     return false unless cherry_pick_tree_id
@@ -865,7 +858,7 @@ class Repository
   end
 
   def check_revert_content(commit, base_branch)
-    source_sha = find_branch(base_branch).target
+    source_sha = find_branch(base_branch).target.sha
     args       = [commit.id, source_sha]
     args << { mainline: 1 } if commit.merge_commit?
 
@@ -879,7 +872,7 @@ class Repository
   end
 
   def check_cherry_pick_content(commit, base_branch)
-    source_sha = find_branch(base_branch).target
+    source_sha = find_branch(base_branch).target.sha
     args       = [commit.id, source_sha]
     args << 1 if commit.merge_commit?
 
@@ -1044,7 +1037,7 @@ class Repository
   end
 
   def tags_sorted_by_committed_date
-    tags.sort_by { |tag| commit(tag.target).committed_date }
+    tags.sort_by { |tag| tag.target.committed_date }
   end
 
   def keep_around_ref_name(sha)

app/services/delete_branch_service.rb

@@ -40,6 +40,6 @@ class DeleteBranchService < BaseService
 
   def build_push_data(branch)
     Gitlab::PushDataBuilder
-      .build(project, current_user, branch.target, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", [])
+      .build(project, current_user, branch.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", [])
   end
 end

app/services/delete_tag_service.rb

@@ -34,6 +34,6 @@ class DeleteTagService < BaseService
 
   def build_push_data(tag)
     Gitlab::PushDataBuilder
-      .build(project, current_user, tag.target, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", [])
+      .build(project, current_user, tag.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", [])
   end
 end

app/services/git_tag_push_service.rb

@@ -27,7 +27,7 @@ class GitTagPushService < BaseService
       tag_name = Gitlab::Git.ref_name(params[:ref])
       tag = project.repository.find_tag(tag_name)
 
-      if tag && tag.target == params[:newrev]
+      if tag && tag.object_sha == params[:newrev]
         commit = project.commit(tag.target)
         commits = [commit].compact
         message = tag.message

app/services/repository_archive_clean_up_service.rb

+class RepositoryArchiveCleanUpService
+  LAST_MODIFIED_TIME_IN_MINUTES = 120
+
+  def initialize(mmin = LAST_MODIFIED_TIME_IN_MINUTES)
+    @mmin = mmin
+    @path = Gitlab.config.gitlab.repository_downloads_path
+  end
+
+  def execute
+    Gitlab::Metrics.measure(:repository_archive_clean_up) do
+      return unless File.directory?(path)
+
+      clean_up_old_archives
+      clean_up_empty_directories
+    end
+  end
+
+  private
+
+  attr_reader :mmin, :path
+
+  def clean_up_old_archives
+    run(%W(find #{path} -not -path #{path} -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -maxdepth 2 -mmin +#{mmin} -delete))
+  end
+
+  def clean_up_empty_directories
+    run(%W(find #{path} -not -path #{path} -type d -empty -name \*.git -maxdepth 1 -delete))
+  end
+
+  def run(cmd)
+    Gitlab::Popen.popen(cmd)
+  end
+end

app/views/discussions/_diff_discussion.html.haml

+%tr.notes_holder
+  %td.notes_line{ colspan: 2 }
+  %td.notes_content
+    %ul.notes{ data: { discussion_id: discussion.id } }
+      = render partial: "projects/notes/note", collection: discussion.notes, as: :note
+    = link_to_reply_discussion(discussion)

app/views/projects/notes/discussions/_diff_with_notes.html.haml → app/views/discussions/_diff_with_notes.html.haml

-- note = discussion_notes.first
-- diff_file = note.diff_file
-- return unless diff_file
-
-- blob = note.blob
+- diff_file = discussion.diff_file
+- blob = discussion.blob
 
 .diff-file.file-holder
   .file-title
-    = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: note.project, url: diff_note_path(note)
+    = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: discussion.project, url: discussion_diff_path(discussion)
 
   .diff-content.code.js-syntax-highlight
     %table
-      - note.truncated_diff_lines.each do |line|
+      - discussion.truncated_diff_lines.each do |line|
         = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true
 
-        - if note.for_line?(line)
-          = render "projects/notes/diff_notes_with_reply", notes: discussion_notes
+        - if discussion.for_line?(line)
+          = render "discussions/diff_discussion", discussion: discussion

app/views/projects/notes/_discussion.html.haml → app/views/discussions/_discussion.html.haml

-- note = discussion_notes.first
-- expanded = !note.diff_note? || note.active?
+- expanded = discussion.expanded?
 %li.note.note-discussion.timeline-entry
   .timeline-entry-inner
     .timeline-icon
-      = link_to user_path(note.author) do
-        = image_tag avatar_icon(note.author), class: "avatar s40"
+      = link_to user_path(discussion.author) do
+        = image_tag avatar_icon(discussion.author), class: "avatar s40"
     .timeline-content
-      .discussion.js-toggle-container{ class: note.discussion_id }
+      .discussion.js-toggle-container{ class: discussion.id }
         .discussion-header
-          = link_to_member(@project, note.author, avatar: false)
+          = link_to_member(@project, discussion.author, avatar: false)
 
           .inline.discussion-headline-light
-            = note.author.to_reference
+            = discussion.author.to_reference
             started a discussion on
 
-            - if note.for_commit?
-              - commit = note.noteable
+            - if discussion.for_commit?
+              - commit = discussion.noteable
               - if commit
                 commit
-                = link_to commit.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code), class: 'monospace'
+                = link_to commit.short_id, namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code), class: 'monospace'
               - else
                 a deleted commit
             - else
-              - if note.active?
-                = link_to diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) do
+              - if discussion.active?
+                = link_to diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code) do
                   the diff
               - else
                 an outdated diff
 
-            = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "note-created-ago")
+            = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
 
           .discussion-actions
             = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
@@ -40,7 +39,7 @@
               Toggle discussion
 
         .discussion-body.js-toggle-content{ class: ("hide" unless expanded) }
-          - if note.diff_note?
-            = render "projects/notes/discussions/diff_with_notes", discussion_notes: discussion_notes
+          - if discussion.diff_discussion? && discussion.diff_file
+            = render "discussions/diff_with_notes", discussion: discussion
           - else
-            = render "projects/notes/discussions/notes", discussion_notes: discussion_notes
+            = render "discussions/notes", discussion: discussion

app/views/projects/notes/discussions/_notes.html.haml → app/views/discussions/_notes.html.haml

-- note = discussion_notes.first
 .panel.panel-default
-  .notes{ data: { discussion_id: note.discussion_id } }
+  .notes{ data: { discussion_id: discussion.id } }
     %ul.notes.timeline
-      = render partial: "projects/notes/note", collection: discussion_notes, as: :note
-  = link_to_reply_discussion(note)
+      = render partial: "projects/notes/note", collection: discussion.notes, as: :note
+  = link_to_reply_discussion(discussion)

app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml → app/views/discussions/_parallel_diff_discussion.html.haml

-- note_left = notes_left.present? ? notes_left.first : nil
-- note_right = notes_right.present? ? notes_right.first : nil
-
 %tr.notes_holder
-  - if note_left
+  - if discussion_left
     %td.notes_line.old
     %td.notes_content.parallel.old
-      %ul.notes{ data: { discussion_id: note_left.discussion_id } }
-        = render partial: "projects/notes/note", collection: notes_left, as: :note
+      %ul.notes{ data: { discussion_id: discussion_left.id } }
+        = render partial: "projects/notes/note", collection: discussion_left.notes, as: :note
 
-      = link_to_reply_discussion(note_left, 'old')
+      = link_to_reply_discussion(discussion_left, 'old')
   - else
     %td.notes_line.old= ""
     %td.notes_content.parallel.old= ""
 
-  - if note_right
+  - if discussion_right
     %td.notes_line.new
     %td.notes_content.parallel.new
-      %ul.notes{ data: { discussion_id: note_right.discussion_id } }
-        = render partial: "projects/notes/note", collection: notes_right, as: :note
+      %ul.notes{ data: { discussion_id: discussion_right.id } }
+        = render partial: "projects/notes/note", collection: discussion_right.notes, as: :note
 
-      = link_to_reply_discussion(note_right, 'new')
+      = link_to_reply_discussion(discussion_right, 'new')
   - else
     %td.notes_line.new= ""
     %td.notes_content.parallel.new= ""

app/views/layouts/nav/_dashboard.html.haml

@@ -30,7 +30,7 @@
       %span
         Merge Requests
         %span.count= number_with_delimiter(current_user.assigned_merge_requests.opened.count)
-  = nav_link(controller: :snippets) do
+  = nav_link(controller: 'dashboard/snippets') do
     = link_to dashboard_snippets_path, title: 'Snippets' do
       %span
         Snippets

app/views/layouts/project.html.haml

@@ -6,7 +6,7 @@
 - content_for :scripts_body_top do
   - project = @target_project || @project
   - if @project_wiki && @page
-    - markdown_preview_path = namespace_project_wiki_markdown_preview_path(project.namespace, project, params[:id])
+    - markdown_preview_path = namespace_project_wiki_markdown_preview_path(project.namespace, project, @page.title)
   - else
     - markdown_preview_path = markdown_preview_namespace_project_path(project.namespace, project)
   - if current_user

app/views/projects/branches/_commit.html.haml

 .branch-commit
-  = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit-id monospace"
+  = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-id monospace"
   ·
   %span.str-truncated
     = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-row-message"

app/views/projects/ci/builds/_build.html.haml

@@ -14,8 +14,10 @@
         %span ##{build.id}
 
       - if build.stuck?
+        .icon-container
           = icon('warning', class: 'text-warning has-tooltip', title: 'Build is stuck. Check runners.')
       - if defined?(retried) && retried
+        .icon-container
           = icon('warning', class: 'text-warning has-tooltip', title: 'Build was retried.')
 
       - if defined?(ref) && ref
@@ -23,6 +25,7 @@
           = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref), class: "monospace branch-name"
         - else
           .light none
+        .icon-container
           = custom_icon("icon_commit")
 
       - if defined?(commit_sha) && commit_sha
@@ -88,4 +91,3 @@
           - elsif build.playable?
             = link_to play_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Play', class: 'btn btn-build' do
               = icon('play')
-

app/views/projects/commit/_pipeline.html.haml

@@ -35,8 +35,8 @@
   .bs-callout.bs-callout-warning
     \.gitlab-ci.yml not found in this commit
 
-.table-holder
-  %table.table.builds
+.table-holder.pipeline-holder
+  %table.table.builds.pipeline
     %thead
       %tr
         %th Status

app/views/projects/commits/_commit.html.haml

@@ -19,13 +19,14 @@
             ·
             = commit.short_id
           - if commit.status
-            = render_commit_status(commit, cssclass: 'visible-xs-inline')
+            .visible-xs-inline
+              = render_commit_status(commit)
           - if commit.description?
             %a.text-expander.hidden-xs.js-toggle-button ...
 
         .commit-actions.hidden-xs
           - if commit.status
-            = render_commit_status(commit, cssclass: 'btn btn-transparent')
+            = render_commit_status(commit)
           = clipboard_button(clipboard_text: commit.id)
           = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit-short-id btn btn-transparent"
           = link_to_browse_code(project, commit)

app/views/projects/diffs/_match_line_parallel.html.haml

-%td.old_line.diff-line-num.empty-cell
-%td.line_content.parallel.match= line
-%td.new_line.diff-line-num.empty-cell
-%td.line_content.parallel.match= line

app/views/projects/diffs/_parallel_view.html.haml

@@ -5,32 +5,35 @@
       - left = line[:left]
       - right = line[:right]
       %tr.line_holder.parallel
-        - if left[:type] == 'match'
-          = render "projects/diffs/match_line_parallel", { line: left[:text] }
-        - elsif left[:type] == 'nonewline'
+        - if left
+          - if left.meta?
             %td.old_line.diff-line-num.empty-cell
-          %td.line_content.parallel.match= left[:text]
-          %td.new_line.diff-line-num.empty-cell
-          %td.line_content.parallel.match= left[:text]
+            %td.line_content.parallel.match= left.text
           - else
-          %td.old_line.diff-line-num{id: left[:line_code], class: [left[:type], ('empty-cell' unless left[:number])], data: { linenumber: left[:number] }}
-            %a{href: "##{left[:line_code]}" }= raw(left[:number])
-          %td.line_content.parallel.noteable_line{class: [left[:type], ('empty-cell' if left[:text].empty?)], data: diff_view_line_data(left[:line_code], left[:position], 'old')}= diff_line_content(left[:text])
-
-          - if right[:type] == 'new'
-            - new_line_type = 'new'
-            - new_line_code = right[:line_code]
-            - new_position = right[:position]
+            - left_line_code = diff_file.line_code(left)
+            - left_position = diff_file.position(left)
+            %td.old_line.diff-line-num{id: left_line_code, class: left.type, data: { linenumber: left.old_pos }}
+              %a{href: "##{left_line_code}" }= raw(left.old_pos)
+            %td.line_content.parallel.noteable_line{class: left.type, data: diff_view_line_data(left_line_code, left_position, 'old')}= diff_line_content(left.text)
         - else
-            - new_line_type = nil
-            - new_line_code = left[:line_code]
-            - new_position = left[:position]
+          %td.old_line.diff-line-num.empty-cell
+          %td.line_content.parallel
 
-          %td.new_line.diff-line-num{id: new_line_code, class: [new_line_type, ('empty-cell' unless right[:number])], data: { linenumber: right[:number] }}
-            %a{href: "##{new_line_code}" }= raw(right[:number])
-          %td.line_content.parallel.noteable_line{class: [new_line_type, ('empty-cell' if right[:text].empty?)], data: diff_view_line_data(new_line_code, new_position, 'new')}= diff_line_content(right[:text])
+        - if right
+          - if right.meta?
+            %td.old_line.diff-line-num.empty-cell
+            %td.line_content.parallel.match= left.text
+          - else
+            - right_line_code = diff_file.line_code(right)
+            - right_position = diff_file.position(right)
+            %td.new_line.diff-line-num{id: right_line_code, class: right.type, data: { linenumber: right.new_pos }}
+              %a{href: "##{right_line_code}" }= raw(right.new_pos)
+            %td.line_content.parallel.noteable_line{class: right.type, data: diff_view_line_data(right_line_code, right_position, 'new')}= diff_line_content(right.text)
+        - else
+          %td.old_line.diff-line-num.empty-cell
+          %td.line_content.parallel
 
       - unless @diff_notes_disabled
-        - notes_left, notes_right = organize_comments(left, right)
-        - if notes_left.present? || notes_right.present?
-          = render "projects/notes/diff_notes_with_reply_parallel", notes_left: notes_left, notes_right: notes_right
+        - discussion_left, discussion_right = parallel_diff_discussions(left, right, diff_file)
+        - if discussion_left || discussion_right
+          = render "discussions/parallel_diff_discussion", discussion_left: discussion_left, discussion_right: discussion_right

app/views/projects/diffs/_text_file.html.haml

@@ -11,9 +11,9 @@
 
     - unless @diff_notes_disabled
       - line_code = diff_file.line_code(line)
-      - diff_notes = @grouped_diff_notes[line_code] if line_code
-      - if diff_notes
-        = render "projects/notes/diff_notes_with_reply", notes: diff_notes
+      - discussion = @grouped_diff_discussions[line_code] if line_code
+      - if discussion
+        = render "discussions/diff_discussion", discussion: discussion
 
   - if last_line > 0
     = render "projects/diffs/match_line", { line: "",

app/views/projects/notes/_diff_notes_with_reply.html.haml

-- note = notes.first
-%tr.notes_holder
-  %td.notes_line{ colspan: 2 }
-  %td.notes_content
-    %ul.notes{ data: { discussion_id: note.discussion_id } }
-      = render partial: "projects/notes/note", collection: notes, as: :note
-    = link_to_reply_discussion(note)

app/views/projects/notes/_notes.html.haml

 - if @discussions.present?
-  - @discussions.each do |discussion_notes|
-    - note = discussion_notes.first
-    - if note_for_main_target?(note)
-      = render partial: "projects/notes/note", object: note, as: :note
+  - @discussions.each do |discussion|
+    - if discussion.for_target?(@noteable)
+      = render partial: "projects/notes/note", object: discussion.first_note, as: :note
     - else
-      = render 'projects/notes/discussion', discussion_notes: discussion_notes
+      = render 'discussions/discussion', discussion: discussion
 - else
-  - @notes.each do |note|
-    = render partial: "projects/notes/note", object: note, as: :note
+  = render partial: "projects/notes/note", collection: @notes, as: :note

app/views/projects/protected_branches/index.html.haml

@@ -6,12 +6,13 @@
       = page_title
     %p Keep stable branches secure and force developers to use merge requests.
     %p.prepend-top-20
-      Protected branches are designed to:
+      By default, protected branches are designed to:
       %ul
-        %li prevent pushes from everybody except #{link_to "masters", help_page_path("user/permissions"), class: "vlink"}
-        %li prevent anyone from force pushing to the branch
-        %li prevent anyone from deleting the branch
-      %p.append-bottom-0 Read more about #{link_to "project permissions", help_page_path("user/permissions"), class: "underlined-link"}
+        %li prevent their creation, if not already created, from everybody except Masters
+        %li prevent pushes from everybody except Masters
+        %li prevent <strong>anyone</strong> from force pushing to the branch
+        %li prevent <strong>anyone</strong> from deleting the branch
+      %p.append-bottom-0 Read more about #{link_to "protected branches", help_page_path("user/project/protected_branches"), class: "underlined-link"} and #{link_to "project permissions", help_page_path("user/permissions"), class: "underlined-link"}.
   .col-lg-9
     %h5.prepend-top-0
       Protect a branch
@@ -23,7 +24,7 @@
           = f.label :name, "Branch", class: "label-light"
           = render partial: "dropdown", locals: { f: f }
           %p.help-block
-            = link_to "Wildcards", help_page_path('workflow/protected_branches', anchor: "wildcard-protected-branches")
+            = link_to "Wildcards", help_page_path('user/project/protected_branches', anchor: "wildcard-protected-branches")
             such as
             %code *-stable
             or

app/views/shared/icons/_icon_fork.svg

-<svg xmlns="http://www.w3.org/2000/svg" width="40" height="40" viewBox="0 0 40 40" xmlns:xlink="http://www.w3.org/1999/xlink"><defs><circle id="a" cx="4" cy="4" r="4"/><mask id="d" width="8" height="8" x="0" y="0" fill="#fff"><use xlink:href="#a"/></mask><circle id="b" cx="20" cy="4" r="4"/><mask id="e" width="8" height="8" x="0" y="0" fill="#fff"><use xlink:href="#b"/></mask><circle id="c" cx="12" cy="30" r="4"/><mask id="f" width="8" height="8" x="0" y="0" fill="#fff"><use xlink:href="#c"/></mask></defs><g fill="none" fill-rule="evenodd" transform="translate(8 3)"><path fill="#7E7E7E" d="M10 19.667c-4.14-1.29-7.389-5.878-7.389-5.878C2.274 13.353 2 12.545 2 12.01V6h4v5.509c0 .276.166.65.367.831 0 0 1.136 1.028 1.746 1.574C9.617 15.261 11.048 16 12.09 16c1.028 0 2.41-.723 3.858-2.048.588-.54 1.84-1.742 1.84-1.742a.784.784 0 0 0 .211-.502V6h4v6.008c0 .548-.259 1.349-.601 1.795 0 0-3.21 4.707-7.399 5.916V27h-4v-7.333z"/><use stroke="#7E7E7E" stroke-width="4" mask="url(#d)" xlink:href="#a"/><use stroke="#7E7E7E" stroke-width="4" mask="url(#e)" xlink:href="#b"/><use stroke="#7E7E7E" stroke-width="4" mask="url(#f)" xlink:href="#c"/></g></svg>
+<svg xmlns="http://www.w3.org/2000/svg" width="40" height="40" viewBox="0 0 40 40">
\ No newline at end of file
+  <path fill="#7E7E7E" fill-rule="evenodd" d="M22,29.5351288 L22,22.7193602 C26.1888699,21.5098039 29.3985457,16.802989 29.3985457,16.802989 C29.740988,16.3567547 30,15.5559546 30,15.0081969 L30,10.4648712 C31.1956027,9.77325238 32,8.48056471 32,7 C32,4.790861 30.209139,3 28,3 C25.790861,3 24,4.790861 24,7 C24,8.48056471 24.8043973,9.77325238 26,10.4648712 L26,14.7083871 C26,14.8784435 25.9055559,15.0987329 25.7890533,15.2104147 C25.7890533,15.2104147 24.5373893,16.4126202 23.9488702,16.9515733 C22.5015398,18.2770075 21.1191354,19 20.090554,19 C19.0477772,19 17.6172728,18.2608988 16.1128852,16.9142923 C15.5030182,16.3683886 14.3672121,15.3403307 14.3672121,15.3403307 C14.1659605,15.1583364 14.0000086,14.7846305 14.0000192,14.5088473 C14.0000192,14.5088473 14.0000932,12.7539451 14.0001308,10.4647956 C15.1956614,9.77315812 16,8.48051074 16,7 C16,4.790861 14.209139,3 12,3 C9.790861,3 8,4.790861 8,7 C8,8.48056471 8.80439726,9.77325238 10,10.4648712 L10,15.0081969 C10,15.5446944 10.2736352,16.3534183 10.6111812,16.7893819 C10.6111812,16.7893819 13.8599776,21.3779363 18,22.6668724 L18,29.5351288 C16.8043973,30.2267476 16,31.5194353 16,33 C16,35.209139 17.790861,37 20,37 C22.209139,37 24,35.209139 24,33 C24,31.5194353 23.1956027,30.2267476 22,29.5351288 Z M14,7 C14,5.8954305 13.1045695,5 12,5 C10.8954305,5 10,5.8954305 10,7 C10,8.1045695 10.8954305,9 12,9 C13.1045695,9 14,8.1045695 14,7 Z M30,7 C30,5.8954305 29.1045695,5 28,5 C26.8954305,5 26,5.8954305 26,7 C26,8.1045695 26.8954305,9 28,9 C29.1045695,9 30,8.1045695 30,7 Z M22,33 C22,31.8954305 21.1045695,31 20,31 C18.8954305,31 18,31.8954305 18,33 C18,34.1045695 18.8954305,35 20,35 C21.1045695,35 22,34.1045695 22,33 Z"/>
+</svg>

app/workers/repository_archive_cache_worker.rb

@@ -4,6 +4,6 @@ class RepositoryArchiveCacheWorker
   sidekiq_options queue: :default
 
   def perform
-    Repository.clean_old_archives
+    RepositoryArchiveCleanUpService.new.execute
   end
 end

config/gitlab.yml.example

@@ -106,8 +106,8 @@ production: &base
 
     ## Repository downloads directory
     # When a user clicks e.g. 'Download zip' on a project, a temporary zip file is created in the following directory.
-    # The default is 'tmp/repositories' relative to the root of the Rails app.
-    # repository_downloads_path: tmp/repositories
+    # The default is 'shared/cache/archive/' relative to the root of the Rails app.
+    # repository_downloads_path: shared/cache/archive/
 
   ## Reply by email
   # Allow users to comment on issues and merge requests by replying to notification emails.

config/initializers/1_settings.rb

@@ -211,7 +211,6 @@ Settings.gitlab.default_projects_features['snippets']           = false if Setti
 Settings.gitlab.default_projects_features['builds']             = true if Settings.gitlab.default_projects_features['builds'].nil?
 Settings.gitlab.default_projects_features['container_registry'] = true if Settings.gitlab.default_projects_features['container_registry'].nil?
 Settings.gitlab.default_projects_features['visibility_level']   = Settings.send(:verify_constant, Gitlab::VisibilityLevel, Settings.gitlab.default_projects_features['visibility_level'], Gitlab::VisibilityLevel::PRIVATE)
-Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') if Settings.gitlab['repository_downloads_path'].nil?
 Settings.gitlab['domain_whitelist'] ||= []
 Settings.gitlab['import_sources'] ||= %w[github bitbucket gitlab gitorious google_code fogbugz git gitlab_project]
 Settings.gitlab['trusted_proxies'] ||= []
@@ -315,6 +314,21 @@ Settings.repositories['storages'] ||= {}
 # Setting gitlab_shell.repos_path is DEPRECATED and WILL BE REMOVED in version 9.0
 Settings.repositories.storages['default'] ||= Settings.gitlab_shell['repos_path'] || Settings.gitlab['user_home'] + '/repositories/'
 
+#
+# The repository_downloads_path is used to remove outdated repository
+# archives, if someone has it configured incorrectly, and it points
+# to the path where repositories are stored this can cause some
+# data-integrity issue. In this case, we sets it to the default
+# repository_downloads_path value.
+#
+repositories_storages_path     = Settings.repositories.storages.values
+repository_downloads_path      = Settings.gitlab['repository_downloads_path'].to_s.gsub(/\/$/, '')
+repository_downloads_full_path = File.expand_path(repository_downloads_path, Settings.gitlab['user_home'])
+
+if repository_downloads_path.blank? || repositories_storages_path.any? { |path| [repository_downloads_path, repository_downloads_full_path].include?(path.gsub(/\/$/, '')) }
+  Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive')
+end
+
 #
 # Backup
 #

config/initializers/6_validations.rb

@@ -3,22 +3,27 @@ def storage_name_valid?(name)
 end
 
 def find_parent_path(name, path)
+  parent = Pathname.new(path).realpath.parent
   Gitlab.config.repositories.storages.detect do |n, p|
-    name != n && path.chomp('/').start_with?(p.chomp('/'))
+    name != n && Pathname.new(p).realpath == parent
   end
 end
 
-def error(message)
+def storage_validation_error(message)
   raise "#{message}. Please fix this in your gitlab.yml before starting GitLab."
 end
 
-error('No repository storage path defined') if Gitlab.config.repositories.storages.empty?
+def validate_storages
+  storage_validation_error('No repository storage path defined') if Gitlab.config.repositories.storages.empty?
 
-Gitlab.config.repositories.storages.each do |name, path|
-  error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name)
+  Gitlab.config.repositories.storages.each do |name, path|
+    storage_validation_error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name)
 
     parent_name, _parent_path = find_parent_path(name, path)
     if parent_name
-    error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages")
+      storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages")
+    end
   end
 end
+
+validate_storages unless Rails.env.test?

db/migrate/20160721081015_drop_and_readd_has_external_wiki_in_projects.rb

@@ -5,8 +5,9 @@ class DropAndReaddHasExternalWikiInProjects < ActiveRecord::Migration
   DOWNTIME = false
 
   def up
-    remove_column :projects, :has_external_wiki, :boolean
-    add_column :projects, :has_external_wiki, :boolean
+    update_column_in_batches(:projects, :has_external_wiki, nil) do |table, query|
+      query.where(table[:has_external_wiki].not_eq(nil))
+    end
   end
 
   def down

doc/README.md

@@ -21,7 +21,7 @@
 
 ## Administrator documentation
 
-- [Access restrictions](administration/access_restrictions.md) Define which Git access protocols can be used to talk to GitLab
+- [Access restrictions](user/admin_area/settings/visibility_and_access_controls.md#enabled-git-access-protocols) Define which Git access protocols can be used to talk to GitLab
 - [Authentication/Authorization](administration/auth/README.md) Configure
   external authentication with LDAP, SAML, CAS and additional Omniauth providers.
 - [Custom Git hooks](administration/custom_hooks.md) Custom Git hooks (on the filesystem) for when webhooks aren't enough.
@@ -50,6 +50,7 @@
 - [Sidekiq Troubleshooting](administration/troubleshooting/sidekiq.md) Debug when Sidekiq appears hung and is not processing jobs.
 - [High Availability](administration/high_availability/README.md) Configure multiple servers for scaling or high availability.
 - [Container Registry](administration/container_registry.md) Configure Docker Registry with GitLab.
+- [Multiple mountpoints for the repositories storage](administration/repository_storages.md) Define multiple repository storage paths to distribute the storage load.
 
 ## Contributor documentation
 

doc/administration/repository_storages.md

 # Repository storages
 
-GitLab allows you to define repository storage paths to enable distribution of
-storage load between several mount points.
-
-## For installations from source
+> [Introduced][ce-4578] in GitLab 8.10.
 
-Add your repository storage paths in your `gitlab.yml` under repositories -> storages, using key -> value pairs.
+GitLab allows you to define multiple repository storage paths to distribute the
+storage load between several mount points.
 
 >**Notes:**
+>
 - You must have at least one storage path called `default`.
+- The paths are defined in key-value pairs. The key is an arbitrary name you
+  can pick to name the file path.
+- The target directories and any of its subpaths must not be a symlink.
+
+## Configure GitLab
+
+>**Warning:**
 - In order for backups to work correctly the storage path must **not** be a
-mount point and the GitLab user should have correct permissions for the parent
-directory of the path.
+  mount point and the GitLab user should have correct permissions for the parent
+  directory of the path.
+
+Edit the configuration files and add the full paths of the alternative repository
+storage paths. In the example below we added two more mountpoints that we named
+`nfs` and `cephfs` respectively.
+
+**For installations from source**
+
+1. Edit `gitlab.yml` and add the storage paths:
+
+    ```yaml
+    repositories:
+      # Paths where repositories can be stored. Give the canonicalized absolute pathname.
+      # NOTE: REPOS PATHS MUST NOT CONTAIN ANY SYMLINK!!!
+      storages: # You must have at least a 'default' storage path.
+        default: /home/git/repositories
+        nfs: /mnt/nfs/repositories
+        cephfs: /mnt/cephfs/repositories
+    ```
+
+1. [Restart GitLab] for the changes to take effect.
+
+The `gitlab_shell: repos_path` entry in `gitlab.yml` will be deprecated and
+replaced by `repositories: storages` in the future, so if you are upgrading
+from a version prior to 8.10, make sure to add the configuration as described
+in the step above. After you make the changes and confirm they are working,
+you can remove:
+
+```yaml
+repos_path: /home/git/repositories
+```
+
+which is located under the `gitlab_shell` section.
+
+---
+
+**For Omnibus installations**
+
+1. Edit `/etc/gitlab/gitlab.rb` by appending the rest of the paths to the
+   default one:
+
+    ```ruby
+    git_data_dirs({
+      "default" => "/var/opt/gitlab/git-data",
+      "nfs" => "/mnt/nfs/git-data",
+      "cephfs" => "/mnt/cephfs/git-data"
+    })
+    ```
+
+    Note that Omnibus stores the repositories in a `repositories` subdirectory
+    of the `git-data` directory.
+
+## Choose where new project repositories will be stored
+
+Once you set the multiple storage paths, you can choose where new projects will
+be stored via the **Application Settings** in the Admin area.
 
-## For omnibus installations
+![Choose repository storage path in Admin area](img/repository_storages_admin_ui.png)
 
-Follow the instructions at https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/doc/settings/configuration.md#storing-git-data-in-an-alternative-directory
+[ce-4578]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4578
+[restart gitlab]: restart_gitlab.md#installations-from-source
+[reconfigure gitlab]: restart_gitlab.md#omnibus-gitlab-reconfigure

doc/api/builds.md

@@ -283,6 +283,40 @@ Response:
 
 [ce-2893]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2893
 
+## Download the artifacts file
+
+> [Introduced][ce-5347] in GitLab 8.10.
+
+Download the artifacts file from the given reference name and job provided the
+build finished successfully.
+
+```
+GET /projects/:id/builds/artifacts/:ref_name/download?job=name
+```
+
+Parameters
+
+| Attribute   | Type    | Required | Description               |
+|-------------|---------|----------|-------------------------- |
+| `id`        | integer | yes      | The ID of a project       |
+| `ref_name`  | string  | yes      | The ref from a repository |
+| `job`       | string  | yes      | The name of the job       |
+
+Example request:
+
+```
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/artifacts/master/download?job=test"
+```
+
+Example response:
+
+| Status    | Description                     |
+|-----------|---------------------------------|
+| 200       | Serves the artifacts file       |
+| 404       | Build not found or no artifacts |
+
+[ce-5347]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347
+
 ## Get a trace file
 
 Get a trace of a specific build of a project
@@ -409,7 +443,7 @@ POST /projects/:id/builds/:build_id/erase
 
 Parameters
 
-| Attribute   | Type    | required | Description         |
+| Attribute   | Type    | Required | Description         |
 |-------------|---------|----------|---------------------|
 | `id`        | integer | yes      | The ID of a project |
 | `build_id`  | integer | yes      | The ID of a build   |
@@ -459,7 +493,7 @@ POST /projects/:id/builds/:build_id/artifacts/keep
 
 Parameters
 
-| Attribute   | Type    | required | Description         |
+| Attribute   | Type    | Required | Description         |
 |-------------|---------|----------|---------------------|
 | `id`        | integer | yes      | The ID of a project |
 | `build_id`  | integer | yes      | The ID of a build   |

doc/ci/variables/README.md

@@ -38,7 +38,7 @@ The `API_TOKEN` will take the Secure Variable value: `SECURE`.
 | **CI_PIPELINE_ID**      | 8.10   | 0.5    | The unique id of the current pipeline that GitLab CI uses internally |
 | **CI_PROJECT_ID**       | all    | all    | The unique id of the current project that GitLab CI uses internally |
 | **CI_PROJECT_NAME**     | 8.10   | 0.5    | The project name that is currently being built |
-| **CI_PROJECT_NAMESPACE**| 8.10   | 0.5    | The project namespace that is currently being built |
+| **CI_PROJECT_NAMESPACE**| 8.10   | 0.5    | The project namespace (username or groupname) that is currently being built |
 | **CI_PROJECT_PATH**     | 8.10   | 0.5    | The namespace with project name |
 | **CI_PROJECT_URL**      | 8.10   | 0.5    | The HTTP address to access project |
 | **CI_PROJECT_DIR**      | all    | all    | The full path where the repository is cloned and where the build is run |

doc/ci/yaml/README.md

@@ -27,12 +27,14 @@ If you want a quick introduction to GitLab CI, follow our
     - [only and except](#only-and-except)
     - [job variables](#job-variables)
     - [tags](#tags)
+    - [allow_failure](#allow_failure)
     - [when](#when)
+        - [Manual actions](#manual-actions)
     - [environment](#environment)
     - [artifacts](#artifacts)
-    - [artifacts:name](#artifactsname)
-    - [artifacts:when](#artifactswhen)
-    - [artifacts:expire_in](#artifactsexpire_in)
+        - [artifacts:name](#artifacts-name)
+        - [artifacts:when](#artifacts-when)
+        - [artifacts:expire_in](#artifacts-expire_in)
     - [dependencies](#dependencies)
     - [before_script and after_script](#before_script-and-after_script)
 - [Git Strategy](#git-strategy)
@@ -40,7 +42,7 @@ If you want a quick introduction to GitLab CI, follow our
 - [Hidden jobs](#hidden-jobs)
 - [Special YAML features](#special-yaml-features)
     - [Anchors](#anchors)
-- [Validate the .gitlab-ci.yml](#validate-the-gitlab-ciyml)
+- [Validate the .gitlab-ci.yml](#validate-the-gitlab-ci-yml)
 - [Skipping builds](#skipping-builds)
 - [Examples](#examples)
 
@@ -473,6 +475,39 @@ job:
 The specification above, will make sure that `job` is built by a Runner that
 has both `ruby` AND `postgres` tags defined.
 
+### allow_failure
+
+`allow_failure` is used when you want to allow a build to fail without impacting
+the rest of the CI suite. Failed builds don't contribute to the commit status.
+
+When enabled and the build fails, the pipeline will be successful/green for all
+intents and purposes, but a "CI build passed with warnings" message  will be
+displayed on the merge request or commit or build page. This is to be used by
+builds that are allowed to fail, but where failure indicates some other (manual)
+steps should be taken elsewhere.
+
+In the example below, `job1` and `job2` will run in parallel, but if `job1`
+fails, it will not stop the next stage from running, since it's marked with
+`allow_failure: true`:
+
+```yaml
+job1:
+  stage: test
+  script:
+  - execute_script_that_will_fail
+  allow_failure: true
+
+job2:
+  stage: test
+  script:
+  - execute_script_that_will_succeed
+
+job3:
+  stage: deploy
+  script:
+  - deploy_to_staging
+```
+
 ### when
 
 `when` is used to implement jobs that are run in case of failure or despite the

doc/integration/README.md

@@ -11,7 +11,6 @@ See the documentation below for details on how to configure these services.
 - [OmniAuth](omniauth.md) Sign in via Twitter, GitHub, GitLab.com, Google, Bitbucket, Facebook, Shibboleth, SAML, Crowd and Azure
 - [SAML](saml.md) Configure GitLab as a SAML 2.0 Service Provider
 - [CAS](cas.md) Configure GitLab to sign in using CAS
-- [Slack](slack.md) Integrate with the Slack chat service
 - [OAuth2 provider](oauth_provider.md) OAuth2 application creation
 - [Gmail actions buttons](gmail_action_buttons_for_gitlab.md) Adds GitLab actions to messages
 - [reCAPTCHA](recaptcha.md) Configure GitLab to use Google reCAPTCHA for new users

doc/integration/slack.md

-# Slack integration
-
-## On Slack
-
-To enable Slack integration you must create an Incoming WebHooks integration on Slack:
-
-1.  [Sign in to Slack](https://slack.com/signin)
-
-1.  Visit [Incoming WebHooks](https://my.slack.com/services/new/incoming-webhook/)
-
-1.  Choose the channel name you want to send notifications to.
-
-1.  Click **Add Incoming WebHooks Integration**
-    - Optional step; You can change bot's name and avatar by clicking modifying the bot name or avatar under **Integration Settings**.
-
-1. Copy the **Webhook URL**, we'll need this later for GitLab.
-
-
-## On GitLab
-
-After Slack is ready we need to setup GitLab. Here are the steps to achieve this.
-
-1.  Sign in to GitLab
-
-1.  Pick the repository you want.
-
-1.  Navigate to Settings -> Services -> Slack
-
-1. Pick the triggers you want to activate and respective channel (`#general` by default).
-
-1.  Fill in your Slack details
-    - Webhook: Paste the Webhook URL from the step above
-    - Username: Fill this in if you want to change the username of the bot
-    - Mark it as active
-
-1. Save your settings
-
-Have fun :)
-
-*P.S. You can set "branch,pushed,Compare changes" as highlight words on your Slack profile settings, so that you can be aware of new commits when somebody pushes them.*
+This document was moved to [project_services/slack.md](../project_services/slack.md).

doc/project_services/slack.md

 # Slack Service
 
-Go to your project's **Settings > Services > Slack** and you will see a checkbox with the following events that can be triggered:
+## On Slack
 
-* Push
-* Issue
-* Merge request
-* Note
-* Tag push
-* Build
-* Wiki page
+To enable Slack integration you must create an incoming webhook integration on
+Slack:
 
-Below each of these event checkboxes you will have an input to insert which Slack channel do you want to send that event message,
-`#general` channel is default.
+1. [Sign in to Slack](https://slack.com/signin)
+1. Visit [Incoming WebHooks](https://my.slack.com/services/new/incoming-webhook/)
+1. Choose the channel name you want to send notifications to.
+1. Click **Add Incoming WebHooks Integration**
+1. Copy the **Webhook URL**, we'll need this later for GitLab.
 
+## On GitLab
 
-![Slack configuration](img/slack_configuration.png)
+After you set up Slack, it's time to set up GitLab.
+
+Go to your project's **Settings > Services > Slack** and you will see a
+checkbox with the following events that can be triggered:
+
+- Push
+- Issue
+- Merge request
+- Note
+- Tag push
+- Build
+- Wiki page
+
+Bellow each of these event checkboxes, you will have an input field to insert
+which Slack channel you want to send that event message, with `#general`
+being the default. Enter your preferred channel **without** the hash sign (`#`).
 
+At the end, fill in your Slack details:
 
 | Field | Description |
 | ----- | ----------- |
-| `Webhook`   | The incoming webhook url which you have to setup on slack. (https://my.slack.com/services/new/incoming-webhook/) |
-| `Username`   | Optional username which can be on messages sent to slack. |
-| `notify only broken builds`    | Notify only about broken builds, when build events are marked to be sent.|
+| **Webhook**  | The [incoming webhook URL][slackhook] which you have to setup on Slack. |
+| **Username** | Optional username which can be on messages sent to slack. Fill this in if you want to change the username of the bot. |
+| **Notify only broken builds** | If you choose to enable the **Build** event and you want to be only notified about failed builds. |
 
+After you are all done, click **Save changes** for the changes to take effect.
+
+>**Note:**
+You can set "branch,pushed,Compare changes" as highlight words on your Slack
+profile settings, so that you can be aware of new commits when somebody pushes
+them.
+
+![Slack configuration](img/slack_configuration.png)
 
+[slackhook]: https://my.slack.com/services/new/incoming-webhook

doc/raketasks/cleanup.md

@@ -2,7 +2,7 @@
 
 ## Remove garbage from filesystem. Important! Data loss!
 
-Remove namespaces(dirs) from `/home/git/repositories` if they don't exist in GitLab database.
+Remove namespaces(dirs) from all repository storage paths if they don't exist in GitLab database.
 
 ```
 # omnibus-gitlab
@@ -12,7 +12,7 @@ sudo gitlab-rake gitlab:cleanup:dirs
 bundle exec rake gitlab:cleanup:dirs RAILS_ENV=production
 ```
 
-Rename repositories from `/home/git/repositories` if they don't exist in GitLab database.
+Rename repositories from all repository storage paths if they don't exist in GitLab database.
 The repositories get a `+orphaned+TIMESTAMP` suffix so that they cannot block new repositories from being created.
 
 ```

doc/user/admin_area/settings/sign_up_restrictions.md

+# Sign-up restrictions
+
+## Blacklist email domains
+
+> [Introduced][ce-5259] in GitLab 8.10.
+
+With this feature enabled, you can block email addresses of a specific domain
+from creating an account on your GitLab server. This is particularly useful to
+prevent spam. Disposable email addresses are usually used by malicious users to
+create dummy accounts and spam issues.
+
+This feature can be activated via the **Application Settings** in the Admin area,
+and you have the option of entering the list manually, or uploading a file with
+the list.
+
+The blacklist accepts wildcards, so you can use `*.test.com` to block every
+`test.com` subdomain, or `*.io` to block all domains ending in `.io`. Domains
+should be separated by a whitespace, semicolon, comma, or a new line.
+
+![Domain Blacklist](img/domain_blacklist.png)
+
+[ce-5259]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5259

doc/administration/access_restrictions.md → doc/user/admin_area/settings/visibility_and_access_controls.md

-# Access Restrictions
+# Visibility and access controls
 
-> **Note:** These features are only available on versions 8.10 and above.
+## Enabled Git access protocols
+
+> [Introduced][ce-4696] in GitLab 8.10.
 
 With GitLab's Access restrictions you can choose which Git access protocols you
 want your users to use to communicate with GitLab. This feature can be enabled
@@ -15,8 +17,6 @@ to choose between:
 
 ![Settings Overview](img/access_restrictions.png)
 
-## Enabled Protocol
-
 When both SSH and HTTP(S) are enabled, GitLab will behave as usual, it will give
 your users the option to choose which protocol they would like to use.
 
@@ -37,20 +37,4 @@ not selected.
   HTTP, will still be accessible. What GitLab does is restrict access on the
   application level.
 
-## Blacklist email domains
-
-With this feature enabled, you can block email addresses of a specific domain
-from creating an account on your GitLab server. This is particularly useful to
-prevent spam. Disposable email addresses are usually used by malicious users to
-create dummy accounts and spam issues.
-
-This feature can be activated via the `Application Settings` in the Admin area,
-and you have the option of entering the list manually, or uploading a file with
-the list.
-
-The blacklist accepts wildcards, so you can use `*.test.com` to block every
-`test.com` subdomain, or `*.io` to block all domains ending in `.io`. Domains
-should be separated by a whitespace, semicolon, comma, or a new line.
-
-![Domain Blacklist](img/domain_blacklist.png)
-
+[ce-4696]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4696

doc/user/project/protected_branches.md

+# Protected Branches
+
+[Permissions](../permissions.md) in GitLab are fundamentally defined around the
+idea of having read or write permission to the repository and branches. To
+prevent people from messing with history or pushing code without review, we've
+created protected branches.
+
+By default, a protected branch does four simple things:
+
+- it prevents its creation, if not already created, from everybody except users
+  with Master permission
+- it prevents pushes from everybody except users with Master permission
+- it prevents **anyone** from force pushing to the branch
+- it prevents **anyone** from deleting the branch
+
+See the [Changelog](#changelog) section for changes over time.
+
+## Configuring protected branches
+
+To protect a branch, you need to have at least Master permission level. Note
+that the `master` branch is protected by default.
+
+1. Navigate to the main page of the project.
+1. In the upper right corner, click the settings wheel and select **Protected branches**.
+
+    ![Project settings list](img/project_settings_list.png)
+
+1. From the **Branch** dropdown menu, select the branch you want to protect and
+   click **Protect**. In the screenshot below, we chose the `develop` branch.
+
+    ![Choose protected branch](img/protected_branches_choose_branch.png)
+
+1. Once done, the protected branch will appear in the "Already protected" list.
+
+    ![Protected branches list](img/protected_branches_list.png)
+
+
+Since GitLab 8.10, we added another layer of branch protection which provides
+more granular management of protected branches. You can now choose the option
+"Developers can merge" so that Developer users can merge a merge request but
+not directly push. In that case, your branches are protected from direct pushes,
+yet Developers don't need elevated permissions or wait for someone with a higher
+permission level to press merge.
+
+You can set this option while creating the protected branch or after its
+creation.
+
+## Wildcard protected branches
+
+>**Note:**
+This feature was [introduced][ce-4665] in GitLab 8.10.
+
+You can specify a wildcard protected branch, which will protect all branches
+matching the wildcard. For example:
+
+| Wildcard Protected Branch | Matching Branches                                      |
+|---------------------------+--------------------------------------------------------|
+| `*-stable`                | `production-stable`, `staging-stable`                  |
+| `production/*`            | `production/app-server`, `production/load-balancer`    |
+| `*gitlab*`                | `gitlab`, `gitlab/staging`, `master/gitlab/production` |
+
+Protected branch settings (like "Developers can push") apply to all matching
+branches.
+
+Two different wildcards can potentially match the same branch. For example,
+`*-stable` and `production-*` would both match a `production-stable` branch.
+In that case, if _any_ of these protected branches have a setting like
+"Allowed to push", then `production-stable` will also inherit this setting.
+
+If you click on a protected branch's name that is created using a wildcard,
+you will be presented with a list of all matching branches:
+
+![Protected branch matches](img/protected_branches_matches.png)
+
+## Restrict the creation of protected branches
+
+Creating a protected branch or a list of protected branches using the wildcard
+feature, not only you are restricting pushes to those branches, but also their
+creation if not already created.
+
+## Error messages when pushing to a protected branch
+
+A user with insufficient permissions will be presented with an error when
+creating or pushing to a branch that's prohibited, either through GitLab's UI:
+
+![Protected branch error GitLab UI](img/protected_branches_error_ui.png)
+
+or using Git from their terminal:
+
+```bash
+remote: GitLab: You are not allowed to push code to protected branches on this project.
+To https://gitlab.example.com/thedude/bowling.git
+ ! [remote rejected] staging-stable -> staging-stable (pre-receive hook declined)
+error: failed to push some refs to 'https://gitlab.example.com/thedude/bowling.git'
+```
+
+## Changelog
+
+**8.10.0**
+
+- Allow specifying protected branches using wildcards [gitlab-org/gitlab-ce!5081][ce-4665]
+
+---
+
+[ce-4665]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4665 "Allow specifying protected branches using wildcards"
+[ce-5081]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5081 "Allow creating protected branches that can't be pushed to"

doc/workflow/README.md

@@ -12,7 +12,7 @@
 - [Project Features](project_features.md)
 - [Project forking workflow](forking_workflow.md)
 - [Project users](add-user/add-user.md)
-- [Protected branches](protected_branches.md)
+- [Protected branches](../user/project/protected_branches.md)
 - [Sharing a project with a group](share_with_group.md)
 - [Share projects with other groups](share_projects_with_other_groups.md)
 - [Web Editor](web_editor.md)

doc/workflow/protected_branches.md

-# Protected Branches
-
-Permissions in GitLab are fundamentally defined around the idea of having read or write permission to the repository and branches.
-
-To prevent people from messing with history or pushing code without review, we've created protected branches.
-
-A protected branch does three simple things:
-
-* it prevents pushes from everybody except users with Master permission
-* it prevents anyone from force pushing to the branch
-* it prevents anyone from deleting the branch
-
-You can make any branch a protected branch. GitLab makes the master branch a protected branch by default.
-
-To protect a branch, user needs to have at least a Master permission level, see [permissions document](../user/permissions.md).
-
-![protected branches page](protected_branches/protected_branches1.png)
-
-Navigate to project settings page and select `protected branches`. From the `Branch` dropdown menu select the branch you want to protect.
-
-Some workflows, like [GitLab workflow](gitlab_flow.md), require all users with write access to submit a Merge request in order to get the code into a protected branch.
-
-Since Masters and Owners can already push to protected branches, that means Developers cannot push to protected branch and need to submit a Merge request.
-
-However, there are workflows where that is not needed and only protecting from force pushes and branch removal is useful.
-
-For those workflows, you can allow everyone with write access to push to a protected branch by selecting `Developers can push` check box.
-
-On already protected branches you can also allow developers to push to the repository by selecting the `Developers can push` check box.
-
-![Developers can push](protected_branches/protected_branches2.png)
-
-## Wildcard Protected Branches
-
->**Note:**
-This feature was added in GitLab 8.10.
-
-1. You can specify a wildcard protected branch, which will protect all branches matching the wildcard. For example:
-
-    | Wildcard Protected Branch | Matching Branches                                      |
-    |---------------------------+--------------------------------------------------------|
-    | `*-stable`                | `production-stable`, `staging-stable`                  |
-    | `production/*`            | `production/app-server`, `production/load-balancer`    |
-    | `*gitlab*`                | `gitlab`, `gitlab/staging`, `master/gitlab/production` |
-
-1. Protected branch settings (like "Developers Can Push") apply to all matching branches.
-
-1. Two different wildcards can potentially match the same branch. For example, `*-stable` and `production-*` would both match a `production-stable` branch.
-   >**Note:**
-   If _any_ of these protected branches have "Developers Can Push" set to true, then `production-stable` has it set to true.
-
-1. If you click on a protected branch's name, you will be presented with a list of all matching branches:
-
-    ![protected branch matches](protected_branches/protected_branches3.png)
-
+This document is moved to [user/project/protected_branches.md](../user/project/protected_branches.md)

features/project/wiki.feature

@@ -8,6 +8,12 @@ Feature: Project Wiki
     Given I create the Wiki Home page
     Then I should see the newly created wiki page
 
+  Scenario: Add new page with errors
+    Given I create the Wiki Home page with no content
+    Then I should see a "Content can't be blank" error message
+    When I create the Wiki Home page
+    Then I should see the newly created wiki page
+
   Scenario: Pressing Cancel while editing a brand new Wiki
     Given I click on the Cancel button
     Then I should be redirected back to the Edit Home Wiki page

features/steps/project/wiki.rb

@@ -19,6 +19,11 @@ class Spinach::Features::ProjectWiki < Spinach::FeatureSteps
     click_on "Create page"
   end
 
+  step 'I create the Wiki Home page with no content' do
+    fill_in "wiki_content", with: ''
+    click_on "Create page"
+  end
+
   step 'I should see the newly created wiki page' do
     expect(page).to have_content "Home"
     expect(page).to have_content "link test"
@@ -173,6 +178,11 @@ class Spinach::Features::ProjectWiki < Spinach::FeatureSteps
     find('a[href*="?version_id"]')
   end
 
+  step 'I should see a "Content can\'t be blank" error message' do
+    expect(page).to have_content('The form contains the following error:')
+    expect(page).to have_content('Content can\'t be blank')
+  end
+
   def wiki
     @project_wiki = ProjectWiki.new(project, current_user)
   end

lib/api/builds.rb

@@ -80,7 +80,7 @@ module API
       #   ref_name (required) - The ref from repository
       #   job (required) - The name for the build
       # Example Request:
-      #   GET /projects/:id/artifacts/:ref_name/download?job=name
+      #   GET /projects/:id/builds/artifacts/:ref_name/download?job=name
       get ':id/builds/artifacts/:ref_name/download',
         requirements: { ref_name: /.+/ } do
         authorize_read_builds!

lib/banzai/reference_extractor.rb

@@ -2,11 +2,11 @@ module Banzai
   # Extract possible GFM references from an arbitrary String for further processing.
   class ReferenceExtractor
     def initialize
-      @texts = []
+      @texts_and_contexts = []
     end
 
     def analyze(text, context = {})
-      @texts << Renderer.render(text, context)
+      @texts_and_contexts << { text: text, context: context }
     end
 
     def references(type, project, current_user = nil)
@@ -21,9 +21,10 @@ module Banzai
     def html_documents
       # This ensures that we don't memoize anything until we have a number of
       # text blobs to parse.
-      return [] if @texts.empty?
+      return [] if @texts_and_contexts.empty?
 
-      @html_documents ||= @texts.map { |html| Nokogiri::HTML.fragment(html) }
+      @html_documents ||= Renderer.cache_collection_render(@texts_and_contexts)
+        .map { |html| Nokogiri::HTML.fragment(html) }
     end
   end
 end

lib/gitlab/diff/parallel_diff.rb

@@ -8,72 +8,35 @@ module Gitlab
       end
 
       def parallelize
-
         i = 0
         free_right_index = nil
 
         lines = []
         highlighted_diff_lines = diff_file.highlighted_diff_lines
         highlighted_diff_lines.each do |line|
-          line_code = diff_file.line_code(line)
-          position = diff_file.position(line)
-
-          case line.type
-          when 'match', nil
+          if line.meta? || line.unchanged?
             # line in the right panel is the same as in the left one
             lines << {
-              left: {
-                type:       line.type,
-                number:     line.old_pos,
-                text:       line.text,
-                line_code:  line_code,
-                position:   position
-              },
-              right: {
-                type:       line.type,
-                number:     line.new_pos,
-                text:       line.text,
-                line_code:  line_code,
-                position:   position
-              }
+              left: line,
+              right: line
             }
 
             free_right_index = nil
             i += 1
-          when 'old'
+          elsif line.removed?
             lines << {
-              left: {
-                type:       line.type,
-                number:     line.old_pos,
-                text:       line.text,
-                line_code:  line_code,
-                position:   position
-              },
-              right: {
-                type:       nil,
-                number:     nil,
-                text:       "",
-                line_code:  line_code,
-                position:   position
-              }
+              left: line,
+              right: nil
             }
 
             # Once we come upon a new line it can be put on the right of this old line
             free_right_index ||= i
             i += 1
-          when 'new'
-            data = {
-              type:       line.type,
-              number:     line.new_pos,
-              text:       line.text,
-              line_code:  line_code,
-              position:   position
-            }
-
+          elsif line.added?
             if free_right_index
               # If an old line came before this without a line on the right, this
               # line can be put to the right of it.
-              lines[free_right_index][:right] = data
+              lines[free_right_index][:right] = line
 
               # If there are any other old lines on the left that don't yet have
               # a new counterpart on the right, update the free_right_index
@@ -81,14 +44,8 @@ module Gitlab
               free_right_index = next_free_right_index < i ? next_free_right_index : nil
             else
               lines << {
-                left: {
-                  type:       nil,
-                  number:     nil,
-                  text:       "",
-                  line_code:  line_code,
-                  position:   position
-                },
-                right: data
+                left: nil,
+                right: line
               }
 
               free_right_index = nil

spec/controllers/help_controller_spec.rb

@@ -36,7 +36,7 @@ describe HelpController do
       context 'when requested file exists' do
         it 'renders the raw file' do
           get :show,
-              path: 'workflow/protected_branches/protected_branches1',
+              path: 'user/project/img/labels_filter',
               format: :png
           expect(response).to be_success
           expect(response.content_type).to eq 'image/png'

spec/factories/issues.rb

@@ -18,5 +18,15 @@ FactoryGirl.define do
 
     factory :closed_issue, traits: [:closed]
     factory :reopened_issue, traits: [:reopened]
+
+    factory :labeled_issue do
+      transient do
+        labels []
+      end
+
+      after(:create) do |issue, evaluator|
+        issue.update_attributes(labels: evaluator.labels)
+      end
+    end
   end
 end

spec/initializers/6_validations_spec.rb

 require 'spec_helper'
+require_relative '../../config/initializers/6_validations.rb'
 
 describe '6_validations', lib: true do
+  before :all do
+    FileUtils.mkdir_p('tmp/tests/paths/a/b/c/d')
+    FileUtils.mkdir_p('tmp/tests/paths/a/b/c2')
+    FileUtils.mkdir_p('tmp/tests/paths/a/b/d')
+  end
+
+  after :all do
+    FileUtils.rm_rf('tmp/tests/paths')
+  end
+
   context 'with correct settings' do
     before do
-      mock_storages('foo' => '/a/b/c', 'bar' => 'a/b/d')
+      mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/d')
     end
 
     it 'passes through' do
-      expect { load_validations }.not_to raise_error
+      expect { validate_storages }.not_to raise_error
     end
   end
 
   context 'with invalid storage names' do
     before do
-      mock_storages('name with spaces' => '/a/b/c')
+      mock_storages('name with spaces' => 'tmp/tests/paths/a/b/c')
     end
 
     it 'throws an error' do
-      expect { load_validations }.to raise_error('"name with spaces" is not a valid storage name. Please fix this in your gitlab.yml before starting GitLab.')
+      expect { validate_storages }.to raise_error('"name with spaces" is not a valid storage name. Please fix this in your gitlab.yml before starting GitLab.')
     end
   end
 
   context 'with nested storage paths' do
     before do
-      mock_storages('foo' => '/a/b/c', 'bar' => '/a/b/c/d')
+      mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c/d')
     end
 
     it 'throws an error' do
-      expect { load_validations }.to raise_error('bar is a nested path of foo. Nested paths are not supported for repository storages. Please fix this in your gitlab.yml before starting GitLab.')
+      expect { validate_storages }.to raise_error('bar is a nested path of foo. Nested paths are not supported for repository storages. Please fix this in your gitlab.yml before starting GitLab.')
     end
   end
 
-  def mock_storages(storages)
-    allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+  context 'with similar but un-nested storage paths' do
+    before do
+      mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c2')
     end
 
-  def load_validations
-    load File.join(__dir__, '../../config/initializers/6_validations.rb')
+    it 'passes through' do
+      expect { validate_storages }.not_to raise_error
+    end
+  end
+
+  def mock_storages(storages)
+    allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
   end
 end

spec/lib/gitlab/diff/parallel_diff_spec.rb

@@ -11,11 +11,51 @@ describe Gitlab::Diff::ParallelDiff, lib: true do
   let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) }
   subject { described_class.new(diff_file) }
 
-  let(:parallel_diff_result_array) { YAML.load_file("#{Rails.root}/spec/fixtures/parallel_diff_result.yml") }
-
   describe '#parallelize' do
     it 'should return an array of arrays containing the parsed diff' do
-      expect(subject.parallelize).to match_array(parallel_diff_result_array)
+      diff_lines = diff_file.highlighted_diff_lines
+      expected = [
+        # Unchanged lines
+        { left: diff_lines[0], right: diff_lines[0] },
+        { left: diff_lines[1], right: diff_lines[1] },
+        { left: diff_lines[2], right: diff_lines[2] },
+        { left: diff_lines[3], right: diff_lines[3] },
+        { left: diff_lines[4], right: diff_lines[5] },
+        { left: diff_lines[6], right: diff_lines[6] },
+        { left: diff_lines[7], right: diff_lines[7] },
+        { left: diff_lines[8], right: diff_lines[8] },
+
+        # Changed lines
+        { left: diff_lines[9], right: diff_lines[11] },
+        { left: diff_lines[10], right: diff_lines[12] },
+
+        # Added lines
+        { left: nil, right: diff_lines[13] },
+        { left: nil, right: diff_lines[14] },
+        { left: nil, right: diff_lines[15] },
+        { left: nil, right: diff_lines[16] },
+        { left: nil, right: diff_lines[17] },
+        { left: nil, right: diff_lines[18] },
+
+        # Unchanged lines
+        { left: diff_lines[19], right: diff_lines[19] },
+        { left: diff_lines[20], right: diff_lines[20] },
+        { left: diff_lines[21], right: diff_lines[21] },
+        { left: diff_lines[22], right: diff_lines[22] },
+        { left: diff_lines[23], right: diff_lines[23] },
+        { left: diff_lines[24], right: diff_lines[24] },
+        { left: diff_lines[25], right: diff_lines[25] },
+
+        # Added line
+        { left: nil, right: diff_lines[26] },
+
+        # Unchanged lines
+        { left: diff_lines[27], right: diff_lines[27] },
+        { left: diff_lines[28], right: diff_lines[28] },
+        { left: diff_lines[29], right: diff_lines[29] }
+      ]
+
+      expect(subject.parallelize).to eq(expected)
     end
   end
 end

spec/mailers/notify_spec.rb

@@ -12,7 +12,7 @@ describe Notify do
   context 'for a project' do
     describe 'items that are assignable, the email' do
       let(:current_user) { create(:user, email: "current@email.com") }
-      let(:assignee) { create(:user, email: 'assignee@example.com') }
+      let(:assignee) { create(:user, email: 'assignee@example.com', name: 'John Doe') }
       let(:previous_assignee) { create(:user, name: 'Previous Assignee') }
 
       shared_examples 'an assignee email' do

spec/models/note_spec.rb

@@ -135,22 +135,30 @@ describe Note, models: true do
     let!(:note2) { create(:note_on_issue) }
 
     it "reads the rendered note body from the cache" do
-      expect(Banzai::Renderer).to receive(:render).
-        with(note1.note,
+      expect(Banzai::Renderer).to receive(:cache_collection_render).
+        with([{
+          text: note1.note,
+          context: {
             pipeline: :note,
             cache_key: [note1, "note"],
             project: note1.project,
-             author: note1.author)
-
-      expect(Banzai::Renderer).to receive(:render).
-        with(note2.note,
+            author: note1.author
+          }
+        }]).and_call_original
+
+      expect(Banzai::Renderer).to receive(:cache_collection_render).
+        with([{
+          text: note2.note,
+          context: {
             pipeline: :note,
             cache_key: [note2, "note"],
             project: note2.project,
-             author: note2.author)
+            author: note2.author
+          }
+        }]).and_call_original
 
-      note1.all_references
-      note2.all_references
+      note1.all_references.users
+      note2.all_references.users
     end
   end
 

spec/models/repository_spec.rb

@@ -50,8 +50,9 @@ describe Repository, models: true do
           double_first = double(committed_date: Time.now)
           double_last = double(committed_date: Time.now - 1.second)
 
-          allow(repository).to receive(:commit).with(tag_a.target).and_return(double_first)
-          allow(repository).to receive(:commit).with(tag_b.target).and_return(double_last)
+          allow(tag_a).to receive(:target).and_return(double_first)
+          allow(tag_b).to receive(:target).and_return(double_last)
+          allow(repository).to receive(:tags).and_return([tag_a, tag_b])
         end
 
         it { is_expected.to eq(['v1.0.0', 'v1.1.0']) }
@@ -64,8 +65,9 @@ describe Repository, models: true do
           double_first = double(committed_date: Time.now - 1.second)
           double_last = double(committed_date: Time.now)
 
-          allow(repository).to receive(:commit).with(tag_a.target).and_return(double_last)
-          allow(repository).to receive(:commit).with(tag_b.target).and_return(double_first)
+          allow(tag_a).to receive(:target).and_return(double_last)
+          allow(tag_b).to receive(:target).and_return(double_first)
+          allow(repository).to receive(:tags).and_return([tag_a, tag_b])
         end
 
         it { is_expected.to eq(['v1.1.0', 'v1.0.0']) }
@@ -1161,41 +1163,6 @@ describe Repository, models: true do
     end
   end
 
-  describe '#local_branches' do
-    it 'returns the local branches' do
-      masterrev = repository.find_branch('master').target
-      create_remote_branch('joe', 'remote_branch', masterrev)
-      repository.add_branch(user, 'local_branch', masterrev)
-
-      expect(repository.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false)
-      expect(repository.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true)
-    end
-  end
-
-  describe '.clean_old_archives' do
-    let(:path) { Gitlab.config.gitlab.repository_downloads_path }
-
-    context 'when the downloads directory does not exist' do
-      it 'does not remove any archives' do
-        expect(File).to receive(:directory?).with(path).and_return(false)
-
-        expect(Gitlab::Popen).not_to receive(:popen)
-
-        described_class.clean_old_archives
-      end
-    end
-
-    context 'when the downloads directory exists' do
-      it 'removes old archives' do
-        expect(File).to receive(:directory?).with(path).and_return(true)
-
-        expect(Gitlab::Popen).to receive(:popen)
-
-        described_class.clean_old_archives
-      end
-    end
-  end
-
   describe "#keep_around" do
     it "stores a reference to the specified commit sha so it isn't garbage collected" do
       repository.keep_around(sample_commit.id)
@@ -1203,9 +1170,4 @@ describe Repository, models: true do
       expect(repository.kept_around?(sample_commit.id)).to be_truthy
     end
   end
-
-  def create_remote_branch(remote_name, branch_name, target)
-    rugged = repository.rugged
-    rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target)
-  end
 end

spec/services/issues/bulk_update_service_spec.rb

@@ -2,89 +2,81 @@ require 'spec_helper'
 
 describe Issues::BulkUpdateService, services: true do
   let(:user)    { create(:user) }
-  let(:project) { Projects::CreateService.new(user, namespace: user.namespace, name: 'test').execute }
+  let(:project) { create(:empty_project, namespace: user.namespace) }
 
-  let!(:result) { Issues::BulkUpdateService.new(project, user, params).execute }
+  def bulk_update(issues, extra_params = {})
+    bulk_update_params = extra_params
+      .reverse_merge(issues_ids: Array(issues).map(&:id).join(','))
 
-  describe :close_issue do
-    let(:issues) { create_list(:issue, 5, project: project) }
-    let(:params) do
-      {
-        state_event: 'close',
-        issues_ids: issues.map(&:id).join(',')
-      }
+    Issues::BulkUpdateService.new(project, user, bulk_update_params).execute
   end
 
+  describe 'close issues' do
+    let(:issues) { create_list(:issue, 2, project: project) }
+
     it 'succeeds and returns the correct number of issues updated' do
+      result = bulk_update(issues, state_event: 'close')
+
       expect(result[:success]).to be_truthy
       expect(result[:count]).to eq(issues.count)
     end
 
     it 'closes all the issues passed' do
+      bulk_update(issues, state_event: 'close')
+
       expect(project.issues.opened).to be_empty
       expect(project.issues.closed).not_to be_empty
     end
   end
 
-  describe :reopen_issues do
-    let(:issues) { create_list(:closed_issue, 5, project: project) }
-    let(:params) do
-      {
-        state_event: 'reopen',
-        issues_ids: issues.map(&:id).join(',')
-      }
-    end
+  describe 'reopen issues' do
+    let(:issues) { create_list(:closed_issue, 2, project: project) }
 
     it 'succeeds and returns the correct number of issues updated' do
+      result = bulk_update(issues, state_event: 'reopen')
+
       expect(result[:success]).to be_truthy
       expect(result[:count]).to eq(issues.count)
     end
 
     it 'reopens all the issues passed' do
+      bulk_update(issues, state_event: 'reopen')
+
       expect(project.issues.closed).to be_empty
       expect(project.issues.opened).not_to be_empty
     end
   end
 
   describe 'updating assignee' do
-    let(:issue) do
-      create(:issue, project: project) { |issue| issue.update_attributes(assignee: user) }
-    end
-
-    let(:params) do
-      {
-        assignee_id: assignee_id,
-        issues_ids: issue.id.to_s
-      }
-    end
+    let(:issue) { create(:issue, project: project, assignee: user) }
 
     context 'when the new assignee ID is a valid user' do
-      let(:new_assignee) { create(:user) }
-      let(:assignee_id) { new_assignee.id }
-
       it 'succeeds' do
+        result = bulk_update(issue, assignee_id: create(:user).id)
+
         expect(result[:success]).to be_truthy
         expect(result[:count]).to eq(1)
       end
 
       it 'updates the assignee to the use ID passed' do
-        expect(issue.reload.assignee).to eq(new_assignee)
+        assignee = create(:user)
+
+        expect { bulk_update(issue, assignee_id: assignee.id) }
+          .to change { issue.reload.assignee }.from(user).to(assignee)
       end
     end
 
     context 'when the new assignee ID is -1' do
-      let(:assignee_id) { -1 }
-
       it 'unassigns the issues' do
-        expect(issue.reload.assignee).to be_nil
+        expect { bulk_update(issue, assignee_id: -1) }
+          .to change { issue.reload.assignee }.to(nil)
       end
     end
 
     context 'when the new assignee ID is not present' do
-      let(:assignee_id) { nil }
-
       it 'does not unassign' do
-        expect(issue.reload.assignee).to eq(user)
+        expect { bulk_update(issue, assignee_id: nil) }
+          .not_to change { issue.reload.assignee }
       end
     end
   end
@@ -93,26 +85,22 @@ describe Issues::BulkUpdateService, services: true do
     let(:issue)     { create(:issue, project: project) }
     let(:milestone) { create(:milestone, project: project) }
 
-    let(:params) do
-      {
-        issues_ids: issue.id.to_s,
-        milestone_id: milestone.id
-      }
-    end
-
     it 'succeeds' do
+      result = bulk_update(issue, milestone_id: milestone.id)
+
       expect(result[:success]).to be_truthy
       expect(result[:count]).to eq(1)
     end
 
     it 'updates the issue milestone' do
-      expect(project.issues.first.milestone).to eq(milestone)
+      expect { bulk_update(issue, milestone_id: milestone.id) }
+        .to change { issue.reload.milestone }.from(nil).to(milestone)
     end
   end
 
   describe 'updating labels' do
     def create_issue_with_labels(labels)
-      create(:issue, project: project) { |issue| issue.update_attributes(labels: labels) }
+      create(:labeled_issue, project: project, labels: labels)
     end
 
     let(:bug) { create(:label, project: project) }
@@ -129,15 +117,18 @@ describe Issues::BulkUpdateService, services: true do
     let(:add_labels) { [] }
     let(:remove_labels) { [] }
 
-    let(:params) do
+    let(:bulk_update_params) do
       {
         label_ids:        labels.map(&:id),
         add_label_ids:    add_labels.map(&:id),
         remove_label_ids: remove_labels.map(&:id),
-        issues_ids: issues.map(&:id).join(',')
       }
     end
 
+    before do
+      bulk_update(issues, bulk_update_params)
+    end
+
     context 'when label_ids are passed' do
       let(:issues) { [issue_all_labels, issue_no_labels] }
       let(:labels) { [bug, regression] }
@@ -263,40 +254,28 @@ describe Issues::BulkUpdateService, services: true do
     end
   end
 
-  describe :subscribe_issues do
-    let(:issues) { create_list(:issue, 5, project: project) }
-    let(:params) do
-      {
-        subscription_event: 'subscribe',
-        issues_ids: issues.map(&:id).join(',')
-      }
-    end
+  describe 'subscribe to issues' do
+    let(:issues) { create_list(:issue, 2, project: project) }
 
     it 'subscribes the given user' do
-      issues.each do |issue|
-        expect(issue.subscribed?(user)).to be_truthy
-      end
-    end
-  end
+      bulk_update(issues, subscription_event: 'subscribe')
 
-  describe :unsubscribe_issues do
-    let(:issues) { create_list(:closed_issue, 5, project: project) }
-    let(:params) do
-      {
-        subscription_event: 'unsubscribe',
-        issues_ids: issues.map(&:id).join(',')
-      }
+      expect(issues).to all(be_subscribed(user))
+    end
   end
 
-    before do
-      issues.each do |issue|
+  describe 'unsubscribe from issues' do
+    let(:issues) do
+      create_list(:closed_issue, 2, project: project) do |issue|
         issue.subscriptions.create(user: user, subscribed: true)
       end
     end
 
     it 'unsubscribes the given user' do
+      bulk_update(issues, subscription_event: 'unsubscribe')
+
       issues.each do |issue|
-        expect(issue.subscribed?(user)).to be_falsey
+        expect(issue).not_to be_subscribed(user)
       end
     end
   end

spec/services/repository_archive_clean_up_service_spec.rb

+require 'spec_helper'
+
+describe RepositoryArchiveCleanUpService, services: true do
+  describe '#execute' do
+    subject(:service) { described_class.new }
+
+    context 'when the downloads directory does not exist' do
+      it 'does not remove any archives' do
+        path = '/invalid/path/'
+        stub_repository_downloads_path(path)
+
+        expect(File).to receive(:directory?).with(path).and_return(false)
+        expect(service).not_to receive(:clean_up_old_archives)
+        expect(service).not_to receive(:clean_up_empty_directories)
+
+        service.execute
+      end
+    end
+
+    context 'when the downloads directory exists' do
+      shared_examples 'invalid archive files' do |dirname, extensions, mtime|
+        it 'does not remove files and directoy' do
+          in_directory_with_files(dirname, extensions, mtime) do |dir, files|
+            service.execute
+
+            files.each { |file| expect(File.exist?(file)).to eq true }
+            expect(File.directory?(dir)).to eq true
+          end
+        end
+      end
+
+      it 'removes files older than 2 hours that matches valid archive extensions' do
+        in_directory_with_files('sample.git', %w[tar tar.bz2 tar.gz zip], 2.hours) do |dir, files|
+          service.execute
+
+          files.each { |file| expect(File.exist?(file)).to eq false }
+          expect(File.directory?(dir)).to eq false
+        end
+      end
+
+      context 'with files older than 2 hours that does not matches valid archive extensions' do
+        it_behaves_like 'invalid archive files', 'sample.git', %w[conf rb], 2.hours
+      end
+
+      context 'with files older than 2 hours inside invalid directories' do
+        it_behaves_like 'invalid archive files', 'john_doe/sample.git', %w[conf rb tar tar.gz], 2.hours
+      end
+
+      context 'with files newer than 2 hours that matches valid archive extensions' do
+        it_behaves_like 'invalid archive files', 'sample.git', %w[tar tar.bz2 tar.gz zip], 1.hour
+      end
+
+      context 'with files newer than 2 hours that does not matches valid archive extensions' do
+        it_behaves_like 'invalid archive files', 'sample.git', %w[conf rb], 1.hour
+      end
+
+      context 'with files newer than 2 hours inside invalid directories' do
+        it_behaves_like 'invalid archive files', 'sample.git', %w[conf rb tar tar.gz], 1.hour
+      end
+    end
+
+    def in_directory_with_files(dirname, extensions, mtime)
+      Dir.mktmpdir do |tmpdir|
+        stub_repository_downloads_path(tmpdir)
+        dir = File.join(tmpdir, dirname)
+        files = create_temporary_files(dir, extensions, mtime)
+
+        yield(dir, files)
+      end
+    end
+
+    def stub_repository_downloads_path(path)
+      allow(Gitlab.config.gitlab).to receive(:repository_downloads_path).and_return(path)
+    end
+
+    def create_temporary_files(dir, extensions, mtime)
+      FileUtils.mkdir_p(dir)
+      FileUtils.touch(extensions.map { |ext| File.join(dir, "sample.#{ext}") }, mtime: Time.now - mtime)
+    end
+  end
+end