Add `source_branch` option for various git operations
If `source_branch` option is passed, and target branch cannot be found, `Repository#update_branch_with_hooks` would try to create a new branch from `source_branch`. This way, we could make changes in the new branch while only firing the hooks once for the changes. Previously, we can only create a new branch first then make changes to the new branch, firing hooks twice. This behaviour is bad for CI. Fixes #7237
This commit is contained in:
parent
3128641f7e
commit
0b5a2eef8f
|
|
@ -786,8 +786,12 @@ class Repository
|
|||
@root_ref ||= cache.fetch(:root_ref) { raw_repository.root_ref }
|
||||
end
|
||||
|
||||
def commit_dir(user, path, message, branch, author_email: nil, author_name: nil)
|
||||
update_branch_with_hooks(user, branch) do |ref|
|
||||
def commit_dir(user, path, message, branch,
|
||||
author_email: nil, author_name: nil, source_branch: nil)
|
||||
update_branch_with_hooks(
|
||||
user,
|
||||
branch,
|
||||
source_branch: source_branch) do |ref|
|
||||
options = {
|
||||
commit: {
|
||||
branch: ref,
|
||||
|
|
@ -802,8 +806,12 @@ class Repository
|
|||
end
|
||||
end
|
||||
|
||||
def commit_file(user, path, content, message, branch, update, author_email: nil, author_name: nil)
|
||||
update_branch_with_hooks(user, branch) do |ref|
|
||||
def commit_file(user, path, content, message, branch, update,
|
||||
author_email: nil, author_name: nil, source_branch: nil)
|
||||
update_branch_with_hooks(
|
||||
user,
|
||||
branch,
|
||||
source_branch: source_branch) do |ref|
|
||||
options = {
|
||||
commit: {
|
||||
branch: ref,
|
||||
|
|
@ -823,8 +831,13 @@ class Repository
|
|||
end
|
||||
end
|
||||
|
||||
def update_file(user, path, content, branch:, previous_path:, message:, author_email: nil, author_name: nil)
|
||||
update_branch_with_hooks(user, branch) do |ref|
|
||||
def update_file(user, path, content,
|
||||
branch:, previous_path:, message:,
|
||||
author_email: nil, author_name: nil, source_branch: nil)
|
||||
update_branch_with_hooks(
|
||||
user,
|
||||
branch,
|
||||
source_branch: source_branch) do |ref|
|
||||
options = {
|
||||
commit: {
|
||||
branch: ref,
|
||||
|
|
@ -849,8 +862,12 @@ class Repository
|
|||
end
|
||||
end
|
||||
|
||||
def remove_file(user, path, message, branch, author_email: nil, author_name: nil)
|
||||
update_branch_with_hooks(user, branch) do |ref|
|
||||
def remove_file(user, path, message, branch,
|
||||
author_email: nil, author_name: nil, source_branch: nil)
|
||||
update_branch_with_hooks(
|
||||
user,
|
||||
branch,
|
||||
source_branch: source_branch) do |ref|
|
||||
options = {
|
||||
commit: {
|
||||
branch: ref,
|
||||
|
|
@ -868,17 +885,18 @@ class Repository
|
|||
end
|
||||
end
|
||||
|
||||
def multi_action(user:, branch:, message:, actions:, author_email: nil, author_name: nil)
|
||||
update_branch_with_hooks(user, branch) do |ref|
|
||||
def multi_action(user:, branch:, message:, actions:,
|
||||
author_email: nil, author_name: nil, source_branch: nil)
|
||||
update_branch_with_hooks(
|
||||
user,
|
||||
branch,
|
||||
source_branch: source_branch) do |ref|
|
||||
index = rugged.index
|
||||
parents = []
|
||||
branch = find_branch(ref)
|
||||
|
||||
if branch
|
||||
last_commit = branch.dereferenced_target
|
||||
index.read_tree(last_commit.raw_commit.tree)
|
||||
parents = [last_commit.sha]
|
||||
end
|
||||
last_commit = find_branch(ref).dereferenced_target
|
||||
index.read_tree(last_commit.raw_commit.tree)
|
||||
parents = [last_commit.sha]
|
||||
|
||||
actions.each do |action|
|
||||
case action[:action]
|
||||
|
|
@ -967,7 +985,10 @@ class Repository
|
|||
|
||||
return false unless revert_tree_id
|
||||
|
||||
update_branch_with_hooks(user, base_branch) do
|
||||
update_branch_with_hooks(
|
||||
user,
|
||||
base_branch,
|
||||
source_branch: revert_tree_id) do
|
||||
committer = user_to_committer(user)
|
||||
source_sha = Rugged::Commit.create(rugged,
|
||||
message: commit.revert_message,
|
||||
|
|
@ -984,7 +1005,10 @@ class Repository
|
|||
|
||||
return false unless cherry_pick_tree_id
|
||||
|
||||
update_branch_with_hooks(user, base_branch) do
|
||||
update_branch_with_hooks(
|
||||
user,
|
||||
base_branch,
|
||||
source_branch: cherry_pick_tree_id) do
|
||||
committer = user_to_committer(user)
|
||||
source_sha = Rugged::Commit.create(rugged,
|
||||
message: commit.message,
|
||||
|
|
@ -1082,11 +1106,11 @@ class Repository
|
|||
fetch_ref(path_to_repo, ref, ref_path)
|
||||
end
|
||||
|
||||
def update_branch_with_hooks(current_user, branch)
|
||||
def update_branch_with_hooks(current_user, branch, source_branch: nil)
|
||||
update_autocrlf_option
|
||||
|
||||
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
|
||||
target_branch = find_branch(branch)
|
||||
target_branch, new_branch_added = raw_ensure_branch(branch, source_branch)
|
||||
was_empty = empty?
|
||||
|
||||
# Make commit
|
||||
|
|
@ -1096,7 +1120,7 @@ class Repository
|
|||
raise CommitError.new('Failed to create commit')
|
||||
end
|
||||
|
||||
if rugged.lookup(newrev).parent_ids.empty? || target_branch.nil?
|
||||
if rugged.lookup(newrev).parent_ids.empty?
|
||||
oldrev = Gitlab::Git::BLANK_SHA
|
||||
else
|
||||
oldrev = rugged.merge_base(newrev, target_branch.dereferenced_target.sha)
|
||||
|
|
@ -1105,11 +1129,9 @@ class Repository
|
|||
GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
|
||||
update_ref!(ref, newrev, oldrev)
|
||||
|
||||
if was_empty || !target_branch
|
||||
# If repo was empty expire cache
|
||||
after_create if was_empty
|
||||
after_create_branch
|
||||
end
|
||||
# If repo was empty expire cache
|
||||
after_create if was_empty
|
||||
after_create_branch if was_empty || new_branch_added
|
||||
end
|
||||
|
||||
newrev
|
||||
|
|
@ -1169,4 +1191,28 @@ class Repository
|
|||
def repository_event(event, tags = {})
|
||||
Gitlab::Metrics.add_event(event, { path: path_with_namespace }.merge(tags))
|
||||
end
|
||||
|
||||
def raw_ensure_branch(branch_name, source_branch)
|
||||
old_branch = find_branch(branch_name)
|
||||
|
||||
if old_branch
|
||||
[old_branch, false]
|
||||
elsif source_branch
|
||||
oldrev = Gitlab::Git::BLANK_SHA
|
||||
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
|
||||
target = commit(source_branch).try(:id)
|
||||
|
||||
unless target
|
||||
raise CommitError.new(
|
||||
"Cannot find branch #{branch_name} nor #{source_branch}")
|
||||
end
|
||||
|
||||
update_ref!(ref, target, oldrev)
|
||||
|
||||
[find_branch(branch_name), true]
|
||||
else
|
||||
raise CommitError.new(
|
||||
"Cannot find branch #{branch_name} and source_branch is not set")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ module Commits
|
|||
tree_id = repository.public_send("check_#{action}_content", @commit, @target_branch)
|
||||
|
||||
if tree_id
|
||||
create_target_branch(into) if @create_merge_request
|
||||
validate_target_branch(into) if @create_merge_request
|
||||
|
||||
repository.public_send(action, current_user, @commit, into, tree_id)
|
||||
success
|
||||
|
|
@ -50,12 +50,12 @@ module Commits
|
|||
true
|
||||
end
|
||||
|
||||
def create_target_branch(new_branch)
|
||||
def validate_target_branch(new_branch)
|
||||
# Temporary branch exists and contains the change commit
|
||||
return success if repository.find_branch(new_branch)
|
||||
return if repository.find_branch(new_branch)
|
||||
|
||||
result = CreateBranchService.new(@project, current_user)
|
||||
.execute(new_branch, @target_branch, source_project: @source_project)
|
||||
result = ValidateNewBranchService.new(@project, current_user).
|
||||
execute(new_branch)
|
||||
|
||||
if result[:status] == :error
|
||||
raise ChangeError, "There was an error creating the source branch: #{result[:message]}"
|
||||
|
|
|
|||
|
|
@ -2,18 +2,9 @@ require_relative 'base_service'
|
|||
|
||||
class CreateBranchService < BaseService
|
||||
def execute(branch_name, ref, source_project: @project)
|
||||
valid_branch = Gitlab::GitRefValidator.validate(branch_name)
|
||||
failure = validate_new_branch(branch_name)
|
||||
|
||||
unless valid_branch
|
||||
return error('Branch name is invalid')
|
||||
end
|
||||
|
||||
repository = project.repository
|
||||
existing_branch = repository.find_branch(branch_name)
|
||||
|
||||
if existing_branch
|
||||
return error('Branch already exists')
|
||||
end
|
||||
return failure if failure
|
||||
|
||||
new_branch = if source_project != @project
|
||||
repository.fetch_ref(
|
||||
|
|
@ -41,4 +32,13 @@ class CreateBranchService < BaseService
|
|||
def success(branch)
|
||||
super().merge(branch: branch)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def validate_new_branch(branch_name)
|
||||
result = ValidateNewBranchService.new(project, current_user).
|
||||
execute(branch_name)
|
||||
|
||||
error(result[:message]) if result[:status] == :error
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -23,9 +23,7 @@ module Files
|
|||
validate
|
||||
|
||||
# Create new branch if it different from source_branch
|
||||
if different_branch?
|
||||
create_target_branch
|
||||
end
|
||||
validate_target_branch if different_branch?
|
||||
|
||||
result = commit
|
||||
if result
|
||||
|
|
@ -73,10 +71,11 @@ module Files
|
|||
end
|
||||
end
|
||||
|
||||
def create_target_branch
|
||||
result = CreateBranchService.new(project, current_user).execute(@target_branch, @source_branch, source_project: @source_project)
|
||||
def validate_target_branch
|
||||
result = ValidateNewBranchService.new(project, current_user).
|
||||
execute(@target_branch)
|
||||
|
||||
unless result[:status] == :success
|
||||
if result[:status] == :error
|
||||
raise_error("Something went wrong when we tried to create #{@target_branch} for you: #{result[:message]}")
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -3,7 +3,14 @@ require_relative "base_service"
|
|||
module Files
|
||||
class CreateDirService < Files::BaseService
|
||||
def commit
|
||||
repository.commit_dir(current_user, @file_path, @commit_message, @target_branch, author_email: @author_email, author_name: @author_name)
|
||||
repository.commit_dir(
|
||||
current_user,
|
||||
@file_path,
|
||||
@commit_message,
|
||||
@target_branch,
|
||||
author_email: @author_email,
|
||||
author_name: @author_name,
|
||||
source_branch: @source_branch)
|
||||
end
|
||||
|
||||
def validate
|
||||
|
|
|
|||
|
|
@ -3,7 +3,16 @@ 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, false, author_email: @author_email, author_name: @author_name)
|
||||
repository.commit_file(
|
||||
current_user,
|
||||
@file_path,
|
||||
@file_content,
|
||||
@commit_message,
|
||||
@target_branch,
|
||||
false,
|
||||
author_email: @author_email,
|
||||
author_name: @author_name,
|
||||
source_branch: @source_branch)
|
||||
end
|
||||
|
||||
def validate
|
||||
|
|
|
|||
|
|
@ -3,7 +3,14 @@ require_relative "base_service"
|
|||
module Files
|
||||
class DeleteService < Files::BaseService
|
||||
def commit
|
||||
repository.remove_file(current_user, @file_path, @commit_message, @target_branch, author_email: @author_email, author_name: @author_name)
|
||||
repository.remove_file(
|
||||
current_user,
|
||||
@file_path,
|
||||
@commit_message,
|
||||
@target_branch,
|
||||
author_email: @author_email,
|
||||
author_name: @author_name,
|
||||
source_branch: @source_branch)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -11,7 +11,8 @@ module Files
|
|||
message: @commit_message,
|
||||
actions: params[:actions],
|
||||
author_email: @author_email,
|
||||
author_name: @author_name
|
||||
author_name: @author_name,
|
||||
source_branch: @source_branch
|
||||
)
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -10,7 +10,8 @@ module Files
|
|||
previous_path: @previous_path,
|
||||
message: @commit_message,
|
||||
author_email: @author_email,
|
||||
author_name: @author_name)
|
||||
author_name: @author_name,
|
||||
source_branch: @source_branch)
|
||||
end
|
||||
|
||||
private
|
||||
|
|
|
|||
|
|
@ -0,0 +1,22 @@
|
|||
require_relative 'base_service'
|
||||
|
||||
class ValidateNewBranchService < BaseService
|
||||
def execute(branch_name)
|
||||
valid_branch = Gitlab::GitRefValidator.validate(branch_name)
|
||||
|
||||
unless valid_branch
|
||||
return error('Branch name is invalid')
|
||||
end
|
||||
|
||||
repository = project.repository
|
||||
existing_branch = repository.find_branch(branch_name)
|
||||
|
||||
if existing_branch
|
||||
return error('Branch already exists')
|
||||
end
|
||||
|
||||
success
|
||||
rescue GitHooksService::PreReceiveError => ex
|
||||
error(ex.message)
|
||||
end
|
||||
end
|
||||
Loading…
Reference in New Issue