diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 316ba4bd9e6..2f4c74eb2dd 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.55.0 +0.56.0 diff --git a/Gemfile b/Gemfile index 1ea500d23b3..044778498a2 100644 --- a/Gemfile +++ b/Gemfile @@ -400,7 +400,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.54.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.58.0', require: 'gitaly' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index bee0ccb48da..fdb81b101f5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -276,7 +276,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly-proto (0.54.0) + gitaly-proto (0.58.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -1037,7 +1037,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly-proto (~> 0.54.0) + gitaly-proto (~> 0.58.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.6.2) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index d5518814483..c85dcfa0475 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -418,6 +418,20 @@ module Gitlab parent_ids.size > 1 end + def to_gitaly_commit + return raw_commit if raw_commit.is_a?(Gitaly::GitCommit) + + message_split = raw_commit.message.split("\n", 2) + Gitaly::GitCommit.new( + id: raw_commit.oid, + subject: message_split[0] ? message_split[0].chomp.b : "", + body: raw_commit.message.b, + parent_ids: raw_commit.parent_ids, + author: gitaly_commit_author_from_rugged(raw_commit.author), + committer: gitaly_commit_author_from_rugged(raw_commit.committer) + ) + end + private def init_from_hash(hash) @@ -463,6 +477,14 @@ module Gitlab def serialize_keys SERIALIZE_KEYS end + + def gitaly_commit_author_from_rugged(author_or_committer) + Gitaly::CommitAuthor.new( + name: author_or_committer[:name].b, + email: author_or_committer[:email].b, + date: Google::Protobuf::Timestamp.new(seconds: author_or_committer[:time].to_i) + ) + end end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 867fc2a42f6..f2bbaed64a7 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -809,44 +809,24 @@ module Gitlab end def cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:) - OperationService.new(user, self).with_branch( - branch_name, - start_branch_name: start_branch_name, - start_repository: start_repository - ) do |start_commit| + gitaly_migrate(:cherry_pick) do |is_enabled| + args = { + user: user, + commit: commit, + branch_name: branch_name, + message: message, + start_branch_name: start_branch_name, + start_repository: start_repository + } - Gitlab::Git.check_namespace!(commit, start_repository) - - cherry_pick_tree_id = check_cherry_pick_content(commit, start_commit.sha) - raise CreateTreeError unless cherry_pick_tree_id - - committer = user_to_committer(user) - - create_commit(message: message, - author: { - email: commit.author_email, - name: commit.author_name, - time: commit.authored_date - }, - committer: committer, - tree: cherry_pick_tree_id, - parents: [start_commit.sha]) + if is_enabled + gitaly_operations_client.user_cherry_pick(args) + else + rugged_cherry_pick(args) + end end end - def check_cherry_pick_content(target_commit, source_sha) - args = [target_commit.sha, source_sha] - args << 1 if target_commit.merge_commit? - - cherry_pick_index = rugged.cherrypick_commit(*args) - return false if cherry_pick_index.conflicts? - - tree_id = cherry_pick_index.write_tree(rugged) - return false unless diff_exists?(source_sha, tree_id) - - tree_id - end - def diff_exists?(sha1, sha2) rugged.diff(sha1, sha2).size > 0 end @@ -1673,6 +1653,45 @@ module Gitlab raise InvalidRef, ex end + def rugged_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:) + OperationService.new(user, self).with_branch( + branch_name, + start_branch_name: start_branch_name, + start_repository: start_repository + ) do |start_commit| + + Gitlab::Git.check_namespace!(commit, start_repository) + + cherry_pick_tree_id = check_cherry_pick_content(commit, start_commit.sha) + raise CreateTreeError unless cherry_pick_tree_id + + committer = user_to_committer(user) + + create_commit(message: message, + author: { + email: commit.author_email, + name: commit.author_name, + time: commit.authored_date + }, + committer: committer, + tree: cherry_pick_tree_id, + parents: [start_commit.sha]) + end + end + + def check_cherry_pick_content(target_commit, source_sha) + args = [target_commit.sha, source_sha] + args << 1 if target_commit.merge_commit? + + cherry_pick_index = rugged.cherrypick_commit(*args) + return false if cherry_pick_index.conflicts? + + tree_id = cherry_pick_index.write_tree(rugged) + return false unless diff_exists?(source_sha, tree_id) + + tree_id + 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 526d44a8b77..51de6a9a75d 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -122,6 +122,36 @@ module Gitlab ).branch_update Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update) end + + def user_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:) + request = Gitaly::UserCherryPickRequest.new( + repository: @gitaly_repo, + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, + commit: commit.to_gitaly_commit, + branch_name: GitalyClient.encode(branch_name), + message: GitalyClient.encode(message), + start_branch_name: GitalyClient.encode(start_branch_name.to_s), + start_repository: start_repository.gitaly_repository + ) + + response = GitalyClient.call( + @repository.storage, + :operation_service, + :user_cherry_pick, + request, + remote_storage: start_repository.storage + ) + + if response.pre_receive_error.presence + raise Gitlab::Git::HooksService::PreReceiveError, response.pre_receive_error + elsif response.commit_error.presence + raise Gitlab::Git::CommitError, response.commit_error + elsif response.create_tree_error.presence + raise Gitlab::Git::Repository::CreateTreeError, response.create_tree_error + else + Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) + end + end end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 27f0a99b2fa..af0c86abe86 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1408,41 +1408,51 @@ describe Repository do end describe '#cherry_pick' do - let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') } - let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } - let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') } - let(:message) { 'cherry-pick message' } + shared_examples 'cherry-picking a commit' do + let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') } + let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } + let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') } + let(:message) { 'cherry-pick message' } - context 'when there is a conflict' do - it 'raises an error' do - expect { repository.cherry_pick(user, conflict_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) + context 'when there is a conflict' do + it 'raises an error' do + expect { repository.cherry_pick(user, conflict_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) + end + end + + context 'when commit was already cherry-picked' do + it 'raises an error' do + repository.cherry_pick(user, pickable_commit, 'master', message) + + expect { repository.cherry_pick(user, pickable_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) + end + end + + context 'when commit can be cherry-picked' do + it 'cherry-picks the changes' do + expect(repository.cherry_pick(user, pickable_commit, 'master', message)).to be_truthy + end + end + + context 'cherry-picking a merge commit' do + it 'cherry-picks the changes' do + expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil + + cherry_pick_commit_sha = repository.cherry_pick(user, pickable_merge, 'improve/awesome', message) + cherry_pick_commit_message = project.commit(cherry_pick_commit_sha).message + + expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil + expect(cherry_pick_commit_message).to eq(message) + end end end - context 'when commit was already cherry-picked' do - it 'raises an error' do - repository.cherry_pick(user, pickable_commit, 'master', message) - - expect { repository.cherry_pick(user, pickable_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError) - end + context 'when Gitaly cherry_pick feature is enabled' do + it_behaves_like 'cherry-picking a commit' end - context 'when commit can be cherry-picked' do - it 'cherry-picks the changes' do - expect(repository.cherry_pick(user, pickable_commit, 'master', message)).to be_truthy - end - end - - context 'cherry-picking a merge commit' do - it 'cherry-picks the changes' do - expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil - - cherry_pick_commit_sha = repository.cherry_pick(user, pickable_merge, 'improve/awesome', message) - cherry_pick_commit_message = project.commit(cherry_pick_commit_sha).message - - expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil - expect(cherry_pick_commit_message).to eq(message) - end + context 'when Gitaly cherry_pick feature is disabled', :disable_gitaly do + it_behaves_like 'cherry-picking a commit' end end