Commit 44f89eaf authored by Rémy Coutable's avatar Rémy Coutable

Use Rugged's TagCollection#create instead of gitlab-shell's Repository#add_tag...

Use Rugged's TagCollection#create instead of gitlab-shell's Repository#add_tag for better performance
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 72e4bd5f
...@@ -13,6 +13,7 @@ v 8.8.0 (unreleased) ...@@ -13,6 +13,7 @@ v 8.8.0 (unreleased)
- Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea) - Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea)
- Added button to toggle whitespaces changes on diff view - Added button to toggle whitespaces changes on diff view
- Backport GitLab Enterprise support from EE - 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 - 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) - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
- Added multiple colors for labels in dropdowns when dups happen. - Added multiple colors for labels in dropdowns when dups happen.
......
...@@ -146,10 +146,17 @@ class Repository ...@@ -146,10 +146,17 @@ class Repository
find_branch(branch_name) find_branch(branch_name)
end end
def add_tag(tag_name, ref, message = nil) def add_tag(user, tag_name, ref, message = nil)
before_push_tag before_push_tag
gitlab_shell.add_tag(path_with_namespace, tag_name, ref, message) options = { message: message, tagger: user_to_committer(user) } if message
tag = rugged.tags.create(tag_name, ref, options)
if tag.annotated?
Gitlab::Git::Tag.new(tag_name, ref, tag.annotation.message)
else
Gitlab::Git::Tag.new(tag_name, ref)
end
end end
def rm_branch(user, branch_name) def rm_branch(user, branch_name)
......
...@@ -8,18 +8,17 @@ class CreateTagService < BaseService ...@@ -8,18 +8,17 @@ class CreateTagService < BaseService
end end
repository = project.repository repository = project.repository
existing_tag = repository.find_tag(tag_name)
if existing_tag
return error('Tag already exists')
end
message.strip! if message message.strip! if message
begin
new_tag = repository.add_tag(current_user, tag_name, ref, message)
rescue Rugged::TagError
return error("Tag #{tag_name} already exists")
rescue Rugged::ReferenceError
return error("Target #{ref} is invalid")
end
repository.add_tag(tag_name, ref, message)
new_tag = repository.find_tag(tag_name)
if new_tag
push_data = create_push_data(project, current_user, new_tag) push_data = create_push_data(project, current_user, new_tag)
EventCreateService.new.push(project, current_user, push_data) EventCreateService.new.push(project, current_user, push_data)
project.execute_hooks(push_data.dup, :tag_push_hooks) project.execute_hooks(push_data.dup, :tag_push_hooks)
project.execute_services(push_data.dup, :tag_push_hooks) project.execute_services(push_data.dup, :tag_push_hooks)
...@@ -31,9 +30,6 @@ class CreateTagService < BaseService ...@@ -31,9 +30,6 @@ class CreateTagService < BaseService
end end
success(new_tag) success(new_tag)
else
error('Invalid reference name')
end
end end
def success(branch) def success(branch)
......
...@@ -57,11 +57,11 @@ class Spinach::Features::ProjectCommitsTags < Spinach::FeatureSteps ...@@ -57,11 +57,11 @@ class Spinach::Features::ProjectCommitsTags < Spinach::FeatureSteps
end end
step 'I should see new an error that tag ref is invalid' do 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 end
step 'I should see new an error that tag already exists' do 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 end
step "I visit tag 'v1.1.0' page" do step "I visit tag 'v1.1.0' page" do
......
...@@ -79,24 +79,6 @@ module Gitlab ...@@ -79,24 +79,6 @@ module Gitlab
'rm-project', "#{name}.git"]) 'rm-project', "#{name}.git"])
end 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 # Gc repository
# #
# path - project path with namespace # path - project path with namespace
......
...@@ -859,12 +859,15 @@ describe Repository, models: true do ...@@ -859,12 +859,15 @@ describe Repository, models: true do
describe '#add_tag' do describe '#add_tag' do
it 'adds a tag' do it 'adds a tag' do
user = build_stubbed(:user)
expect(repository).to receive(:before_push_tag) expect(repository).to receive(:before_push_tag)
expect(repository.rugged.tags).to receive(:create).
with('8.5', 'master',
hash_including(message: 'foo',
tagger: hash_including(name: user.name, email: user.email))).
and_call_original
expect_any_instance_of(Gitlab::Shell).to receive(:add_tag). repository.add_tag(user, '8.5', 'master', 'foo')
with(repository.path_with_namespace, '8.5', 'master', 'foo')
repository.add_tag('8.5', 'master', 'foo')
end end
end end
......
...@@ -147,7 +147,7 @@ describe API::API, api: true do ...@@ -147,7 +147,7 @@ describe API::API, api: true do
tag_name: 'v8.0.0', tag_name: 'v8.0.0',
ref: 'master' ref: 'master'
expect(response.status).to eq(400) 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 end
it 'should return 400 if ref name is invalid' do it 'should return 400 if ref name is invalid' do
...@@ -155,7 +155,7 @@ describe API::API, api: true do ...@@ -155,7 +155,7 @@ describe API::API, api: true do
tag_name: 'mytag', tag_name: 'mytag',
ref: 'foo' ref: 'foo'
expect(response.status).to eq(400) 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
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