From 501ce37f838205cfd588eed47d57ca4a90c68e51 Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Mon, 18 Jul 2016 17:06:49 +0300
Subject: [PATCH] Fix of 'Commits being passed to custom hooks are already
reachable when using the UI'
---
CHANGELOG | 2 +
app/models/repository.rb | 72 +++++++++++----------------
app/services/create_branch_service.rb | 28 +++++------
3 files changed, 43 insertions(+), 59 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index 1b35a67832c64..56b7d2a485f1a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
Please view this file on the master branch, on stable branches it's out of date.
+v 8.11.0 (unreleased)
+ - Fix of 'Commits being passed to custom hooks are already reachable when using the UI'
v 8.10.0 (unreleased)
- Fix profile activity heatmap to show correct day name (eanplatter)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 09487b62f989f..c187bad39ad60 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -704,6 +704,7 @@ def commit_dir(user, path, message, branch)
options[:commit] = {
message: message,
branch: ref,
+ update_ref: false,
}
raw_repository.mkdir(path, options)
@@ -719,6 +720,7 @@ def commit_file(user, path, content, message, branch, update)
options[:commit] = {
message: message,
branch: ref,
+ update_ref: false,
}
options[:file] = {
@@ -739,7 +741,8 @@ def remove_file(user, path, message, branch)
options[:author] = committer
options[:commit] = {
message: message,
- branch: ref
+ branch: ref,
+ update_ref: false,
}
options[:file] = {
@@ -779,11 +782,10 @@ def merge(user, merge_request, options = {})
merge_index = rugged.merge_commits(our_commit, their_commit)
return false if merge_index.conflicts?
- commit_with_hooks(user, merge_request.target_branch) do |tmp_ref|
+ commit_with_hooks(user, merge_request.target_branch) do
actual_options = options.merge(
parents: [our_commit, their_commit],
tree: merge_index.write_tree(rugged),
- update_ref: tmp_ref
)
commit_id = Rugged::Commit.create(rugged, actual_options)
@@ -798,15 +800,14 @@ def revert(user, commit, base_branch, revert_tree_id = nil)
return false unless revert_tree_id
- commit_with_hooks(user, base_branch) do |ref|
+ commit_with_hooks(user, base_branch) do
committer = user_to_committer(user)
source_sha = Rugged::Commit.create(rugged,
message: commit.revert_message,
author: committer,
committer: committer,
tree: revert_tree_id,
- parents: [rugged.lookup(source_sha)],
- update_ref: ref)
+ parents: [rugged.lookup(source_sha)])
end
end
@@ -816,7 +817,7 @@ def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil)
return false unless cherry_pick_tree_id
- commit_with_hooks(user, base_branch) do |ref|
+ commit_with_hooks(user, base_branch) do
committer = user_to_committer(user)
source_sha = Rugged::Commit.create(rugged,
message: commit.message,
@@ -827,8 +828,7 @@ def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil)
},
committer: committer,
tree: cherry_pick_tree_id,
- parents: [rugged.lookup(source_sha)],
- update_ref: ref)
+ parents: [rugged.lookup(source_sha)])
end
end
@@ -929,20 +929,6 @@ def fetch_ref(source_path, source_ref, target_ref)
Gitlab::Popen.popen(args, path_to_repo)
end
- def with_tmp_ref(oldrev = nil)
- random_string = SecureRandom.hex
- tmp_ref = "refs/tmp/#{random_string}/head"
-
- if oldrev && !Gitlab::Git.blank_ref?(oldrev)
- rugged.references.create(tmp_ref, oldrev)
- end
-
- # Make commit in tmp ref
- yield(tmp_ref)
- ensure
- rugged.references.delete(tmp_ref) rescue nil
- end
-
def commit_with_hooks(current_user, branch)
update_autocrlf_option
@@ -955,33 +941,31 @@ def commit_with_hooks(current_user, branch)
oldrev = target_branch.target
end
- with_tmp_ref(oldrev) do |tmp_ref|
- # Make commit in tmp ref
- newrev = yield(tmp_ref)
+ # Make commit
+ newrev = yield(ref)
- unless newrev
- raise CommitError.new('Failed to create commit')
- end
+ unless newrev
+ raise CommitError.new('Failed to create commit')
+ end
+
+ GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
+ if was_empty || !target_branch
+ # Create branch
+ rugged.references.create(ref, newrev)
+ else
+ # Update head
+ current_head = find_branch(branch).target
- GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
- if was_empty || !target_branch
- # Create branch
- rugged.references.create(ref, newrev)
+ # Make sure target branch was not changed during pre-receive hook
+ if current_head == oldrev
+ rugged.references.update(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
+ raise CommitError.new('Commit was rejected because branch received new push')
end
end
-
- newrev
end
+
+ newrev
end
def ls_files(ref)
diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb
index d874582d54ffe..757fc35a78fdf 100644
--- a/app/services/create_branch_service.rb
+++ b/app/services/create_branch_service.rb
@@ -15,21 +15,19 @@ def execute(branch_name, ref, source_project: @project)
return error('Branch already exists')
end
- new_branch = nil
-
- if source_project != @project
- repository.with_tmp_ref do |tmp_ref|
- repository.fetch_ref(
- source_project.repository.path_to_repo,
- "refs/heads/#{ref}",
- tmp_ref
- )
-
- new_branch = repository.add_branch(current_user, branch_name, tmp_ref)
- end
- else
- new_branch = repository.add_branch(current_user, branch_name, ref)
- end
+ new_branch = if source_project != @project
+ repository.fetch_ref(
+ source_project.repository.path_to_repo,
+ "refs/heads/#{ref}",
+ "refs/heads/#{branch_name}"
+ )
+
+ repository.after_create_branch
+
+ repository.find_branch(branch_name)
+ else
+ repository.add_branch(current_user, branch_name, ref)
+ end
if new_branch
success(new_branch)
--
GitLab