NXD Teach GitLab about patches

Teach GitLab not only to merge changes from a merge-request, but also to
apply patches posted to merge-request in a way like `git am` would do -
without merge commit and directly on top of current branch. Which way to
go is selected by user in web UI, and apply patches is the first option.

There are 3 cases:

- only 1 commit is present in MR -> the only available option is to
  apply that single commit as one patch without a merge

  ( There is no need for merge commit in this case at all: information
    about user who applied the patch goes to "Committer" field in resultant
    commit. Avoiding 1 merge per 1 patch results in cleaner history )

  It is also possible to review patch description directly in web UI,
  before doing the actual application, and correct / amend it as needed.

- several commits are present in MR:

  * it is possible to apply the patches directly on top of current
    branch. Again information about who applied what goes to "Committer"
    field.

  * it is possible to merge MR changes with making a merge commit.

    This variant is useful, when patches from a MR do several logical
    steps to reach one goal, and MR description contain cover letter for
    whole patch series.

    in this case original commits stay untouched and resulting merge
    will contain MR author as author, user who accepted MR as committer,
    and cover letter as merge commit message.

    NOTE we avoid useless "Merge branch X into Y" in merge message, and
        just put MR title into merge subject and MR description into merge
        description.

        This way it is more logical with more important information in
        merge subject and thus e.g. more handy to oversee what a merge brings,
        just by it subject, e.g. via looking at updates via

            gitk --first-parent ...

        or via web.

NOTE for pre-generated references to merge-request we now use full MR
    URL, instead of !<MR-n>. Full URLs work everywhere, not only on
    original site where MR was created, or even only in original repo
    and not its fork on the same site.

app/assets/stylesheets/pages/merge_requests.scss

@@ -54,6 +54,12 @@
       padding-top: 8px;
       line-height: 20px;
 
+      // text in-between buttons
+      &.inbetween-text {
+        margin-left: 0px;  // disable `margin-left: 20px` in parent
+        margin-right: 7px; // compensate for margin-right in btn-grouped
+      }
+
       &.right {
         float: right;
         padding-right: 0;

app/controllers/projects/merge_requests_controller.rb

@@ -659,7 +659,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def merge_params
-    params.permit(:should_remove_source_branch, :commit_message)
+    params.permit(:should_remove_source_branch, :commit_message, :apply_patches)
   end
 
   # Make sure merge requests created before 8.0

app/models/merge_request.rb

@@ -598,8 +598,11 @@ class MergeRequest < ActiveRecord::Base
       issue.to_reference(target_project)
     end
 
+    #message = [
+    #  "Merge branch '#{source_branch}' into '#{target_branch}'",
+    #  title
+    #]
     message = [
-      "Merge branch '#{source_branch}' into '#{target_branch}'",
       title
     ]
 
@@ -608,7 +611,7 @@ class MergeRequest < ActiveRecord::Base
     end
 
     message << "#{description}" if include_description && description.present?
-    message << "See merge request #{to_reference}"
+    message << "/reviewed-on #{Gitlab::UrlBuilder.build(self)}"
 
     message.join("\n\n")
   end

app/models/repository.rb

@@ -854,6 +854,51 @@ class Repository
     false
   end
 
+  # apply patches represented by [] of commits
+  def apply_patches(user, patches, target_branch, options = {})
+    base_commit = rugged.branches[target_branch].target
+    raise "Invalid apply target" if base_commit.nil?
+
+    res = nil
+    patches.each_with_index do |patch, i|
+      patch = rugged.lookup(patch.id) # Gitlab::Commit -> Rugged::Commit
+      args = [patch, base_commit]
+      args << { mainline: 1 } if patch.parents.size > 1
+
+      index = rugged.cherrypick_commit(*args)
+      raise "Conflict while applying #{patch.oid} to #{base_commit.oid}" if index.conflicts?
+
+      tree_id = index.write_tree(rugged)
+      # no check for empty diff: i.e. cherry-pick --allow-empty
+
+      committer = user_to_committer(user)
+      commit_options = {
+        message:    patch.message,
+        author:     patch.author,
+        committer:  committer,
+        tree:       tree_id,
+        parents:    [base_commit],
+      }.merge(options)
+
+      # for all patches except last just create commit object without updating
+      # ref and invoking hooks
+      if i + 1 < patches.length then
+        base_commit = Rugged::Commit.create(rugged, commit_options)
+
+      # for last patch make sure to update ref and invoke hooks
+      else
+        res = commit_with_hooks(user, target_branch) do |ref|
+          commit_options["update_ref"] = ref
+          Rugged::Commit.create(rugged, commit_options)
+        end
+      end
+
+    end
+
+    fail if res.nil? # assert
+    res
+  end
+
   def revert(
     user, commit, branch_name,
     start_branch_name: nil, start_project: project)

app/services/merge_requests/merge_service.rb

@@ -37,16 +37,34 @@ module MergeRequests
 
     def commit
       committer = repository.user_to_committer(current_user)
+      # MR author with corrected time to be MR creation time.
+      # (NOTE rugged wants Time, not ActiveSupport::TimeWithZone)
+      mr_author = repository.user_to_committer(merge_request.author)
+                  .merge({time: Time.parse(merge_request.created_at.to_s)})
 
+      mr_message= params[:commit_message] || merge_request.merge_commit_message
+      mr_1patch = merge_request.commits.size == 1
+
+      do_apply = params[:apply_patches] == "on" || mr_1patch
+      do_merge = !do_apply
+
+      # merge -> author=mr_author, message=mr_message
+      # apply -> author=patch author, message=mr_message if only 1 patch
       options = {
-        message: params[:commit_message] || merge_request.merge_commit_message,
-        author: committer,
+        #message: params[:commit_message] || merge_request.merge_commit_message,
+        #author: commiter,
         committer: committer
       }
+      options[:author]  = mr_author   if do_merge
+      options[:message] = mr_message  if do_merge || (do_apply && merge_request.commits.size == 1)
+
+      commit_id = do_merge \
+        ? repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options)
+        : repository.apply_patches(current_user, merge_request.commits.reverse, merge_request.target_branch, options)
 
-      commit_id = repository.merge(current_user, source, merge_request, options)
+      #commit_id = repository.merge(current_user, source, merge_request, options)
 
-      raise MergeError, 'Conflicts detected during merge' unless commit_id
+      #raise MergeError, 'Conflicts detected during merge' unless commit_id
 
       merge_request.update(merge_commit_sha: commit_id)
     rescue GitHooksService::PreReceiveError => e

app/views/projects/merge_requests/widget/open/_accept.html.haml

@@ -26,8 +26,18 @@
                     = icon('warning fw')
                     Merge immediately
         - else
-          = f.button class: "btn btn-grouped js-merge-button accept-merge-request" do
-            Accept merge request
+          = f.button class: "btn btn-create btn-grouped js-apply-button accept_merge_request accept_merge_request_apply #{status_class}" do
+            - if @merge_request.commits.size == 1
+              Apply Patch
+            - else
+              Apply Patches
+
+          - if @merge_request.commits.size > 1
+            // do we really need accept-control.inbetween-text? make it simpler?
+            .accept-control.inbetween-text
+              or
+            = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request_merge #{status_class}" do
+              Merge as Topic
       - if @merge_request.force_remove_source_branch?
         .accept-control
           The source branch will be removed.
@@ -39,12 +49,40 @@
       .accept-control
         %button.modify-merge-commit-link.js-toggle-button{ type: "button" }
           = icon('edit')
-          Modify commit message
+          Modify #{@merge_request.commits.size > 1 ? "merge" : "commit"} message
     .js-toggle-content.hide.prepend-top-default
       = render 'shared/commit_message_container', params: params,
           message_with_description: @merge_request.merge_commit_message(include_description: true),
           message_without_description: @merge_request.merge_commit_message,
+          label_text: @merge_request.commits.size > 1 ? "Merge message" : "Commit message",
           text: @merge_request.merge_commit_message,
           rows: 14, hint: true
 
     = hidden_field_tag :merge_when_pipeline_succeeds, "", autocomplete: "off"
+    = hidden_field_tag :merge_when_build_succeeds, "", autocomplete: "off"
+    = hidden_field_tag :apply_patches, ""
+
+  :javascript
+    $('.accept-mr-form').on('ajax:send', function() {
+      $(".accept-mr-form :input").disable();
+    });
+
+    $('.accept_merge_request_apply').on('click', function() {
+      $("#apply_patches").val("on");
+      $('.js-apply-button').html("<i class='fa fa-spinner fa-spin'></i> Apply in progress");
+    });
+
+    $('.accept_merge_request_merge').on('click', function() {
+      $("#apply_patches").val("off");
+      $('.js-merge-button').html("<i class='fa fa-spinner fa-spin'></i> Merge in progress");
+    });
+
+    $('.merge_when_build_succeeds').on('click', function() {
+      $("#merge_when_build_succeeds").val("1");
+    });
+
+    $('.js-merge-dropdown a').on('click', function(e) {
+      e.preventDefault();
+      $(this).closest("form").submit();
+    });
+>>>>>>> b27dc77... NXD Teach GitLab about patches

app/views/shared/_commit_message_container.html.haml

@@ -2,7 +2,7 @@
   - nonce = SecureRandom.hex
   - descriptions = local_assigns.slice(:message_with_description, :message_without_description)
   = label_tag "commit_message-#{nonce}", class: 'control-label' do
-    Commit message
+    = local_assigns[:label_text] || "Commit message"
   .col-sm-10
     .commit-message-container
       .max-width-marker