Commit 343301bc authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'gitaly-prepare-cherry-pick-revert' into 'master'

Prepare cherry-pick and revert for migration to Gitaly

See merge request gitlab-org/gitlab-ce!14339
parents f13158a0 75509fac
...@@ -834,10 +834,6 @@ class Repository ...@@ -834,10 +834,6 @@ class Repository
} }
end end
def user_to_committer(user)
Gitlab::Git.committer_hash(email: user.email, name: user.name)
end
def can_be_merged?(source_sha, target_branch) def can_be_merged?(source_sha, target_branch)
our_commit = rugged.branches[target_branch].target our_commit = rugged.branches[target_branch].target
their_commit = rugged.lookup(source_sha) their_commit = rugged.lookup(source_sha)
...@@ -859,54 +855,34 @@ class Repository ...@@ -859,54 +855,34 @@ class Repository
end end
def revert( def revert(
user, commit, branch_name, user, commit, branch_name, message,
start_branch_name: nil, start_project: project) start_branch_name: nil, start_project: project)
with_branch(
user,
branch_name,
start_branch_name: start_branch_name,
start_repository: start_project.repository.raw_repository) do |start_commit|
revert_tree_id = check_revert_content(commit, start_commit.sha)
unless revert_tree_id
raise Repository::CreateTreeError.new('Failed to revert commit')
end
committer = user_to_committer(user) with_cache_hooks do
raw_repository.revert(
create_commit(message: commit.revert_message(user), user: user,
author: committer, commit: commit.raw,
committer: committer, branch_name: branch_name,
tree: revert_tree_id, message: message,
parents: [start_commit.sha]) start_branch_name: start_branch_name,
start_repository: start_project.repository.raw_repository
)
end end
end end
def cherry_pick( def cherry_pick(
user, commit, branch_name, user, commit, branch_name, message,
start_branch_name: nil, start_project: project) start_branch_name: nil, start_project: project)
with_branch(
user,
branch_name,
start_branch_name: start_branch_name,
start_repository: start_project.repository.raw_repository) do |start_commit|
cherry_pick_tree_id = check_cherry_pick_content(commit, start_commit.sha) with_cache_hooks do
unless cherry_pick_tree_id raw_repository.cherry_pick(
raise Repository::CreateTreeError.new('Failed to cherry-pick commit') user: user,
end commit: commit.raw,
branch_name: branch_name,
committer = user_to_committer(user) message: message,
start_branch_name: start_branch_name,
create_commit(message: commit.cherry_pick_message(user), start_repository: start_project.repository.raw_repository
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
end end
...@@ -918,36 +894,6 @@ class Repository ...@@ -918,36 +894,6 @@ class Repository
end end
end end
def check_revert_content(target_commit, source_sha)
args = [target_commit.sha, source_sha]
args << { mainline: 1 } if target_commit.merge_commit?
revert_index = rugged.revert_commit(*args)
return false if revert_index.conflicts?
tree_id = revert_index.write_tree(rugged)
return false unless diff_exists?(source_sha, tree_id)
tree_id
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
def merged_to_root_ref?(branch_name) def merged_to_root_ref?(branch_name)
branch_commit = commit(branch_name) branch_commit = commit(branch_name)
root_ref_commit = commit(root_ref) root_ref_commit = commit(root_ref)
......
...@@ -11,15 +11,19 @@ module Commits ...@@ -11,15 +11,19 @@ module Commits
def commit_change(action) def commit_change(action)
raise NotImplementedError unless repository.respond_to?(action) raise NotImplementedError unless repository.respond_to?(action)
# rubocop:disable GitlabSecurity/PublicSend
message = @commit.public_send(:"#{action}_message", current_user)
# rubocop:disable GitlabSecurity/PublicSend # rubocop:disable GitlabSecurity/PublicSend
repository.public_send( repository.public_send(
action, action,
current_user, current_user,
@commit, @commit,
@branch_name, @branch_name,
message,
start_project: @start_project, start_project: @start_project,
start_branch_name: @start_branch) start_branch_name: @start_branch)
rescue Repository::CreateTreeError rescue Gitlab::Git::Repository::CreateTreeError
error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically. error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically.
This #{@commit.change_type_title(current_user)} may already have been #{action.to_s.dasherize}ed, or a more recent commit may have updated some of its content." This #{@commit.change_type_title(current_user)} may already have been #{action.to_s.dasherize}ed, or a more recent commit may have updated some of its content."
raise ChangeError, error_msg raise ChangeError, error_msg
......
...@@ -57,6 +57,15 @@ module Gitlab ...@@ -57,6 +57,15 @@ module Gitlab
def version def version
Gitlab::VersionInfo.parse(Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --version)).first) Gitlab::VersionInfo.parse(Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --version)).first)
end end
def check_namespace!(*objects)
expected_namespace = self.name + '::'
objects.each do |object|
unless object.class.name.start_with?(expected_namespace)
raise ArgumentError, "expected object in #{expected_namespace}, got #{object}"
end
end
end
end end
end end
end end
...@@ -413,6 +413,10 @@ module Gitlab ...@@ -413,6 +413,10 @@ module Gitlab
end end
end end
def merge_commit?
parent_ids.size > 1
end
private private
def init_from_hash(hash) def init_from_hash(hash)
......
...@@ -15,9 +15,7 @@ module Gitlab ...@@ -15,9 +15,7 @@ module Gitlab
end end
# Refactoring aid # Refactoring aid
unless new_repository.is_a?(Gitlab::Git::Repository) Gitlab::Git.check_namespace!(new_repository)
raise "expected a Gitlab::Git::Repository, got #{new_repository}"
end
@repository = new_repository @repository = new_repository
end end
......
...@@ -19,6 +19,7 @@ module Gitlab ...@@ -19,6 +19,7 @@ module Gitlab
InvalidRef = Class.new(StandardError) InvalidRef = Class.new(StandardError)
GitError = Class.new(StandardError) GitError = Class.new(StandardError)
DeleteBranchError = Class.new(StandardError) DeleteBranchError = Class.new(StandardError)
CreateTreeError = Class.new(StandardError)
class << self class << self
# Unlike `new`, `create` takes the storage path, not the storage name # Unlike `new`, `create` takes the storage path, not the storage name
...@@ -684,6 +685,88 @@ module Gitlab ...@@ -684,6 +685,88 @@ module Gitlab
nil nil
end end
def revert(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)
revert_tree_id = check_revert_content(commit, start_commit.sha)
raise CreateTreeError unless revert_tree_id
committer = user_to_committer(user)
create_commit(message: message,
author: committer,
committer: committer,
tree: revert_tree_id,
parents: [start_commit.sha])
end
end
def check_revert_content(target_commit, source_sha)
args = [target_commit.sha, source_sha]
args << { mainline: 1 } if target_commit.merge_commit?
revert_index = rugged.revert_commit(*args)
return false if revert_index.conflicts?
tree_id = revert_index.write_tree(rugged)
return false unless diff_exists?(source_sha, tree_id)
tree_id
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|
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 diff_exists?(sha1, sha2)
rugged.diff(sha1, sha2).size > 0
end
def user_to_committer(user)
Gitlab::Git.committer_hash(email: user.email, name: user.name)
end
def create_commit(params = {}) def create_commit(params = {})
params[:message].delete!("\r") params[:message].delete!("\r")
...@@ -835,7 +918,7 @@ module Gitlab ...@@ -835,7 +918,7 @@ module Gitlab
end end
def with_repo_branch_commit(start_repository, start_branch_name) def with_repo_branch_commit(start_repository, start_branch_name)
raise "expected Gitlab::Git::Repository, got #{start_repository}" unless start_repository.is_a?(Gitlab::Git::Repository) Gitlab::Git.check_namespace!(start_repository)
return yield nil if start_repository.empty_repo? return yield nil if start_repository.empty_repo?
......
...@@ -1311,24 +1311,25 @@ describe Repository, models: true do ...@@ -1311,24 +1311,25 @@ describe Repository, models: true do
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') }
let(:message) { 'revert message' }
context 'when there is a conflict' do context 'when there is a conflict' do
it 'raises an error' do it 'raises an error' do
expect { repository.revert(user, new_image_commit, 'master') }.to raise_error(/Failed to/) expect { repository.revert(user, new_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError)
end end
end end
context 'when commit was already reverted' do context 'when commit was already reverted' do
it 'raises an error' do it 'raises an error' do
repository.revert(user, update_image_commit, 'master') repository.revert(user, update_image_commit, 'master', message)
expect { repository.revert(user, update_image_commit, 'master') }.to raise_error(/Failed to/) expect { repository.revert(user, update_image_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError)
end end
end end
context 'when commit can be reverted' do context 'when commit can be reverted' do
it 'reverts the changes' do it 'reverts the changes' do
expect(repository.revert(user, update_image_commit, 'master')).to be_truthy expect(repository.revert(user, update_image_commit, 'master', message)).to be_truthy
end end
end end
...@@ -1337,7 +1338,7 @@ describe Repository, models: true do ...@@ -1337,7 +1338,7 @@ describe Repository, models: true do
merge_commit merge_commit
expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present
repository.revert(user, merge_commit, 'master') repository.revert(user, merge_commit, 'master', message)
expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present
end end
end end
...@@ -1347,24 +1348,25 @@ describe Repository, models: true do ...@@ -1347,24 +1348,25 @@ describe Repository, models: true do
let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') } let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') }
let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') }
let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') } let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') }
let(:message) { 'cherry-pick message' }
context 'when there is a conflict' do context 'when there is a conflict' do
it 'raises an error' do it 'raises an error' do
expect { repository.cherry_pick(user, conflict_commit, 'master') }.to raise_error(/Failed to/) expect { repository.cherry_pick(user, conflict_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError)
end end
end end
context 'when commit was already cherry-picked' do context 'when commit was already cherry-picked' do
it 'raises an error' do it 'raises an error' do
repository.cherry_pick(user, pickable_commit, 'master') repository.cherry_pick(user, pickable_commit, 'master', message)
expect { repository.cherry_pick(user, pickable_commit, 'master') }.to raise_error(/Failed to/) expect { repository.cherry_pick(user, pickable_commit, 'master', message) }.to raise_error(Gitlab::Git::Repository::CreateTreeError)
end end
end end
context 'when commit can be cherry-picked' do context 'when commit can be cherry-picked' do
it 'cherry-picks the changes' do it 'cherry-picks the changes' do
expect(repository.cherry_pick(user, pickable_commit, 'master')).to be_truthy expect(repository.cherry_pick(user, pickable_commit, 'master', message)).to be_truthy
end end
end end
...@@ -1372,11 +1374,11 @@ describe Repository, models: true do ...@@ -1372,11 +1374,11 @@ describe Repository, models: true do
it 'cherry-picks the changes' do it 'cherry-picks the changes' do
expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil 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') cherry_pick_commit_sha = repository.cherry_pick(user, pickable_merge, 'improve/awesome', message)
cherry_pick_commit_message = project.commit(cherry_pick_commit_sha).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(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil
expect(cherry_pick_commit_message).to include('cherry picked from') expect(cherry_pick_commit_message).to eq(message)
end end
end 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