Commit 1ad69967 authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Support merge to ref for merge-commit and squash

Adds the ground work for writing into
the merge ref refs/merge-requests/:iid/merge the
merge result between source and target branches of
a MR, without further side-effects such as
mailing, MR updates and target branch changes.
parent 99218353
...@@ -419,7 +419,8 @@ group :ed25519 do ...@@ -419,7 +419,8 @@ group :ed25519 do
end end
# Gitaly GRPC client # Gitaly GRPC client
gem 'gitaly-proto', '~> 1.10.0', require: 'gitaly' gem 'gitaly-proto', '~> 1.12.0', require: 'gitaly'
gem 'grpc', '~> 1.15.0' gem 'grpc', '~> 1.15.0'
gem 'google-protobuf', '~> 3.6' gem 'google-protobuf', '~> 3.6'
......
...@@ -278,7 +278,7 @@ GEM ...@@ -278,7 +278,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.10.0) gitaly-proto (1.12.0)
grpc (~> 1.0) grpc (~> 1.0)
github-markup (1.7.0) github-markup (1.7.0)
gitlab-default_value_for (3.1.1) gitlab-default_value_for (3.1.1)
...@@ -1017,7 +1017,7 @@ DEPENDENCIES ...@@ -1017,7 +1017,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.10.0) gitaly-proto (~> 1.12.0)
github-markup (~> 1.7.0) github-markup (~> 1.7.0)
gitlab-default_value_for (~> 3.1.1) gitlab-default_value_for (~> 3.1.1)
gitlab-markup (~> 1.6.5) gitlab-markup (~> 1.6.5)
......
...@@ -764,6 +764,16 @@ class MergeRequest < ActiveRecord::Base ...@@ -764,6 +764,16 @@ class MergeRequest < ActiveRecord::Base
true true
end end
def mergeable_to_ref?
return false if merged?
return false if broken?
# Given the `merge_ref_path` will have the same
# state the `target_branch` would have. Ideally
# we need to check if it can be merged to it.
project.repository.can_be_merged?(diff_head_sha, target_branch)
end
def ff_merge_possible? def ff_merge_possible?
project.repository.ancestor?(target_branch_sha, diff_head_sha) project.repository.ancestor?(target_branch_sha, diff_head_sha)
end end
...@@ -1077,6 +1087,10 @@ class MergeRequest < ActiveRecord::Base ...@@ -1077,6 +1087,10 @@ class MergeRequest < ActiveRecord::Base
"refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/head" "refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/head"
end end
def merge_ref_path
"refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/merge"
end
def in_locked_state def in_locked_state
begin begin
lock_mr lock_mr
......
...@@ -1925,6 +1925,14 @@ class Project < ActiveRecord::Base ...@@ -1925,6 +1925,14 @@ class Project < ActiveRecord::Base
persisted? && path_changed? persisted? && path_changed?
end end
def human_merge_method
if merge_method == :ff
'Fast-forward'
else
merge_method.to_s.humanize
end
end
def merge_method def merge_method
if self.merge_requests_ff_only_enabled if self.merge_requests_ff_only_enabled
:ff :ff
......
...@@ -854,6 +854,12 @@ class Repository ...@@ -854,6 +854,12 @@ class Repository
end end
end end
def merge_to_ref(user, source_sha, merge_request, target_ref, message)
branch = merge_request.target_branch
raw.merge_to_ref(user, source_sha, branch, target_ref, message)
end
def ff_merge(user, source, target_branch, merge_request: nil) def ff_merge(user, source, target_branch, merge_request: nil)
their_commit_id = commit(source)&.id their_commit_id = commit(source)&.id
raise 'Invalid merge source' if their_commit_id.nil? raise 'Invalid merge source' if their_commit_id.nil?
......
# frozen_string_literal: true
module MergeRequests
class MergeBaseService < MergeRequests::BaseService
include Gitlab::Utils::StrongMemoize
MergeError = Class.new(StandardError)
attr_reader :merge_request
# Overridden in EE.
def hooks_validation_pass?(_merge_request)
true
end
# Overridden in EE.
def hooks_validation_error(_merge_request)
end
def source
if merge_request.squash
squash_sha!
else
merge_request.diff_head_sha
end
end
private
def handle_merge_error(*args)
# No-op
end
def commit_message
params[:commit_message] ||
merge_request.default_merge_commit_message
end
def squash_sha!
strong_memoize(:squash_sha) do
params[:merge_request] = merge_request
squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute
case squash_result[:status]
when :success
squash_result[:squash_sha]
when :error
raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
end
end
end
end
end
...@@ -7,13 +7,7 @@ module MergeRequests ...@@ -7,13 +7,7 @@ module MergeRequests
# mark merge request as merged and execute all hooks and notifications # mark merge request as merged and execute all hooks and notifications
# Executed when you do merge via GitLab UI # Executed when you do merge via GitLab UI
# #
class MergeService < MergeRequests::BaseService class MergeService < MergeRequests::MergeBaseService
include Gitlab::Utils::StrongMemoize
MergeError = Class.new(StandardError)
attr_reader :merge_request, :source
delegate :merge_jid, :state, to: :@merge_request delegate :merge_jid, :state, to: :@merge_request
def execute(merge_request) def execute(merge_request)
...@@ -38,19 +32,6 @@ module MergeRequests ...@@ -38,19 +32,6 @@ module MergeRequests
handle_merge_error(log_message: e.message, save_message_on_model: true) handle_merge_error(log_message: e.message, save_message_on_model: true)
end end
def source
if merge_request.squash
squash_sha!
else
merge_request.diff_head_sha
end
end
# Overridden in EE.
def hooks_validation_pass?(_merge_request)
true
end
private private
def error_check! def error_check!
...@@ -79,24 +60,8 @@ module MergeRequests ...@@ -79,24 +60,8 @@ module MergeRequests
merge_request.update!(merge_commit_sha: commit_id) merge_request.update!(merge_commit_sha: commit_id)
end end
def squash_sha!
strong_memoize(:squash_sha) do
params[:merge_request] = merge_request
squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute
case squash_result[:status]
when :success
squash_result[:squash_sha]
when :error
raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
end
end
end
def try_merge def try_merge
message = params[:commit_message] || merge_request.default_merge_commit_message repository.merge(current_user, source, merge_request, commit_message)
repository.merge(current_user, source, merge_request, message)
rescue Gitlab::Git::PreReceiveError => e rescue Gitlab::Git::PreReceiveError => e
handle_merge_error(log_message: e.message) handle_merge_error(log_message: e.message)
raise MergeError, 'Something went wrong during merge pre-receive hook' raise MergeError, 'Something went wrong during merge pre-receive hook'
......
# frozen_string_literal: true
module MergeRequests
# Performs the merge between source SHA and the target branch. 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
# target branch would have if we used the regular `MergeService`, but without
# every side-effect that comes with it (MR updates, mails, source branch
# deletion, etc). This service should be kept idempotent (i.e. can
# be executed regardless of the `target_ref` current state).
#
class MergeToRefService < MergeRequests::MergeBaseService
def execute(merge_request)
@merge_request = merge_request
error_check!
commit_id = commit
raise MergeError, 'Conflicts detected during merge' unless commit_id
success(commit_id: commit_id)
rescue MergeError => error
error(error.message)
end
private
def error_check!
error =
if !merge_method_supported?
"#{project.human_merge_method} to #{target_ref} is currently not supported."
elsif !hooks_validation_pass?(merge_request)
hooks_validation_error(merge_request)
elsif @merge_request.should_be_rebased?
'Fast-forward merge is not possible. Please update your source branch.'
elsif !@merge_request.mergeable_to_ref?
"Merge request is not mergeable to #{target_ref}"
elsif !source
'No source for merge'
end
raise MergeError, error if error
end
def target_ref
merge_request.merge_ref_path
end
def commit
repository.merge_to_ref(current_user, source, merge_request, target_ref, commit_message)
rescue Gitlab::Git::PreReceiveError => error
raise MergeError, error.message
end
def merge_method_supported?
[:merge, :rebase_merge].include?(project.merge_method)
end
end
end
---
title: Support merge ref writing (without merging to target branch)
merge_request: 24692
author:
type: added
...@@ -556,6 +556,12 @@ module Gitlab ...@@ -556,6 +556,12 @@ 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)
wrapped_gitaly_errors do
gitaly_operation_client.user_merge_to_ref(user, source_sha, branch, target_ref, message)
end
end
def merge(user, source_sha, target_branch, message, &block) def merge(user, source_sha, target_branch, message, &block)
wrapped_gitaly_errors do wrapped_gitaly_errors do
gitaly_operation_client.user_merge_branch(user, source_sha, target_branch, message, &block) gitaly_operation_client.user_merge_branch(user, source_sha, target_branch, message, &block)
......
...@@ -100,6 +100,25 @@ module Gitlab ...@@ -100,6 +100,25 @@ module Gitlab
end end
end end
def user_merge_to_ref(user, source_sha, branch, target_ref, message)
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: message
)
response = GitalyClient.call(@repository.storage, :operation_service, :user_merge_to_ref, request)
if pre_receive_error = response.pre_receive_error.presence
raise Gitlab::Git::PreReceiveError, pre_receive_error
end
response.commit_id
end
def user_merge_branch(user, source_sha, target_branch, message) def user_merge_branch(user, source_sha, target_branch, message)
request_enum = QueueEnumerator.new request_enum = QueueEnumerator.new
response_enum = GitalyClient.call( response_enum = GitalyClient.call(
......
...@@ -1704,6 +1704,37 @@ describe Gitlab::Git::Repository, :seed_helper do ...@@ -1704,6 +1704,37 @@ describe Gitlab::Git::Repository, :seed_helper do
end end
end end
describe '#merge_to_ref' do
let(:repository) { mutable_repository }
let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' }
let(:left_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' }
let(:right_branch) { '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)
end
def merge_to_ref
repository.merge_to_ref(user, left_sha, right_branch, target_ref, 'Merge message')
end
it 'generates a commit in the target_ref' do
expect(repository.ref_exists?(target_ref)).to be(false)
commit_sha = merge_to_ref
ref_head = repository.commit(target_ref)
expect(commit_sha).to be_present
expect(repository.ref_exists?(target_ref)).to be(true)
expect(ref_head.id).to eq(commit_sha)
end
it 'does not change the right branch HEAD' do
expect { merge_to_ref }.not_to change { repository.find_branch(right_branch).target }
end
end
describe '#merge' do describe '#merge' do
let(:repository) { mutable_repository } let(:repository) { mutable_repository }
let(:source_sha) { '913c66a37b4a45b9769037c55c2d238bd0942d2e' } let(:source_sha) { '913c66a37b4a45b9769037c55c2d238bd0942d2e' }
......
...@@ -1373,6 +1373,29 @@ describe Repository do ...@@ -1373,6 +1373,29 @@ describe Repository do
end end
end end
describe '#merge_to_ref' do
let(:merge_request) do
create(:merge_request, source_branch: 'feature',
target_branch: 'master',
source_project: project)
end
it 'writes merge of source and target 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')
merge_commit = repository.commit(merge_commit_id)
expect(merge_commit.message).to eq('Custom message')
expect(merge_commit.author_name).to eq(user.name)
expect(merge_commit.author_email).to eq(user.commit_email)
expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present
end
end
describe '#ff_merge' do describe '#ff_merge' do
before do before do
repository.add_branch(user, 'ff-target', 'feature~5') repository.add_branch(user, 'ff-target', 'feature~5')
......
# frozen_string_literal: true
require 'spec_helper'
describe MergeRequests::MergeToRefService do
set(:user) { create(:user) }
let(:merge_request) { create(:merge_request, :simple) }
let(:project) { merge_request.project }
before do
project.add_maintainer(user)
end
describe '#execute' do
let(:service) do
described_class.new(project, user,
commit_message: 'Awesome message',
'should_remove_source_branch' => true)
end
def process_merge_to_ref
perform_enqueued_jobs do
service.execute(merge_request)
end
end
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)
result = service.execute(merge_request)
ref_head = repository.commit(target_ref)
expect(result[:status]).to eq(:success)
expect(result[:commit_id]).to be_present
expect(repository.ref_exists?(target_ref)).to be(true)
expect(ref_head.id).to eq(result[:commit_id])
end
it 'does not send any mail' do
expect { process_merge_to_ref }.not_to change { ActionMailer::Base.deliveries.count }
end
it 'does not change the MR state' do
expect { process_merge_to_ref }.not_to change { merge_request.state }
end
it 'does not create notes' do
expect { process_merge_to_ref }.not_to change { merge_request.notes.count }
end
it 'does not delete the source branch' do
expect(DeleteBranchService).not_to receive(:new)
process_merge_to_ref
end
it 'returns an error when the failing to process the merge' do
allow(project.repository).to receive(:merge_to_ref).and_return(nil)
result = service.execute(merge_request)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Conflicts detected during merge')
end
context 'commit history comparison with regular MergeService' do
let(:merge_ref_service) do
described_class.new(project, user, {})
end
let(:merge_service) do
MergeRequests::MergeService.new(project, user, {})
end
shared_examples_for 'MergeService for target ref' do
it 'target_ref has the same state of target branch' do
repo = merge_request.target_project.repository
process_merge_to_ref
merge_service.execute(merge_request)
ref_commits = repo.commits(merge_request.merge_ref_path, limit: 3)
target_branch_commits = repo.commits(merge_request.target_branch, limit: 3)
ref_commits.zip(target_branch_commits).each do |ref_commit, target_branch_commit|
expect(ref_commit.parents).to eq(target_branch_commit.parents)
end
end
end
context 'when merge commit' do
it_behaves_like 'MergeService for target ref'
end
context 'when merge commit with squash' do
before do
merge_request.update!(squash: true, source_branch: 'master', target_branch: 'feature')
end
it_behaves_like 'MergeService for target ref'
end
end
context 'merge pre-condition checks' do
before do
merge_request.project.update!(merge_method: merge_method)
end
context 'when semi-linear merge method' do
let(:merge_method) { :rebase_merge }
it 'return error when MR should be able to fast-forward' do
allow(merge_request).to receive(:should_be_rebased?) { true }
error_message = 'Fast-forward merge is not possible. Please update your source branch.'
result = service.execute(merge_request)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq(error_message)
end
end
context 'when fast-forward merge method' do
let(:merge_method) { :ff }
it 'returns error' do
error_message = "Fast-forward to #{merge_request.merge_ref_path} is currently not supported."
result = service.execute(merge_request)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq(error_message)
end
end
context 'when MR is not mergeable to ref' do
let(:merge_method) { :merge }
it 'returns error' do
allow(merge_request).to receive(:mergeable_to_ref?) { false }
error_message = "Merge request is not mergeable to #{merge_request.merge_ref_path}"
result = service.execute(merge_request)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq(error_message)
end
end
end
context 'does not close related todos' do
let(:merge_request) { create(:merge_request, assignee: user, author: user) }
let(:project) { merge_request.project }
let!(:todo) do
create(:todo, :assigned,
project: project,
author: user,
user: user,
target: merge_request)
end
before do
allow(service).to receive(:execute_hooks)
perform_enqueued_jobs do
service.execute(merge_request)
todo.reload
end
end
it { expect(todo).not_to be_done }
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