Commit 3d6ba3b1 authored by Minqi Pan's avatar Minqi Pan

Add support to cherry-pick any commit

Issue: #12785
Merge Request: !3514
parent e9f20f59
......@@ -76,6 +76,7 @@ v 8.7.0 (unreleased)
- Add encrypted credentials for imported projects and migrate old ones
- Author and participants are displayed first on users autocompletion
- Show number sign on external issue reference text (Florent Baldino)
- Add support to cherry-pick any commit into any branch in the web interface (Minqi Pan)
v 8.6.6
- Expire the exists cache before deletion to ensure project dir actually exists (Stan Hu). !3413
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -547,15 +547,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
......@@ -758,6 +761,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]
......@@ -772,6 +797,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
......
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
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
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
#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'))
......@@ -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
......
......@@ -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
......@@ -86,7 +86,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;
......
......@@ -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')
......@@ -549,6 +549,7 @@ Rails.application.routes.draw do
post :cancel_builds
post :retry_builds
post :revert
post :cherry_pick
end
end
......
......@@ -25,6 +25,7 @@ Create merge requests and review code.
- [Automatically close issues from merge requests](../customization/issue_closing.md)
- [Automatically merge when your builds succeed](../workflow/merge_when_build_succeeds.md)
- [Revert any commit](../workflow/revert_changes.md)
- [Cherry-pick any commit](../workflow/cherry_pick_changes.md)
## Test and Deploy
......
......@@ -20,6 +20,7 @@
- [Milestones](milestones.md)
- [Merge Requests](merge_requests.md)
- [Revert changes](revert_changes.md)
- [Cherry-pick changes](cherry_pick_changes.md)
- ["Work In Progress" Merge Requests](wip_merge_requests.md)
- [Merge When Build Succeeds](merge_when_build_succeeds.md)
- [Manage large binaries with Git LFS](lfs/manage_large_binaries_with_git_lfs.md)
......
# Cherry-pick changes
_**Note:** This feature was [introduced][ce-3514] in GitLab 8.7._
---
GitLab implements Git's powerful feature to [cherry-pick any commit][git-cherry-pick]
with introducing a **Cherry-pick** button in Merge Requests and commit details.
## Cherry-picking a Merge Request
After the Merge Request has been merged, a **Cherry-pick** button will be available
to cherry-pick the changes introduced by that Merge Request:
![Cherry-pick Merge Request](img/cherry_pick_changes_mr.png)
---
You can cherry-pick the changes directly into the selected branch or you can opt to
create a new Merge Request with the cherry-pick changes:
![Cherry-pick Merge Request modal](img/cherry_pick_changes_mr_modal.png)
## Cherry-picking a Commit
You can cherry-pick a Commit from the Commit details page:
![Cherry-pick commit](img/cherry_pick_changes_commit.png)
---
Similar to cherry-picking a Merge Request, you can opt to cherry-pick the changes
directly into the target branch or create a new Merge Request to cherry-pick the
changes:
![Cherry-pick commit modal](img/cherry_pick_changes_commit_modal.png)
---
Please note that when cherry-picking merge commits, the mainline will always be the
first parent. If you want to use a different mainline then you need to do that
from the command line.
Here is a quick example to cherry-pick a merge commit using the second parent as the
mainline:
```bash
git cherry-pick -m 2 7a39eb0
```
[ce-3514]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3514 "Cherry-pick button Merge Request"
[git-cherry-pick]: https://git-scm.com/docs/git-cherry-pick "Git cherry-pick documentation"
......@@ -4,6 +4,8 @@ describe Projects::CommitController do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:commit) { project.commit("master") }
let(:master_pickable_sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' }
let(:master_pickable_commit) { project.commit(master_pickable_sha) }
before do
sign_in(user)
......@@ -192,4 +194,53 @@ describe Projects::CommitController do
end
end
end
describe '#cherry_pick' do
context 'when target branch is not provided' do
it 'should render the 404 page' do
post(:cherry_pick,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
id: master_pickable_commit.id)
expect(response).not_to be_success
expect(response.status).to eq(404)
end
end
context 'when the cherry-pick was successful' do
it 'should redirect to the commits page' do
post(:cherry_pick,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
target_branch: 'master',
id: master_pickable_commit.id)
expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master')
expect(flash[:notice]).to eq('The commit has been successfully cherry-picked.')
end
end
context 'when the cherry_pick failed' do
before do
post(:cherry_pick,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
target_branch: 'master',
id: master_pickable_commit.id)
end
it 'should redirect to the commit page' do
# Cherry-picking a commit that has been already cherry-picked.
post(:cherry_pick,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
target_branch: 'master',
id: master_pickable_commit.id)
expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, master_pickable_commit.id)
expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.')
end
end
end
end
require 'spec_helper'
describe 'Cherry-pick Commits' do
let(:project) { create(:project) }
let(:master_pickable_commit) { project.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') }
let(:master_pickable_merge) { project.commit('e56497bb5f03a90a51293fc6d516788730953899') }
before do
login_as :user
project.team << [@user, :master]
visit namespace_project_commits_path(project.namespace, project, project.repository.root_ref, { limit: 5 })
end
context "I cherry-pick a commit" do
it do
visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id)
find("a[href='#modal-cherry-pick-commit']").click
page.within('#modal-cherry-pick-commit') do
uncheck 'create_merge_request'
click_button 'Cherry-pick'
end
expect(page).to have_content('The commit has been successfully cherry-picked.')
end
end
context "I cherry-pick a merge commit" do
it do
visit namespace_project_commit_path(project.namespace, project, master_pickable_merge.id)
find("a[href='#modal-cherry-pick-commit']").click
page.within('#modal-cherry-pick-commit') do
uncheck 'create_merge_request'
click_button 'Cherry-pick'
end
expect(page).to have_content('The commit has been successfully cherry-picked.')
end
end
context "I cherry-pick a commit that was previously cherry-picked" do
it do
visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id)
find("a[href='#modal-cherry-pick-commit']").click
page.within('#modal-cherry-pick-commit') do
uncheck 'create_merge_request'
click_button 'Cherry-pick'
end
visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id)
find("a[href='#modal-cherry-pick-commit']").click
page.within('#modal-cherry-pick-commit') do
uncheck 'create_merge_request'
click_button 'Cherry-pick'
end
expect(page).to have_content('Sorry, we cannot cherry-pick this commit automatically.')
end
end
context "I cherry-pick a commit in a new merge request" do
it do
visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id)
find("a[href='#modal-cherry-pick-commit']").click
page.within('#modal-cherry-pick-commit') do
click_button 'Cherry-pick'
end
expect(page).to have_content('The commit has been successfully cherry-picked. You can now submit a merge request to get this change into the original branch.')
end
end
end
......@@ -541,6 +541,41 @@ describe Repository, models: true do
end
end
describe '#cherry_pick' do
let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') }
let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') }
let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') }
context 'when there is a conflict' do
it 'should abort the operation' do
expect(repository.cherry_pick(user, conflict_commit, 'master')).to eq(false)
end
end
context 'when commit was already cherry-picked' do
it 'should abort the operation' do
repository.cherry_pick(user, pickable_commit, 'master')
expect(repository.cherry_pick(user, pickable_commit, 'master')).to eq(false)
end
end
context 'when commit can be cherry-picked' do
it 'should cherry-pick the changes' do
expect(repository.cherry_pick(user, pickable_commit, 'master')).to be_truthy
end
end
context 'cherry-picking a merge commit' do
it 'should cherry-pick the changes' do
expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).to be_nil
repository.cherry_pick(user, pickable_merge, 'master')
expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).not_to be_nil
end
end
end
describe '#before_delete' do
describe 'when a repository does not exist' do
before do
......
......@@ -42,7 +42,7 @@ Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
eos
)
end
def another_sample_commit
OpenStruct.new(
id: "e56497bb5f03a90a51293fc6d516788730953899",
......