Merge branch 'pmq20/gitlab-ce-issue_12785'

13 files changed, 180 insertions, 79 deletions

diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index 576fa3cedb2..4d64a2d9884 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -12,7 +12,7 @@ class Projects::CommitController < Projects::ApplicationController
   before_action :authorize_read_commit_status!, only: [:builds]
   before_action :commit
   before_action :define_show_vars, only: [:show, :builds]
-  before_action :authorize_edit_tree!, only: [:revert]
+  before_action :authorize_edit_tree!, only: [:revert, :cherry_pick]
 
   def show
     apply_diff_view_cookie!
@@ -60,27 +60,32 @@ class Projects::CommitController < Projects::ApplicationController
   end
 
   def revert
-    assign_revert_commit_vars
+    assign_change_commit_vars(@commit.revert_branch_name)
 
     return render_404 if @target_branch.blank?
 
-    create_commit(Commits::RevertService, success_notice: "The #{revert_type_title} has been successfully reverted.",
-                                          success_path: successful_revert_path, failure_path: failed_revert_path)
+    create_commit(Commits::RevertService, success_notice: "The #{@commit.change_type_title} has been successfully reverted.",
+                                          success_path: successful_change_path, failure_path: failed_change_path)
   end
+  
+  def cherry_pick
+    assign_change_commit_vars(@commit.cherry_pick_branch_name)
+    
+    return render_404 if @target_branch.blank?
 
-  private
-
-  def revert_type_title
-    @commit.merged_merge_request ? 'merge request' : 'commit'
+    create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title} has been successfully cherry-picked.",
+                                              success_path: successful_change_path, failure_path: failed_change_path)
   end
 
-  def successful_revert_path
+  private
+
+  def successful_change_path
     return referenced_merge_request_url if @commit.merged_merge_request
 
     namespace_project_commits_url(@project.namespace, @project, @target_branch)
   end
 
-  def failed_revert_path
+  def failed_change_path
     return referenced_merge_request_url if @commit.merged_merge_request
 
     namespace_project_commit_url(@project.namespace, @project, params[:id])
@@ -111,14 +116,13 @@ class Projects::CommitController < Projects::ApplicationController
     @statuses = ci_commit.statuses if ci_commit
   end
 
-  def assign_revert_commit_vars
+  def assign_change_commit_vars(mr_source_branch)
     @commit = project.commit(params[:id])
     @target_branch = params[:target_branch]
-    @mr_source_branch = @commit.revert_branch_name
+    @mr_source_branch = mr_source_branch
     @mr_target_branch = @target_branch
     @commit_params = {
       commit: @commit,
-      revert_type_title: revert_type_title,
       create_merge_request: params[:create_merge_request].present? || different_project?
     }
   end
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 5394347bd15..b59c3982edd 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -126,12 +126,10 @@ module CommitsHelper
   def revert_commit_link(commit, continue_to_path, btn_class: nil)
     return unless current_user
 
-    tooltip = "Revert this #{revert_commit_type(commit)} in a new merge request"
+    tooltip = "Revert this #{commit.change_type_title} in a new merge request"
 
     if can_collaborate_with_project?
-      content_tag :span, 'data-toggle' => 'modal', 'data-target' => '#modal-revert-commit' do
-        link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip', 'data-container' => 'body', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class}"
-      end
+      link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'modal', 'data-container' => 'body', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class} has-tooltip"
     elsif can?(current_user, :fork_project, @project)
       continue_params = {
         to: continue_to_path,
@@ -146,11 +144,24 @@ module CommitsHelper
     end
   end
 
-  def revert_commit_type(commit)
-    if commit.merged_merge_request
-      'merge request'
-    else
-      'commit'
+  def cherry_pick_commit_link(commit, continue_to_path, btn_class: nil)
+    return unless current_user
+
+    tooltip = "Cherry-pick this #{commit.change_type_title} in a new merge request"
+
+    if can_collaborate_with_project?
+      link_to 'Cherry-pick', '#modal-cherry-pick-commit', 'data-toggle' => 'modal', 'data-container' => 'body', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class} has-tooltip"
+    elsif can?(current_user, :fork_project, @project)
+      continue_params = {
+        to: continue_to_path,
+        notice: edit_in_new_fork_notice + ' Try to cherry-pick this commit again.',
+        notice_now: edit_in_new_fork_notice_now
+      }
+      fork_path = namespace_project_forks_path(@project.namespace, @project,
+        namespace_key: current_user.namespace.id,
+        continue: continue_params)
+
+      link_to 'Cherry-pick', fork_path, class: 'btn btn-grouped btn-close', method: :post, 'data-toggle' => 'tooltip', 'data-container' => 'body', title: tooltip
     end
   end
 
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index 4920ca5af6e..dbedf417fa5 100644
--- a/app/helpers/tree_helper.rb
+++ b/app/helpers/tree_helper.rb
@@ -66,7 +66,7 @@ module TreeHelper
       ref
     else
       project = tree_edit_project(project)
-      project.repository.next_patch_branch
+      project.repository.next_branch('patch')
     end
   end
 
diff --git a/app/models/commit.rb b/app/models/commit.rb
index d1f07ccd55c..b406a4dd8d2 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -218,6 +218,10 @@ class Commit
   def revert_branch_name
     "revert-#{short_id}"
   end
+  
+  def cherry_pick_branch_name
+    project.repository.next_branch("cherry-pick-#{short_id}", mild: true)
+  end
 
   def revert_description
     if merged_merge_request
@@ -253,6 +257,10 @@ class Commit
     end.any? { |commit_ref| commit_ref.reverts_commit?(self) }
   end
 
+  def change_type_title
+    merged_merge_request ? 'merge request' : 'commit'
+  end
+
   private
 
   def repo_changes
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 589756f8531..c6d7a439c05 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -549,15 +549,18 @@ class Repository
     commit(sha)
   end
 
-  def next_patch_branch
-    patch_branch_ids = self.branch_names.map do |n|
-      result = n.match(/\Apatch-([0-9]+)\z/)
+  def next_branch(name, opts={})
+    branch_ids = self.branch_names.map do |n|
+      next 1 if n == name
+      result = n.match(/\A#{name}-([0-9]+)\z/)
       result[1].to_i if result
     end.compact
 
-    highest_patch_branch_id = patch_branch_ids.max || 0
+    highest_branch_id = branch_ids.max || 0
 
-    "patch-#{highest_patch_branch_id + 1}"
+    return name if opts[:mild] && 0 == highest_branch_id
+
+    "#{name}-#{highest_branch_id + 1}"
   end
 
   # Remove archives older than 2 hours
@@ -760,6 +763,28 @@ class Repository
     end
   end
 
+  def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil)
+    source_sha = find_branch(base_branch).target
+    cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch)
+
+    return false unless cherry_pick_tree_id
+
+    commit_with_hooks(user, base_branch) do |ref|
+      committer = user_to_committer(user)
+      source_sha = Rugged::Commit.create(rugged,
+        message: commit.message,
+        author: {
+          email: commit.author_email,
+          name: commit.author_name,
+          time: commit.authored_date
+        },
+        committer: committer,
+        tree: cherry_pick_tree_id,
+        parents: [rugged.lookup(source_sha)],
+        update_ref: ref)
+    end
+  end
+
   def check_revert_content(commit, base_branch)
     source_sha = find_branch(base_branch).target
     args       = [commit.id, source_sha]
@@ -774,6 +799,20 @@ class Repository
     tree_id
   end
 
+  def check_cherry_pick_content(commit, base_branch)
+    source_sha = find_branch(base_branch).target
+    args       = [commit.id, source_sha]
+    args       << 1 if commit.merge_commit?
+
+    cherry_pick_index = rugged.cherrypick_commit(*args)
+    return false if cherry_pick_index.conflicts?
+
+    tree_id = cherry_pick_index.write_tree(rugged)
+    return false unless diff_exists?(source_sha, tree_id)
+
+    tree_id
+  end
+
   def diff_exists?(sha1, sha2)
     rugged.diff(sha1, sha2).size > 0
   end
diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
new file mode 100644
index 00000000000..6b69cb53b2c
--- /dev/null
+++ b/app/services/commits/change_service.rb
@@ -0,0 +1,47 @@
+module Commits
+  class ChangeService < ::BaseService
+    class ValidationError < StandardError; end
+    class ChangeError < StandardError; end
+
+    def execute
+      @source_project = params[:source_project] || @project
+      @target_branch = params[:target_branch]
+      @commit = params[:commit]
+      @create_merge_request = params[:create_merge_request].present?
+
+      check_push_permissions unless @create_merge_request
+      commit
+    rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError,
+           ValidationError, ChangeError => ex
+      error(ex.message)
+    end
+
+    def commit
+      raise NotImplementedError
+    end
+
+    private
+
+    def check_push_permissions
+      allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch)
+
+      unless allowed
+        raise ValidationError.new('You are not allowed to push into this branch')
+      end
+
+      true
+    end
+    
+    def create_target_branch(new_branch)
+      # Temporary branch exists and contains the change commit
+      return success if repository.find_branch(new_branch)
+
+      result = CreateBranchService.new(@project, current_user)
+                                  .execute(new_branch, @target_branch, source_project: @source_project)
+
+      if result[:status] == :error
+        raise ChangeError, "There was an error creating the source branch: #{result[:message]}"
+      end
+    end
+  end
+end
diff --git a/app/services/commits/cherry_pick_service.rb b/app/services/commits/cherry_pick_service.rb
new file mode 100644
index 00000000000..f9a4efa7182
--- /dev/null
+++ b/app/services/commits/cherry_pick_service.rb
@@ -0,0 +1,19 @@
+module Commits
+  class CherryPickService < ChangeService
+    def commit
+      cherry_pick_into = @create_merge_request ? @commit.cherry_pick_branch_name : @target_branch
+      cherry_pick_tree_id = repository.check_cherry_pick_content(@commit, @target_branch)
+
+      if cherry_pick_tree_id
+        create_target_branch(cherry_pick_into) if @create_merge_request
+
+        repository.cherry_pick(current_user, @commit, cherry_pick_into, cherry_pick_tree_id)
+        success
+      else
+        error_msg = "Sorry, we cannot cherry-pick this #{@commit.change_type_title} automatically.
+                     It may have already been cherry-picked, or a more recent commit may have updated some of its content."
+        raise ChangeError, error_msg
+      end
+    end
+  end
+end
diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb
index a3c950ede1f..c7de9f6f35e 100644
--- a/app/services/commits/revert_service.rb
+++ b/app/services/commits/revert_service.rb
@@ -1,21 +1,5 @@
 module Commits
-  class RevertService < ::BaseService
-    class ValidationError < StandardError; end
-    class ReversionError < StandardError; end
-
-    def execute
-      @source_project = params[:source_project] || @project
-      @target_branch = params[:target_branch]
-      @commit = params[:commit]
-      @create_merge_request = params[:create_merge_request].present?
-
-      check_push_permissions unless @create_merge_request
-      commit
-    rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError,
-           ValidationError, ReversionError => ex
-      error(ex.message)
-    end
-
+  class RevertService < ChangeService
     def commit
       revert_into = @create_merge_request ? @commit.revert_branch_name : @target_branch
       revert_tree_id = repository.check_revert_content(@commit, @target_branch)
@@ -26,34 +10,10 @@ module Commits
         repository.revert(current_user, @commit, revert_into, revert_tree_id)
         success
       else
-        error_msg = "Sorry, we cannot revert this #{params[:revert_type_title]} automatically.
+        error_msg = "Sorry, we cannot revert this #{@commit.change_type_title} automatically.
                      It may have already been reverted, or a more recent commit may have updated some of its content."
-        raise ReversionError, error_msg
+        raise ChangeError, error_msg
       end
     end
-
-    private
-
-    def create_target_branch(new_branch)
-      # Temporary branch exists and contains the revert commit
-      return success if repository.find_branch(new_branch)
-
-      result = CreateBranchService.new(@project, current_user)
-                                  .execute(new_branch, @target_branch, source_project: @source_project)
-
-      if result[:status] == :error
-        raise ReversionError, "There was an error creating the source branch: #{result[:message]}"
-      end
-    end
-
-    def check_push_permissions
-      allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch)
-
-      unless allowed
-        raise ValidationError.new('You are not allowed to push into this branch')
-      end
-
-      true
-    end
   end
 end
diff --git a/app/views/projects/commit/_revert.html.haml b/app/views/projects/commit/_change.html.haml
index 52ca3ed5b14..44ef1fdbbe3 100644
--- a/app/views/projects/commit/_revert.html.haml
+++ b/app/views/projects/commit/_change.html.haml
@@ -1,13 +1,21 @@
-#modal-revert-commit.modal
+- case type.to_s
+- when 'revert'
+  - label = 'Revert'
+  - target_label = 'Revert in branch'
+- when 'cherry-pick'
+  - label = 'Cherry-pick'
+  - target_label = 'Pick into branch'
+
+.modal{id: "modal-#{type}-commit"}
   .modal-dialog
     .modal-content
       .modal-header
         %a.close{href: "#", "data-dismiss" => "modal"} ×
-        %h3.page-title== Revert this #{revert_commit_type(commit)}
+        %h3.page-title== #{label} this #{commit.change_type_title}
       .modal-body
-        = form_tag revert_namespace_project_commit_path(@project.namespace, @project, commit.id), method: :post, remote: false, class: 'form-horizontal js-create-dir-form js-requires-input' do
+        = form_tag send("#{type.underscore}_namespace_project_commit_path", @project.namespace, @project, commit.id), method: :post, remote: false, class: 'form-horizontal js-#{type}-form js-requires-input' do
           .form-group.branch
-            = label_tag 'target_branch', 'Revert in branch', class: 'control-label'
+            = label_tag 'target_branch', target_label, class: 'control-label'
             .col-sm-10
               = select_tag "target_branch", grouped_options_refs, class: "select2 select2-sm js-target-branch"
               - if can?(current_user, :push_code, @project)
@@ -20,7 +28,7 @@
               - else
                 = hidden_field_tag 'create_merge_request', 1
           .form-actions
-            = submit_tag "Revert", class: 'btn btn-create'
+            = submit_tag label, class: 'btn btn-create'
             = link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal"
 
             - unless can?(current_user, :push_code, @project)
@@ -28,4 +36,4 @@
                 = commit_in_fork_help
 
 :javascript
-  new NewCommitForm($('.js-create-dir-form'))
+  new NewCommitForm($('.js-#{type}-form'))
diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml
index 71995fcc487..d6c9e54e657 100644
--- a/app/views/projects/commit/_commit_box.html.haml
+++ b/app/views/projects/commit/_commit_box.html.haml
@@ -18,6 +18,7 @@
       Browse Files
     - unless @commit.has_been_reverted?(current_user)
       = revert_commit_link(@commit, namespace_project_commit_path(@project.namespace, @project, @commit.id))
+    = cherry_pick_commit_link(@commit, namespace_project_commit_path(@project.namespace, @project, @commit.id))
   %div
 
 %p
diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml
index 21e186120c3..e550af7888a 100644
--- a/app/views/projects/commit/show.html.haml
+++ b/app/views/projects/commit/show.html.haml
@@ -13,4 +13,5 @@
          diff_refs: @diff_refs
 = render "projects/notes/notes_with_form"
 - if can_collaborate_with_project?
-  = render "projects/commit/revert", commit: @commit, title: @commit.title
+  - %w(revert cherry-pick).each do |type|
+    = render "projects/commit/change", type: type, commit: @commit, title: @commit.title
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index 7125d7d9d1c..65da3712a24 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -87,7 +87,9 @@
 
 = render 'shared/issuable/sidebar', issuable: @merge_request
 - if @merge_request.can_be_reverted?
-  = render "projects/commit/revert", commit: @merge_request.merge_commit, title: @merge_request.title
+  = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit, title: @merge_request.title
+- if @merge_request.merge_commit
+  = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit, title: @merge_request.title
 
 :javascript
   var merge_request;
diff --git a/app/views/projects/merge_requests/widget/_merged_buttons.haml b/app/views/projects/merge_requests/widget/_merged_buttons.haml
index 85a3a6ba9e2..361acf7d27f 100644
--- a/app/views/projects/merge_requests/widget/_merged_buttons.haml
+++ b/app/views/projects/merge_requests/widget/_merged_buttons.haml
@@ -9,3 +9,4 @@
         Remove Source Branch
     - if mr_can_be_reverted
       = revert_commit_link(@merge_request.merge_commit, namespace_project_merge_request_path(@project.namespace, @project, @merge_request), btn_class: 'sm')
+    = cherry_pick_commit_link(@merge_request.merge_commit, namespace_project_merge_request_path(@project.namespace, @project, @merge_request), btn_class: 'sm')