Merge remote-tracking branch 'upstream/master' into pipeline-notifications

* upstream/master: (216 commits)
  Remove invalid changelog entries
  require rails/generators for generators
  Adds variable. Fixes changelog
  Fix haml_lint
  Fixes after review
  Update changelog
  Remove tooltips from project header
  Makes table pagination responsive
  23545 Fix distorted project and group avatars
  Fix spinach tests
  Update links in side nav and header
  Allow to search for user by secondary email address in the admin interface
  Rename :name search parameter to :search_query at /admin/users
  Initialize Sidekiq with the list of queues used by GitLab
  Fix project features default values
  Add a link to the Issue Boards API in main README
  Add validation errors to Merge Request form
  Fix Markdown styling inside reference links
  Fix relative links in Markdown wiki when displayed in "Project" tab
  Adds label description to issue board title
  ...

10 files changed, 93 insertions, 49 deletions

diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb
index d3dd30b2588..8face432d97 100644
--- a/app/services/ci/process_pipeline_service.rb
+++ b/app/services/ci/process_pipeline_service.rb
@@ -10,17 +10,14 @@ module Ci
         create_builds!
       end
 
-      @pipeline.with_lock do
-        new_builds =
-          stage_indexes_of_created_builds.map do |index|
-            process_stage(index)
-          end
+      new_builds =
+        stage_indexes_of_created_builds.map do |index|
+          process_stage(index)
+        end
 
-        @pipeline.update_status
+      @pipeline.update_status
 
-        # Return a flag if a when builds got enqueued
-        new_builds.flatten.any?
-      end
+      new_builds.flatten.any?
     end
 
     private
@@ -32,9 +29,11 @@ module Ci
     def process_stage(index)
       current_status = status_for_prior_stages(index)
 
-      created_builds_in_stage(index).select do |build|
-        if HasStatus::COMPLETED_STATUSES.include?(current_status)
-          process_build(build, current_status)
+      if HasStatus::COMPLETED_STATUSES.include?(current_status)
+        created_builds_in_stage(index).select do |build|
+          Gitlab::OptimisticLocking.retry_lock(build) do |subject|
+            process_build(subject, current_status)
+          end
         end
       end
     end
diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb
index 6973191b203..74b5ebf372b 100644
--- a/app/services/ci/register_build_service.rb
+++ b/app/services/ci/register_build_service.rb
@@ -28,17 +28,14 @@ module Ci
 
       if build
         # In case when 2 runners try to assign the same build, second runner will be declined
-        # with StateMachines::InvalidTransition in run! method.
-        build.with_lock do
-          build.runner_id = current_runner.id
-          build.save!
-          build.run!
-        end
+        # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method.
+        build.runner_id = current_runner.id
+        build.run!
       end
 
       build
 
-    rescue StateMachines::InvalidTransition
+    rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError
       nil
     end
 
diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb
index 918eddaa53a..3e5dd4ebb86 100644
--- a/app/services/delete_branch_service.rb
+++ b/app/services/delete_branch_service.rb
@@ -42,7 +42,7 @@ class DeleteBranchService < BaseService
     Gitlab::DataBuilder::Push.build(
       project,
       current_user,
-      branch.target.sha,
+      branch.dereferenced_target.sha,
       Gitlab::Git::BLANK_SHA,
       "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}",
       [])
diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb
index d0cb151a010..d824406cb49 100644
--- a/app/services/delete_tag_service.rb
+++ b/app/services/delete_tag_service.rb
@@ -36,7 +36,7 @@ class DeleteTagService < BaseService
     Gitlab::DataBuilder::Push.build(
       project,
       current_user,
-      tag.target.sha,
+      tag.dereferenced_target.sha,
       Gitlab::Git::BLANK_SHA,
       "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}",
       [])
diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb
index e6002b03b93..20a4445bddf 100644
--- a/app/services/git_tag_push_service.rb
+++ b/app/services/git_tag_push_service.rb
@@ -27,8 +27,8 @@ class GitTagPushService < BaseService
       tag_name = Gitlab::Git.ref_name(params[:ref])
       tag = project.repository.find_tag(tag_name)
 
-      if tag && tag.object_sha == params[:newrev]
-        commit = project.commit(tag.target)
+      if tag && tag.target == params[:newrev]
+        commit = project.commit(tag.dereferenced_target)
         commits = [commit].compact
         message = tag.message
       end
diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb
index 74291312c4e..d622f9edd33 100644
--- a/app/services/labels/find_or_create_service.rb
+++ b/app/services/labels/find_or_create_service.rb
@@ -2,21 +2,24 @@ module Labels
   class FindOrCreateService
     def initialize(current_user, project, params = {})
       @current_user = current_user
-      @group = project.group
       @project = project
       @params = params.dup
     end
 
-    def execute
+    def execute(skip_authorization: false)
+      @skip_authorization = skip_authorization
       find_or_create_label
     end
 
     private
 
-    attr_reader :current_user, :group, :project, :params
+    attr_reader :current_user, :project, :params, :skip_authorization
 
     def available_labels
-      @available_labels ||= LabelsFinder.new(current_user, project_id: project.id).execute
+      @available_labels ||= LabelsFinder.new(
+        current_user,
+        project_id: project.id
+      ).execute(skip_authorization: skip_authorization)
     end
 
     def find_or_create_label
diff --git a/app/services/members/approve_access_request_service.rb b/app/services/members/approve_access_request_service.rb
index 416aee2ab51..c13f289f61e 100644
--- a/app/services/members/approve_access_request_service.rb
+++ b/app/services/members/approve_access_request_service.rb
@@ -4,17 +4,25 @@ module Members
 
     attr_accessor :source
 
+    # source - The source object that respond to `#requesters` (i.g. project or group)
+    # current_user - The user that performs the access request approval
+    # params - A hash of parameters
+    #   :user_id - User ID used to retrieve the access requester
+    #   :id - Member ID used to retrieve the access requester
+    #   :access_level - Optional access level set when the request is accepted
     def initialize(source, current_user, params = {})
       @source = source
       @current_user = current_user
-      @params = params
+      @params = params.slice(:user_id, :id, :access_level)
     end
 
-    def execute
+    # opts - A hash of options
+    #   :force - Bypass permission check: current_user can be nil in that case
+    def execute(opts = {})
       condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] }
       access_requester = source.requesters.find_by!(condition)
 
-      raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester)
+      raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester, opts)
 
       access_requester.access_level = params[:access_level] if params[:access_level]
       access_requester.accept_request
@@ -24,8 +32,11 @@ module Members
 
     private
 
-    def can_update_access_requester?(access_requester)
-      access_requester && can?(current_user, action_member_permission(:update, access_requester), access_requester)
+    def can_update_access_requester?(access_requester, opts = {})
+      access_requester && (
+        opts[:force] ||
+        can?(current_user, action_member_permission(:update, access_requester), access_requester)
+      )
     end
   end
 end
diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb
new file mode 100644
index 00000000000..e4b24ccef92
--- /dev/null
+++ b/app/services/members/create_service.rb
@@ -0,0 +1,16 @@
+module Members
+  class CreateService < BaseService
+    def execute
+      return false if params[:user_ids].blank?
+
+      project.team.add_users(
+        params[:user_ids].split(','),
+        params[:access_level],
+        expires_at: params[:expires_at],
+        current_user: current_user
+      )
+
+      true
+    end
+  end
+end
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 404f75616b5..f415244068b 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -13,20 +13,8 @@ module MergeRequests
       merge_request.target_project ||= (project.forked_from_project || project)
       merge_request.target_branch ||= merge_request.target_project.default_branch
 
-      if merge_request.target_branch.blank? || merge_request.source_branch.blank?
-        message =
-          if params[:source_branch] || params[:target_branch]
-            "You must select source and target branch"
-          end
-
-        return build_failed(merge_request, message)
-      end
-
-      if merge_request.source_project == merge_request.target_project &&
-         merge_request.target_branch == merge_request.source_branch
-
-        return build_failed(merge_request, 'You must select different branches')
-      end
+      messages = validate_branches(merge_request)
+      return build_failed(merge_request, messages) unless messages.empty?
 
       compare = CompareService.new.execute(
         merge_request.source_project,
@@ -43,6 +31,34 @@ module MergeRequests
 
     private
 
+    def validate_branches(merge_request)
+      messages = []
+
+      if merge_request.target_branch.blank? || merge_request.source_branch.blank?
+        messages <<
+          if params[:source_branch] || params[:target_branch]
+            "You must select source and target branch"
+          end
+      end
+
+      if merge_request.source_project == merge_request.target_project &&
+         merge_request.target_branch == merge_request.source_branch
+
+        messages << 'You must select different branches'
+      end
+
+      # See if source and target branches exist
+      unless merge_request.source_project.commit(merge_request.source_branch)
+        messages << "Source branch \"#{merge_request.source_branch}\" does not exist"
+      end
+
+      unless merge_request.target_project.commit(merge_request.target_branch)
+        messages << "Target branch \"#{merge_request.target_branch}\" does not exist"
+      end
+
+      messages
+    end
+
     # When your branch name starts with an iid followed by a dash this pattern will be
     # interpreted as the user wants to close that issue on this project.
     #
@@ -91,8 +107,10 @@ module MergeRequests
       merge_request
     end
 
-    def build_failed(merge_request, message)
-      merge_request.errors.add(:base, message) unless message.nil?
+    def build_failed(merge_request, messages)
+      messages.compact.each do |message|
+        merge_request.errors.add(:base, message)
+      end
       merge_request.compare_commits = []
       merge_request.can_be_created = false
       merge_request
diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb
index e466ffa60eb..d7221fe993c 100644
--- a/app/services/projects/import_service.rb
+++ b/app/services/projects/import_service.rb
@@ -29,7 +29,7 @@ module Projects
       if unknown_url?
         # In this case, we only want to import issues, not a repository.
         create_repository
-      else
+      elsif !project.repository_exists?
         import_repository
       end
     end