Commit 86cbf60c authored by Robert Speicher's avatar Robert Speicher

Merge branch 'feature/migrate-branch-operations-to-gitaly' into 'master'

Migrate creating/deleting a branch to Gitaly

See merge request !13864
parents 81da789e 41ef94e7
......@@ -41,7 +41,7 @@ module Github
def remove!(name)
repository.delete_branch(name)
rescue Rugged::ReferenceError => e
rescue Gitlab::Git::Repository::DeleteBranchError => e
Rails.logger.error("#{self.class.name}: Could not remove branch #{name}: #{e}")
end
......
......@@ -18,6 +18,7 @@ module Gitlab
InvalidBlobName = Class.new(StandardError)
InvalidRef = Class.new(StandardError)
GitError = Class.new(StandardError)
DeleteBranchError = Class.new(StandardError)
class << self
# Unlike `new`, `create` takes the storage path, not the storage name
......@@ -653,10 +654,16 @@ module Gitlab
end
# Delete the specified branch from the repository
#
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/476
def delete_branch(branch_name)
rugged.branches.delete(branch_name)
gitaly_migrate(:delete_branch) do |is_enabled|
if is_enabled
gitaly_ref_client.delete_branch(branch_name)
else
rugged.branches.delete(branch_name)
end
end
rescue Rugged::ReferenceError, CommandError => e
raise DeleteBranchError, e
end
def delete_refs(*ref_names)
......@@ -681,15 +688,14 @@ module Gitlab
# Examples:
# create_branch("feature")
# create_branch("other-feature", "master")
#
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/476
def create_branch(ref, start_point = "HEAD")
rugged_ref = rugged.branches.create(ref, start_point)
target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target)
Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit)
rescue Rugged::ReferenceError => e
raise InvalidRef.new("Branch #{ref} already exists") if e.to_s =~ /'refs\/heads\/#{ref}'/
raise InvalidRef.new("Invalid reference #{start_point}")
gitaly_migrate(:create_branch) do |is_enabled|
if is_enabled
gitaly_ref_client.create_branch(ref, start_point)
else
rugged_create_branch(ref, start_point)
end
end
end
# Delete the specified remote from this repository.
......@@ -1226,6 +1232,15 @@ module Gitlab
false
end
def rugged_create_branch(ref, start_point)
rugged_ref = rugged.branches.create(ref, start_point)
target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target)
Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit)
rescue Rugged::ReferenceError => e
raise InvalidRef.new("Branch #{ref} already exists") if e.to_s =~ /'refs\/heads\/#{ref}'/
raise InvalidRef.new("Invalid reference #{start_point}")
end
def gitaly_copy_gitattributes(revision)
gitaly_repository_client.apply_gitattributes(revision)
end
......
......@@ -79,7 +79,7 @@ module Gitlab
end
def find_branch(branch_name)
request = Gitaly::DeleteBranchRequest.new(
request = Gitaly::FindBranchRequest.new(
repository: @gitaly_repo,
name: GitalyClient.encode(branch_name)
)
......@@ -92,6 +92,40 @@ module Gitlab
Gitlab::Git::Branch.new(@repository, encode!(branch.name.dup), branch.target_commit.id, target_commit)
end
def create_branch(ref, start_point)
request = Gitaly::CreateBranchRequest.new(
repository: @gitaly_repo,
name: GitalyClient.encode(ref),
start_point: GitalyClient.encode(start_point)
)
response = GitalyClient.call(@repository.storage, :ref_service, :create_branch, request)
case response.status
when :OK
branch = response.branch
target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit)
Gitlab::Git::Branch.new(@repository, branch.name, branch.target_commit.id, target_commit)
when :ERR_INVALID
invalid_ref!("Invalid ref name")
when :ERR_EXISTS
invalid_ref!("Branch #{ref} already exists")
when :ERR_INVALID_START_POINT
invalid_ref!("Invalid reference #{start_point}")
else
raise "Unknown response status: #{response.status}"
end
end
def delete_branch(branch_name)
request = Gitaly::DeleteBranchRequest.new(
repository: @gitaly_repo,
name: GitalyClient.encode(branch_name)
)
GitalyClient.call(@repository.storage, :ref_service, :delete_branch, request)
end
private
def consume_refs_response(response)
......@@ -163,6 +197,10 @@ module Gitlab
Gitlab::Git::Commit.decorate(@repository, hash)
end
def invalid_ref!(message)
raise Gitlab::Git::Repository::InvalidRef.new(message)
end
end
end
end
......@@ -166,7 +166,7 @@ module Gitlab
def remove_branch(name)
project.repository.delete_branch(name)
rescue Rugged::ReferenceError
rescue Gitlab::Git::Repository::DeleteBranchFailed
errors << { type: :remove_branch, name: name }
end
......
......@@ -390,46 +390,73 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
describe "#delete_branch" do
before(:all) do
@repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
@repo.delete_branch("feature")
shared_examples "deleting a branch" do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
after do
FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
it "removes the branch from the repo" do
branch_name = "to-be-deleted-soon"
repository.create_branch(branch_name)
expect(repository.rugged.branches[branch_name]).not_to be_nil
repository.delete_branch(branch_name)
expect(repository.rugged.branches[branch_name]).to be_nil
end
context "when branch does not exist" do
it "raises a DeleteBranchError exception" do
expect { repository.delete_branch("this-branch-does-not-exist") }.to raise_error(Gitlab::Git::Repository::DeleteBranchError)
end
end
end
it "should remove the branch from the repo" do
expect(@repo.rugged.branches["feature"]).to be_nil
context "when Gitaly delete_branch is enabled" do
it_behaves_like "deleting a branch"
end
after(:all) do
FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
context "when Gitaly delete_branch is disabled", skip_gitaly_mock: true do
it_behaves_like "deleting a branch"
end
end
describe "#create_branch" do
before(:all) do
@repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
end
shared_examples 'creating a branch' do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
it "should create a new branch" do
expect(@repo.create_branch('new_branch', 'master')).not_to be_nil
end
after do
FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
it "should create a new branch with the right name" do
expect(@repo.create_branch('another_branch', 'master').name).to eq('another_branch')
end
it "should create a new branch" do
expect(repository.create_branch('new_branch', 'master')).not_to be_nil
end
it "should fail if we create an existing branch" do
@repo.create_branch('duplicated_branch', 'master')
expect {@repo.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists")
it "should create a new branch with the right name" do
expect(repository.create_branch('another_branch', 'master').name).to eq('another_branch')
end
it "should fail if we create an existing branch" do
repository.create_branch('duplicated_branch', 'master')
expect {repository.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists")
end
it "should fail if we create a branch from a non existing ref" do
expect {repository.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge")
end
end
it "should fail if we create a branch from a non existing ref" do
expect {@repo.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge")
context 'when Gitaly create_branch feature is enabled' do
it_behaves_like 'creating a branch'
end
after(:all) do
FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
context 'when Gitaly create_branch feature is disabled', skip_gitaly_mock: true do
it_behaves_like 'creating a branch'
end
end
......@@ -905,7 +932,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
it 'should set the autocrlf option to the provided option' do
@repo.autocrlf = :input
File.open(File.join(SEED_STORAGE_PATH, TEST_MUTABLE_REPO_PATH, '.git', 'config')) do |config_file|
File.open(File.join(SEED_STORAGE_PATH, TEST_MUTABLE_REPO_PATH, 'config')) do |config_file|
expect(config_file.read).to match('autocrlf = input')
end
end
......@@ -977,7 +1004,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
context 'with local and remote branches' do
let(:repository) do
Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '')
Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
end
before do
......@@ -1024,7 +1051,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
context 'with local and remote branches' do
let(:repository) do
Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '')
Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
end
before do
......@@ -1230,7 +1257,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
describe '#local_branches' do
before(:all) do
@repo = Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '')
@repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
end
after(:all) do
......
......@@ -41,7 +41,7 @@ module SeedHelper
end
def create_mutable_seeds
system(git_env, *%W(#{Gitlab.config.git.bin_path} clone #{TEST_REPO_PATH} #{TEST_MUTABLE_REPO_PATH}),
system(git_env, *%W(#{Gitlab.config.git.bin_path} clone --bare #{TEST_REPO_PATH} #{TEST_MUTABLE_REPO_PATH}),
chdir: SEED_STORAGE_PATH,
out: '/dev/null',
err: '/dev/null')
......
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