Merge branch 'ce-to-ee-2018-07-04' into 'master'

CE upstream - 2018-07-04 09:23 UTC

Closes gitaly#949, gitaly#911, gitaly#820, gitaly#514, gitaly#551, gitaly#321, and gitaly#565

See merge request gitlab-org/gitlab-ee!6375

Gemfile

@@ -142,7 +142,7 @@ gem 'aws-sdk'
 gem 'faraday_middleware-aws-signers-v4'
 
 # Markdown and HTML processing
-gem 'html-pipeline', '~> 2.7.1'
+gem 'html-pipeline', '~> 2.8'
 gem 'deckar01-task_list', '2.0.0'
 gem 'gitlab-markup', '~> 1.6.4'
 gem 'redcarpet', '~> 3.4'

Gemfile.lock

@@ -421,7 +421,7 @@ GEM
     hipchat (1.5.2)
       httparty
       mimemagic
-    html-pipeline (2.7.1)
+    html-pipeline (2.8.3)
       activesupport (>= 2)
       nokogiri (>= 1.4)
     html2text (0.2.0)
@@ -1097,7 +1097,7 @@ DEPENDENCIES
   hashie-forbidden_attributes
   health_check (~> 2.6.0)
   hipchat (~> 1.5.0)
-  html-pipeline (~> 2.7.1)
+  html-pipeline (~> 2.8)
   html2text
   httparty (~> 0.13.3)
   icalendar

Gemfile.rails5.lock

@@ -424,7 +424,7 @@ GEM
     hipchat (1.5.2)
       httparty
       mimemagic
-    html-pipeline (2.7.1)
+    html-pipeline (2.8.3)
       activesupport (>= 2)
       nokogiri (>= 1.4)
     html2text (0.2.0)
@@ -1107,7 +1107,7 @@ DEPENDENCIES
   hashie-forbidden_attributes
   health_check (~> 2.6.0)
   hipchat (~> 1.5.0)
-  html-pipeline (~> 2.7.1)
+  html-pipeline (~> 2.8)
   html2text
   httparty (~> 0.13.3)
   icalendar

app/controllers/projects/jobs_controller.rb

@@ -44,12 +44,10 @@ class Projects::JobsController < Projects::ApplicationController
   end
 
   def show
-    @builds = @project.pipelines
-      .find_by_sha(@build.sha)
-      .builds
+    @pipeline = @build.pipeline
+    @builds = @pipeline.builds
       .order('id DESC')
       .present(current_user: current_user)
-    @pipeline = @build.pipeline
 
     respond_to do |format|
       format.html

changelogs/unreleased/47040-inconsistent-job-list-in-job-details-view.yml

+---
+title: Show jobs from same pipeline in sidebar in job details view.
+merge_request: 20243
+author:
+type: fixed

doc/api/jobs.md

@@ -32,15 +32,12 @@ Example of response
       "title": "Test the CI integration."
     },
     "coverage": null,
-    "created_at": "2015-12-24T15:51:21.802Z",
-    "artifacts_file": {
-      "filename": "artifacts.zip",
-      "size": 1000
-    },
-    "finished_at": "2015-12-24T17:54:27.895Z",
-    "artifacts_expire_at": "2016-01-23T17:54:27.895Z"
-    "id": 7,
-    "name": "teaspoon",
+    "created_at": "2015-12-24T15:51:21.727Z",
+    "artifacts_file": null,
+    "finished_at": "2015-12-24T17:54:24.921Z",
+    "artifacts_expire_at": "2016-01-23T17:54:24.921Z",
+    "id": 6,
+    "name": "rspec:other",
     "pipeline": {
       "id": 6,
       "ref": "master",
@@ -50,7 +47,7 @@ Example of response
     "ref": "master",
     "runner": null,
     "stage": "test",
-    "started_at": "2015-12-24T17:54:27.722Z",
+    "started_at": "2015-12-24T17:54:24.729Z",
     "status": "failed",
     "tag": false,
     "user": {
@@ -79,12 +76,15 @@ Example of response
       "title": "Test the CI integration."
     },
     "coverage": null,
-    "created_at": "2015-12-24T15:51:21.727Z",
-    "artifacts_file": null,
-    "finished_at": "2015-12-24T17:54:24.921Z",
-    "artifacts_expire_at": "2016-01-23T17:54:24.921Z",
-    "id": 6,
-    "name": "rspec:other",
+    "created_at": "2015-12-24T15:51:21.802Z",
+    "artifacts_file": {
+      "filename": "artifacts.zip",
+      "size": 1000
+    },
+    "finished_at": "2015-12-24T17:54:27.895Z",
+    "artifacts_expire_at": "2016-01-23T17:54:27.895Z"
+    "id": 7,
+    "name": "teaspoon",
     "pipeline": {
       "id": 6,
       "ref": "master",
@@ -94,7 +94,7 @@ Example of response
     "ref": "master",
     "runner": null,
     "stage": "test",
-    "started_at": "2015-12-24T17:54:24.729Z",
+    "started_at": "2015-12-24T17:54:27.722Z",
     "status": "failed",
     "tag": false,
     "user": {
@@ -148,15 +148,12 @@ Example of response
       "title": "Test the CI integration."
     },
     "coverage": null,
-    "created_at": "2015-12-24T15:51:21.802Z",
-    "artifacts_file": {
-      "filename": "artifacts.zip",
-      "size": 1000
-    },
-    "finished_at": "2015-12-24T17:54:27.895Z",
-    "artifacts_expire_at": "2016-01-23T17:54:27.895Z"
-    "id": 7,
-    "name": "teaspoon",
+    "created_at": "2015-12-24T15:51:21.727Z",
+    "artifacts_file": null,
+    "finished_at": "2015-12-24T17:54:24.921Z",
+    "artifacts_expire_at": "2016-01-23T17:54:24.921Z"
+    "id": 6,
+    "name": "rspec:other",
     "pipeline": {
       "id": 6,
       "ref": "master",
@@ -166,7 +163,7 @@ Example of response
     "ref": "master",
     "runner": null,
     "stage": "test",
-    "started_at": "2015-12-24T17:54:27.722Z",
+    "started_at": "2015-12-24T17:54:24.729Z",
     "status": "failed",
     "tag": false,
     "user": {
@@ -195,12 +192,15 @@ Example of response
       "title": "Test the CI integration."
     },
     "coverage": null,
-    "created_at": "2015-12-24T15:51:21.727Z",
-    "artifacts_file": null,
-    "finished_at": "2015-12-24T17:54:24.921Z",
-    "artifacts_expire_at": "2016-01-23T17:54:24.921Z"
-    "id": 6,
-    "name": "rspec:other",
+    "created_at": "2015-12-24T15:51:21.802Z",
+    "artifacts_file": {
+      "filename": "artifacts.zip",
+      "size": 1000
+    },
+    "finished_at": "2015-12-24T17:54:27.895Z",
+    "artifacts_expire_at": "2016-01-23T17:54:27.895Z"
+    "id": 7,
+    "name": "teaspoon",
     "pipeline": {
       "id": 6,
       "ref": "master",
@@ -210,7 +210,7 @@ Example of response
     "ref": "master",
     "runner": null,
     "stage": "test",
-    "started_at": "2015-12-24T17:54:24.729Z",
+    "started_at": "2015-12-24T17:54:27.722Z",
     "status": "failed",
     "tag": false,
     "user": {

doc/integration/google.md

@@ -77,7 +77,7 @@ On your GitLab server:
 
     For installations from source:
 
-    ```
+    ```yaml
     - { name: 'google_oauth2', app_id: 'YOUR_APP_ID',
       app_secret: 'YOUR_APP_SECRET',
       args: { access_type: 'offline', approval_prompt: '' } }

lib/banzai/filter/sanitization_filter.rb

@@ -4,27 +4,20 @@ module Banzai
     #
     # Extends HTML::Pipeline::SanitizationFilter with a custom whitelist.
     class SanitizationFilter < HTML::Pipeline::SanitizationFilter
+      include Gitlab::Utils::StrongMemoize
+
       UNSAFE_PROTOCOLS = %w(data javascript vbscript).freeze
       TABLE_ALIGNMENT_PATTERN = /text-align: (?<alignment>center|left|right)/
 
       def whitelist
-        whitelist = super
-
-        customize_whitelist(whitelist)
-
-        whitelist
+        strong_memoize(:whitelist) do
+          customize_whitelist(super.dup)
+        end
       end
 
       private
 
-      def customized?(transformers)
-        transformers.last.source_location[0] == __FILE__
-      end
-
       def customize_whitelist(whitelist)
-        # Only push these customizations once
-        return if customized?(whitelist[:transformers])
-
         # Allow table alignment; we whitelist specific text-align values in a
         # transformer below
         whitelist[:attributes]['th'] = %w(style)

lib/gitlab/git/commit.rb

@@ -63,12 +63,8 @@ module Gitlab
           # This saves us an RPC round trip.
           return nil if commit_id.include?(':')
 
-          commit = repo.gitaly_migrate(:find_commit) do |is_enabled|
-            if is_enabled
-              repo.gitaly_commit_client.find_commit(commit_id)
-            else
-              rugged_find(repo, commit_id)
-            end
+          commit = repo.wrapped_gitaly_errors do
+            repo.gitaly_commit_client.find_commit(commit_id)
           end
 
           decorate(repo, commit) if commit
@@ -78,12 +74,6 @@ module Gitlab
           nil
         end
 
-        def rugged_find(repo, commit_id)
-          obj = repo.rev_parse_target(commit_id)
-
-          obj.is_a?(Rugged::Commit) ? obj : nil
-        end
-
         # Get last commit for HEAD
         #
         # Ex.

lib/gitlab/git/conflict/resolver.rb

@@ -12,14 +12,8 @@ module Gitlab
         end
 
         def conflicts
-          @conflicts ||= begin
-            @target_repository.gitaly_migrate(:conflicts_list_conflict_files) do |is_enabled|
-              if is_enabled
-                gitaly_conflicts_client(@target_repository).list_conflict_files.to_a
-              else
-                rugged_list_conflict_files
-              end
-            end
+          @conflicts ||= @target_repository.wrapped_gitaly_errors do
+            gitaly_conflicts_client(@target_repository).list_conflict_files.to_a
           end
         rescue GRPC::FailedPrecondition => e
           raise Gitlab::Git::Conflict::Resolver::ConflictSideMissing.new(e.message)
@@ -28,12 +22,8 @@ module Gitlab
         end
 
         def resolve_conflicts(source_repository, resolution, source_branch:, target_branch:)
-          source_repository.gitaly_migrate(:conflicts_resolve_conflicts) do |is_enabled|
-            if is_enabled
-              gitaly_conflicts_client(source_repository).resolve_conflicts(@target_repository, resolution, source_branch, target_branch)
-            else
-              rugged_resolve_conflicts(source_repository, resolution, source_branch, target_branch)
-            end
+          source_repository.wrapped_gitaly_errors do
+            gitaly_conflicts_client(source_repository).resolve_conflicts(@target_repository, resolution, source_branch, target_branch)
           end
         end
 
@@ -61,57 +51,6 @@ module Gitlab
         def gitaly_conflicts_client(repository)
           repository.gitaly_conflicts_client(@our_commit_oid, @their_commit_oid)
         end
-
-        def write_resolved_file_to_index(repository, index, file, params)
-          if params[:sections]
-            resolved_lines = file.resolve_lines(params[:sections])
-            new_file = resolved_lines.map { |line| line[:full_line] }.join("\n")
-
-            new_file << "\n" if file.our_blob.data.end_with?("\n")
-          elsif params[:content]
-            new_file = file.resolve_content(params[:content])
-          end
-
-          our_path = file.our_path
-
-          oid = repository.rugged.write(new_file, :blob)
-          index.add(path: our_path, oid: oid, mode: file.our_mode)
-          index.conflict_remove(our_path)
-        end
-
-        def rugged_list_conflict_files
-          target_index = @target_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid)
-
-          # We don't need to do `with_repo_branch_commit` here, because the target
-          # project always fetches source refs when creating merge request diffs.
-          conflict_files(@target_repository, target_index)
-        end
-
-        def rugged_resolve_conflicts(source_repository, resolution, source_branch, target_branch)
-          source_repository.with_repo_branch_commit(@target_repository, target_branch) do
-            index = source_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid)
-            conflicts = conflict_files(source_repository, index)
-
-            resolution.files.each do |file_params|
-              conflict_file = conflict_for_path(conflicts, file_params[:old_path], file_params[:new_path])
-
-              write_resolved_file_to_index(source_repository, index, conflict_file, file_params)
-            end
-
-            unless index.conflicts.empty?
-              missing_files = index.conflicts.map { |file| file[:ours][:path] }
-
-              raise ResolutionError, "Missing resolutions for the following files: #{missing_files.join(', ')}"
-            end
-
-            commit_params = {
-              message: resolution.commit_message,
-              parents: [@our_commit_oid, @their_commit_oid]
-            }
-
-            source_repository.commit_index(resolution.user, source_branch, index, commit_params)
-          end
-        end
       end
     end
   end

lib/gitlab/git/repository.rb

@@ -556,12 +556,8 @@ module Gitlab
       # diff options.  The +options+ hash can also include :break_rewrites to
       # split larger rewrites into delete/add pairs.
       def diff(from, to, options = {}, *paths)
-        iterator = gitaly_migrate(:diff_between) do |is_enabled|
-          if is_enabled
-            gitaly_commit_client.diff(from, to, options.merge(paths: paths))
-          else
-            diff_patches(from, to, options, *paths)
-          end
+        iterator = wrapped_gitaly_errors do
+          gitaly_commit_client.diff(from, to, options.merge(paths: paths))
         end
 
         Gitlab::Git::DiffCollection.new(iterator, options)
@@ -1608,17 +1604,6 @@ module Gitlab
         tmp_entry
       end
 
-      # Return the Rugged patches for the diff between +from+ and +to+.
-      def diff_patches(from, to, options = {}, *paths)
-        options ||= {}
-        break_rewrites = options[:break_rewrites]
-        actual_options = Gitlab::Git::Diff.filter_diff_options(options.merge(paths: paths))
-
-        diff = rugged.diff(from, to, actual_options)
-        diff.find_similar!(break_rewrites: break_rewrites)
-        diff.each_patch
-      end
-
       def sort_branches(branches, sort_by)
         case sort_by
         when 'name'

spec/controllers/projects/jobs_controller_spec.rb

@@ -102,6 +102,8 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
 
   describe 'GET show' do
     let!(:job) { create(:ci_build, :failed, pipeline: pipeline) }
+    let!(:second_job) { create(:ci_build, :failed, pipeline: pipeline) }
+    let!(:third_job) { create(:ci_build, :failed) }
 
     context 'when requesting HTML' do
       context 'when job exists' do
@@ -113,6 +115,13 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
           expect(response).to have_gitlab_http_status(:ok)
           expect(assigns(:build).id).to eq(job.id)
         end
+
+        it 'has the correct build collection' do
+          builds = assigns(:builds).map(&:id)
+
+          expect(builds).to include(job.id, second_job.id)
+          expect(builds).not_to include(third_job.id)
+        end
       end
 
       context 'when job does not exist' do

spec/services/merge_requests/conflicts/list_service_spec.rb

@@ -84,23 +84,5 @@ describe MergeRequests::Conflicts::ListService do
 
       expect(service.can_be_resolved_in_ui?).to be_falsey
     end
-
-    context 'with gitaly disabled', :skip_gitaly_mock do
-      it 'returns a falsey value when the MR has a missing ref after a force push' do
-        merge_request = create_merge_request('conflict-resolvable')
-        service = conflicts_service(merge_request)
-        allow_any_instance_of(Rugged::Repository).to receive(:merge_commits).and_raise(Rugged::OdbError)
-
-        expect(service.can_be_resolved_in_ui?).to be_falsey
-      end
-
-      it 'returns a falsey value when the MR has a missing revision after a force push' do
-        merge_request = create_merge_request('conflict-resolvable')
-        service = conflicts_service(merge_request)
-        allow(merge_request).to receive_message_chain(:target_branch_head, :raw, :id).and_return(Gitlab::Git::BLANK_SHA)
-
-        expect(service.can_be_resolved_in_ui?).to be_falsey
-      end
-    end
   end
 end

spec/services/merge_requests/conflicts/resolve_service_spec.rb

@@ -123,17 +123,6 @@ describe MergeRequests::Conflicts::ResolveService do
           expect(merge_request_from_fork.source_branch_head.parents.map(&:id))
             .to eq(['404fa3fc7c2c9b5dacff102f353bdf55b1be2813', target_head])
         end
-
-        context 'when gitaly is disabled', :skip_gitaly_mock do
-          it 'gets conflicts from the source project' do
-            # REFACTOR NOTE: We used to test that `project.repository.rugged` wasn't
-            # used in this case, but since the refactor, for simplification,
-            # we always use that repository for read only operations.
-            expect(forked_project.repository.rugged).to receive(:merge_commits).and_call_original
-
-            subject
-          end
-        end
       end
     end
 

spec/tasks/gitlab/git_rake_spec.rb

 require 'rake_helper'
 
 describe 'gitlab:git rake tasks' do
+  let(:base_path) { 'tmp/tests/default_storage' }
+
   before(:all) do
     @default_storage_hash = Gitlab.config.repositories.storages.default.to_h
   end
 
   before do
     Rake.application.rake_require 'tasks/gitlab/git'
-    storages = { 'default' => Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/default_storage')) }
+    storages = { 'default' => Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => base_path)) }
+
+    path = Settings.absolute("#{base_path}/@hashed/1/2/test.git")
+    FileUtils.mkdir_p(path)
+    Gitlab::Popen.popen(%W[git -C #{path} init --bare])
 
-    FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git'))
     allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
     allow_any_instance_of(String).to receive(:color) { |string, _color| string }
 
@@ -17,7 +22,7 @@ describe 'gitlab:git rake tasks' do
   end
 
   after do
-    FileUtils.rm_rf(Settings.absolute('tmp/tests/default_storage'))
+    FileUtils.rm_rf(Settings.absolute(base_path))
   end
 
   describe 'fsck' do
@@ -26,14 +31,14 @@ describe 'gitlab:git rake tasks' do
     end
 
     it 'errors out about config.lock issues' do
-      FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/config.lock'))
+      FileUtils.touch(Settings.absolute("#{base_path}/@hashed/1/2/test.git/config.lock"))
 
       expect { run_rake_task('gitlab:git:fsck') }.to output(/file exists\? ... yes/).to_stdout
     end
 
     it 'errors out about ref lock issues' do
-      FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/refs/heads'))
-      FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/refs/heads/blah.lock'))
+      FileUtils.mkdir_p(Settings.absolute("#{base_path}/@hashed/1/2/test.git/refs/heads"))
+      FileUtils.touch(Settings.absolute("#{base_path}/@hashed/1/2/test.git/refs/heads/blah.lock"))
 
       expect { run_rake_task('gitlab:git:fsck') }.to output(/Ref lock files exist:/).to_stdout
     end