Automatic merge of gitlab-org/gitlab-ce master

GITALY_SERVER_VERSION

-1.24.0
+1.25.0

app/services/merge_requests/merge_to_ref_service.rb

@@ -20,7 +20,12 @@ module MergeRequests
 
       raise_error('Conflicts detected during merge') unless commit_id
 
-      success(commit_id: commit_id)
+      commit = project.commit(commit_id)
+      target_id, source_id = commit.parent_ids
+
+      success(commit_id: commit.id,
+              target_id: target_id,
+              source_id: source_id)
     rescue MergeError => error
       error(error.message)
     end
@@ -38,12 +43,8 @@ module MergeRequests
       error =
         if Feature.disabled?(:merge_to_tmp_merge_ref_path, project)
           'Feature is not enabled'
-        elsif !merge_method_supported?
-          "#{project.human_merge_method} to #{target_ref} is currently not supported."
         elsif !hooks_validation_pass?(merge_request)
           hooks_validation_error(merge_request)
-        elsif @merge_request.should_be_rebased?
-          'Fast-forward merge is not possible. Please update your source branch.'
         elsif !@merge_request.mergeable_to_ref?
           "Merge request is not mergeable to #{target_ref}"
         elsif !source
@@ -68,9 +69,5 @@ module MergeRequests
     rescue Gitlab::Git::PreReceiveError => error
       raise MergeError, error.message
     end
-
-    def merge_method_supported?
-      [:merge, :rebase_merge].include?(project.merge_method)
-    end
   end
 end

changelogs/unreleased/gt-update-activity-filter-for-issues.yml

+---
+title: Update activity filter for issues
+merge_request: 23423
+author: George Tsiolis
+type: changed

changelogs/unreleased/osw-merge-to-ref-changes-for-ci-team.yml

+---
+title: Make merge to refs/merge-requests/:iid/merge not raise when FF-only enabled
+merge_request: 25653
+author:
+type: fixed

changelogs/unreleased/sh-rugged-commit-is-ancestor.yml

+---
+title: Bring back Rugged implementation of CommitIsAncestor
+merge_request: 25702
+author:
+type: other

doc/development/profiling.md

@@ -10,6 +10,10 @@ There is a `Gitlab::Profiler.profile` method, and corresponding
 `bin/profile-url` script, that enable profiling a GET or POST request to a
 specific URL, either as an anonymous user (the default) or as a specific user.
 
+NOTE: **Note:** The first argument to the profiler is either a full URL
+(including the instance hostname) or an absolute path, including the
+leading slash.
+
 When using the script, command-line documentation is available by passing no
 arguments.
 

lib/event_filter.rb

@@ -33,7 +33,7 @@ class EventFilter
     when TEAM
       events.where(action: [Event::JOINED, Event::LEFT, Event::EXPIRED])
     when ISSUE
-      events.where(action: [Event::CREATED, Event::UPDATED, Event::CLOSED, Event::REOPENED])
+      events.where(action: [Event::CREATED, Event::UPDATED, Event::CLOSED, Event::REOPENED], target_type: 'Issue')
     else
       events
     end

lib/gitlab/git/repository.rb

@@ -11,7 +11,7 @@ module Gitlab
       include Gitlab::Git::WrapsGitalyErrors
       include Gitlab::EncodingHelper
       include Gitlab::Utils::StrongMemoize
-      include Gitlab::Git::RuggedImpl::Repository
+      prepend Gitlab::Git::RuggedImpl::Repository
 
       SEARCH_CONTEXT_LINES = 3
       REV_LIST_COMMIT_LIMIT = 2_000

lib/gitlab/git/rugged_impl/repository.rb

@@ -10,7 +10,9 @@ module Gitlab
   module Git
     module RuggedImpl
       module Repository
-        FEATURE_FLAGS = %i(rugged_find_commit).freeze
+        extend ::Gitlab::Utils::Override
+
+        FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor).freeze
 
         def alternate_object_directories
           relative_object_directories.map { |d| File.join(path, d) }
@@ -41,6 +43,29 @@ module Gitlab
           obj = rugged.rev_parse(revspec)
           Ref.dereference_object(obj)
         end
+
+        override :ancestor?
+        def ancestor?(from, to)
+          if Feature.enabled?(:rugged_commit_is_ancestor)
+            rugged_is_ancestor?(from, to)
+          else
+            super
+          end
+        end
+
+        def rugged_is_ancestor?(ancestor_id, descendant_id)
+          return false if ancestor_id.nil? || descendant_id.nil?
+
+          rugged_merge_base(ancestor_id, descendant_id) == ancestor_id
+        rescue Rugged::OdbError
+          false
+        end
+
+        def rugged_merge_base(from, to)
+          rugged.merge_base(from, to)
+        rescue Rugged::ReferenceError
+          nil
+        end
       end
     end
   end

spec/features/dashboard/activity_spec.rb

@@ -39,6 +39,8 @@ describe 'Dashboard > Activity' do
       event
     end
 
+    let(:issue) { create(:issue, project: project) }
+
     let!(:merged_event) do
       create(:event, :merged, project: project, target: merge_request, author: user)
     end
@@ -59,6 +61,10 @@ describe 'Dashboard > Activity' do
       create(:event, :closed, project: project, target: milestone, author: user)
     end
 
+    let!(:issue_event) do
+      create(:event, :created, project: project, target: issue, author: user)
+    end
+
     before do
       project.add_maintainer(user)
 
@@ -74,6 +80,7 @@ describe 'Dashboard > Activity' do
         expect(page).to have_content('closed')
         expect(page).to have_content('commented on')
         expect(page).to have_content('closed milestone')
+        expect(page).to have_content('opened issue')
       end
     end
 
@@ -87,6 +94,7 @@ describe 'Dashboard > Activity' do
         expect(page).not_to have_content('accepted')
         expect(page).not_to have_content('closed')
         expect(page).not_to have_content('commented on')
+        expect(page).not_to have_content('opened issue')
       end
     end
 
@@ -100,6 +108,7 @@ describe 'Dashboard > Activity' do
         expect(page).to have_content('accepted')
         expect(page).not_to have_content('closed')
         expect(page).not_to have_content('commented on')
+        expect(page).not_to have_content('opened issue')
       end
     end
 
@@ -111,9 +120,10 @@ describe 'Dashboard > Activity' do
         expect(page).not_to have_content('pushed new branch')
         expect(page).not_to have_content('joined')
         expect(page).not_to have_content('accepted')
-        expect(page).to have_content('closed')
+        expect(page).not_to have_content('closed')
         expect(page).not_to have_content('commented on')
-        expect(page).to have_content('closed milestone')
+        expect(page).not_to have_content('closed milestone')
+        expect(page).to have_content('opened issue')
       end
     end
 
@@ -127,6 +137,7 @@ describe 'Dashboard > Activity' do
         expect(page).not_to have_content('accepted')
         expect(page).not_to have_content('closed')
         expect(page).to have_content('commented on')
+        expect(page).not_to have_content('opened issue')
       end
     end
 
@@ -140,6 +151,7 @@ describe 'Dashboard > Activity' do
         expect(page).not_to have_content('accepted')
         expect(page).not_to have_content('closed')
         expect(page).not_to have_content('commented on')
+        expect(page).not_to have_content('opened issue')
       end
     end
 
@@ -155,6 +167,7 @@ describe 'Dashboard > Activity' do
         expect(page).not_to have_content('accepted')
         expect(page).not_to have_content('closed')
         expect(page).not_to have_content('commented on')
+        expect(page).not_to have_content('opened issue')
       end
     end
   end

spec/lib/event_filter_spec.rb

@@ -26,10 +26,10 @@ describe EventFilter do
 
     set(:push_event)     { create(:push_event,        project: public_project) }
     set(:merged_event)   { create(:event, :merged,    project: public_project, target: public_project) }
-    set(:created_event)  { create(:event, :created,   project: public_project, target: public_project) }
-    set(:updated_event)  { create(:event, :updated,   project: public_project, target: public_project) }
-    set(:closed_event)   { create(:event, :closed,    project: public_project, target: public_project) }
-    set(:reopened_event) { create(:event, :reopened,  project: public_project, target: public_project) }
+    set(:created_event)  { create(:event, :created,   project: public_project, target: create(:issue, project: public_project)) }
+    set(:updated_event)  { create(:event, :updated,   project: public_project, target: create(:issue, project: public_project)) }
+    set(:closed_event)   { create(:event, :closed,    project: public_project, target: create(:issue, project: public_project)) }
+    set(:reopened_event) { create(:event, :reopened,  project: public_project, target: create(:issue, project: public_project)) }
     set(:comments_event) { create(:event, :commented, project: public_project, target: public_project) }
     set(:joined_event)   { create(:event, :joined,    project: public_project, target: public_project) }
     set(:left_event)     { create(:event, :left,      project: public_project, target: public_project) }

spec/models/repository_spec.rb

@@ -2237,7 +2237,7 @@ describe Repository do
     rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id)
   end
 
-  describe '#ancestor?' do
+  shared_examples '#ancestor?' do
     let(:commit) { repository.commit }
     let(:ancestor) { commit.parents.first }
 
@@ -2261,6 +2261,20 @@ describe Repository do
     end
   end
 
+  describe '#ancestor? with Gitaly enabled' do
+    it_behaves_like "#ancestor?"
+  end
+
+  describe '#ancestor? with Rugged enabled', :enable_rugged do
+    it 'calls out to the Rugged implementation' do
+      allow_any_instance_of(Rugged).to receive(:merge_base).with(repository.commit.id, Gitlab::Git::BLANK_SHA).and_call_original
+
+      repository.ancestor?(repository.commit.id, Gitlab::Git::BLANK_SHA)
+    end
+
+    it_behaves_like '#ancestor?'
+  end
+
   describe '#archive_metadata' do
     let(:ref) { 'master' }
     let(:storage_path) { '/tmp' }

spec/requests/api/merge_requests_spec.rb

@@ -1153,18 +1153,6 @@ describe API::MergeRequests do
 
       expect(response).to have_gitlab_http_status(404)
     end
-
-    it "returns 400 when merge method is not supported" do
-      merge_request.project.update!(merge_method: 'ff')
-
-      put api(url, user)
-
-      expected_error =
-        'Fast-forward to refs/merge-requests/1/merge is currently not supported.'
-
-      expect(response).to have_gitlab_http_status(400)
-      expect(json_response['message']).to eq(expected_error)
-    end
   end
 
   describe "PUT /projects/:id/merge_requests/:merge_request_iid" do

spec/services/merge_requests/merge_to_ref_service_spec.rb

@@ -19,27 +19,7 @@ describe MergeRequests::MergeToRefService do
     end
   end
 
-  set(:user) { create(:user) }
-  let(:merge_request) { create(:merge_request, :simple) }
-  let(:project) { merge_request.project }
-
-  before do
-    project.add_maintainer(user)
-  end
-
-  describe '#execute' do
-    let(:service) do
-      described_class.new(project, user,
-                          commit_message: 'Awesome message',
-                          'should_remove_source_branch' => true)
-    end
-
-    def process_merge_to_ref
-      perform_enqueued_jobs do
-        service.execute(merge_request)
-      end
-    end
-
+  shared_examples_for 'successfully merges to ref with merge method' do
     it 'writes commit to merge ref' do
       repository = project.repository
       target_ref = merge_request.merge_ref_path
@@ -52,9 +32,31 @@ describe MergeRequests::MergeToRefService do
 
       expect(result[:status]).to eq(:success)
       expect(result[:commit_id]).to be_present
+      expect(result[:source_id]).to eq(merge_request.source_branch_sha)
+      expect(result[:target_id]).to eq(merge_request.target_branch_sha)
       expect(repository.ref_exists?(target_ref)).to be(true)
       expect(ref_head.id).to eq(result[:commit_id])
     end
+  end
+
+  shared_examples_for 'successfully evaluates pre-condition checks' do
+    it 'returns error when feature is disabled' do
+      stub_feature_flags(merge_to_tmp_merge_ref_path: false)
+
+      result = service.execute(merge_request)
+
+      expect(result[:status]).to eq(:error)
+      expect(result[:message]).to eq('Feature is not enabled')
+    end
+
+    it 'returns an error when the failing to process the merge' do
+      allow(project.repository).to receive(:merge_to_ref).and_return(nil)
+
+      result = service.execute(merge_request)
+
+      expect(result[:status]).to eq(:error)
+      expect(result[:message]).to eq('Conflicts detected during merge')
+    end
 
     it 'does not send any mail' do
       expect { process_merge_to_ref }.not_to change { ActionMailer::Base.deliveries.count }
@@ -73,25 +75,31 @@ describe MergeRequests::MergeToRefService do
 
       process_merge_to_ref
     end
+  end
 
-    it 'returns error when feature is disabled' do
-      stub_feature_flags(merge_to_tmp_merge_ref_path: false)
+  set(:user) { create(:user) }
+  let(:merge_request) { create(:merge_request, :simple) }
+  let(:project) { merge_request.project }
 
-      result = service.execute(merge_request)
+  before do
+    project.add_maintainer(user)
+  end
 
-      expect(result[:status]).to eq(:error)
-      expect(result[:message]).to eq('Feature is not enabled')
+  describe '#execute' do
+    let(:service) do
+      described_class.new(project, user, commit_message: 'Awesome message',
+                                         should_remove_source_branch: true)
     end
 
-    it 'returns an error when the failing to process the merge' do
-      allow(project.repository).to receive(:merge_to_ref).and_return(nil)
-
-      result = service.execute(merge_request)
-
-      expect(result[:status]).to eq(:error)
-      expect(result[:message]).to eq('Conflicts detected during merge')
+    def process_merge_to_ref
+      perform_enqueued_jobs do
+        service.execute(merge_request)
+      end
     end
 
+    it_behaves_like 'successfully merges to ref with merge method'
+    it_behaves_like 'successfully evaluates pre-condition checks'
+
     context 'commit history comparison with regular MergeService' do
       let(:merge_ref_service) do
         described_class.new(project, user, {})
@@ -122,29 +130,15 @@ describe MergeRequests::MergeToRefService do
       context 'when semi-linear merge method' do
         let(:merge_method) { :rebase_merge }
 
-        it 'return error when MR should be able to fast-forward' do
-          allow(merge_request).to receive(:should_be_rebased?) { true }
-
-          error_message = 'Fast-forward merge is not possible. Please update your source branch.'
-
-          result = service.execute(merge_request)
-
-          expect(result[:status]).to eq(:error)
-          expect(result[:message]).to eq(error_message)
-        end
+        it_behaves_like 'successfully merges to ref with merge method'
+        it_behaves_like 'successfully evaluates pre-condition checks'
       end
 
       context 'when fast-forward merge method' do
         let(:merge_method) { :ff }
 
-        it 'returns error' do
-          error_message = "Fast-forward to #{merge_request.merge_ref_path} is currently not supported."
-
-          result = service.execute(merge_request)
-
-          expect(result[:status]).to eq(:error)
-          expect(result[:message]).to eq(error_message)
-        end
+        it_behaves_like 'successfully merges to ref with merge method'
+        it_behaves_like 'successfully evaluates pre-condition checks'
       end
 
       context 'when MR is not mergeable to ref' do