Commit 48307fac authored by Shinya Maeda's avatar Shinya Maeda

Extend MergeToRefService for creating merge ref from the other ref

Currently, MergeToRefService is specifically designed for
createing merge commits from source branch and target branch of
merge reqeusts. We extend this behavior to source branch and any
target ref paths.
parent 9414a41f
...@@ -429,7 +429,7 @@ group :ed25519 do ...@@ -429,7 +429,7 @@ group :ed25519 do
end end
# Gitaly GRPC client # Gitaly GRPC client
gem 'gitaly-proto', '~> 1.32.0', require: 'gitaly' gem 'gitaly-proto', '~> 1.36.0', require: 'gitaly'
gem 'grpc', '~> 1.19.0' gem 'grpc', '~> 1.19.0'
......
...@@ -303,7 +303,7 @@ GEM ...@@ -303,7 +303,7 @@ GEM
gettext_i18n_rails (>= 0.7.1) gettext_i18n_rails (>= 0.7.1)
po_to_json (>= 1.0.0) po_to_json (>= 1.0.0)
rails (>= 3.2.0) rails (>= 3.2.0)
gitaly-proto (1.32.0) gitaly-proto (1.36.0)
grpc (~> 1.0) grpc (~> 1.0)
github-markup (1.7.0) github-markup (1.7.0)
gitlab-labkit (0.3.0) gitlab-labkit (0.3.0)
...@@ -1092,7 +1092,7 @@ DEPENDENCIES ...@@ -1092,7 +1092,7 @@ DEPENDENCIES
gettext (~> 3.2.2) gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3) gettext_i18n_rails_js (~> 1.3)
gitaly-proto (~> 1.32.0) gitaly-proto (~> 1.36.0)
github-markup (~> 1.7.0) github-markup (~> 1.7.0)
gitlab-labkit (~> 0.3.0) gitlab-labkit (~> 0.3.0)
gitlab-markup (~> 1.7.0) gitlab-markup (~> 1.7.0)
......
...@@ -839,10 +839,10 @@ class Repository ...@@ -839,10 +839,10 @@ class Repository
end end
end end
def merge_to_ref(user, source_sha, merge_request, target_ref, message) def merge_to_ref(user, source_sha, merge_request, target_ref, message, first_parent_ref)
branch = merge_request.target_branch branch = merge_request.target_branch
raw.merge_to_ref(user, source_sha, branch, target_ref, message) raw.merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
end end
def ff_merge(user, source, target_branch, merge_request: nil) def ff_merge(user, source, target_branch, merge_request: nil)
......
# frozen_string_literal: true # frozen_string_literal: true
module MergeRequests module MergeRequests
# Performs the merge between source SHA and the target branch. Instead # Performs the merge between source SHA and the target branch or the specified first parent ref. Instead
# of writing the result to the MR target branch, it targets the `target_ref`. # of writing the result to the MR target branch, it targets the `target_ref`.
# #
# Ideally this should leave the `target_ref` state with the same state the # Ideally this should leave the `target_ref` state with the same state the
...@@ -56,12 +56,22 @@ module MergeRequests ...@@ -56,12 +56,22 @@ module MergeRequests
raise_error(error) if error raise_error(error) if error
end end
##
# The parameter `target_ref` is where the merge result will be written.
# Default is the merge ref i.e. `refs/merge-requests/:iid/merge`.
def target_ref def target_ref
merge_request.merge_ref_path params[:target_ref] || merge_request.merge_ref_path
end
##
# The parameter `first_parent_ref` is the main line of the merge commit.
# Default is the target branch ref of the merge request.
def first_parent_ref
params[:first_parent_ref] || merge_request.target_branch_ref
end end
def commit def commit
repository.merge_to_ref(current_user, source, merge_request, target_ref, commit_message) repository.merge_to_ref(current_user, source, merge_request, target_ref, commit_message, first_parent_ref)
rescue Gitlab::Git::PreReceiveError => error rescue Gitlab::Git::PreReceiveError => error
raise MergeError, error.message raise MergeError, error.message
end end
......
---
title: Extend `MergeToRefService` to create merge ref from an arbitrary ref
merge_request: 30361
author:
type: added
...@@ -536,9 +536,9 @@ module Gitlab ...@@ -536,9 +536,9 @@ module Gitlab
tags.find { |tag| tag.name == name } tags.find { |tag| tag.name == name }
end end
def merge_to_ref(user, source_sha, branch, target_ref, message) def merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
wrapped_gitaly_errors do wrapped_gitaly_errors do
gitaly_operation_client.user_merge_to_ref(user, source_sha, branch, target_ref, message) gitaly_operation_client.user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
end end
end end
......
...@@ -100,14 +100,15 @@ module Gitlab ...@@ -100,14 +100,15 @@ module Gitlab
end end
end end
def user_merge_to_ref(user, source_sha, branch, target_ref, message) def user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
request = Gitaly::UserMergeToRefRequest.new( request = Gitaly::UserMergeToRefRequest.new(
repository: @gitaly_repo, repository: @gitaly_repo,
source_sha: source_sha, source_sha: source_sha,
branch: encode_binary(branch), branch: encode_binary(branch),
target_ref: encode_binary(target_ref), target_ref: encode_binary(target_ref),
user: Gitlab::Git::User.from_gitlab(user).to_gitaly, user: Gitlab::Git::User.from_gitlab(user).to_gitaly,
message: encode_binary(message) message: encode_binary(message),
first_parent_ref: encode_binary(first_parent_ref)
) )
response = GitalyClient.call(@repository.storage, :operation_service, :user_merge_to_ref, request) response = GitalyClient.call(@repository.storage, :operation_service, :user_merge_to_ref, request)
......
...@@ -1694,14 +1694,15 @@ describe Gitlab::Git::Repository, :seed_helper do ...@@ -1694,14 +1694,15 @@ describe Gitlab::Git::Repository, :seed_helper do
let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' } let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' }
let(:left_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:left_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' }
let(:right_branch) { 'test-master' } let(:right_branch) { 'test-master' }
let(:first_parent_ref) { 'refs/heads/test-master' }
let(:target_ref) { 'refs/merge-requests/999/merge' } let(:target_ref) { 'refs/merge-requests/999/merge' }
before do before do
repository.create_branch(right_branch, branch_head) unless repository.branch_exists?(right_branch) repository.create_branch(right_branch, branch_head) unless repository.ref_exists?(first_parent_ref)
end end
def merge_to_ref def merge_to_ref
repository.merge_to_ref(user, left_sha, right_branch, target_ref, 'Merge message') repository.merge_to_ref(user, left_sha, right_branch, target_ref, 'Merge message', first_parent_ref)
end end
it 'generates a commit in the target_ref' do it 'generates a commit in the target_ref' do
...@@ -1716,7 +1717,7 @@ describe Gitlab::Git::Repository, :seed_helper do ...@@ -1716,7 +1717,7 @@ describe Gitlab::Git::Repository, :seed_helper do
end end
it 'does not change the right branch HEAD' do it 'does not change the right branch HEAD' do
expect { merge_to_ref }.not_to change { repository.find_branch(right_branch).target } expect { merge_to_ref }.not_to change { repository.commit(first_parent_ref).sha }
end end
end end
......
...@@ -79,13 +79,13 @@ describe Gitlab::GitalyClient::OperationService do ...@@ -79,13 +79,13 @@ describe Gitlab::GitalyClient::OperationService do
end end
describe '#user_merge_to_ref' do describe '#user_merge_to_ref' do
let(:branch) { 'my-branch' } let(:first_parent_ref) { 'refs/heads/my-branch' }
let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' }
let(:ref) { 'refs/merge-requests/x/merge' } let(:ref) { 'refs/merge-requests/x/merge' }
let(:message) { 'validación' } let(:message) { 'validación' }
let(:response) { Gitaly::UserMergeToRefResponse.new(commit_id: 'new-commit-id') } let(:response) { Gitaly::UserMergeToRefResponse.new(commit_id: 'new-commit-id') }
subject { client.user_merge_to_ref(user, source_sha, branch, ref, message) } subject { client.user_merge_to_ref(user, source_sha, nil, ref, message, first_parent_ref) }
it 'sends a user_merge_to_ref message' do it 'sends a user_merge_to_ref message' do
expect_any_instance_of(Gitaly::OperationService::Stub) expect_any_instance_of(Gitaly::OperationService::Stub)
......
...@@ -1420,12 +1420,13 @@ describe Repository do ...@@ -1420,12 +1420,13 @@ describe Repository do
source_project: project) source_project: project)
end end
it 'writes merge of source and target to MR merge_ref_path' do it 'writes merge of source SHA and first parent ref to MR merge_ref_path' do
merge_commit_id = repository.merge_to_ref(user, merge_commit_id = repository.merge_to_ref(user,
merge_request.diff_head_sha, merge_request.diff_head_sha,
merge_request, merge_request,
merge_request.merge_ref_path, merge_request.merge_ref_path,
'Custom message') 'Custom message',
merge_request.target_branch_ref)
merge_commit = repository.commit(merge_commit_id) merge_commit = repository.commit(merge_commit_id)
......
...@@ -22,7 +22,6 @@ describe MergeRequests::MergeToRefService do ...@@ -22,7 +22,6 @@ describe MergeRequests::MergeToRefService do
shared_examples_for 'successfully merges to ref with merge method' do shared_examples_for 'successfully merges to ref with merge method' do
it 'writes commit to merge ref' do it 'writes commit to merge ref' do
repository = project.repository repository = project.repository
target_ref = merge_request.merge_ref_path
expect(repository.ref_exists?(target_ref)).to be(false) expect(repository.ref_exists?(target_ref)).to be(false)
...@@ -33,7 +32,7 @@ describe MergeRequests::MergeToRefService do ...@@ -33,7 +32,7 @@ describe MergeRequests::MergeToRefService do
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
expect(result[:commit_id]).to be_present expect(result[:commit_id]).to be_present
expect(result[:source_id]).to eq(merge_request.source_branch_sha) expect(result[:source_id]).to eq(merge_request.source_branch_sha)
expect(result[:target_id]).to eq(merge_request.target_branch_sha) expect(result[:target_id]).to eq(repository.commit(first_parent_ref).sha)
expect(repository.ref_exists?(target_ref)).to be(true) expect(repository.ref_exists?(target_ref)).to be(true)
expect(ref_head.id).to eq(result[:commit_id]) expect(ref_head.id).to eq(result[:commit_id])
end end
...@@ -74,17 +73,22 @@ describe MergeRequests::MergeToRefService do ...@@ -74,17 +73,22 @@ describe MergeRequests::MergeToRefService do
describe '#execute' do describe '#execute' do
let(:service) do let(:service) do
described_class.new(project, user, commit_message: 'Awesome message', described_class.new(project, user, **params)
should_remove_source_branch: true)
end end
let(:params) { { commit_message: 'Awesome message', should_remove_source_branch: true } }
def process_merge_to_ref def process_merge_to_ref
perform_enqueued_jobs do perform_enqueued_jobs do
service.execute(merge_request) service.execute(merge_request)
end end
end end
it_behaves_like 'successfully merges to ref with merge method' it_behaves_like 'successfully merges to ref with merge method' do
let(:first_parent_ref) { 'refs/heads/master' }
let(:target_ref) { merge_request.merge_ref_path }
end
it_behaves_like 'successfully evaluates pre-condition checks' it_behaves_like 'successfully evaluates pre-condition checks'
context 'commit history comparison with regular MergeService' do context 'commit history comparison with regular MergeService' do
...@@ -129,14 +133,22 @@ describe MergeRequests::MergeToRefService do ...@@ -129,14 +133,22 @@ describe MergeRequests::MergeToRefService do
context 'when semi-linear merge method' do context 'when semi-linear merge method' do
let(:merge_method) { :rebase_merge } let(:merge_method) { :rebase_merge }
it_behaves_like 'successfully merges to ref with merge method' it_behaves_like 'successfully merges to ref with merge method' do
let(:first_parent_ref) { 'refs/heads/master' }
let(:target_ref) { merge_request.merge_ref_path }
end
it_behaves_like 'successfully evaluates pre-condition checks' it_behaves_like 'successfully evaluates pre-condition checks'
end end
context 'when fast-forward merge method' do context 'when fast-forward merge method' do
let(:merge_method) { :ff } let(:merge_method) { :ff }
it_behaves_like 'successfully merges to ref with merge method' it_behaves_like 'successfully merges to ref with merge method' do
let(:first_parent_ref) { 'refs/heads/master' }
let(:target_ref) { merge_request.merge_ref_path }
end
it_behaves_like 'successfully evaluates pre-condition checks' it_behaves_like 'successfully evaluates pre-condition checks'
end end
...@@ -178,5 +190,34 @@ describe MergeRequests::MergeToRefService do ...@@ -178,5 +190,34 @@ describe MergeRequests::MergeToRefService do
it { expect(todo).not_to be_done } it { expect(todo).not_to be_done }
end end
describe 'cascading merge refs' do
set(:project) { create(:project, :repository) }
let(:params) { { commit_message: 'Cascading merge', first_parent_ref: first_parent_ref, target_ref: target_ref } }
context 'when first merge happens' do
let(:merge_request) do
create(:merge_request, source_project: project, source_branch: 'feature',
target_project: project, target_branch: 'master')
end
it_behaves_like 'successfully merges to ref with merge method' do
let(:first_parent_ref) { 'refs/heads/master' }
let(:target_ref) { 'refs/merge-requests/1/train' }
end
context 'when second merge happens' do
let(:merge_request) do
create(:merge_request, source_project: project, source_branch: 'improve/awesome',
target_project: project, target_branch: 'master')
end
it_behaves_like 'successfully merges to ref with merge method' do
let(:first_parent_ref) { 'refs/merge-requests/1/train' }
let(:target_ref) { 'refs/merge-requests/2/train' }
end
end
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment