From 4e4866f2559262b3c858de15890eb864f18eeca8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 14 Aug 2015 16:00:36 +0200 Subject: [PATCH] Refactor pre/post receive commit services into one class --- app/services/commit_service.rb | 27 ++++++----- app/services/post_commit_service.rb | 71 ----------------------------- app/services/pre_commit_service.rb | 71 ----------------------------- lib/gitlab/git/hook.rb | 59 ++++++++++++++++++++++++ spec/support/test_env.rb | 2 +- 5 files changed, 76 insertions(+), 154 deletions(-) delete mode 100644 app/services/post_commit_service.rb delete mode 100644 app/services/pre_commit_service.rb create mode 100644 lib/gitlab/git/hook.rb diff --git a/app/services/commit_service.rb b/app/services/commit_service.rb index d158f103530..5000e73b473 100644 --- a/app/services/commit_service.rb +++ b/app/services/commit_service.rb @@ -4,48 +4,53 @@ class CommitService class PreReceiveError < StandardError; end class CommitError < StandardError; end - def self.transaction(project, current_user, ref) + def self.transaction(project, current_user, branch) repository = project.repository path_to_repo = repository.path_to_repo empty_repo = repository.empty? + oldrev = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + branch + gl_id = Gitlab::ShellEnv.gl_id(current_user) # Create temporary ref random_string = SecureRandom.hex tmp_ref = "refs/tmp/#{random_string}/head" unless empty_repo - target = repository.find_branch(ref).target - repository.rugged.references.create(tmp_ref, target) + oldrev = repository.find_branch(branch).target + repository.rugged.references.create(tmp_ref, oldrev) end # Make commit in tmp ref - sha = yield(tmp_ref) + newrev = yield(tmp_ref) - unless sha + unless newrev raise CommitError.new('Failed to create commit') end # Run GitLab pre-receive hook - status = PreCommitService.new(project, current_user).execute(sha, ref) + pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', path_to_repo) + status = pre_receive_hook.trigger(gl_id, oldrev, newrev, ref) if status if empty_repo # Create branch - repository.rugged.references.create(Gitlab::Git::BRANCH_REF_PREFIX + ref, sha) + repository.rugged.references.create(ref, newrev) else # Update head - current_target = repository.find_branch(ref).target + current_head = repository.find_branch(branch).target # Make sure target branch was not changed during pre-receive hook - if current_target == target - repository.rugged.references.update(Gitlab::Git::BRANCH_REF_PREFIX + ref, sha) + if current_head == oldrev + repository.rugged.references.update(ref, newrev) else raise CommitError.new('Commit was rejected because branch received new push') end end # Run GitLab post receive hook - PostCommitService.new(project, current_user).execute(sha, ref) + post_receive_hook = Gitlab::Git::Hook.new('post-receive', path_to_repo) + status = post_receive_hook.trigger(gl_id, oldrev, newrev, ref) else # Remove tmp ref and return error to user repository.rugged.references.delete(tmp_ref) diff --git a/app/services/post_commit_service.rb b/app/services/post_commit_service.rb deleted file mode 100644 index 8592c8d238b..00000000000 --- a/app/services/post_commit_service.rb +++ /dev/null @@ -1,71 +0,0 @@ -class PostCommitService < BaseService - include Gitlab::Popen - - attr_reader :changes, :repo_path - - def execute(sha, branch) - commit = repository.commit(sha) - full_ref = Gitlab::Git::BRANCH_REF_PREFIX + branch - old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA - @changes = "#{old_sha} #{sha} #{full_ref}" - @repo_path = repository.path_to_repo - - post_receive - end - - private - - def post_receive - hook = hook_file('post-receive', repo_path) - return true if hook.nil? - call_receive_hook(hook) - end - - def call_receive_hook(hook) - # function will return true if succesful - exit_status = false - - vars = { - 'GL_ID' => Gitlab::ShellEnv.gl_id(current_user), - 'PWD' => repo_path - } - - options = { - chdir: repo_path - } - - # we combine both stdout and stderr as we don't know what stream - # will be used by the custom hook - Open3.popen2e(vars, hook, options) do |stdin, stdout_stderr, wait_thr| - exit_status = true - stdin.sync = true - - # in git, pre- and post- receive hooks may just exit without - # reading stdin. We catch the exception to avoid a broken pipe - # warning - begin - # inject all the changes as stdin to the hook - changes.lines do |line| - stdin.puts line - end - rescue Errno::EPIPE - end - - # need to close stdin before reading stdout - stdin.close - - # only output stdut_stderr if scripts doesn't return 0 - unless wait_thr.value == 0 - exit_status = false - end - end - - exit_status - end - - def hook_file(hook_type, repo_path) - hook_path = File.join(repo_path.strip, 'hooks') - hook_file = "#{hook_path}/#{hook_type}" - hook_file if File.exist?(hook_file) - end -end diff --git a/app/services/pre_commit_service.rb b/app/services/pre_commit_service.rb deleted file mode 100644 index ed8331ddce4..00000000000 --- a/app/services/pre_commit_service.rb +++ /dev/null @@ -1,71 +0,0 @@ -class PreCommitService < BaseService - include Gitlab::Popen - - attr_reader :changes, :repo_path - - def execute(sha, branch) - commit = repository.commit(sha) - full_ref = Gitlab::Git::BRANCH_REF_PREFIX + branch - old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA - @changes = "#{old_sha} #{sha} #{full_ref}" - @repo_path = repository.path_to_repo - - pre_receive - end - - private - - def pre_receive - hook = hook_file('pre-receive', repo_path) - return true if hook.nil? - call_receive_hook(hook) - end - - def call_receive_hook(hook) - # function will return true if succesful - exit_status = false - - vars = { - 'GL_ID' => Gitlab::ShellEnv.gl_id(current_user), - 'PWD' => repo_path - } - - options = { - chdir: repo_path - } - - # we combine both stdout and stderr as we don't know what stream - # will be used by the custom hook - Open3.popen2e(vars, hook, options) do |stdin, stdout_stderr, wait_thr| - exit_status = true - stdin.sync = true - - # in git, pre- and post- receive hooks may just exit without - # reading stdin. We catch the exception to avoid a broken pipe - # warning - begin - # inject all the changes as stdin to the hook - changes.lines do |line| - stdin.puts line - end - rescue Errno::EPIPE - end - - # need to close stdin before reading stdout - stdin.close - - # only output stdut_stderr if scripts doesn't return 0 - unless wait_thr.value == 0 - exit_status = false - end - end - - exit_status - end - - def hook_file(hook_type, repo_path) - hook_path = File.join(repo_path.strip, 'hooks') - hook_file = "#{hook_path}/#{hook_type}" - hook_file if File.exist?(hook_file) - end -end diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb new file mode 100644 index 00000000000..dd393fe09d2 --- /dev/null +++ b/lib/gitlab/git/hook.rb @@ -0,0 +1,59 @@ +module Gitlab + module Git + class Hook + attr_reader :name, :repo_path, :path + + def initialize(name, repo_path) + @name = name + @repo_path = repo_path + @path = File.join(repo_path.strip, 'hooks', name) + end + + def exists? + File.exist?(path) + end + + def trigger(gl_id, oldrev, newrev, ref) + return true unless exists? + + changes = [oldrev, newrev, ref].join(" ") + + # function will return true if succesful + exit_status = false + + vars = { + 'GL_ID' => gl_id, + 'PWD' => repo_path + } + + options = { + chdir: repo_path + } + + Open3.popen2(vars, path, options) do |stdin, _, wait_thr| + exit_status = true + stdin.sync = true + + # in git, pre- and post- receive hooks may just exit without + # reading stdin. We catch the exception to avoid a broken pipe + # warning + begin + # inject all the changes as stdin to the hook + changes.lines do |line| + stdin.puts line + end + rescue Errno::EPIPE + end + + stdin.close + + unless wait_thr.value == 0 + exit_status = false + end + end + + exit_status + end + end + end +end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 3a678db2dfd..3eab74ba986 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -58,7 +58,7 @@ module TestEnv end def disable_pre_receive - allow_any_instance_of(PreCommitService).to receive(:execute).and_return(true) + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true) end # Clean /tmp/tests