Merge remote-tracking branch 'origin/master' into...

Merge remote-tracking branch 'origin/master' into 39549-label-list-page-redesign-with-draggable-labels

CONTRIBUTING.md

@@ -182,7 +182,7 @@ Assigning a team label makes sure issues get the attention of the appropriate
 people.
 
 The current team labels are ~Distribution, ~"CI/CD", ~Discussion, ~Documentation, ~Quality,
-~Geo, ~Gitaly, ~Monitoring, ~Platform, ~Release, ~"Security Products" and ~"UX".
+~Geo, ~Gitaly, ~Monitoring, ~Platform, ~Release, ~"Security Products", ~"Configuration", and ~"UX".
 
 The descriptions on the [labels page][labels-page] explain what falls under the
 responsibility of each team.

README.md

@@ -126,5 +126,5 @@ Please see [Getting help for GitLab](https://about.gitlab.com/getting-help/) on
 
 ## Is it awesome?
 
-Thanks for [asking this question](https://twitter.com/supersloth/status/489462789384056832) Joshua.
 [These people](https://twitter.com/gitlab/likes) seem to like it.
+

app/assets/stylesheets/framework/layout.scss

@@ -115,9 +115,3 @@ body {
 .with-performance-bar .layout-page {
   margin-top: $header-height + $performance-bar-height;
 }
-
-.vertical-center {
-  min-height: 100vh;
-  display: flex;
-  align-items: center;
-}

app/helpers/projects_helper.rb

@@ -238,6 +238,14 @@ module ProjectsHelper
     "git push --set-upstream #{repository_url}/$(git rev-parse --show-toplevel | xargs basename).git $(git rev-parse --abbrev-ref HEAD)"
   end
 
+  def show_xcode_link?(project = @project)
+    browser.platform.mac? && project.repository.xcode_project?
+  end
+
+  def xcode_uri_to_repo(project = @project)
+    "xcode://clone?repo=#{CGI.escape(default_url_to_repo(project))}"
+  end
+
   private
 
   def get_project_nav_tabs(project, current_user)

app/models/ci/build.rb

@@ -55,6 +55,11 @@ module Ci
       where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)',
         '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive)
     end
+
+    scope :without_archived_trace, ->() do
+      where('NOT EXISTS (?)', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').trace)
+    end
+
     scope :with_artifacts_stored_locally, -> { with_artifacts_archive.where(artifacts_file_store: [nil, LegacyArtifactUploader::Store::LOCAL]) }
     scope :with_artifacts_not_expired, ->() { with_artifacts_archive.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts_archive.where('artifacts_expire_at < ?', Time.now) }
@@ -144,6 +149,7 @@ module Ci
       after_transition any => [:success] do |build|
         build.run_after_commit do
           BuildSuccessWorker.perform_async(id)
+          PagesWorker.perform_async(:deploy, id) if build.pages_generator?
         end
       end
 
@@ -183,6 +189,11 @@ module Ci
       pipeline.manual_actions.where.not(name: name)
     end
 
+    def pages_generator?
+      Gitlab.config.pages.enabled &&
+        self.name == 'pages'
+    end
+
     def playable?
       action? && (manual? || retryable?)
     end
@@ -402,8 +413,6 @@ module Ci
       build_data = Gitlab::DataBuilder::Build.build(self)
       project.execute_hooks(build_data.dup, :job_hooks)
       project.execute_services(build_data.dup, :job_hooks)
-      PagesService.new(build_data).execute
-      project.running_or_pending_build_count(force: true)
     end
 
     def browsable_artifacts?

app/models/project.rb

@@ -1649,12 +1649,6 @@ class Project < ActiveRecord::Base
     import_state.update_column(:jid, nil)
   end
 
-  def running_or_pending_build_count(force: false)
-    Rails.cache.fetch(['projects', id, 'running_or_pending_build_count'], force: force) do
-      builds.running_or_pending.count(:all)
-    end
-  end
-
   # Lazy loading of the `pipeline_status` attribute
   def pipeline_status
     @pipeline_status ||= Gitlab::Cache::Ci::ProjectPipelineStatus.load_for_project(self)

app/services/pages_service.rb

-class PagesService
-  attr_reader :data
-
-  def initialize(data)
-    @data = data
-  end
-
-  def execute
-    return unless Settings.pages.enabled
-    return unless data[:build_name] == 'pages'
-    return unless data[:build_status] == 'success'
-
-    PagesWorker.perform_async(:deploy, data[:build_id])
-  end
-end

app/services/projects/update_service.rb

@@ -17,6 +17,8 @@ module Projects
 
       ensure_wiki_exists if enabling_wiki?
 
+      yield if block_given?
+
       if project.update_attributes(params.except(:default_branch))
         if project.previous_changes.include?('path')
           project.rename_repo
@@ -36,7 +38,7 @@ module Projects
     end
 
     def run_auto_devops_pipeline?
-      return false if project.repository.gitlab_ci_yml || !project.auto_devops.previous_changes.include?('enabled')
+      return false if project.repository.gitlab_ci_yml || !project.auto_devops&.previous_changes&.include?('enabled')
 
       project.auto_devops.enabled? || (project.auto_devops.enabled.nil? && Gitlab::CurrentSettings.auto_devops_enabled?)
     end
@@ -53,8 +55,8 @@ module Projects
     def changing_default_branch?
       new_branch = params[:default_branch]
 
-      project.repository.exists? &&
-        new_branch && new_branch != project.default_branch
+      new_branch && project.repository.exists? &&
+        new_branch != project.default_branch
     end
 
     def enabling_wiki?

app/views/admin/users/_access_levels.html.haml

 %fieldset
   %legend Access
-  .form-group
-    = f.label :projects_limit, class: 'col-form-label'
+  .form-group.row
+    = f.label :projects_limit, class: 'col-form-label col-sm-2'
     .col-sm-10= f.number_field :projects_limit, min: 0, max: Gitlab::Database::MAX_INT_VALUE, class: 'form-control'
 
-  .form-group
-    = f.label :can_create_group, class: 'col-form-label'
+  .form-group.row
+    = f.label :can_create_group, class: 'col-form-label col-sm-2'
     .col-sm-10= f.check_box :can_create_group
 
-  .form-group
-    = f.label :access_level, class: 'col-form-label'
+  .form-group.row
+    = f.label :access_level, class: 'col-form-label col-sm-2'
     .col-sm-10
       - editing_current_user = (current_user == @user)
 
       = f.radio_button :access_level, :regular, disabled: editing_current_user
-      = label_tag :regular do
+      = label_tag :regular, class: 'font-weight-bold' do
         Regular
       %p.light
         Regular users have access to their groups and projects
 
       = f.radio_button :access_level, :admin, disabled: editing_current_user
-      = label_tag :admin do
+      = label_tag :admin, class: 'font-weight-bold' do
         Admin
       %p.light
         Administrators have access to all groups, projects and users and can manage all features in this installation
@@ -28,8 +28,8 @@
         %p.light
           You cannot remove your own admin rights.
 
-  .form-group
-    = f.label :external, class: 'col-form-label'
+  .form-group.row
+    = f.label :external, class: 'col-form-label col-sm-2'
     .col-sm-10
       = f.check_box :external do
         External

app/views/admin/users/_form.html.haml

@@ -56,7 +56,7 @@
         = f.label :linkedin, class: 'col-form-label col-sm-2'
         .col-sm-10= f.text_field :linkedin, class: 'form-control'
       .form-group.row
-        = f.label :twitter, class: 'col-form-label'
+        = f.label :twitter, class: 'col-form-label col-sm-2'
         .col-sm-10= f.text_field :twitter, class: 'form-control'
       .form-group.row
         = f.label :website_url, 'Website', class: 'col-form-label col-sm-2'

app/views/projects/_home_panel.html.haml

@@ -42,6 +42,10 @@
           .project-clone-holder
             = render "shared/clone_panel"
 
+          - if show_xcode_link?(@project)
+            .project-action-button.project-xcode.inline
+              = render "projects/buttons/xcode_link"
+
         - if current_user
           - if can?(current_user, :download_code, @project)
             = render 'projects/buttons/download', project: @project, ref: @ref

app/views/projects/blob/viewers/_download.html.haml

 .file-content.blob_file.blob-no-preview
-  .center.render-error.vertical-center
+  .center.render-error
     = link_to blob_raw_path do
       %h1.light
         = sprite_icon('download')

app/views/projects/buttons/_xcode_link.html.haml

+%a.btn.btn-default{ href: xcode_uri_to_repo(@project) }
+  = _("Open in Xcode")

changelogs/unreleased/45820-add-xcode-link.yml

+---
+title: Add Open in Xcode link for xcode repositories
+merge_request:
+author:
+type: added

changelogs/unreleased/46452-nomethoderror-undefined-method-previous_changes-for-nil-nilclass.yml

+---
+title: Check for nil AutoDevOps when saving project CI/CD settings.
+merge_request: 19190
+author:
+type: fixed

changelogs/unreleased/add-background-migrations-for-not-archived-traces.yml

+---
+title: Add background migrations for archiving legacy job traces
+merge_request: 19194
+author:
+type: performance

changelogs/unreleased/gh-importer-transactions.yml

+---
+title: Move PR IO operations out of a transaction
+merge_request:
+author:
+type: performance

changelogs/unreleased/optimise-pages-service-calling.yml

+---
+title: Optimise PagesWorker usage
+merge_request:
+author:
+type: performance

changelogs/unreleased/rails5-fix-46236.yml

+---
+title: Support rails5 in postgres indexes function and fix some migrations
+merge_request: 19400
+author: Jasper Maes
+type: fixed

changelogs/unreleased/remove-unused-query-in-hooks.yml

+---
+title: Remove unused running_or_pending_build_count
+merge_request:
+author:
+type: performance

changelogs/unreleased/sh-fix-pipeline-jobs-nplus-one.yml

+---
+title: Eliminate N+1 queries for CI job artifacts in /api/prjoects/:id/pipelines/:pipeline_id/jobs
+merge_request:
+author:
+type: performance

config/initializers/postgresql_opclasses_support.rb

@@ -107,8 +107,15 @@ module ActiveRecord
 
           result.map do |row|
             index_name = row[0]
-            unique = row[1] == 't'
+            unique = if Gitlab.rails5?
+                       row[1]
+                     else
+                       row[1] == 't'
+                     end
             indkey = row[2].split(" ")
+            if Gitlab.rails5?
+              indkey = indkey.map(&:to_i)
+            end
             inddef = row[3]
             oid = row[4]
 

db/migrate/20160226114608_add_trigram_indexes_for_searching.rb

@@ -37,7 +37,12 @@ class AddTrigramIndexesForSearching < ActiveRecord::Migration
     res = execute("SELECT true AS enabled FROM pg_available_extensions WHERE name = 'pg_trgm' AND installed_version IS NOT NULL;")
     row = res.first
 
-    row && row['enabled'] == 't' ? true : false
+    check = if Gitlab.rails5?
+              true
+            else
+              't'
+            end
+    row && row['enabled'] == check ? true : false
   end
 
   def create_trigrams_extension

db/migrate/20170622135728_add_unique_constraint_to_ci_variables.rb

@@ -2,12 +2,13 @@ class AddUniqueConstraintToCiVariables < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
   DOWNTIME = false
+  INDEX_NAME = 'index_ci_variables_on_project_id_and_key_and_environment_scope'
 
   disable_ddl_transaction!
 
   def up
     unless this_index_exists?
-      add_concurrent_index(:ci_variables, columns, name: index_name, unique: true)
+      add_concurrent_index(:ci_variables, columns, name: INDEX_NAME, unique: true)
     end
   end
 
@@ -18,21 +19,17 @@ class AddUniqueConstraintToCiVariables < ActiveRecord::Migration
         add_concurrent_index(:ci_variables, :project_id)
       end
 
-      remove_concurrent_index(:ci_variables, columns, name: index_name)
+      remove_concurrent_index(:ci_variables, columns, name: INDEX_NAME)
     end
   end
 
   private
 
   def this_index_exists?
-    index_exists?(:ci_variables, columns, name: index_name)
+    index_exists?(:ci_variables, columns, name: INDEX_NAME)
   end
 
   def columns
     @columns ||= [:project_id, :key, :environment_scope]
   end
-
-  def index_name
-    'index_ci_variables_on_project_id_and_key_and_environment_scope'
-  end
 end

db/migrate/20171106155656_turn_issues_due_date_index_to_partial_index.rb

@@ -20,9 +20,7 @@ class TurnIssuesDueDateIndexToPartialIndex < ActiveRecord::Migration
       name: NEW_INDEX_NAME
     )
 
-    # We set the column name to nil as otherwise Rails will ignore the custom
-    # index name and remove the wrong index.
-    remove_concurrent_index(:issues, nil, name: OLD_INDEX_NAME)
+    remove_concurrent_index_by_name(:issues, OLD_INDEX_NAME)
   end
 
   def down
@@ -32,6 +30,6 @@ class TurnIssuesDueDateIndexToPartialIndex < ActiveRecord::Migration
       name: OLD_INDEX_NAME
     )
 
-    remove_concurrent_index(:issues, nil, name: NEW_INDEX_NAME)
+    remove_concurrent_index_by_name(:issues, NEW_INDEX_NAME)
   end
 end

db/migrate/20180201110056_add_foreign_keys_to_todos.rb

@@ -31,7 +31,7 @@ class AddForeignKeysToTodos < ActiveRecord::Migration
   end
 
   def down
-    remove_foreign_key :todos, :users
+    remove_foreign_key :todos, column: :user_id
     remove_foreign_key :todos, column: :author_id
     remove_foreign_key :todos, :notes
   end

db/post_migrate/20180529152628_schedule_to_archive_legacy_traces.rb

+class ScheduleToArchiveLegacyTraces < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 5000
+  BACKGROUND_MIGRATION_CLASS = 'ArchiveLegacyTraces'
+
+  disable_ddl_transaction!
+
+  class Build < ActiveRecord::Base
+    include EachBatch
+    self.table_name = 'ci_builds'
+    self.inheritance_column = :_type_disabled # Disable STI
+
+    scope :type_build, -> { where(type: 'Ci::Build') }
+
+    scope :finished, -> { where(status: [:success, :failed, :canceled]) }
+
+    scope :without_archived_trace, -> do
+      where('NOT EXISTS (SELECT 1 FROM ci_job_artifacts WHERE ci_builds.id = ci_job_artifacts.job_id AND ci_job_artifacts.file_type = 3)')
+    end
+  end
+
+  def up
+    queue_background_migration_jobs_by_range_at_intervals(
+      ::ScheduleToArchiveLegacyTraces::Build.type_build.finished.without_archived_trace,
+      BACKGROUND_MIGRATION_CLASS,
+      5.minutes,
+      batch_size: BATCH_SIZE)
+  end
+
+  def down
+    # noop
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180529093006) do
+ActiveRecord::Schema.define(version: 20180529152628) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

doc/topics/autodevops/index.md

@@ -497,10 +497,10 @@ also be customized, and you can easily use a [custom buildpack](#custom-buildpac
 | `CANARY_ENABLED`             | From GitLab 11.0, this variable can be used to define a [deploy policy for canary environments](#deploy-policy-for-canary-environments). |
 | `INCREMENTAL_ROLLOUT_ENABLED`| From GitLab 10.8, this variable can be used to enable an [incremental rollout](#incremental-rollout-to-production) of your application for the production environment. |
 | `TEST_DISABLED`              | From GitLab 11.0, this variable can be used to disable the `test` job. If the variable is present, the job will not be created. |
-| `CODEQUALITY_DISABLED`       | From GitLab 11.0, this variable can be used to disable the `codequality` job. If the variable is present, the job will not be created. |
+| `CODE_QUALITY_DISABLED`      | From GitLab 11.0, this variable can be used to disable the `code_quality` job. If the variable is present, the job will not be created. |
 | `SAST_DISABLED`              | From GitLab 11.0, this variable can be used to disable the `sast` job. If the variable is present, the job will not be created. |
 | `DEPENDENCY_SCANNING_DISABLED` | From GitLab 11.0, this variable can be used to disable the `dependency_scanning` job. If the variable is present, the job will not be created. |
-| `CONTAINER_SCANNING_DISABLED` | From GitLab 11.0, this variable can be used to disable the `sast:container` job. If the variable is present, the job will not be created. |
+| `CONTAINER_SCANNING_DISABLED` | From GitLab 11.0, this variable can be used to disable the `container_scanning` job. If the variable is present, the job will not be created. |
 | `REVIEW_DISABLED`            | From GitLab 11.0, this variable can be used to disable the `review` and the manual `review:stop` job. If the variable is present, these jobs will not be created. |
 | `DAST_DISABLED`              | From GitLab 11.0, this variable can be used to disable the `dast` job. If the variable is present, the job will not be created. |
 | `PERFORMANCE_DISABLED`       | From GitLab 11.0, this variable can be used to disable the `performance` job. If the variable is present, the job will not be created. |

lib/api/jobs.rb

@@ -54,6 +54,7 @@ module API
         pipeline = user_project.pipelines.find(params[:pipeline_id])
         builds = pipeline.builds
         builds = filter_builds(builds, params[:scope])
+        builds = builds.preload(:job_artifacts_archive)
 
         present paginate(builds), with: Entities::Job
       end

lib/gitlab/background_migration/archive_legacy_traces.rb

+# frozen_string_literal: true
+# rubocop:disable Metrics/AbcSize
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    class ArchiveLegacyTraces
+      def perform(start_id, stop_id)
+        # This background migration directly refers to ::Ci::Build model which is defined in application code.
+        # In general, migration code should be isolated as much as possible in order to be idempotent.
+        # However, `archive!` method is too complicated to be replicated by coping its subsequent code.
+        # So we chose a way to use ::Ci::Build directly and we don't change the `archive!` method until 11.1
+        ::Ci::Build.finished.without_archived_trace
+          .where(id: start_id..stop_id).find_each do |build|
+            begin
+              build.trace.archive!
+            rescue => e
+              Rails.logger.error "Failed to archive live trace. id: #{build.id} message: #{e.message}"
+            end
+          end
+      end
+    end
+  end
+end

lib/gitlab/git/repository.rb

@@ -1397,6 +1397,11 @@ module Gitlab
       def write_config(full_path:)
         return unless full_path.present?
 
+        # This guard avoids Gitaly log/error spam
+        unless exists?
+          raise NoRepository, 'repository does not exist'
+        end
+
         gitaly_migrate(:write_config) do |is_enabled|
           if is_enabled
             gitaly_repository_client.write_config(full_path: full_path)

lib/gitlab/github_import/importer/pull_request_importer.rb

@@ -22,15 +22,22 @@ module Gitlab
         end
 
         def execute
-          if (mr_id = create_merge_request)
-            issuable_finder.cache_database_id(mr_id)
+          mr, already_exists = create_merge_request
+
+          if mr
+            insert_git_data(mr, already_exists)
+            issuable_finder.cache_database_id(mr.id)
           end
         end
 
         # Creates the merge request and returns its ID.
         #
         # This method will return `nil` if the merge request could not be
-        # created.
+        # created, otherwise it will return an Array containing the following
+        # values:
+        #
+        # 1. A MergeRequest instance.
+        # 2. A boolean indicating if the MR already exists.
         def create_merge_request
           author_id, author_found = user_finder.author_id_for(pull_request)
 
@@ -69,21 +76,42 @@ module Gitlab
             merge_request_id = GithubImport
               .insert_and_return_id(attributes, project.merge_requests)
 
-            merge_request = project.merge_requests.find(merge_request_id)
+            [project.merge_requests.find(merge_request_id), false]
+          end
+        rescue ActiveRecord::InvalidForeignKey
+          # It's possible the project has been deleted since scheduling this
+          # job. In this case we'll just skip creating the merge request.
+          []
+        rescue ActiveRecord::RecordNotUnique
+          # It's possible we previously created the MR, but failed when updating
+          # the Git data. In this case we'll just continue working on the
+          # existing row.
+          [project.merge_requests.find_by(iid: pull_request.iid), true]
+        end
 
+        def insert_git_data(merge_request, already_exists = false)
           # These fields are set so we can create the correct merge request
           # diffs.
           merge_request.source_branch_sha = pull_request.source_branch_sha
           merge_request.target_branch_sha = pull_request.target_branch_sha
 
           merge_request.keep_around_commit
-            merge_request.merge_request_diffs.create
 
-            merge_request.id
+          # MR diffs normally use an "after_save" hook to pull data from Git.
+          # All of this happens in the transaction started by calling
+          # create/save/etc. This in turn can lead to these transactions being
+          # held open for much longer than necessary. To work around this we
+          # first save the diff, then populate it.
+          diff =
+            if already_exists
+              merge_request.merge_request_diffs.take
+            else
+              merge_request.merge_request_diffs.build
             end
-        rescue ActiveRecord::InvalidForeignKey
-          # It's possible the project has been deleted since scheduling this
-          # job. In this case we'll just skip creating the merge request.
+
+          diff.importing = true
+          diff.save
+          diff.save_git_content
         end
       end
     end

lib/gitlab/utils/override.rb

@@ -87,18 +87,28 @@ module Gitlab
       end
 
       def included(base = nil)
-        return super if base.nil? # Rails concern, ignoring it
+        super
+
+        queue_verification(base)
+      end
 
+      alias_method :prepended, :included
+
+      def extended(mod)
         super
 
+        queue_verification(mod.singleton_class)
+      end
+
+      def queue_verification(base)
+        return unless ENV['STATIC_VERIFICATION']
+
         if base.is_a?(Class) # We could check for Class in `override`
           # This could be `nil` if `override` was never called
           Override.extensions[self]&.add_class(base)
         end
       end
 
-      alias_method :prepended, :included
-
       def self.extensions
         @extensions ||= {}
       end

lib/tasks/gitlab/traces.rake

@@ -8,9 +8,7 @@ namespace :gitlab do
       logger = Logger.new(STDOUT)
       logger.info('Archiving legacy traces')
 
-      Ci::Build.finished
-        .where('NOT EXISTS (?)',
-          Ci::JobArtifact.select(1).trace.where('ci_builds.id = ci_job_artifacts.job_id'))
+      Ci::Build.finished.without_archived_trace
         .order(id: :asc)
         .find_in_batches(batch_size: 1000) do |jobs|
         job_ids = jobs.map { |job| [job.id] }

spec/features/projects/jobs/user_browses_job_spec.rb

@@ -32,8 +32,6 @@ describe 'User browses a job', :js do
     page.within('.erased') do
       expect(page).to have_content('Job has been erased')
     end
-
-    expect(build.project.running_or_pending_build_count).to eq(build.project.builds.running_or_pending.count(:all))
   end
 
   context 'with a failed job' do

spec/helpers/projects_helper_spec.rb

@@ -435,4 +435,46 @@ describe ProjectsHelper do
       expect(helper.send(:git_user_name)).to eq('John \"A\" Doe53')
     end
   end
+
+  describe 'show_xcode_link' do
+    let!(:project) { create(:project) }
+    let(:mac_ua) { 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36' }
+    let(:ios_ua) { 'Mozilla/5.0 (iPad; CPU OS 5_1_1 like Mac OS X) AppleWebKit/534.46 (KHTML, like Gecko) Version/5.1 Mobile/9B206 Safari/7534.48.3' }
+
+    context 'when the repository is xcode compatible' do
+      before do
+        allow(project.repository).to receive(:xcode_project?).and_return(true)
+      end
+
+      it 'returns false if the visitor is not using macos' do
+        allow(helper).to receive(:browser).and_return(Browser.new(ios_ua))
+
+        expect(helper.show_xcode_link?(project)).to eq(false)
+      end
+
+      it 'returns true if the visitor is using macos' do
+        allow(helper).to receive(:browser).and_return(Browser.new(mac_ua))
+
+        expect(helper.show_xcode_link?(project)).to eq(true)
+      end
+    end
+
+    context 'when the repository is not xcode compatible' do
+      before do
+        allow(project.repository).to receive(:xcode_project?).and_return(false)
+      end
+
+      it 'returns false if the visitor is not using macos' do
+        allow(helper).to receive(:browser).and_return(Browser.new(ios_ua))
+
+        expect(helper.show_xcode_link?(project)).to eq(false)
+      end
+
+      it 'returns false if the visitor is using macos' do
+        allow(helper).to receive(:browser).and_return(Browser.new(mac_ua))
+
+        expect(helper.show_xcode_link?(project)).to eq(false)
+      end
+    end
+  end
 end

spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb

+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 20180529152628 do
+  include TraceHelpers
+
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:builds) { table(:ci_builds) }
+  let(:job_artifacts) { table(:ci_job_artifacts) }
+
+  before do
+    namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
+    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
+    @build = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build')
+  end
+
+  context 'when trace file exsits at the right place' do
+    before do
+      create_legacy_trace(@build, 'trace in file')
+    end
+
+    it 'correctly archive legacy traces' do
+      expect(job_artifacts.count).to eq(0)
+      expect(File.exist?(legacy_trace_path(@build))).to be_truthy
+
+      described_class.new.perform(1, 1)
+
+      expect(job_artifacts.count).to eq(1)
+      expect(File.exist?(legacy_trace_path(@build))).to be_falsy
+      expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in file')
+    end
+  end
+
+  context 'when trace file does not exsits at the right place' do
+    it 'does not raise errors nor create job artifact' do
+      expect { described_class.new.perform(1, 1) }.not_to raise_error
+
+      expect(job_artifacts.count).to eq(0)
+    end
+  end
+
+  context 'when trace data exsits in database' do
+    before do
+      create_legacy_trace_in_db(@build, 'trace in db')
+    end
+
+    it 'correctly archive legacy traces' do
+      expect(job_artifacts.count).to eq(0)
+      expect(@build.read_attribute(:trace)).not_to be_empty
+
+      described_class.new.perform(1, 1)
+
+      @build.reload
+      expect(job_artifacts.count).to eq(1)
+      expect(@build.read_attribute(:trace)).to be_nil
+      expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in db')
+    end
+  end
+end

spec/lib/gitlab/git/repository_spec.rb

@@ -2002,6 +2002,18 @@ describe Gitlab::Git::Repository, seed_helper: true do
           expect(config).to include("fullpath = #{repository_path}")
         end
       end
+
+      context 'repository does not exist' do
+        it 'raises NoRepository and does not call Gitaly WriteConfig' do
+          repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '')
+
+          expect(repository.gitaly_repository_client).not_to receive(:write_config)
+
+          expect do
+            repository.write_config(full_path: 'foo/bar.git')
+          end.to raise_error(Gitlab::Git::Repository::NoRepository)
+        end
+      end
     end
 
     context "when gitaly_write_config is enabled" do

spec/lib/gitlab/github_import/importer/issue_importer_spec.rb

@@ -180,12 +180,12 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
 
       allow(importer.user_finder)
         .to receive(:user_id_for)
-        .ordered.with(issue.assignees[0])
+        .with(issue.assignees[0])
         .and_return(4)
 
       allow(importer.user_finder)
         .to receive(:user_id_for)
-        .ordered.with(issue.assignees[1])
+        .with(issue.assignees[1])
         .and_return(5)
 
       expect(Gitlab::Database)

spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb

@@ -40,13 +40,19 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
 
   describe '#execute' do
     it 'imports the pull request' do
+      mr = double(:merge_request, id: 10)
+
       expect(importer)
         .to receive(:create_merge_request)
-        .and_return(10)
+        .and_return([mr, false])
+
+      expect(importer)
+        .to receive(:insert_git_data)
+        .with(mr, false)
 
       expect_any_instance_of(Gitlab::GithubImport::IssuableFinder)
         .to receive(:cache_database_id)
-        .with(10)
+        .with(mr.id)
 
       importer.execute
     end
@@ -99,18 +105,11 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
         importer.create_merge_request
       end
 
-      it 'returns the ID of the created merge request' do
-        id = importer.create_merge_request
-
-        expect(id).to be_a_kind_of(Numeric)
-      end
-
-      it 'creates the merge request diffs' do
-        importer.create_merge_request
-
-        mr = project.merge_requests.take
+      it 'returns the created merge request' do
+        mr, exists = importer.create_merge_request
 
-        expect(mr.merge_request_diffs.exists?).to eq(true)
+        expect(mr).to be_instance_of(MergeRequest)
+        expect(exists).to eq(false)
       end
     end
 
@@ -217,5 +216,65 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
         expect { importer.create_merge_request }.not_to raise_error
       end
     end
+
+    context 'when the merge request already exists' do
+      before do
+        allow(importer.user_finder)
+          .to receive(:author_id_for)
+          .with(pull_request)
+          .and_return([user.id, true])
+
+        allow(importer.user_finder)
+          .to receive(:assignee_id_for)
+          .with(pull_request)
+          .and_return(user.id)
+      end
+
+      it 'returns the existing merge request' do
+        mr1, exists1 = importer.create_merge_request
+        mr2, exists2 = importer.create_merge_request
+
+        expect(mr2).to eq(mr1)
+        expect(exists1).to eq(false)
+        expect(exists2).to eq(true)
+      end
+    end
+  end
+
+  describe '#insert_git_data' do
+    before do
+      allow(importer.milestone_finder)
+        .to receive(:id_for)
+        .with(pull_request)
+        .and_return(milestone.id)
+
+      allow(importer.user_finder)
+        .to receive(:author_id_for)
+        .with(pull_request)
+        .and_return([user.id, true])
+
+      allow(importer.user_finder)
+        .to receive(:assignee_id_for)
+        .with(pull_request)
+        .and_return(user.id)
+    end
+
+    it 'creates the merge request diffs' do
+      mr, exists = importer.create_merge_request
+
+      importer.insert_git_data(mr, exists)
+
+      expect(mr.merge_request_diffs.exists?).to eq(true)
+    end
+
+    it 'creates the merge request diff commits' do
+      mr, exists = importer.create_merge_request
+
+      importer.insert_git_data(mr, exists)
+
+      diff = mr.merge_request_diffs.take
+
+      expect(diff.merge_request_diff_commits.exists?).to eq(true)
+    end
   end
 end

spec/lib/gitlab/utils/override_spec.rb

-require 'spec_helper'
+require 'fast_spec_helper'
 
 describe Gitlab::Utils::Override do
-  let(:base) { Struct.new(:good) }
+  let(:base) do
+    Struct.new(:good) do
+      def self.good
+        0
+      end
+    end
+  end
 
   let(:derived) { Class.new(base).tap { |m| m.extend described_class } }
   let(:extension) { Module.new.tap { |m| m.extend described_class } }
@@ -9,6 +15,14 @@ describe Gitlab::Utils::Override do
   let(:prepending_class) { base.tap { |m| m.prepend extension } }
   let(:including_class) { base.tap { |m| m.include extension } }
 
+  let(:prepending_class_methods) do
+    base.tap { |m| m.singleton_class.prepend extension }
+  end
+
+  let(:extending_class_methods) do
+    base.tap { |m| m.extend extension }
+  end
+
   let(:klass) { subject }
 
   def good(mod)
@@ -36,7 +50,7 @@ describe Gitlab::Utils::Override do
   shared_examples 'checking as intended' do
     it 'checks ok for overriding method' do
       good(subject)
-      result = klass.new(0).good
+      result = instance.good
 
       expect(result).to eq(1)
       described_class.verify!
@@ -45,7 +59,25 @@ describe Gitlab::Utils::Override do
     it 'raises NotImplementedError when it is not overriding anything' do
       expect do
         bad(subject)
-        klass.new(0).bad
+        instance.bad
+        described_class.verify!
+      end.to raise_error(NotImplementedError)
+    end
+  end
+
+  shared_examples 'checking as intended, nothing was overridden' do
+    it 'raises NotImplementedError because it is not overriding it' do
+      expect do
+        good(subject)
+        instance.good
+        described_class.verify!
+      end.to raise_error(NotImplementedError)
+    end
+
+    it 'raises NotImplementedError when it is not overriding anything' do
+      expect do
+        bad(subject)
+        instance.bad
         described_class.verify!
       end.to raise_error(NotImplementedError)
     end
@@ -54,7 +86,7 @@ describe Gitlab::Utils::Override do
   shared_examples 'nothing happened' do
     it 'does not complain when it is overriding something' do
       good(subject)
-      result = klass.new(0).good
+      result = instance.good
 
       expect(result).to eq(1)
       described_class.verify!
@@ -62,7 +94,7 @@ describe Gitlab::Utils::Override do
 
     it 'does not complain when it is not overriding anything' do
       bad(subject)
-      result = klass.new(0).bad
+      result = instance.bad
 
       expect(result).to eq(true)
       described_class.verify!
@@ -75,6 +107,9 @@ describe Gitlab::Utils::Override do
   end
 
   describe '#override' do
+    context 'when instance is klass.new(0)' do
+      let(:instance) { klass.new(0) }
+
       context 'when STATIC_VERIFICATION is set' do
         before do
           stub_env('STATIC_VERIFICATION', 'true')
@@ -97,22 +132,7 @@ describe Gitlab::Utils::Override do
           subject { extension }
           let(:klass) { including_class }
 
-        it 'raises NotImplementedError because it is not overriding it' do
-          expect do
-            good(subject)
-            klass.new(0).good
-            described_class.verify!
-          end.to raise_error(NotImplementedError)
-        end
-
-        it 'raises NotImplementedError when it is not overriding anything' do
-          expect do
-            bad(subject)
-            klass.new(0).bad
-            described_class.verify!
-          end.to raise_error(NotImplementedError)
-        end
-      end
+          it_behaves_like 'checking as intended, nothing was overridden'
         end
       end
 
@@ -140,7 +160,7 @@ describe Gitlab::Utils::Override do
 
           it 'does not complain when it is overriding something' do
             good(subject)
-        result = klass.new(0).good
+            result = instance.good
 
             expect(result).to eq(0)
             described_class.verify!
@@ -148,11 +168,37 @@ describe Gitlab::Utils::Override do
 
           it 'does not complain when it is not overriding anything' do
             bad(subject)
-        result = klass.new(0).bad
+            result = instance.bad
 
             expect(result).to eq(true)
             described_class.verify!
           end
         end
       end
+    end
+
+    context 'when instance is klass' do
+      let(:instance) { klass }
+
+      context 'when STATIC_VERIFICATION is set' do
+        before do
+          stub_env('STATIC_VERIFICATION', 'true')
+        end
+
+        context 'when subject is a module, and class is prepending it' do
+          subject { extension }
+          let(:klass) { prepending_class_methods }
+
+          it_behaves_like 'checking as intended'
+        end
+
+        context 'when subject is a module, and class is extending it' do
+          subject { extension }
+          let(:klass) { extending_class_methods }
+
+          it_behaves_like 'checking as intended, nothing was overridden'
+        end
+      end
+    end
+  end
 end

spec/migrations/schedule_to_archive_legacy_traces_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180529152628_schedule_to_archive_legacy_traces')
+
+describe ScheduleToArchiveLegacyTraces, :migration do
+  include TraceHelpers
+
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:builds) { table(:ci_builds) }
+  let(:job_artifacts) { table(:ci_job_artifacts) }
+
+  before do
+    namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
+    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
+    @build_success = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build')
+    @build_failed = builds.create!(id: 2, project_id: 123, status: 'failed', type: 'Ci::Build')
+    @builds_canceled = builds.create!(id: 3, project_id: 123, status: 'canceled', type: 'Ci::Build')
+    @build_running = builds.create!(id: 4, project_id: 123, status: 'running', type: 'Ci::Build')
+
+    create_legacy_trace(@build_success, 'This job is done')
+    create_legacy_trace(@build_failed, 'This job is done')
+    create_legacy_trace(@builds_canceled, 'This job is done')
+    create_legacy_trace(@build_running, 'This job is not done yet')
+  end
+
+  it 'correctly archive legacy traces' do
+    expect(job_artifacts.count).to eq(0)
+    expect(File.exist?(legacy_trace_path(@build_success))).to be_truthy
+    expect(File.exist?(legacy_trace_path(@build_failed))).to be_truthy
+    expect(File.exist?(legacy_trace_path(@builds_canceled))).to be_truthy
+    expect(File.exist?(legacy_trace_path(@build_running))).to be_truthy
+
+    migrate!
+
+    expect(job_artifacts.count).to eq(3)
+    expect(File.exist?(legacy_trace_path(@build_success))).to be_falsy
+    expect(File.exist?(legacy_trace_path(@build_failed))).to be_falsy
+    expect(File.exist?(legacy_trace_path(@builds_canceled))).to be_falsy
+    expect(File.exist?(legacy_trace_path(@build_running))).to be_truthy
+    expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @build_success.id).first))).to be_truthy
+    expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @build_failed.id).first))).to be_truthy
+    expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @builds_canceled.id).first))).to be_truthy
+    expect(job_artifacts.where(job_id: @build_running.id)).not_to be_exist
+  end
+end

spec/models/ci/build_spec.rb

@@ -2506,4 +2506,76 @@ describe Ci::Build do
       end
     end
   end
+
+  describe 'pages deployments' do
+    set(:build) { create(:ci_build, project: project, user: user) }
+
+    context 'when job is "pages"' do
+      before do
+        build.name = 'pages'
+      end
+
+      context 'when pages are enabled' do
+        before do
+          allow(Gitlab.config.pages).to receive_messages(enabled: true)
+        end
+
+        it 'is marked as pages generator' do
+          expect(build).to be_pages_generator
+        end
+
+        context 'job succeeds' do
+          it "calls pages worker" do
+            expect(PagesWorker).to receive(:perform_async).with(:deploy, build.id)
+
+            build.success!
+          end
+        end
+
+        context 'job fails' do
+          it "does not call pages worker" do
+            expect(PagesWorker).not_to receive(:perform_async)
+
+            build.drop!
+          end
+        end
+      end
+
+      context 'when pages are disabled' do
+        before do
+          allow(Gitlab.config.pages).to receive_messages(enabled: false)
+        end
+
+        it 'is not marked as pages generator' do
+          expect(build).not_to be_pages_generator
+        end
+
+        context 'job succeeds' do
+          it "does not call pages worker" do
+            expect(PagesWorker).not_to receive(:perform_async)
+
+            build.success!
+          end
+        end
+      end
+    end
+
+    context 'when job is not "pages"' do
+      before do
+        build.name = 'other-job'
+      end
+
+      it 'is not marked as pages generator' do
+        expect(build).not_to be_pages_generator
+      end
+
+      context 'job succeeds' do
+        it "does not call pages worker" do
+          expect(PagesWorker).not_to receive(:perform_async)
+
+          build.success
+        end
+      end
+    end
+  end
 end

spec/requests/api/jobs_spec.rb

@@ -177,6 +177,18 @@ describe API::Jobs do
           json_response.each { |job| expect(job['pipeline']['id']).to eq(pipeline.id) }
         end
       end
+
+      it 'avoids N+1 queries' do
+        control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
+          get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), query
+        end.count
+
+        3.times { create(:ci_build, :artifacts, pipeline: pipeline) }
+
+        expect do
+          get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), query
+        end.not_to exceed_all_query_limit(control_count)
+      end
     end
 
     context 'unauthorized user' do

spec/services/pages_service_spec.rb

-require 'spec_helper'
-
-describe PagesService do
-  let(:build) { create(:ci_build) }
-  let(:data) { Gitlab::DataBuilder::Build.build(build) }
-  let(:service) { described_class.new(data) }
-
-  before do
-    allow(Gitlab.config.pages).to receive(:enabled).and_return(true)
-  end
-
-  context 'execute asynchronously for pages job' do
-    before do
-      build.name = 'pages'
-    end
-
-    context 'on success' do
-      before do
-        build.success
-      end
-
-      it 'executes worker' do
-        expect(PagesWorker).to receive(:perform_async)
-        service.execute
-      end
-    end
-
-    %w(pending running failed canceled).each do |status|
-      context "on #{status}" do
-        before do
-          build.status = status
-        end
-
-        it 'does not execute worker' do
-          expect(PagesWorker).not_to receive(:perform_async)
-          service.execute
-        end
-      end
-    end
-  end
-
-  context 'for other jobs' do
-    before do
-      build.name = 'other job'
-      build.success
-    end
-
-    it 'does not execute worker' do
-      expect(PagesWorker).not_to receive(:perform_async)
-      service.execute
-    end
-  end
-end

spec/services/projects/update_service_spec.rb

@@ -275,6 +275,10 @@ describe Projects::UpdateService do
       it { is_expected.to eq(false) }
     end
 
+    context 'when auto devops is nil' do
+      it { is_expected.to eq(false) }
+    end
+
     context 'when auto devops is explicitly enabled' do
       before do
         project.create_auto_devops!(enabled: true)

spec/support/trace/trace_helpers.rb

+module TraceHelpers
+  def create_legacy_trace(build, content)
+    File.open(legacy_trace_path(build), 'wb') { |stream| stream.write(content) }
+  end
+
+  def create_legacy_trace_in_db(build, content)
+    build.update_column(:trace, content)
+  end
+
+  def legacy_trace_path(build)
+    legacy_trace_dir = File.join(Settings.gitlab_ci.builds_path,
+      build.created_at.utc.strftime("%Y_%m"),
+      build.project_id.to_s)
+
+    FileUtils.mkdir_p(legacy_trace_dir)
+
+    File.join(legacy_trace_dir, "#{build.id}.log")
+  end
+
+  def archived_trace_path(job_artifact)
+    disk_hash = Digest::SHA2.hexdigest(job_artifact.project_id.to_s)
+    creation_date = job_artifact.created_at.utc.strftime('%Y_%m_%d')
+
+    File.join(Gitlab.config.artifacts.path, disk_hash[0..1], disk_hash[2..3], disk_hash,
+      creation_date, job_artifact.job_id.to_s, job_artifact.id.to_s, 'job.log')
+  end
+end