Commit 0c3d7830 authored by Douwe Maan's avatar Douwe Maan

Merge branch '5966-rebase-with-block' into 'master'

Update rebasing to use the new two-phase Gitaly Rebase RPC

Closes gitlab-ee#5966

See merge request gitlab-org/gitlab-ce!27446
parents 4e67122e 49cb4b3d
...@@ -417,7 +417,7 @@ group :ed25519 do ...@@ -417,7 +417,7 @@ group :ed25519 do
end end
# Gitaly GRPC client # Gitaly GRPC client
gem 'gitaly-proto', '~> 1.26.0', require: 'gitaly' gem 'gitaly-proto', '~> 1.27.0', require: 'gitaly'
gem 'grpc', '~> 1.19.0' gem 'grpc', '~> 1.19.0'
......
...@@ -283,7 +283,7 @@ GEM ...@@ -283,7 +283,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.26.0) gitaly-proto (1.27.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)
...@@ -1056,7 +1056,7 @@ DEPENDENCIES ...@@ -1056,7 +1056,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.26.0) gitaly-proto (~> 1.27.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-labkit (~> 0.2.0) gitlab-labkit (~> 0.2.0)
......
...@@ -1037,11 +1037,41 @@ class Repository ...@@ -1037,11 +1037,41 @@ class Repository
raw_repository.fetch_ref(source_repository.raw_repository, source_ref: source_ref, target_ref: target_ref) raw_repository.fetch_ref(source_repository.raw_repository, source_ref: source_ref, target_ref: target_ref)
end end
# DEPRECATED: https://gitlab.com/gitlab-org/gitaly/issues/1628
def rebase_deprecated(user, merge_request)
rebase_sha = raw.rebase_deprecated(
user,
merge_request.id,
branch: merge_request.source_branch,
branch_sha: merge_request.source_branch_sha,
remote_repository: merge_request.target_project.repository.raw,
remote_branch: merge_request.target_branch
)
# To support the full deprecated behaviour, set the
# `rebase_commit_sha` for the merge_request here and return the value
merge_request.update(rebase_commit_sha: rebase_sha)
rebase_sha
end
def rebase(user, merge_request) def rebase(user, merge_request)
raw.rebase(user, merge_request.id, branch: merge_request.source_branch, if Feature.disabled?(:two_step_rebase, default_enabled: true)
branch_sha: merge_request.source_branch_sha, return rebase_deprecated(user, merge_request)
remote_repository: merge_request.target_project.repository.raw, end
remote_branch: merge_request.target_branch)
MergeRequest.transaction do
raw.rebase(
user,
merge_request.id,
branch: merge_request.source_branch,
branch_sha: merge_request.source_branch_sha,
remote_repository: merge_request.target_project.repository.raw,
remote_branch: merge_request.target_branch
) do |commit_id|
merge_request.update!(rebase_commit_sha: commit_id)
end
end
end end
def squash(user, merge_request, message) def squash(user, merge_request, message)
......
...@@ -20,17 +20,7 @@ module MergeRequests ...@@ -20,17 +20,7 @@ module MergeRequests
return false return false
end end
log_prefix = "#{self.class.name} info (#{merge_request.to_reference(full: true)}):" repository.rebase(current_user, merge_request)
Gitlab::GitLogger.info("#{log_prefix} rebase started")
rebase_sha = repository.rebase(current_user, merge_request)
Gitlab::GitLogger.info("#{log_prefix} rebased to #{rebase_sha}")
merge_request.update(rebase_commit_sha: rebase_sha)
Gitlab::GitLogger.info("#{log_prefix} rebase SHA saved: #{rebase_sha}")
true true
rescue => e rescue => e
......
---
title: Fix approvals sometimes being reset after a merge request is rebased
merge_request: 27446
author:
type: fixed
...@@ -834,7 +834,8 @@ module Gitlab ...@@ -834,7 +834,8 @@ module Gitlab
gitaly_repository_client.create_from_snapshot(url, auth) gitaly_repository_client.create_from_snapshot(url, auth)
end end
def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:) # DEPRECATED: https://gitlab.com/gitlab-org/gitaly/issues/1628
def rebase_deprecated(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:)
wrapped_gitaly_errors do wrapped_gitaly_errors do
gitaly_operation_client.user_rebase(user, rebase_id, gitaly_operation_client.user_rebase(user, rebase_id,
branch: branch, branch: branch,
...@@ -844,6 +845,20 @@ module Gitlab ...@@ -844,6 +845,20 @@ module Gitlab
end end
end end
def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:, &block)
wrapped_gitaly_errors do
gitaly_operation_client.rebase(
user,
rebase_id,
branch: branch,
branch_sha: branch_sha,
remote_repository: remote_repository,
remote_branch: remote_branch,
&block
)
end
end
def rebase_in_progress?(rebase_id) def rebase_in_progress?(rebase_id)
wrapped_gitaly_errors do wrapped_gitaly_errors do
gitaly_repository_client.rebase_in_progress?(rebase_id) gitaly_repository_client.rebase_in_progress?(rebase_id)
......
...@@ -197,6 +197,7 @@ module Gitlab ...@@ -197,6 +197,7 @@ module Gitlab
start_repository: start_repository) start_repository: start_repository)
end end
# DEPRECATED: https://gitlab.com/gitlab-org/gitaly/issues/1628
def user_rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:) def user_rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:)
request = Gitaly::UserRebaseRequest.new( request = Gitaly::UserRebaseRequest.new(
repository: @gitaly_repo, repository: @gitaly_repo,
...@@ -225,6 +226,49 @@ module Gitlab ...@@ -225,6 +226,49 @@ module Gitlab
end end
end end
def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:)
request_enum = QueueEnumerator.new
rebase_sha = nil
response_enum = GitalyClient.call(
@repository.storage,
:operation_service,
:user_rebase_confirmable,
request_enum.each,
remote_storage: remote_repository.storage
)
# First request
request_enum.push(
Gitaly::UserRebaseConfirmableRequest.new(
header: Gitaly::UserRebaseConfirmableRequest::Header.new(
repository: @gitaly_repo,
user: Gitlab::Git::User.from_gitlab(user).to_gitaly,
rebase_id: rebase_id.to_s,
branch: encode_binary(branch),
branch_sha: branch_sha,
remote_repository: remote_repository.gitaly_repository,
remote_branch: encode_binary(remote_branch)
)
)
)
perform_next_gitaly_rebase_request(response_enum) do |response|
rebase_sha = response.rebase_sha
end
yield rebase_sha
# Second request confirms with gitaly to finalize the rebase
request_enum.push(Gitaly::UserRebaseConfirmableRequest.new(apply: true))
perform_next_gitaly_rebase_request(response_enum)
rebase_sha
ensure
request_enum.close
end
def user_squash(user, squash_id, branch, start_sha, end_sha, author, message) def user_squash(user, squash_id, branch, start_sha, end_sha, author, message)
request = Gitaly::UserSquashRequest.new( request = Gitaly::UserSquashRequest.new(
repository: @gitaly_repo, repository: @gitaly_repo,
...@@ -346,6 +390,20 @@ module Gitlab ...@@ -346,6 +390,20 @@ module Gitlab
private private
def perform_next_gitaly_rebase_request(response_enum)
response = response_enum.next
if response.pre_receive_error.present?
raise Gitlab::Git::PreReceiveError, response.pre_receive_error
elsif response.git_error.present?
raise Gitlab::Git::Repository::GitError, response.git_error
end
yield response if block_given?
response
end
def call_cherry_pick_or_revert(rpc, user:, commit:, branch_name:, message:, start_branch_name:, start_repository:) def call_cherry_pick_or_revert(rpc, user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
request_class = "Gitaly::User#{rpc.to_s.camelcase}Request".constantize request_class = "Gitaly::User#{rpc.to_s.camelcase}Request".constantize
......
...@@ -1451,6 +1451,91 @@ describe Repository do ...@@ -1451,6 +1451,91 @@ describe Repository do
end end
end end
describe '#rebase' do
let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) }
shared_examples_for 'a method that can rebase successfully' do
it 'returns the rebase commit sha' do
rebase_commit_sha = repository.rebase(user, merge_request)
head_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha
expect(rebase_commit_sha).to eq(head_sha)
end
it 'sets the `rebase_commit_sha` for the given merge request' do
rebase_commit_sha = repository.rebase(user, merge_request)
expect(rebase_commit_sha).not_to be_nil
expect(merge_request.rebase_commit_sha).to eq(rebase_commit_sha)
end
end
context 'when two_step_rebase feature is enabled' do
before do
stub_feature_flags(two_step_rebase: true)
end
it_behaves_like 'a method that can rebase successfully'
it 'executes the new Gitaly RPC' do
expect_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:rebase)
expect_any_instance_of(Gitlab::GitalyClient::OperationService).not_to receive(:user_rebase)
repository.rebase(user, merge_request)
end
describe 'rolling back the `rebase_commit_sha`' do
let(:new_sha) { Digest::SHA1.hexdigest('foo') }
it 'does not rollback when there are no errors' do
second_response = double(pre_receive_error: nil, git_error: nil)
mock_gitaly(second_response)
repository.rebase(user, merge_request)
expect(merge_request.reload.rebase_commit_sha).to eq(new_sha)
end
it 'does rollback when an error is encountered in the second step' do
second_response = double(pre_receive_error: 'my_error', git_error: nil)
mock_gitaly(second_response)
expect do
repository.rebase(user, merge_request)
end.to raise_error(Gitlab::Git::PreReceiveError)
expect(merge_request.reload.rebase_commit_sha).to be_nil
end
def mock_gitaly(second_response)
responses = [
double(rebase_sha: new_sha).as_null_object,
second_response
]
expect_any_instance_of(
Gitaly::OperationService::Stub
).to receive(:user_rebase_confirmable).and_return(responses.each)
end
end
end
context 'when two_step_rebase feature is disabled' do
before do
stub_feature_flags(two_step_rebase: false)
end
it_behaves_like 'a method that can rebase successfully'
it 'executes the deprecated Gitaly RPC' do
expect_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:user_rebase)
expect_any_instance_of(Gitlab::GitalyClient::OperationService).not_to receive(:rebase)
repository.rebase(user, merge_request)
end
end
end
describe '#revert' do describe '#revert' do
let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') } let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') }
let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
......
...@@ -73,7 +73,7 @@ describe MergeRequests::RebaseService do ...@@ -73,7 +73,7 @@ describe MergeRequests::RebaseService do
end end
context 'valid params' do context 'valid params' do
describe 'successful rebase' do shared_examples_for 'a service that can execute a successful rebase' do
before do before do
service.execute(merge_request) service.execute(merge_request)
end end
...@@ -99,6 +99,22 @@ describe MergeRequests::RebaseService do ...@@ -99,6 +99,22 @@ describe MergeRequests::RebaseService do
end end
end end
context 'when the two_step_rebase feature is enabled' do
before do
stub_feature_flags(two_step_rebase: true)
end
it_behaves_like 'a service that can execute a successful rebase'
end
context 'when the two_step_rebase feature is disabled' do
before do
stub_feature_flags(two_step_rebase: false)
end
it_behaves_like 'a service that can execute a successful rebase'
end
context 'fork' do context 'fork' do
describe 'successful fork rebase' do describe 'successful fork rebase' do
let(:forked_project) do let(:forked_project) do
......
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