Commit 1a9d5059 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'use-rugged-to-create-tag' into 'master'

Use Rugged's TagCollection#create instead of gitlab-shell's Repository#add_tag for better performance

This was originally opened at !1757 by @pcarranza but I changed it to use Rugged instead of gitlab_git, following @DouweM's request.

Once this is merged, https://gitlab.com/gitlab-org/gitlab-shell/merge_requests/26 will be mergeable too.

See merge request !3745
parents c2c70cfc 209f2f1e
......@@ -13,6 +13,7 @@ v 8.8.0 (unreleased)
- Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea)
- Added button to toggle whitespaces changes on diff view
- Backport GitLab Enterprise support from EE
- Create tags using Rugged for performance reasons. !3745
- Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718
- Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
- Added multiple colors for labels in dropdowns when dups happen.
......
......@@ -148,10 +148,20 @@ class Repository
find_branch(branch_name)
end
def add_tag(tag_name, ref, message = nil)
before_push_tag
def add_tag(user, tag_name, target, message = nil)
oldrev = Gitlab::Git::BLANK_SHA
ref = Gitlab::Git::TAG_REF_PREFIX + tag_name
target = commit(target).try(:id)
return false unless target
options = { message: message, tagger: user_to_committer(user) } if message
GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do
rugged.tags.create(tag_name, target, options)
end
gitlab_shell.add_tag(path_with_namespace, tag_name, ref, message)
find_tag(tag_name)
end
def rm_branch(user, branch_name)
......
......@@ -43,9 +43,4 @@ class CreateBranchService < BaseService
out[:branch] = branch
out
end
def build_push_data(project, user, branch)
Gitlab::PushDataBuilder.
build(project, user, Gitlab::Git::BLANK_SHA, branch.target, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", [])
end
end
require_relative 'base_service'
class CreateTagService < BaseService
def execute(tag_name, ref, message, release_description = nil)
def execute(tag_name, target, message, release_description = nil)
valid_tag = Gitlab::GitRefValidator.validate(tag_name)
if valid_tag == false
return error('Tag name invalid')
end
return error('Tag name invalid') unless valid_tag
repository = project.repository
existing_tag = repository.find_tag(tag_name)
if existing_tag
return error('Tag already exists')
end
message.strip! if message
repository.add_tag(tag_name, ref, message)
new_tag = repository.find_tag(tag_name)
new_tag = nil
begin
new_tag = repository.add_tag(current_user, tag_name, target, message)
rescue Rugged::TagError
return error("Tag #{tag_name} already exists")
rescue GitHooksService::PreReceiveError
return error('Tag creation was rejected by Git hook')
end
if new_tag
push_data = create_push_data(project, current_user, new_tag)
EventCreateService.new.push(project, current_user, push_data)
project.execute_hooks(push_data.dup, :tag_push_hooks)
project.execute_services(push_data.dup, :tag_push_hooks)
CreateCommitBuildsService.new.execute(project, current_user, push_data)
if release_description
CreateReleaseService.new(@project, @current_user).
execute(tag_name, release_description)
end
success(new_tag)
success.merge(tag: new_tag)
else
error('Invalid reference name')
end
error("Target #{target} is invalid")
end
def success(branch)
out = super()
out[:tag] = branch
out
end
def create_push_data(project, user, tag)
commits = [project.commit(tag.target)].compact
Gitlab::PushDataBuilder.
build(project, user, Gitlab::Git::BLANK_SHA, tag.target, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", commits, tag.message)
end
end
......@@ -57,11 +57,11 @@ class Spinach::Features::ProjectCommitsTags < Spinach::FeatureSteps
end
step 'I should see new an error that tag ref is invalid' do
expect(page).to have_content 'Invalid reference name'
expect(page).to have_content 'Target foo is invalid'
end
step 'I should see new an error that tag already exists' do
expect(page).to have_content 'Tag already exists'
expect(page).to have_content 'Tag v1.0.0 already exists'
end
step "I visit tag 'v1.1.0' page" do
......
......@@ -79,24 +79,6 @@ module Gitlab
'rm-project', "#{name}.git"])
end
# Add repository tag from passed ref
#
# path - project path with namespace
# tag_name - new tag name
# ref - HEAD for new tag
# message - optional message for tag (annotated tag)
#
# Ex.
# add_tag("gitlab/gitlab-ci", "v4.0", "master")
# add_tag("gitlab/gitlab-ci", "v4.0", "master", "message")
#
def add_tag(path, tag_name, ref, message = nil)
cmd = %W(#{gitlab_shell_path}/bin/gitlab-projects create-tag #{path}.git
#{tag_name} #{ref})
cmd << message unless message.nil? || message.empty?
Gitlab::Utils.system_silent(cmd)
end
# Gc repository
#
# path - project path with namespace
......
......@@ -858,13 +858,30 @@ describe Repository, models: true do
end
describe '#add_tag' do
it 'adds a tag' do
expect(repository).to receive(:before_push_tag)
context 'with a valid target' do
let(:user) { build_stubbed(:user) }
expect_any_instance_of(Gitlab::Shell).to receive(:add_tag).
with(repository.path_with_namespace, '8.5', 'master', 'foo')
it 'creates the tag using rugged' do
expect(repository.rugged.tags).to receive(:create).
with('8.5', repository.commit('master').id,
hash_including(message: 'foo',
tagger: hash_including(name: user.name, email: user.email))).
and_call_original
repository.add_tag('8.5', 'master', 'foo')
repository.add_tag(user, '8.5', 'master', 'foo')
end
it 'returns a Gitlab::Git::Tag object' do
tag = repository.add_tag(user, '8.5', 'master', 'foo')
expect(tag).to be_a(Gitlab::Git::Tag)
end
end
context 'with an invalid target' do
it 'returns false' do
expect(repository.add_tag(user, '8.5', 'bar', 'foo')).to be false
end
end
end
......
......@@ -147,7 +147,7 @@ describe API::API, api: true do
tag_name: 'v8.0.0',
ref: 'master'
expect(response.status).to eq(400)
expect(json_response['message']).to eq('Tag already exists')
expect(json_response['message']).to eq('Tag v8.0.0 already exists')
end
it 'should return 400 if ref name is invalid' do
......@@ -155,7 +155,7 @@ describe API::API, api: true do
tag_name: 'mytag',
ref: 'foo'
expect(response.status).to eq(400)
expect(json_response['message']).to eq('Invalid reference name')
expect(json_response['message']).to eq('Target foo is invalid')
end
end
......
require 'spec_helper'
describe CreateTagService, services: true do
let(:project) { create(:project) }
let(:repository) { project.repository }
let(:user) { create(:user) }
let(:service) { described_class.new(project, user) }
describe '#execute' do
it 'creates the tag and returns success' do
response = service.execute('v42.42.42', 'master', 'Foo')
expect(response[:status]).to eq(:success)
expect(response[:tag]).to be_a Gitlab::Git::Tag
expect(response[:tag].name).to eq('v42.42.42')
end
context 'when target is invalid' do
it 'returns an error' do
response = service.execute('v1.1.0', 'foo', 'Foo')
expect(response).to eq(status: :error,
message: 'Target foo is invalid')
end
end
context 'when tag already exists' do
it 'returns an error' do
expect(repository).to receive(:add_tag).
with(user, 'v1.1.0', 'master', 'Foo').
and_raise(Rugged::TagError)
response = service.execute('v1.1.0', 'master', 'Foo')
expect(response).to eq(status: :error,
message: 'Tag v1.1.0 already exists')
end
end
context 'when pre-receive hook fails' do
it 'returns an error' do
expect(repository).to receive(:add_tag).
with(user, 'v1.1.0', 'master', 'Foo').
and_raise(GitHooksService::PreReceiveError)
response = service.execute('v1.1.0', 'master', 'Foo')
expect(response).to eq(status: :error,
message: 'Tag creation was rejected by Git hook')
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