Commit 5d449d4a authored by Valery Sizov's avatar Valery Sizov

Merge branch 'backport_from_ee_easy_commit_creation' into 'master'

Fix of 'Commits being passed to custom hooks are already reachable when using the UI'

This is back-port of https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/563

## What does this MR do?
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19771
This MR changes the way how we create commit, we create it directly in the branch and we don't use temporary ref anymore. This also works similar to how `git-receive-pack` works so it's a natural way to create commit.


## What are the relevant issue numbers?
https://gitlab.com/gitlab-org/gitlab-ce/issues/19771

See merge request !5337
parents a27212ab 501ce37f
Please view this file on the master branch, on stable branches it's out of date. 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) v 8.10.0 (unreleased)
- Fix profile activity heatmap to show correct day name (eanplatter) - Fix profile activity heatmap to show correct day name (eanplatter)
......
...@@ -704,6 +704,7 @@ class Repository ...@@ -704,6 +704,7 @@ class Repository
options[:commit] = { options[:commit] = {
message: message, message: message,
branch: ref, branch: ref,
update_ref: false,
} }
raw_repository.mkdir(path, options) raw_repository.mkdir(path, options)
...@@ -719,6 +720,7 @@ class Repository ...@@ -719,6 +720,7 @@ class Repository
options[:commit] = { options[:commit] = {
message: message, message: message,
branch: ref, branch: ref,
update_ref: false,
} }
options[:file] = { options[:file] = {
...@@ -739,7 +741,8 @@ class Repository ...@@ -739,7 +741,8 @@ class Repository
options[:author] = committer options[:author] = committer
options[:commit] = { options[:commit] = {
message: message, message: message,
branch: ref branch: ref,
update_ref: false,
} }
options[:file] = { options[:file] = {
...@@ -779,11 +782,10 @@ class Repository ...@@ -779,11 +782,10 @@ class Repository
merge_index = rugged.merge_commits(our_commit, their_commit) merge_index = rugged.merge_commits(our_commit, their_commit)
return false if merge_index.conflicts? 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( actual_options = options.merge(
parents: [our_commit, their_commit], parents: [our_commit, their_commit],
tree: merge_index.write_tree(rugged), tree: merge_index.write_tree(rugged),
update_ref: tmp_ref
) )
commit_id = Rugged::Commit.create(rugged, actual_options) commit_id = Rugged::Commit.create(rugged, actual_options)
...@@ -798,15 +800,14 @@ class Repository ...@@ -798,15 +800,14 @@ class Repository
return false unless revert_tree_id 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) committer = user_to_committer(user)
source_sha = Rugged::Commit.create(rugged, source_sha = Rugged::Commit.create(rugged,
message: commit.revert_message, message: commit.revert_message,
author: committer, author: committer,
committer: committer, committer: committer,
tree: revert_tree_id, tree: revert_tree_id,
parents: [rugged.lookup(source_sha)], parents: [rugged.lookup(source_sha)])
update_ref: ref)
end end
end end
...@@ -816,7 +817,7 @@ class Repository ...@@ -816,7 +817,7 @@ class Repository
return false unless cherry_pick_tree_id 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) committer = user_to_committer(user)
source_sha = Rugged::Commit.create(rugged, source_sha = Rugged::Commit.create(rugged,
message: commit.message, message: commit.message,
...@@ -827,8 +828,7 @@ class Repository ...@@ -827,8 +828,7 @@ class Repository
}, },
committer: committer, committer: committer,
tree: cherry_pick_tree_id, tree: cherry_pick_tree_id,
parents: [rugged.lookup(source_sha)], parents: [rugged.lookup(source_sha)])
update_ref: ref)
end end
end end
...@@ -929,20 +929,6 @@ class Repository ...@@ -929,20 +929,6 @@ class Repository
Gitlab::Popen.popen(args, path_to_repo) Gitlab::Popen.popen(args, path_to_repo)
end 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) def commit_with_hooks(current_user, branch)
update_autocrlf_option update_autocrlf_option
...@@ -955,33 +941,31 @@ class Repository ...@@ -955,33 +941,31 @@ class Repository
oldrev = target_branch.target oldrev = target_branch.target
end end
with_tmp_ref(oldrev) do |tmp_ref| # Make commit
# Make commit in tmp ref newrev = yield(ref)
newrev = yield(tmp_ref)
unless newrev unless newrev
raise CommitError.new('Failed to create commit') raise CommitError.new('Failed to create commit')
end 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 # Make sure target branch was not changed during pre-receive hook
if was_empty || !target_branch if current_head == oldrev
# Create branch rugged.references.update(ref, newrev)
rugged.references.create(ref, newrev)
else else
# Update head raise CommitError.new('Commit was rejected because branch received new push')
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 end
end end
newrev
end end
newrev
end end
def ls_files(ref) def ls_files(ref)
......
...@@ -15,21 +15,19 @@ class CreateBranchService < BaseService ...@@ -15,21 +15,19 @@ class CreateBranchService < BaseService
return error('Branch already exists') return error('Branch already exists')
end end
new_branch = nil new_branch = if source_project != @project
repository.fetch_ref(
if source_project != @project source_project.repository.path_to_repo,
repository.with_tmp_ref do |tmp_ref| "refs/heads/#{ref}",
repository.fetch_ref( "refs/heads/#{branch_name}"
source_project.repository.path_to_repo, )
"refs/heads/#{ref}",
tmp_ref repository.after_create_branch
)
repository.find_branch(branch_name)
new_branch = repository.add_branch(current_user, branch_name, tmp_ref) else
end repository.add_branch(current_user, branch_name, ref)
else end
new_branch = repository.add_branch(current_user, branch_name, ref)
end
if new_branch if new_branch
success(new_branch) success(new_branch)
......
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