diff options
author | Shinya Maeda <shinya@gitlab.com> | 2019-06-20 19:45:46 +0700 |
---|---|---|
committer | Shinya Maeda <shinya@gitlab.com> | 2019-07-05 17:15:10 +0700 |
commit | 48307fac1ec7cd207fbd53762fd1226a9d6fb1a2 (patch) | |
tree | 92b3d9b65dd6e541dd09acda541e5c291448fb09 | |
parent | 9414a41f8390511005702ab4fec99239b6c3c6dd (diff) | |
download | gitlab-ce-48307fac1ec7cd207fbd53762fd1226a9d6fb1a2.tar.gz |
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.
-rw-r--r-- | Gemfile | 2 | ||||
-rw-r--r-- | Gemfile.lock | 4 | ||||
-rw-r--r-- | app/models/repository.rb | 4 | ||||
-rw-r--r-- | app/services/merge_requests/merge_to_ref_service.rb | 16 | ||||
-rw-r--r-- | changelogs/unreleased/create-merge-train-ref-ce.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/git/repository.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/operation_service.rb | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 7 | ||||
-rw-r--r-- | spec/lib/gitlab/gitaly_client/operation_service_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 5 | ||||
-rw-r--r-- | spec/services/merge_requests/merge_to_ref_service_spec.rb | 55 |
11 files changed, 85 insertions, 26 deletions
@@ -429,7 +429,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 1.32.0', require: 'gitaly' +gem 'gitaly-proto', '~> 1.36.0', require: 'gitaly' gem 'grpc', '~> 1.19.0' diff --git a/Gemfile.lock b/Gemfile.lock index 5b648d43137..5099167e03f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -303,7 +303,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (1.32.0) + gitaly-proto (1.36.0) grpc (~> 1.0) github-markup (1.7.0) gitlab-labkit (0.3.0) @@ -1092,7 +1092,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 1.32.0) + gitaly-proto (~> 1.36.0) github-markup (~> 1.7.0) gitlab-labkit (~> 0.3.0) gitlab-markup (~> 1.7.0) diff --git a/app/models/repository.rb b/app/models/repository.rb index d087a5a7bbd..a408db7ebbe 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -839,10 +839,10 @@ class Repository 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 - 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 def ff_merge(user, source, target_branch, merge_request: nil) diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index efe4dcd6255..0ea50a5dbf5 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true 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`. # # Ideally this should leave the `target_ref` state with the same state the @@ -56,12 +56,22 @@ module MergeRequests raise_error(error) if error 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 - 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 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 raise MergeError, error.message end diff --git a/changelogs/unreleased/create-merge-train-ref-ce.yml b/changelogs/unreleased/create-merge-train-ref-ce.yml new file mode 100644 index 00000000000..b0b95275f58 --- /dev/null +++ b/changelogs/unreleased/create-merge-train-ref-ce.yml @@ -0,0 +1,5 @@ +--- +title: Extend `MergeToRefService` to create merge ref from an arbitrary ref +merge_request: 30361 +author: +type: added diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 19b6aab1c4f..060a29be782 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -536,9 +536,9 @@ module Gitlab tags.find { |tag| tag.name == name } 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 - 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 diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index b42e6cbad8d..783c2ff0915 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -100,14 +100,15 @@ module Gitlab 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( repository: @gitaly_repo, source_sha: source_sha, branch: encode_binary(branch), target_ref: encode_binary(target_ref), 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) diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index cceeae8afe6..a28b95e5bff 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1694,14 +1694,15 @@ describe Gitlab::Git::Repository, :seed_helper do let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' } let(:left_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:right_branch) { 'test-master' } + let(:first_parent_ref) { 'refs/heads/test-master' } let(:target_ref) { 'refs/merge-requests/999/merge' } 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 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 it 'generates a commit in the target_ref' do @@ -1716,7 +1717,7 @@ describe Gitlab::Git::Repository, :seed_helper do end 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 diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 18663a72fcd..f38b8d31237 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -79,13 +79,13 @@ describe Gitlab::GitalyClient::OperationService do end describe '#user_merge_to_ref' do - let(:branch) { 'my-branch' } + let(:first_parent_ref) { 'refs/heads/my-branch' } let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:ref) { 'refs/merge-requests/x/merge' } let(:message) { 'validaciĆ³n' } 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 expect_any_instance_of(Gitaly::OperationService::Stub) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 13da7bd7407..3d967aa4ab8 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1420,12 +1420,13 @@ describe Repository do source_project: project) 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_request.diff_head_sha, merge_request, merge_request.merge_ref_path, - 'Custom message') + 'Custom message', + merge_request.target_branch_ref) merge_commit = repository.commit(merge_commit_id) diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index 61f99f82a76..e2f201677fa 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -22,7 +22,6 @@ describe MergeRequests::MergeToRefService do shared_examples_for 'successfully merges to ref with merge method' do it 'writes commit to merge ref' do repository = project.repository - target_ref = merge_request.merge_ref_path expect(repository.ref_exists?(target_ref)).to be(false) @@ -33,7 +32,7 @@ describe MergeRequests::MergeToRefService do expect(result[:status]).to eq(:success) expect(result[:commit_id]).to be_present 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(ref_head.id).to eq(result[:commit_id]) end @@ -74,17 +73,22 @@ describe MergeRequests::MergeToRefService do describe '#execute' do let(:service) do - described_class.new(project, user, commit_message: 'Awesome message', - should_remove_source_branch: true) + described_class.new(project, user, **params) end + let(:params) { { commit_message: 'Awesome message', should_remove_source_branch: true } } + def process_merge_to_ref perform_enqueued_jobs do service.execute(merge_request) 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' context 'commit history comparison with regular MergeService' do @@ -129,14 +133,22 @@ describe MergeRequests::MergeToRefService do context 'when semi-linear merge method' do 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' end context 'when fast-forward merge method' do 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' end @@ -178,5 +190,34 @@ describe MergeRequests::MergeToRefService do it { expect(todo).not_to be_done } 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 |