From 5ecd0c81af85476c2328d3836cc68b17ebd5a8a6 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 7 Dec 2016 22:37:43 +0800
Subject: [PATCH] Commit outside the hooks if possible:
So we still commit outside the hooks, and only
update ref inside the hooks. There are only two
exceptions:
* Whenever it's adding a tag. We can't add a tag
without committing, unfortunately. See !7700
* Whenever source project is in another repository.
We'll need to fetch ref otherwise commits can't be made.
See the whole discussion starting from:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7237#note_19210942
---
app/models/repository.rb | 16 +++-
app/services/git_operation_service.rb | 111 ++++++++++++--------------
spec/models/repository_spec.rb | 2 +-
3 files changed, 65 insertions(+), 64 deletions(-)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 36cbb0d051e..9393d6b461e 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -973,7 +973,8 @@ class Repository
base_branch,
source_branch: source_branch, source_project: source_project) do
- source_sha = find_branch(base_branch).dereferenced_target.sha
+ source_sha = source_project.repository.find_source_sha(
+ source_branch || base_branch)
committer = user_to_committer(user)
Rugged::Commit.create(rugged,
@@ -996,7 +997,8 @@ class Repository
base_branch,
source_branch: source_branch, source_project: source_project) do
- source_sha = find_branch(base_branch).dereferenced_target.sha
+ source_sha = source_project.repository.find_source_sha(
+ source_branch || base_branch)
committer = user_to_committer(user)
Rugged::Commit.create(rugged,
@@ -1162,6 +1164,16 @@ class Repository
end
end
+ protected
+
+ def find_source_sha(branch_name)
+ if branch_exists?(branch_name)
+ find_branch(branch_name).dereferenced_target.sha
+ else
+ Gitlab::Git::BLANK_SHA
+ end
+ end
+
private
def refs_directory_exists?
diff --git a/app/services/git_operation_service.rb b/app/services/git_operation_service.rb
index c9e2c21737a..3a102a9276b 100644
--- a/app/services/git_operation_service.rb
+++ b/app/services/git_operation_service.rb
@@ -34,43 +34,10 @@ GitOperationService = Struct.new(:user, :repository) do
source_branch: nil,
source_project: repository.project)
- ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
- oldrev = Gitlab::Git::BLANK_SHA
+ check_with_branch_arguments!(branch_name, source_branch, source_project)
- if repository.branch_exists?(branch_name)
- oldrev = newrev = repository.commit(branch_name).sha
-
- elsif repository.project != source_project
- unless source_branch
- raise ArgumentError,
- 'Should also pass :source_branch if' +
- ' :source_project is different from current project'
- end
-
- newrev = source_project.repository.commit(source_branch).try(:sha)
-
- unless newrev
- raise Repository::CommitError.new(
- "Cannot find branch #{branch_name} nor" \
- " #{source_branch} from" \
- " #{source_project.path_with_namespace}")
- end
-
- elsif source_branch
- newrev = repository.commit(source_branch).try(:sha)
-
- unless newrev
- raise Repository::CommitError.new(
- "Cannot find branch #{branch_name} nor" \
- " #{source_branch} from" \
- " #{repository.project.path_with_namespace}")
- end
-
- else # we want an orphan empty branch
- newrev = Gitlab::Git::BLANK_SHA
- end
-
- commit_with_hooks(ref, oldrev, newrev) do
+ update_branch_with_hooks(
+ branch_name, source_branch, source_project) do |ref|
if repository.project != source_project
repository.fetch_ref(
source_project.repository.path_to_repo,
@@ -85,39 +52,37 @@ GitOperationService = Struct.new(:user, :repository) do
private
- def commit_with_hooks(ref, oldrev, newrev)
- with_hooks_and_update_ref(ref, oldrev, newrev) do |service|
- was_empty = repository.empty?
-
- # Make commit
- nextrev = yield(ref)
-
- unless nextrev
- raise Repository::CommitError.new('Failed to create commit')
- end
+ def update_branch_with_hooks(branch_name, source_branch, source_project)
+ update_autocrlf_option
- branch =
- repository.find_branch(ref[Gitlab::Git::BRANCH_REF_PREFIX.size..-1])
+ ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
+ oldrev = Gitlab::Git::BLANK_SHA
+ was_empty = repository.empty?
- prevrev = if branch &&
- !repository.rugged.lookup(nextrev).parent_ids.empty?
- repository.rugged.merge_base(
- nextrev, branch.dereferenced_target.sha)
- else
- newrev
- end
+ # Make commit
+ newrev = yield(ref)
- update_ref!(ref, nextrev, prevrev)
+ unless newrev
+ raise Repository::CommitError.new('Failed to create commit')
+ end
- service.newrev = nextrev
+ branch = repository.find_branch(branch_name)
+ oldrev = if repository.rugged.lookup(newrev).parent_ids.empty? ||
+ branch.nil?
+ Gitlab::Git::BLANK_SHA
+ else
+ repository.rugged.merge_base(
+ newrev, branch.dereferenced_target.sha)
+ end
+ with_hooks_and_update_ref(ref, oldrev, newrev) do
# If repo was empty expire cache
repository.after_create if was_empty
repository.after_create_branch if was_empty ||
oldrev == Gitlab::Git::BLANK_SHA
-
- nextrev
end
+
+ newrev
end
def with_hooks_and_update_ref(ref, oldrev, newrev)
@@ -129,8 +94,6 @@ GitOperationService = Struct.new(:user, :repository) do
end
def with_hooks(ref, oldrev, newrev)
- update_autocrlf_option
-
result = nil
GitHooksService.new.execute(
@@ -170,4 +133,30 @@ GitOperationService = Struct.new(:user, :repository) do
repository.raw_repository.autocrlf = :input
end
end
+
+ def check_with_branch_arguments!(branch_name, source_branch, source_project)
+ return if repository.branch_exists?(branch_name)
+
+ if repository.project != source_project
+ unless source_branch
+ raise ArgumentError,
+ 'Should also pass :source_branch if' +
+ ' :source_project is different from current project'
+ end
+
+ unless source_project.repository.commit(source_branch).try(:sha)
+ raise Repository::CommitError.new(
+ "Cannot find branch #{branch_name} nor" \
+ " #{source_branch} from" \
+ " #{source_project.path_with_namespace}")
+ end
+ elsif source_branch
+ unless repository.commit(source_branch).try(:sha)
+ raise Repository::CommitError.new(
+ "Cannot find branch #{branch_name} nor" \
+ " #{source_branch} from" \
+ " #{repository.project.path_with_namespace}")
+ end
+ end
+ end
end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index e3ec4c85a74..ebd05c946cc 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -695,7 +695,7 @@ describe Repository, models: true do
user,
repository.path_to_repo,
old_rev,
- old_rev,
+ new_rev,
'refs/heads/feature').
and_yield(service).and_return(true)
end
--
GitLab