Fix the rebase before merge feature

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
+v 7.14.2
+  - Fix the rebase before merge feature
+
 v 7.14.1
   - Only include base URL in OmniAuth full_host parameter (Stan Hu)
   - Fix Error 500 in API when accessing a group that has an avatar (Stan Hu)

app/models/merge_request.rb

@@ -45,6 +45,9 @@ class MergeRequest < ActiveRecord::Base
   attr_accessor :should_remove_source_branch
+  # If true, merge request should rebase before merging
+  attr_accessor :should_rebase
+
   # When this attribute is true some MR validation is ignored
   # It allows us to close or modify broken merge requests
   attr_accessor :allow_broken

app/workers/auto_merge_worker.rb

@@ -8,6 +8,7 @@ def perform(merge_request_id, current_user_id, params)
     current_user = User.find(current_user_id)
     merge_request = MergeRequest.find(merge_request_id)
     merge_request.should_remove_source_branch = params[:should_remove_source_branch]
+    merge_request.should_rebase = params[:should_rebase]
     merge_request.automerge!(current_user, params[:commit_message])
   end
 end

lib/gitlab/satellite/merge_action.rb

@@ -2,6 +2,8 @@ module Gitlab
   module Satellite
     # GitLab server-side merge
     class MergeAction < Action
+      include Gitlab::Popen
+
       attr_accessor :merge_request
       def initialize(user, merge_request)
@@ -27,17 +29,25 @@ def can_be_merged?
       def merge!(merge_commit_message = nil)
         in_locked_and_timed_satellite do |merge_repo|
           prepare_satellite!(merge_repo)
+
+          # If rebase before merge
+          # Attempt a rebase and return if succesful.
+          # If unsuccesful, continue with regular merge.
+          if merge_request.should_rebase
+            if rebase_in_satellite!(merge_repo)
+              remove_source_branch(merge_repo)
+              return true
+            end
+          end
+
           if merge_in_satellite!(merge_repo, merge_commit_message)
             # push merge back to bare repo
             # will raise CommandFailed when push fails
             merge_repo.git.push(default_options, :origin, merge_request.target_branch)
             # remove source branch
-            if merge_request.remove_source_branch?
-              # will raise CommandFailed when push fails
-              merge_repo.git.push(default_options, :origin, ":#{merge_request.source_branch}")
-              merge_request.source_project.repository.expire_branch_names
-            end
+            remove_source_branch(merge_repo)
+
             # merge, push and branch removal successful
             true
           end
@@ -134,6 +144,40 @@ def merge_in_satellite!(repo, message = nil)
         handle_exception(ex)
       end
+      # Rebases before merging the source_branch into the target_branch in the satellite.
+      # Before starting the rebase, clears out the satellite.
+      # Returns false if rebase cannot be done cleanly and resets the satellite.
+      # Returns true otherwise.
+
+      # Eg. of clean rebase
+      # source_branch: feature
+      # target_branch: master
+      #
+      # git checkout feature
+      # git pull --rebase origin master
+      # git push origin feature:master
+      # git merge feature
+      # git push remote master
+      def rebase_in_satellite!(repo)
+        update_satellite_source_and_target!(repo)
+        repo.git.checkout(default_options({ b: true }), merge_request.source_branch, "source/#{merge_request.source_branch}")
+
+        output, status = popen(%W(git pull --rebase origin #{merge_request.target_branch}), repo.working_dir)
+        if status == 0
+          Gitlab::AppLogger.info "Rebasing before merge in #{merge_request.source_project.path_with_namespace} MR!#{merge_request.id}: #{output}."
+          if merge_request.source_branch && merge_request.target_branch
+            repo.git.push(default_options, "origin", "#{merge_request.source_branch}:#{merge_request.target_branch}")
+          end
+        else
+          repo.git.rebase(default_options, "--abort")
+          Gitlab::AppLogger.info "Rebasing in in #{merge_request.source_project.path_with_namespace} MR!#{merge_request.id} aborted, rebase manually."
+          prepare_satellite!(repo)
+          false
+        end
+      rescue Grit::Git::CommandFailed => ex
+        handle_exception(ex)
+      end
+
       # Assumes a satellite exists that is a fresh clone of the projects repo, prepares satellite for merges, diffs etc
       def update_satellite_source_and_target!(repo)
         repo.remote_add('source', merge_request.source_project.repository.path_to_repo)
@@ -142,6 +186,16 @@ def update_satellite_source_and_target!(repo)
       rescue Grit::Git::CommandFailed => ex
         handle_exception(ex)
       end
+
+      def remove_source_branch(repo)
+        if merge_request.remove_source_branch?
+          # will raise CommandFailed when push fails
+          repo.git.push(default_options, :origin, ":#{merge_request.source_branch}")
+          merge_request.source_project.repository.expire_branch_names
+        end
+      rescue Grit::Git::CommandFailed => ex
+        handle_exception(ex)
+      end
     end
   end
 end

spec/lib/gitlab/satellite/merge_action_spec.rb

@@ -102,6 +102,41 @@ def is_a_matching_diff(diff, diffs)
     end
   end
+  describe '#can_be_rebased?' do
+    context 'on fork' do
+      before(:each) do
+        allow(merge_request).to receive(:should_rebase).and_return(true)
+      end
+
+      it do
+        result = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).merge!
+        expect(result).to be_truthy
+      end
+
+      it do
+        result = Gitlab::Satellite::MergeAction.new(merge_request_fork_with_conflict.author, merge_request_fork_with_conflict).merge!
+        expect(result).to be_falsey
+      end
+    end
+
+    context 'between branches' do
+      before(:each) do
+        allow(merge_request).to receive(:should_rebase).and_return(true)
+      end
+
+      it do
+        result = Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).merge!
+        expect(result).to be_truthy
+      end
+
+      it do
+        result = Gitlab::Satellite::MergeAction.new(merge_request_with_conflict.author, merge_request_with_conflict).merge!
+        expect(result).to be_falsey
+      end
+    end
+  end
+
+
   describe '#merge!' do
     let(:merge_request) { create(:merge_request, source_project: project, target_project: project, source_branch: "markdown", should_remove_source_branch: true) }
     let(:merge_action) { Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request) }