diff options
author | Ahmad Sherif <me@ahmadsherif.com> | 2018-01-03 14:04:58 +0100 |
---|---|---|
committer | Ahmad Sherif <me@ahmadsherif.com> | 2018-01-10 13:28:10 +0100 |
commit | 42265d445f88408501e89b0a9aa959fd6f8d3bcf (patch) | |
tree | 7e90a6b1ebccfabfee2246747a3b267d8f97f69c | |
parent | be623ef3c1a867d23e9625fe372c17fcad3c47ce (diff) | |
download | gitlab-ce-42265d445f88408501e89b0a9aa959fd6f8d3bcf.tar.gz |
Migrate Gitlab::Git::Repository#rebase to Gitalyfeature/migrate-rebase-to-gitaly
Closes gitaly#863
-rw-r--r-- | lib/gitlab/git/repository.rb | 70 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/operation_service.rb | 28 | ||||
-rw-r--r-- | spec/services/merge_requests/rebase_service_spec.rb | 94 |
3 files changed, 133 insertions, 59 deletions
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 283134e043e..cd2ed2d6ae5 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1208,28 +1208,20 @@ module Gitlab end def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:) - rebase_path = worktree_path(REBASE_WORKTREE_PREFIX, rebase_id) - env = git_env_for_user(user) - - if remote_repository.is_a?(RemoteRepository) - env.merge!(remote_repository.fetch_env) - remote_repo_path = GITALY_INTERNAL_URL - else - remote_repo_path = remote_repository.path - end - - with_worktree(rebase_path, branch, env: env) do - run_git!( - %W(pull --rebase #{remote_repo_path} #{remote_branch}), - chdir: rebase_path, env: env - ) - - rebase_sha = run_git!(%w(rev-parse HEAD), chdir: rebase_path, env: env).strip - - Gitlab::Git::OperationService.new(user, self) - .update_branch(branch, rebase_sha, branch_sha) - - rebase_sha + gitaly_migrate(:rebase) do |is_enabled| + if is_enabled + gitaly_rebase(user, rebase_id, + branch: branch, + branch_sha: branch_sha, + remote_repository: remote_repository, + remote_branch: remote_branch) + else + git_rebase(user, rebase_id, + branch: branch, + branch_sha: branch_sha, + remote_repository: remote_repository, + remote_branch: remote_branch) + end end end @@ -2010,6 +2002,40 @@ module Gitlab tree_id end + def gitaly_rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:) + gitaly_operation_client.user_rebase(user, rebase_id, + branch: branch, + branch_sha: branch_sha, + remote_repository: remote_repository, + remote_branch: remote_branch) + end + + def git_rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:) + rebase_path = worktree_path(REBASE_WORKTREE_PREFIX, rebase_id) + env = git_env_for_user(user) + + if remote_repository.is_a?(RemoteRepository) + env.merge!(remote_repository.fetch_env) + remote_repo_path = GITALY_INTERNAL_URL + else + remote_repo_path = remote_repository.path + end + + with_worktree(rebase_path, branch, env: env) do + run_git!( + %W(pull --rebase #{remote_repo_path} #{remote_branch}), + chdir: rebase_path, env: env + ) + + rebase_sha = run_git!(%w(rev-parse HEAD), chdir: rebase_path, env: env).strip + + Gitlab::Git::OperationService.new(user, self) + .update_branch(branch, rebase_sha, branch_sha) + + rebase_sha + end + end + def local_fetch_ref(source_path, source_ref:, target_ref:) args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref}) run_git(args) diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index ae1753ff0ae..3795d527949 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -146,6 +146,34 @@ module Gitlab start_repository: start_repository) end + def user_rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:) + request = Gitaly::UserRebaseRequest.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) + ) + + response = GitalyClient.call( + @repository.storage, + :operation_service, + :user_rebase, + request, + remote_storage: remote_repository.storage + ) + + if response.pre_receive_error.presence + raise Gitlab::Git::HooksService::PreReceiveError, response.pre_receive_error + elsif response.git_error.presence + raise Gitlab::Git::Repository::GitError, response.git_error + else + response.rebase_sha + end + end + private def call_cherry_pick_or_revert(rpc, user:, commit:, branch_name:, message:, start_branch_name:, start_repository:) diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index d1b37cdd073..1a8377f6453 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -36,7 +36,7 @@ describe MergeRequests::RebaseService do end end - context 'when unexpected error occurs' do + context 'when unexpected error occurs', :disable_gitaly do before do allow(repository).to receive(:run_git!).and_raise('Something went wrong') end @@ -53,7 +53,7 @@ describe MergeRequests::RebaseService do end end - context 'with git command failure' do + context 'with git command failure', :disable_gitaly do before do allow(repository).to receive(:run_git!).and_raise(Gitlab::Git::Repository::GitError, 'Something went wrong') end @@ -71,31 +71,41 @@ describe MergeRequests::RebaseService do end context 'valid params' do - before do - service.execute(merge_request) - end + shared_examples 'successful rebase' do + before do + service.execute(merge_request) + end - it 'rebases source branch' do - parent_sha = merge_request.source_project.repository.commit(merge_request.source_branch).parents.first.sha - target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha - expect(parent_sha).to eq(target_branch_sha) - end + it 'rebases source branch' do + parent_sha = merge_request.source_project.repository.commit(merge_request.source_branch).parents.first.sha + target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha + expect(parent_sha).to eq(target_branch_sha) + end - it 'records the new SHA on the merge request' do - head_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha - expect(merge_request.reload.rebase_commit_sha).to eq(head_sha) + it 'records the new SHA on the merge request' do + head_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha + expect(merge_request.reload.rebase_commit_sha).to eq(head_sha) + end + + it 'logs correct author and commiter' do + head_commit = merge_request.source_project.repository.commit(merge_request.source_branch) + + expect(head_commit.author_email).to eq('dmitriy.zaporozhets@gmail.com') + expect(head_commit.author_name).to eq('Dmitriy Zaporozhets') + expect(head_commit.committer_email).to eq(user.email) + expect(head_commit.committer_name).to eq(user.name) + end end - it 'logs correct author and commiter' do - head_commit = merge_request.source_project.repository.commit(merge_request.source_branch) + context 'when Gitaly rebase feature is enabled' do + it_behaves_like 'successful rebase' + end - expect(head_commit.author_email).to eq('dmitriy.zaporozhets@gmail.com') - expect(head_commit.author_name).to eq('Dmitriy Zaporozhets') - expect(head_commit.committer_email).to eq(user.email) - expect(head_commit.committer_name).to eq(user.name) + context 'when Gitaly rebase feature is disabled', :disable_gitaly do + it_behaves_like 'successful rebase' end - context 'git commands' do + context 'git commands', :disable_gitaly do it 'sets GL_REPOSITORY env variable when calling git commands' do expect(repository).to receive(:popen).exactly(3) .with(anything, anything, hash_including('GL_REPOSITORY')) @@ -106,27 +116,37 @@ describe MergeRequests::RebaseService do end context 'fork' do - let(:forked_project) do - fork_project(project, user, repository: true) + shared_examples 'successful fork rebase' do + let(:forked_project) do + fork_project(project, user, repository: true) + end + + let(:merge_request_from_fork) do + forked_project.repository.create_file( + user, + 'new-file-to-target', + '', + message: 'Add new file to target', + branch_name: 'master') + + create(:merge_request, + source_branch: 'master', source_project: forked_project, + target_branch: 'master', target_project: project) + end + + it 'rebases source branch' do + parent_sha = forked_project.repository.commit(merge_request_from_fork.source_branch).parents.first.sha + target_branch_sha = project.repository.commit(merge_request_from_fork.target_branch).sha + expect(parent_sha).to eq(target_branch_sha) + end end - let(:merge_request_from_fork) do - forked_project.repository.create_file( - user, - 'new-file-to-target', - '', - message: 'Add new file to target', - branch_name: 'master') - - create(:merge_request, - source_branch: 'master', source_project: forked_project, - target_branch: 'master', target_project: project) + context 'when Gitaly rebase feature is enabled' do + it_behaves_like 'successful fork rebase' end - it 'rebases source branch' do - parent_sha = forked_project.repository.commit(merge_request_from_fork.source_branch).parents.first.sha - target_branch_sha = project.repository.commit(merge_request_from_fork.target_branch).sha - expect(parent_sha).to eq(target_branch_sha) + context 'when Gitaly rebase feature is disabled', :disable_gitaly do + it_behaves_like 'successful fork rebase' end end end |