From 34690142bf42e0a3d48b1b30075387abefe86318 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 13 Aug 2015 12:57:24 +0200 Subject: [PATCH 01/11] Implement commit transaction with pre-receive and post-receive hooks for web editor Signed-off-by: Dmitriy Zaporozhets --- Gemfile | 2 +- Gemfile.lock | 6 +-- app/models/repository.rb | 2 +- app/services/commit_service.rb | 37 +++++++++++++++ app/services/files/base_service.rb | 5 -- app/services/files/create_service.rb | 4 +- app/services/files/delete_service.rb | 4 +- app/services/files/update_service.rb | 4 +- app/services/pre_commit_service.rb | 71 ++++++++++++++++++++++++++++ 9 files changed, 122 insertions(+), 13 deletions(-) create mode 100644 app/services/commit_service.rb create mode 100644 app/services/pre_commit_service.rb diff --git a/Gemfile b/Gemfile index 2483a7d24a..77bb02a50a 100644 --- a/Gemfile +++ b/Gemfile @@ -38,7 +38,7 @@ gem "browser", '~> 0.8.0' # Extracting information from a git repository # Provide access to Gitlab::Git library -gem "gitlab_git", '~> 7.2.12' +gem "gitlab_git", '~> 7.2.13' # Ruby/Rack Git Smart-HTTP Server Handler # GitLab fork with a lot of changes (improved thread-safety, better memory usage etc) diff --git a/Gemfile.lock b/Gemfile.lock index 335f6777c9..2141067183 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -271,7 +271,7 @@ GEM mime-types (~> 1.19) gitlab_emoji (0.1.0) gemojione (~> 2.0) - gitlab_git (7.2.12) + gitlab_git (7.2.13) activesupport (~> 4.0) charlock_holmes (~> 0.6) gitlab-linguist (~> 3.0) @@ -783,7 +783,7 @@ DEPENDENCIES gitlab-grack (~> 2.0.2) gitlab-linguist (~> 3.0.1) gitlab_emoji (~> 0.1) - gitlab_git (~> 7.2.12) + gitlab_git (~> 7.2.13) gitlab_meta (= 7.0) gitlab_omniauth-ldap (= 1.2.1) gollum-lib (~> 4.0.2) @@ -875,4 +875,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.10.4 + 1.10.6 diff --git a/app/models/repository.rb b/app/models/repository.rb index 46efbede2a..32b3541907 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -373,7 +373,7 @@ class Repository options[:author] = committer options[:commit] = { message: message, - branch: ref + branch: ref, } options[:file] = { diff --git a/app/services/commit_service.rb b/app/services/commit_service.rb new file mode 100644 index 0000000000..a5603f2669 --- /dev/null +++ b/app/services/commit_service.rb @@ -0,0 +1,37 @@ +require 'securerandom' + +class CommitService + def self.transaction(project, current_user, ref) + repository = project.repository + path_to_repo = repository.path_to_repo + + # Create temporary ref + random_string = SecureRandom.hex + tmp_ref = "refs/tmp/#{random_string}/head" + target = repository.find_branch(ref).target + repository.rugged.references.create(tmp_ref, target) + + # Make commit in tmp ref + sha = yield(tmp_ref) + + unless sha + raise 'Failed to create commit' + end + + # Run GitLab pre-receive hook + status = PreCommitService.new(project, current_user).execute(sha, ref) + + if status + # Update head + repository.rugged.references.update(Gitlab::Git::BRANCH_REF_PREFIX + ref, sha) + + # Run GitLab post receive hook + PostCommitService.new(project, current_user).execute(sha, ref) + else + # Remove tmp ref and return error to user + repository.rugged.references.delete(tmp_ref) + + raise 'Commit was rejected by pre-reveive hook' + end + end +end diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index d7b40ee890..1f7b92e222 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -22,7 +22,6 @@ module Files end if sha = commit - after_commit(sha, @target_branch) success else error("Something went wrong. Your changes were not committed") @@ -33,10 +32,6 @@ module Files private - def after_commit(sha, branch) - PostCommitService.new(project, current_user).execute(sha, branch) - end - def current_branch @current_branch ||= params[:current_branch] end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 91d715b2d6..3e00864f00 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -3,7 +3,9 @@ require_relative "base_service" module Files class CreateService < Files::BaseService def commit - repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) + CommitService.transaction(project, current_user, @target_branch) do |tmp_ref| + repository.commit_file(current_user, @file_path, @file_content, @commit_message, tmp_ref) + end end def validate diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index 27c881c343..d61ca31cf9 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -3,7 +3,9 @@ require_relative "base_service" module Files class DeleteService < Files::BaseService def commit - repository.remove_file(current_user, @file_path, @commit_message, @target_branch) + CommitService.transaction(project, current_user, @target_branch) do |tmp_ref| + repository.remove_file(current_user, @file_path, @commit_message, tmp_ref) + end end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index a20903c6f0..69212b3699 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -3,7 +3,9 @@ require_relative "base_service" module Files class UpdateService < Files::BaseService def commit - repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) + CommitService.transaction(project, current_user, @target_branch) do |tmp_ref| + repository.commit_file(current_user, @file_path, @file_content, @commit_message, tmp_ref) + end end end end diff --git a/app/services/pre_commit_service.rb b/app/services/pre_commit_service.rb new file mode 100644 index 0000000000..ed8331ddce --- /dev/null +++ b/app/services/pre_commit_service.rb @@ -0,0 +1,71 @@ +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 From c8614cbbe76a97dc70576451a75907059b4f876c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 13 Aug 2015 14:29:13 +0200 Subject: [PATCH 02/11] Capture pre-receive exception Signed-off-by: Dmitriy Zaporozhets --- app/services/commit_service.rb | 24 ++++++++++++++++++------ app/services/files/base_service.rb | 2 +- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/services/commit_service.rb b/app/services/commit_service.rb index a5603f2669..92d1585840 100644 --- a/app/services/commit_service.rb +++ b/app/services/commit_service.rb @@ -1,29 +1,41 @@ require 'securerandom' class CommitService + class PreReceiveError < StandardError; end + class CommitError < StandardError; end + def self.transaction(project, current_user, ref) repository = project.repository path_to_repo = repository.path_to_repo + empty_repo = repository.empty? # Create temporary ref random_string = SecureRandom.hex tmp_ref = "refs/tmp/#{random_string}/head" - target = repository.find_branch(ref).target - repository.rugged.references.create(tmp_ref, target) + + unless empty_repo + target = repository.find_branch(ref).target + repository.rugged.references.create(tmp_ref, target) + end # Make commit in tmp ref sha = yield(tmp_ref) unless sha - raise 'Failed to create commit' + raise CommitError.new('Failed to create commit') end # Run GitLab pre-receive hook status = PreCommitService.new(project, current_user).execute(sha, ref) if status - # Update head - repository.rugged.references.update(Gitlab::Git::BRANCH_REF_PREFIX + ref, sha) + if empty_repo + # Create branch + repository.rugged.references.create(Gitlab::Git::BRANCH_REF_PREFIX + ref, sha) + else + # Update head + repository.rugged.references.update(Gitlab::Git::BRANCH_REF_PREFIX + ref, sha) + end # Run GitLab post receive hook PostCommitService.new(project, current_user).execute(sha, ref) @@ -31,7 +43,7 @@ class CommitService # Remove tmp ref and return error to user repository.rugged.references.delete(tmp_ref) - raise 'Commit was rejected by pre-reveive hook' + raise PreReceiveError.new('Commit was rejected by pre-reveive hook') end end end diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 1f7b92e222..507e21f581 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -26,7 +26,7 @@ module Files else error("Something went wrong. Your changes were not committed") end - rescue ValidationError => ex + rescue CommitService::CommitError, CommitService::PreReceiveError, ValidationError => ex error(ex.message) end From 9011a3223448a68a77a599c6a38fb338ccfa3f1e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 14 Aug 2015 13:27:51 +0200 Subject: [PATCH 03/11] Disable pre-receive check in test env Signed-off-by: Dmitriy Zaporozhets --- spec/support/test_env.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 8dc687c358..a2e8f9054a 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -40,6 +40,9 @@ module TestEnv # Setup GitLab shell for test instance setup_gitlab_shell + # Skip pre-receive check so we can use web editor + disable_pre_receive + # Create repository for FactoryGirl.create(:project) setup_factory_repo @@ -57,6 +60,10 @@ module TestEnv and_call_original end + def disable_pre_receive + allow_any_instance_of(PreCommitService).to receive(:execute).and_return(true) + end + # Clean /tmp/tests # # Keeps gitlab-shell and gitlab-test From 9649f9387a4ef3c40bc2720053b716c987255363 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 14 Aug 2015 15:08:31 +0200 Subject: [PATCH 04/11] Fix tests for web editor --- features/support/env.rb | 4 ++++ spec/support/test_env.rb | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/features/support/env.rb b/features/support/env.rb index 672251af08..62c80b9c94 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -28,5 +28,9 @@ Spinach.hooks.before_run do RSpec::Mocks.setup TestEnv.init(mailer: false) + # skip pre-receive hook check so we can use + # web editor and merge + TestEnv.disable_pre_receive + include FactoryGirl::Syntax::Methods end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index a2e8f9054a..3a678db2df 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -40,9 +40,6 @@ module TestEnv # Setup GitLab shell for test instance setup_gitlab_shell - # Skip pre-receive check so we can use web editor - disable_pre_receive - # Create repository for FactoryGirl.create(:project) setup_factory_repo From bacad39ef984b99520f8d2a7921acd5c8bdab1ef Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 14 Aug 2015 15:22:48 +0200 Subject: [PATCH 05/11] Make sure target has not changed during pre-receive hook --- app/services/commit_service.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/services/commit_service.rb b/app/services/commit_service.rb index 92d1585840..d158f10353 100644 --- a/app/services/commit_service.rb +++ b/app/services/commit_service.rb @@ -34,7 +34,14 @@ class CommitService repository.rugged.references.create(Gitlab::Git::BRANCH_REF_PREFIX + ref, sha) else # Update head - repository.rugged.references.update(Gitlab::Git::BRANCH_REF_PREFIX + ref, sha) + current_target = repository.find_branch(ref).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) + else + raise CommitError.new('Commit was rejected because branch received new push') + end end # Run GitLab post receive hook From 4e4866f2559262b3c858de15890eb864f18eeca8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 14 Aug 2015 16:00:36 +0200 Subject: [PATCH 06/11] 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 d158f10353..5000e73b47 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 8592c8d238..0000000000 --- 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 ed8331ddce..0000000000 --- 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 0000000000..dd393fe09d --- /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 3a678db2df..3eab74ba98 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 From f4149bcddca9c0e7aac078b3e7c198f5624ea107 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 14 Aug 2015 16:23:40 +0200 Subject: [PATCH 07/11] Refactor how repository makes commit with pre/post receive hooks --- app/models/repository.rb | 119 ++++++++++++++++++++------- app/services/commit_service.rb | 56 ------------- app/services/files/base_service.rb | 2 +- app/services/files/create_service.rb | 4 +- app/services/files/delete_service.rb | 4 +- app/services/files/update_service.rb | 4 +- 6 files changed, 94 insertions(+), 95 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 32b3541907..99b6cd3cad 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1,4 +1,9 @@ +require 'securerandom' + class Repository + class PreReceiveError < StandardError; end + class CommitError < StandardError; end + include Gitlab::ShellAdapter attr_accessor :raw_repository, :path_with_namespace, :project @@ -364,43 +369,47 @@ class Repository @root_ref ||= raw_repository.root_ref end - def commit_file(user, path, content, message, ref) - path[0] = '' if path[0] == '/' + def commit_file(user, path, content, message, branch) + commit_with_hooks(user, branch) do |ref| + path[0] = '' if path[0] == '/' - committer = user_to_comitter(user) - options = {} - options[:committer] = committer - options[:author] = committer - options[:commit] = { - message: message, - branch: ref, - } + committer = user_to_comitter(user) + options = {} + options[:committer] = committer + options[:author] = committer + options[:commit] = { + message: message, + branch: ref, + } - options[:file] = { - content: content, - path: path - } + options[:file] = { + content: content, + path: path + } - Gitlab::Git::Blob.commit(raw_repository, options) + Gitlab::Git::Blob.commit(raw_repository, options) + end end - def remove_file(user, path, message, ref) - path[0] = '' if path[0] == '/' + def remove_file(user, path, message, branch) + commit_with_hooks(user, branch) do |branch| + path[0] = '' if path[0] == '/' - committer = user_to_comitter(user) - options = {} - options[:committer] = committer - options[:author] = committer - options[:commit] = { - message: message, - branch: ref - } + committer = user_to_comitter(user) + options = {} + options[:committer] = committer + options[:author] = committer + options[:commit] = { + message: message, + branch: ref + } - options[:file] = { - path: path - } + options[:file] = { + path: path + } - Gitlab::Git::Blob.remove(raw_repository, options) + Gitlab::Git::Blob.remove(raw_repository, options) + end end def user_to_comitter(user) @@ -479,6 +488,58 @@ class Repository Gitlab::Popen.popen(args, path_to_repo) end + def commit_with_hooks(current_user, branch) + 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? + oldrev = find_branch(branch).target + rugged.references.create(tmp_ref, oldrev) + end + + # Make commit in tmp ref + newrev = yield(tmp_ref) + + unless newrev + raise CommitError.new('Failed to create commit') + end + + # Run GitLab pre-receive hook + 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? + # Create branch + rugged.references.create(ref, newrev) + else + # Update head + current_head = find_branch(branch).target + + # Make sure target branch was not changed during pre-receive hook + if current_head == oldrev + rugged.references.update(ref, newrev) + else + raise CommitError.new('Commit was rejected because branch received new push') + end + end + + # Run GitLab post receive hook + 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 + rugged.references.delete(tmp_ref) + + raise PreReceiveError.new('Commit was rejected by pre-reveive hook') + end + end + private def cache diff --git a/app/services/commit_service.rb b/app/services/commit_service.rb index 5000e73b47..c77da061a9 100644 --- a/app/services/commit_service.rb +++ b/app/services/commit_service.rb @@ -1,61 +1,5 @@ -require 'securerandom' class CommitService - class PreReceiveError < StandardError; end - class CommitError < StandardError; end - 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 - oldrev = repository.find_branch(branch).target - repository.rugged.references.create(tmp_ref, oldrev) - end - - # Make commit in tmp ref - newrev = yield(tmp_ref) - - unless newrev - raise CommitError.new('Failed to create commit') - end - - # Run GitLab pre-receive hook - 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(ref, newrev) - else - # Update head - current_head = repository.find_branch(branch).target - - # Make sure target branch was not changed during pre-receive hook - 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 - 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) - - raise PreReceiveError.new('Commit was rejected by pre-reveive hook') - end end end diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 507e21f581..7aecee217d 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -26,7 +26,7 @@ module Files else error("Something went wrong. Your changes were not committed") end - rescue CommitService::CommitError, CommitService::PreReceiveError, ValidationError => ex + rescue Repository::CommitError, Repository::PreReceiveError, ValidationError => ex error(ex.message) end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 3e00864f00..91d715b2d6 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -3,9 +3,7 @@ require_relative "base_service" module Files class CreateService < Files::BaseService def commit - CommitService.transaction(project, current_user, @target_branch) do |tmp_ref| - repository.commit_file(current_user, @file_path, @file_content, @commit_message, tmp_ref) - end + repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) end def validate diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index d61ca31cf9..27c881c343 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -3,9 +3,7 @@ require_relative "base_service" module Files class DeleteService < Files::BaseService def commit - CommitService.transaction(project, current_user, @target_branch) do |tmp_ref| - repository.remove_file(current_user, @file_path, @commit_message, tmp_ref) - end + repository.remove_file(current_user, @file_path, @commit_message, @target_branch) end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 69212b3699..a20903c6f0 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -3,9 +3,7 @@ require_relative "base_service" module Files class UpdateService < Files::BaseService def commit - CommitService.transaction(project, current_user, @target_branch) do |tmp_ref| - repository.commit_file(current_user, @file_path, @file_content, @commit_message, tmp_ref) - end + repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) end end end From 9ea37cda3fa5e0aacf265bcb7739c1d4240f3bdc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 14 Aug 2015 17:52:49 +0200 Subject: [PATCH 08/11] merge with support of pre-receive and post-receive hooks Signed-off-by: Dmitriy Zaporozhets --- app/models/repository.rb | 18 ++++++++++-------- app/services/merge_requests/merge_service.rb | 14 ++------------ 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 99b6cd3cad..f2f75e0e0f 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -392,7 +392,7 @@ class Repository end def remove_file(user, path, message, branch) - commit_with_hooks(user, branch) do |branch| + commit_with_hooks(user, branch) do |ref| path[0] = '' if path[0] == '/' committer = user_to_comitter(user) @@ -431,7 +431,7 @@ class Repository end end - def merge(source_sha, target_branch, options = {}) + def merge(user, source_sha, target_branch, options = {}) our_commit = rugged.branches[target_branch].target their_commit = rugged.lookup(source_sha) @@ -441,13 +441,15 @@ class Repository merge_index = rugged.merge_commits(our_commit, their_commit) return false if merge_index.conflicts? - actual_options = options.merge( - parents: [our_commit, their_commit], - tree: merge_index.write_tree(rugged), - update_ref: "refs/heads/#{target_branch}" - ) + commit_with_hooks(user, target_branch) do |ref| + actual_options = options.merge( + parents: [our_commit, their_commit], + tree: merge_index.write_tree(rugged), + update_ref: ref + ) - Rugged::Commit.create(rugged, actual_options) + Rugged::Commit.create(rugged, actual_options) + end end def search_files(query, ref) diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 2107529a21..98a67c0bc9 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -17,7 +17,7 @@ module MergeRequests end merge_request.in_locked_state do - if merge_changes + if commit after_merge success else @@ -28,12 +28,6 @@ module MergeRequests private - def merge_changes - if sha = commit - after_commit(sha, merge_request.target_branch) - end - end - def commit committer = repository.user_to_comitter(current_user) @@ -43,11 +37,7 @@ module MergeRequests committer: committer } - repository.merge(merge_request.source_sha, merge_request.target_branch, options) - end - - def after_commit(sha, branch) - PostCommitService.new(project, current_user).execute(sha, branch) + repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options) end def after_merge From a608e45e6b662df438a0884a997c53c5c9125a47 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 14 Aug 2015 17:56:36 +0200 Subject: [PATCH 09/11] Remove unnecessary file Signed-off-by: Dmitriy Zaporozhets --- app/services/commit_service.rb | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 app/services/commit_service.rb diff --git a/app/services/commit_service.rb b/app/services/commit_service.rb deleted file mode 100644 index c77da061a9..0000000000 --- a/app/services/commit_service.rb +++ /dev/null @@ -1,5 +0,0 @@ - -class CommitService - def self.transaction(project, current_user, branch) - end -end From 3b93856e131c3539650444356a259dd93f65cd26 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 17 Aug 2015 10:24:25 +0200 Subject: [PATCH 10/11] save repo empty state into variable for proper result Signed-off-by: Dmitriy Zaporozhets --- app/models/repository.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index f2f75e0e0f..a1f2ea1d7e 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -494,12 +494,13 @@ class Repository oldrev = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + branch gl_id = Gitlab::ShellEnv.gl_id(current_user) + was_empty = empty? # Create temporary ref random_string = SecureRandom.hex tmp_ref = "refs/tmp/#{random_string}/head" - unless empty? + unless was_empty oldrev = find_branch(branch).target rugged.references.create(tmp_ref, oldrev) end @@ -516,7 +517,7 @@ class Repository status = pre_receive_hook.trigger(gl_id, oldrev, newrev, ref) if status - if empty? + if was_empty # Create branch rugged.references.create(ref, newrev) else From e02cff19ed158637bd0199f954f6b40d431e45a4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 17 Aug 2015 10:47:29 +0200 Subject: [PATCH 11/11] Fix typo in text when raise pre-receive exception Signed-off-by: Dmitriy Zaporozhets --- app/models/repository.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index a1f2ea1d7e..ea298d88f2 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -539,7 +539,7 @@ class Repository # Remove tmp ref and return error to user rugged.references.delete(tmp_ref) - raise PreReceiveError.new('Commit was rejected by pre-reveive hook') + raise PreReceiveError.new('Commit was rejected by pre-receive hook') end end