summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG1
-rw-r--r--app/services/create_branch_service.rb4
-rw-r--r--app/services/create_tag_service.rb4
-rw-r--r--app/services/delete_branch_service.rb4
-rw-r--r--app/services/git_hooks_service.rb6
-rw-r--r--lib/gitlab/git/hook.rb13
-rw-r--r--spec/models/repository_spec.rb18
-rw-r--r--spec/services/create_tag_service_spec.rb4
-rw-r--r--spec/services/git_hooks_service_spec.rb10
9 files changed, 36 insertions, 28 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 5f593336895..3b89fa05801 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,6 +32,7 @@ v 8.10.0 (unreleased)
- Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w)
- Add basic system information like memory and disk usage to the admin panel
- Don't garbage collect commits that have related DB records like comments
+ - More descriptive message for git hooks and file locks
v 8.9.5 (unreleased)
- Improve the request / withdraw access button. !4860
diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb
index 9f4481a8153..cc128563437 100644
--- a/app/services/create_branch_service.rb
+++ b/app/services/create_branch_service.rb
@@ -34,8 +34,8 @@ class CreateBranchService < BaseService
else
error('Invalid reference name')
end
- rescue GitHooksService::PreReceiveError
- error('Branch creation was rejected by Git hook')
+ rescue GitHooksService::PreReceiveError => ex
+ error(ex.message)
end
def success(branch)
diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb
index 91ed0e354d0..bd8d982e1fb 100644
--- a/app/services/create_tag_service.rb
+++ b/app/services/create_tag_service.rb
@@ -13,8 +13,8 @@ class CreateTagService < BaseService
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')
+ rescue GitHooksService::PreReceiveError => ex
+ return error(ex.message)
end
if new_tag
diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb
index fae069ee4a5..752a7029952 100644
--- a/app/services/delete_branch_service.rb
+++ b/app/services/delete_branch_service.rb
@@ -30,8 +30,8 @@ class DeleteBranchService < BaseService
else
error('Failed to remove branch')
end
- rescue GitHooksService::PreReceiveError
- error('Branch deletion was rejected by Git hook')
+ rescue GitHooksService::PreReceiveError => ex
+ error(ex.message)
end
def error(message, return_code = 400)
diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb
index d7a0c25a044..172bd85dade 100644
--- a/app/services/git_hooks_service.rb
+++ b/app/services/git_hooks_service.rb
@@ -9,8 +9,10 @@ class GitHooksService
@ref = ref
%w(pre-receive update).each do |hook_name|
- unless run_hook(hook_name)
- raise PreReceiveError.new("Git operation was rejected by #{hook_name} hook")
+ status, message = run_hook(hook_name)
+
+ unless status
+ raise PreReceiveError, message
end
end
diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb
index 07b856ca64c..5415f4844d3 100644
--- a/lib/gitlab/git/hook.rb
+++ b/lib/gitlab/git/hook.rb
@@ -29,8 +29,8 @@ module Gitlab
def call_receive_hook(gl_id, oldrev, newrev, ref)
changes = [oldrev, newrev, ref].join(" ")
- # function will return true if succesful
exit_status = false
+ exit_message = nil
vars = {
'GL_ID' => gl_id,
@@ -41,7 +41,7 @@ module Gitlab
chdir: repo_path
}
- Open3.popen2(vars, path, options) do |stdin, _, wait_thr|
+ Open3.popen3(vars, path, options) do |stdin, _, stderr, wait_thr|
exit_status = true
stdin.sync = true
@@ -60,16 +60,21 @@ module Gitlab
unless wait_thr.value == 0
exit_status = false
+ exit_message = stderr.gets
end
end
- exit_status
+ [exit_status, exit_message]
end
def call_update_hook(gl_id, oldrev, newrev, ref)
+ status = nil
+
Dir.chdir(repo_path) do
- system({ 'GL_ID' => gl_id }, path, ref, oldrev, newrev)
+ status = system({ 'GL_ID' => gl_id }, path, ref, oldrev, newrev)
end
+
+ [status, nil]
end
end
end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index e753306a31f..7975fc64e59 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -308,14 +308,14 @@ describe Repository, models: true do
describe :add_branch do
context 'when pre hooks were successful' do
it 'should run without errors' do
- hook = double(trigger: true)
+ hook = double(trigger: [true, nil])
expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error
end
it 'should create the branch' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+ allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
branch = repository.add_branch(user, 'new_feature', 'master')
@@ -331,7 +331,7 @@ describe Repository, models: true do
context 'when pre hooks failed' do
it 'should get an error' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+ allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
repository.add_branch(user, 'new_feature', 'master')
@@ -339,7 +339,7 @@ describe Repository, models: true do
end
it 'should not create the branch' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+ allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
repository.add_branch(user, 'new_feature', 'master')
@@ -352,13 +352,13 @@ describe Repository, models: true do
describe :rm_branch do
context 'when pre hooks were successful' do
it 'should run without errors' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+ allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
end
it 'should delete the branch' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+ allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
expect { repository.rm_branch(user, 'feature') }.not_to raise_error
@@ -368,7 +368,7 @@ describe Repository, models: true do
context 'when pre hooks failed' do
it 'should get an error' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+ allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
repository.rm_branch(user, 'new_feature')
@@ -376,7 +376,7 @@ describe Repository, models: true do
end
it 'should not delete the branch' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+ allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
repository.rm_branch(user, 'feature')
@@ -408,7 +408,7 @@ describe Repository, models: true do
context 'when pre hooks failed' do
it 'should get an error' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+ allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
repository.commit_with_hooks(user, 'feature') { sample_commit.id }
diff --git a/spec/services/create_tag_service_spec.rb b/spec/services/create_tag_service_spec.rb
index 91f9e663b66..7dc43c50b0d 100644
--- a/spec/services/create_tag_service_spec.rb
+++ b/spec/services/create_tag_service_spec.rb
@@ -41,12 +41,12 @@ describe CreateTagService, services: true do
it 'returns an error' do
expect(repository).to receive(:add_tag).
with(user, 'v1.1.0', 'master', 'Foo').
- and_raise(GitHooksService::PreReceiveError)
+ and_raise(GitHooksService::PreReceiveError, 'something went wrong')
response = service.execute('v1.1.0', 'master', 'Foo')
expect(response).to eq(status: :error,
- message: 'Tag creation was rejected by Git hook')
+ message: 'something went wrong')
end
end
end
diff --git a/spec/services/git_hooks_service_spec.rb b/spec/services/git_hooks_service_spec.rb
index 6367ac832e8..3fc37a315c0 100644
--- a/spec/services/git_hooks_service_spec.rb
+++ b/spec/services/git_hooks_service_spec.rb
@@ -18,16 +18,16 @@ describe GitHooksService, services: true do
describe '#execute' do
context 'when receive hooks were successful' do
it 'should call post-receive hook' do
- hook = double(trigger: true)
+ hook = double(trigger: [true, nil])
expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
- expect(service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }).to eq(true)
+ expect(service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }).to eq([true, nil])
end
end
context 'when pre-receive hook failed' do
it 'should not call post-receive hook' do
- expect(service).to receive(:run_hook).with('pre-receive').and_return(false)
+ expect(service).to receive(:run_hook).with('pre-receive').and_return([false, ''])
expect(service).not_to receive(:run_hook).with('post-receive')
expect do
@@ -38,8 +38,8 @@ describe GitHooksService, services: true do
context 'when update hook failed' do
it 'should not call post-receive hook' do
- expect(service).to receive(:run_hook).with('pre-receive').and_return(true)
- expect(service).to receive(:run_hook).with('update').and_return(false)
+ expect(service).to receive(:run_hook).with('pre-receive').and_return([true, nil])
+ expect(service).to receive(:run_hook).with('update').and_return([false, ''])
expect(service).not_to receive(:run_hook).with('post-receive')
expect do
View Lorry Controller Status