summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2019-06-20 19:45:46 +0700
committerShinya Maeda <shinya@gitlab.com>2019-07-05 15:17:23 +0700
commitf6ea904a1cb99410fb99e59de769e7f062e1c4f6 (patch)
tree6d232bd534e35e54bcc6540c0a08a9f4dbc4cb99
parent9414a41f8390511005702ab4fec99239b6c3c6dd (diff)
downloadgitlab-ce-create-merge-train-ref-ce.tar.gz
Extend MergeToRefService for creating merge ref from the other refcreate-merge-train-ref-ce
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--Gemfile2
-rw-r--r--Gemfile.lock4
-rw-r--r--app/models/repository.rb4
-rw-r--r--app/services/merge_requests/merge_to_ref_service.rb16
-rw-r--r--changelogs/unreleased/create-merge-train-ref-ce.yml5
-rw-r--r--lib/gitlab/git/repository.rb4
-rw-r--r--lib/gitlab/gitaly_client/operation_service.rb5
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb7
-rw-r--r--spec/lib/gitlab/gitaly_client/operation_service_spec.rb4
-rw-r--r--spec/models/repository_spec.rb5
-rw-r--r--spec/services/merge_requests/merge_to_ref_service_spec.rb55
11 files changed, 85 insertions, 26 deletions
diff --git a/Gemfile b/Gemfile
index 1264d75eac6..b1750afec91 100644
--- a/Gemfile
+++ b/Gemfile
@@ -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..de2943b6782
--- /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: 14298
+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