Commit e7d530a6 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fix-git-hooks-when-creating-file' into 'master'

Don't execute git hooks if you create branch as part of other change

Closes #23439

See merge request !7237
parents dce77b1d 54fca951
...@@ -4,13 +4,15 @@ module CreatesCommit ...@@ -4,13 +4,15 @@ module CreatesCommit
def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil)
set_commit_variables set_commit_variables
start_branch = @mr_target_branch unless initial_commit?
commit_params = @commit_params.merge( commit_params = @commit_params.merge(
source_project: @project, start_project: @mr_target_project,
source_branch: @ref, start_branch: start_branch,
target_branch: @target_branch target_branch: @mr_source_branch
) )
result = service.new(@tree_edit_project, current_user, commit_params).execute result = service.new(
@mr_source_project, current_user, commit_params).execute
if result[:status] == :success if result[:status] == :success
update_flash_notice(success_notice) update_flash_notice(success_notice)
...@@ -89,20 +91,18 @@ module CreatesCommit ...@@ -89,20 +91,18 @@ module CreatesCommit
@mr_source_project != @mr_target_project @mr_source_project != @mr_target_project
end end
def different_branch?
@mr_source_branch != @mr_target_branch || different_project?
end
def create_merge_request? def create_merge_request?
params[:create_merge_request].present? && different_branch? # XXX: Even if the field is set, if we're checking the same branch
# as the target branch in the same project,
# we don't want to create a merge request.
params[:create_merge_request].present? &&
(different_project? || @ref != @target_branch)
end end
# TODO: We should really clean this up
def set_commit_variables def set_commit_variables
@mr_source_branch ||= @target_branch
if can?(current_user, :push_code, @project) if can?(current_user, :push_code, @project)
# Edit file in this project # Edit file in this project
@tree_edit_project = @project
@mr_source_project = @project @mr_source_project = @project
if @project.forked? if @project.forked?
...@@ -112,15 +112,34 @@ module CreatesCommit ...@@ -112,15 +112,34 @@ module CreatesCommit
else else
# Merge request to this project # Merge request to this project
@mr_target_project = @project @mr_target_project = @project
@mr_target_branch ||= @ref @mr_target_branch = @ref || @target_branch
end end
else else
# Edit file in fork
@tree_edit_project = current_user.fork_of(@project)
# Merge request from fork to this project # Merge request from fork to this project
@mr_source_project = @tree_edit_project @mr_source_project = current_user.fork_of(@project)
@mr_target_project = @project @mr_target_project = @project
@mr_target_branch ||= @ref @mr_target_branch = @ref || @target_branch
end end
@mr_source_branch = guess_mr_source_branch
end
def initial_commit?
@mr_target_branch.nil? ||
!@mr_target_project.repository.branch_exists?(@mr_target_branch)
end
def guess_mr_source_branch
# XXX: Happens when viewing a commit without a branch. In this case,
# @target_branch would be the default branch for @mr_source_project,
# however we want a generated new branch here. Thus we can't use
# @target_branch, but should pass nil to indicate that we want a new
# branch instead of @target_branch.
return if
create_merge_request? &&
# XXX: Don't understand why rubocop prefers this indention
@mr_source_project.repository.branch_exists?(@target_branch)
@target_branch
end end
end end
...@@ -50,7 +50,7 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -50,7 +50,7 @@ class Projects::CommitController < Projects::ApplicationController
end end
def revert def revert
assign_change_commit_vars(@commit.revert_branch_name) assign_change_commit_vars
return render_404 if @target_branch.blank? return render_404 if @target_branch.blank?
...@@ -59,7 +59,7 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -59,7 +59,7 @@ class Projects::CommitController < Projects::ApplicationController
end end
def cherry_pick def cherry_pick
assign_change_commit_vars(@commit.cherry_pick_branch_name) assign_change_commit_vars
return render_404 if @target_branch.blank? return render_404 if @target_branch.blank?
...@@ -116,11 +116,9 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -116,11 +116,9 @@ class Projects::CommitController < Projects::ApplicationController
} }
end end
def assign_change_commit_vars(mr_source_branch) def assign_change_commit_vars
@commit = project.commit(params[:id]) @commit = project.commit(params[:id])
@target_branch = params[:target_branch] @target_branch = params[:target_branch]
@mr_source_branch = mr_source_branch
@mr_target_branch = @target_branch
@commit_params = { @commit_params = {
commit: @commit, commit: @commit,
create_merge_request: params[:create_merge_request].present? || different_project? create_merge_request: params[:create_merge_request].present? || different_project?
......
...@@ -46,7 +46,8 @@ class Projects::CompareController < Projects::ApplicationController ...@@ -46,7 +46,8 @@ class Projects::CompareController < Projects::ApplicationController
end end
def define_diff_vars def define_diff_vars
@compare = CompareService.new.execute(@project, @head_ref, @project, @start_ref) @compare = CompareService.new(@project, @head_ref)
.execute(@project, @start_ref)
if @compare if @compare
@commits = @compare.commits @commits = @compare.commits
......
...@@ -169,7 +169,8 @@ class MergeRequestDiff < ActiveRecord::Base ...@@ -169,7 +169,8 @@ class MergeRequestDiff < ActiveRecord::Base
# When compare merge request versions we want diff A..B instead of A...B # When compare merge request versions we want diff A..B instead of A...B
# so we handle cases when user does squash and rebase of the commits between versions. # so we handle cases when user does squash and rebase of the commits between versions.
# For this reason we set straight to true by default. # For this reason we set straight to true by default.
CompareService.new.execute(project, head_commit_sha, project, sha, straight: straight) CompareService.new(project, head_commit_sha)
.execute(project, sha, straight: straight)
end end
def commits_count def commits_count
......
...@@ -5,7 +5,7 @@ class Repository ...@@ -5,7 +5,7 @@ class Repository
attr_accessor :path_with_namespace, :project attr_accessor :path_with_namespace, :project
class CommitError < StandardError; end CommitError = Class.new(StandardError)
# Methods that cache data from the Git repository. # Methods that cache data from the Git repository.
# #
...@@ -64,10 +64,6 @@ class Repository ...@@ -64,10 +64,6 @@ class Repository
@raw_repository ||= Gitlab::Git::Repository.new(path_to_repo) @raw_repository ||= Gitlab::Git::Repository.new(path_to_repo)
end end
def update_autocrlf_option
raw_repository.autocrlf = :input if raw_repository.autocrlf != :input
end
# Return absolute path to repository # Return absolute path to repository
def path_to_repo def path_to_repo
@path_to_repo ||= File.expand_path( @path_to_repo ||= File.expand_path(
...@@ -168,63 +164,46 @@ class Repository ...@@ -168,63 +164,46 @@ class Repository
tags.find { |tag| tag.name == name } tags.find { |tag| tag.name == name }
end end
def add_branch(user, branch_name, target) def add_branch(user, branch_name, ref)
oldrev = Gitlab::Git::BLANK_SHA newrev = commit(ref).try(:sha)
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
target = commit(target).try(:id)
return false unless target return false unless newrev
GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do GitOperationService.new(user, self).add_branch(branch_name, newrev)
update_ref!(ref, target, oldrev)
end
after_create_branch after_create_branch
find_branch(branch_name) find_branch(branch_name)
end end
def add_tag(user, tag_name, target, message = nil) def add_tag(user, tag_name, target, message = nil)
oldrev = Gitlab::Git::BLANK_SHA newrev = commit(target).try(:id)
ref = Gitlab::Git::TAG_REF_PREFIX + tag_name
target = commit(target).try(:id)
return false unless target
options = { message: message, tagger: user_to_committer(user) } if message options = { message: message, tagger: user_to_committer(user) } if message
GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do |service| return false unless newrev
raw_tag = rugged.tags.create(tag_name, target, options)
service.newrev = raw_tag.target_id GitOperationService.new(user, self).add_tag(tag_name, newrev, options)
end
find_tag(tag_name) find_tag(tag_name)
end end
def rm_branch(user, branch_name) def rm_branch(user, branch_name)
before_remove_branch before_remove_branch
branch = find_branch(branch_name) branch = find_branch(branch_name)
oldrev = branch.try(:dereferenced_target).try(:id)
newrev = Gitlab::Git::BLANK_SHA
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
GitHooksService.new.execute(user, path_to_repo, oldrev, newrev, ref) do GitOperationService.new(user, self).rm_branch(branch)
update_ref!(ref, newrev, oldrev)
end
after_remove_branch after_remove_branch
true true
end end
def rm_tag(tag_name) def rm_tag(user, tag_name)
before_remove_tag before_remove_tag
tag = find_tag(tag_name)
begin GitOperationService.new(user, self).rm_tag(tag)
rugged.tags.delete(tag_name)
true after_remove_tag
rescue Rugged::ReferenceError true
false
end
end end
def ref_names def ref_names
...@@ -241,21 +220,6 @@ class Repository ...@@ -241,21 +220,6 @@ class Repository
false false
end end
def update_ref!(name, newrev, oldrev)
# We use 'git update-ref' because libgit2/rugged currently does not
# offer 'compare and swap' ref updates. Without compare-and-swap we can
# (and have!) accidentally reset the ref to an earlier state, clobbering
# commits. See also https://github.com/libgit2/libgit2/issues/1534.
command = %W(#{Gitlab.config.git.bin_path} update-ref --stdin -z)
_, status = Gitlab::Popen.popen(command, path_to_repo) do |stdin|
stdin.write("update #{name}\x00#{newrev}\x00#{oldrev}\x00")
end
return if status.zero?
raise CommitError.new("Could not update branch #{name.sub('refs/heads/', '')}. Please refresh and try again.")
end
# Makes sure a commit is kept around when Git garbage collection runs. # Makes sure a commit is kept around when Git garbage collection runs.
# Git GC will delete commits from the repository that are no longer in any # Git GC will delete commits from the repository that are no longer in any
# branches or tags, but we want to keep some of these commits around, for # branches or tags, but we want to keep some of these commits around, for
...@@ -435,6 +399,11 @@ class Repository ...@@ -435,6 +399,11 @@ class Repository
repository_event(:remove_tag) repository_event(:remove_tag)
end end
# Runs code after removing a tag.
def after_remove_tag
expire_tags_cache
end
def before_import def before_import
expire_content_cache expire_content_cache
end end
...@@ -779,121 +748,132 @@ class Repository ...@@ -779,121 +748,132 @@ class Repository
@tags ||= raw_repository.tags @tags ||= raw_repository.tags
end end
def commit_dir(user, path, message, branch, author_email: nil, author_name: nil) # rubocop:disable Metrics/ParameterLists
update_branch_with_hooks(user, branch) do |ref| def commit_dir(
options = { user, path,
commit: { message:, branch_name:,
branch: ref, author_email: nil, author_name: nil,
message: message, start_branch_name: nil, start_project: project)
update_ref: false check_tree_entry_for_dir(branch_name, path)
}
}
options.merge!(get_committer_and_author(user, email: author_email, name: author_name))
raw_repository.mkdir(path, options)
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|
options = {
commit: {
branch: ref,
message: message,
update_ref: false
},
file: {
content: content,
path: path,
update: update
}
}
options.merge!(get_committer_and_author(user, email: author_email, name: author_name))
Gitlab::Git::Blob.commit(raw_repository, options) if start_branch_name
start_project.repository.
check_tree_entry_for_dir(start_branch_name, path)
end 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|
options = {
commit: {
branch: ref,
message: message,
update_ref: false
},
file: {
content: content,
path: path,
update: true
}
}
options.merge!(get_committer_and_author(user, email: author_email, name: author_name))
if previous_path && previous_path != path commit_file(
options[:file][:previous_path] = previous_path user,
Gitlab::Git::Blob.rename(raw_repository, options) "#{path}/.gitkeep",
else '',
Gitlab::Git::Blob.commit(raw_repository, options) message: message,
branch_name: branch_name,
update: false,
author_email: author_email,
author_name: author_name,
start_branch_name: start_branch_name,
start_project: start_project)
end
# rubocop:enable Metrics/ParameterLists
# rubocop:disable Metrics/ParameterLists
def commit_file(
user, path, content,
message:, branch_name:, update: true,
author_email: nil, author_name: nil,
start_branch_name: nil, start_project: project)
unless update
error_message = "Filename already exists; update not allowed"
if tree_entry_at(branch_name, path)
raise Gitlab::Git::Repository::InvalidBlobName.new(error_message)
end end
end
end
def remove_file(user, path, message, branch, author_email: nil, author_name: nil)
update_branch_with_hooks(user, branch) do |ref|
options = {
commit: {
branch: ref,
message: message,
update_ref: false
},
file: {
path: path
}
}
options.merge!(get_committer_and_author(user, email: author_email, name: author_name))
Gitlab::Git::Blob.remove(raw_repository, options) if start_branch_name &&
start_project.repository.tree_entry_at(start_branch_name, path)
raise Gitlab::Git::Repository::InvalidBlobName.new(error_message)
end
end end
end
def multi_action(user:, branch:, message:, actions:, author_email: nil, author_name: nil) multi_action(
update_branch_with_hooks(user, branch) do |ref| user: user,
message: message,
branch_name: branch_name,
author_email: author_email,
author_name: author_name,
start_branch_name: start_branch_name,
start_project: start_project,
actions: [{ action: :create,
file_path: path,
content: content }])
end
# rubocop:enable Metrics/ParameterLists
# rubocop:disable Metrics/ParameterLists
def update_file(
user, path, content,
message:, branch_name:, previous_path:,
author_email: nil, author_name: nil,
start_branch_name: nil, start_project: project)
action = if previous_path && previous_path != path
:move
else
:update
end
multi_action(
user: user,
message: message,
branch_name: branch_name,
author_email: author_email,
author_name: author_name,
start_branch_name: start_branch_name,
start_project: start_project,
actions: [{ action: action,
file_path: path,
content: content,
previous_path: previous_path }])
end
# rubocop:enable Metrics/ParameterLists
# rubocop:disable Metrics/ParameterLists
def remove_file(
user, path,
message:, branch_name:,
author_email: nil, author_name: nil,
start_branch_name: nil, start_project: project)
multi_action(
user: user,
message: message,
branch_name: branch_name,
author_email: author_email,
author_name: author_name,
start_branch_name: start_branch_name,
start_project: start_project,
actions: [{ action: :delete,
file_path: path }])
end
# rubocop:enable Metrics/ParameterLists
# rubocop:disable Metrics/ParameterLists
def multi_action(
user:, branch_name:, message:, actions:,
author_email: nil, author_name: nil,
start_branch_name: nil, start_project: project)
GitOperationService.new(user, self).with_branch(
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
index = rugged.index index = rugged.index
parents = []
branch = find_branch(ref)
if branch parents = if start_commit
last_commit = branch.dereferenced_target index.read_tree(start_commit.raw_commit.tree)
index.read_tree(last_commit.raw_commit.tree) [start_commit.sha]
parents = [last_commit.sha] else
end []
end
actions.each do |action| actions.each do |act|
case action[:action] git_action(index, act)
when :create, :update, :move
mode =
case action[:action]
when :update
index.get(action[:file_path])[:mode]
when :move
index.get(action[:previous_path])[:mode]
end
mode ||= 0o100644
index.remove(action[:previous_path]) if action[:action] == :move
content = action[:encoding] == 'base64' ? Base64.decode64(action[:content]) : action[:content]
oid = rugged.write(content, :blob)
index.add(path: action[:file_path], oid: oid, mode: mode)
when :delete
index.remove(action[:file_path])
end
end end
options = { options = {
...@@ -906,6 +886,7 @@ class Repository ...@@ -906,6 +886,7 @@ class Repository
Rugged::Commit.create(rugged, options) Rugged::Commit.create(rugged, options)
end end
end end
# rubocop:enable Metrics/ParameterLists
def get_committer_and_author(user, email: nil, name: nil) def get_committer_and_author(user, email: nil, name: nil)
committer = user_to_committer(user) committer = user_to_committer(user)
...@@ -918,7 +899,7 @@ class Repository ...@@ -918,7 +899,7 @@ class Repository
end end
def user_to_committer(user) def user_to_committer(user)
Gitlab::Git::committer_hash(email: user.email, name: user.name) Gitlab::Git.committer_hash(email: user.email, name: user.name)
end end
def can_be_merged?(source_sha, target_branch) def can_be_merged?(source_sha, target_branch)
...@@ -933,16 +914,17 @@ class Repository ...@@ -933,16 +914,17 @@ class Repository
end end
def merge(user, merge_request, options = {}) def merge(user, merge_request, options = {})
our_commit = rugged.branches[merge_request.target_branch].target GitOperationService.new(user, self).with_branch(
their_commit = rugged.lookup(merge_request.diff_head_sha) merge_request.target_branch) do |start_commit|
our_commit = start_commit.sha
their_commit = merge_request.diff_head_sha
raise "Invalid merge target" if our_commit.nil? raise 'Invalid merge target' unless our_commit
raise "Invalid merge source" if their_commit.nil? raise 'Invalid merge source' unless their_commit
merge_index = rugged.merge_commits(our_commit, their_commit) merge_index = rugged.merge_commits(our_commit, their_commit)
return false if merge_index.conflicts? break if merge_index.conflicts?
update_branch_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),
...@@ -952,34 +934,48 @@ class Repository ...@@ -952,34 +934,48 @@ class Repository
merge_request.update(in_progress_merge_commit_sha: commit_id) merge_request.update(in_progress_merge_commit_sha: commit_id)
commit_id commit_id
end end
rescue Repository::CommitError # when merge_index.conflicts?
false
end end
def revert(user, commit, base_branch, revert_tree_id = nil) def revert(
source_sha = find_branch(base_branch).dereferenced_target.sha user, commit, branch_name, revert_tree_id = nil,
revert_tree_id ||= check_revert_content(commit, base_branch) start_branch_name: nil, start_project: project)
revert_tree_id ||= check_revert_content(commit, branch_name)
return false unless revert_tree_id return false unless revert_tree_id
update_branch_with_hooks(user, base_branch) do GitOperationService.new(user, self).with_branch(
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
committer = user_to_committer(user) committer = user_to_committer(user)
source_sha = Rugged::Commit.create(rugged,
Rugged::Commit.create(rugged,
message: commit.revert_message(user), message: commit.revert_message(user),
author: committer, author: committer,
committer: committer, committer: committer,
tree: revert_tree_id, tree: revert_tree_id,
parents: [rugged.lookup(source_sha)]) parents: [start_commit.sha])
end end
end end
def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil) def cherry_pick(
source_sha = find_branch(base_branch).dereferenced_target.sha user, commit, branch_name, cherry_pick_tree_id = nil,
cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch) start_branch_name: nil, start_project: project)
cherry_pick_tree_id ||= check_cherry_pick_content(commit, branch_name)
return false unless cherry_pick_tree_id return false unless cherry_pick_tree_id
update_branch_with_hooks(user, base_branch) do GitOperationService.new(user, self).with_branch(
branch_name,
start_branch_name: start_branch_name,
start_project: start_project) do |start_commit|
committer = user_to_committer(user) committer = user_to_committer(user)
source_sha = Rugged::Commit.create(rugged,
Rugged::Commit.create(rugged,
message: commit.message, message: commit.message,
author: { author: {
email: commit.author_email, email: commit.author_email,
...@@ -988,22 +984,22 @@ class Repository ...@@ -988,22 +984,22 @@ class Repository
}, },
committer: committer, committer: committer,
tree: cherry_pick_tree_id, tree: cherry_pick_tree_id,
parents: [rugged.lookup(source_sha)]) parents: [start_commit.sha])
end end
end end
def resolve_conflicts(user, branch, params) def resolve_conflicts(user, branch_name, params)
update_branch_with_hooks(user, branch) do GitOperationService.new(user, self).with_branch(branch_name) do
committer = user_to_committer(user) committer = user_to_committer(user)
Rugged::Commit.create(rugged, params.merge(author: committer, committer: committer)) Rugged::Commit.create(rugged, params.merge(author: committer, committer: committer))
end end
end end
def check_revert_content(commit, base_branch) def check_revert_content(target_commit, branch_name)
source_sha = find_branch(base_branch).dereferenced_target.sha source_sha = commit(branch_name).sha
args = [commit.id, source_sha] args = [target_commit.sha, source_sha]
args << { mainline: 1 } if commit.merge_commit? args << { mainline: 1 } if target_commit.merge_commit?
revert_index = rugged.revert_commit(*args) revert_index = rugged.revert_commit(*args)
return false if revert_index.conflicts? return false if revert_index.conflicts?
...@@ -1014,10 +1010,10 @@ class Repository ...@@ -1014,10 +1010,10 @@ class Repository
tree_id tree_id
end end
def check_cherry_pick_content(commit, base_branch) def check_cherry_pick_content(target_commit, branch_name)
source_sha = find_branch(base_branch).dereferenced_target.sha source_sha = commit(branch_name).sha
args = [commit.id, source_sha] args = [target_commit.sha, source_sha]
args << 1 if commit.merge_commit? args << 1 if target_commit.merge_commit?
cherry_pick_index = rugged.cherrypick_commit(*args) cherry_pick_index = rugged.cherrypick_commit(*args)
return false if cherry_pick_index.conflicts? return false if cherry_pick_index.conflicts?
...@@ -1075,6 +1071,28 @@ class Repository ...@@ -1075,6 +1071,28 @@ class Repository
Gitlab::Popen.popen(args, path_to_repo).first.lines.map(&:strip) Gitlab::Popen.popen(args, path_to_repo).first.lines.map(&:strip)
end end
def with_repo_branch_commit(start_repository, start_branch_name)
branch_name_or_sha =
if start_repository == self
start_branch_name
else
tmp_ref = "refs/tmp/#{SecureRandom.hex}/head"
fetch_ref(
start_repository.path_to_repo,
"#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}",
tmp_ref
)
start_repository.commit(start_branch_name).sha
end
yield(commit(branch_name_or_sha))
ensure
rugged.references.delete(tmp_ref) if tmp_ref
end
def fetch_ref(source_path, source_ref, target_ref) def fetch_ref(source_path, source_ref, target_ref)
args = %W(#{Gitlab.config.git.bin_path} fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref}) args = %W(#{Gitlab.config.git.bin_path} fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
Gitlab::Popen.popen(args, path_to_repo) Gitlab::Popen.popen(args, path_to_repo)
...@@ -1084,39 +1102,6 @@ class Repository ...@@ -1084,39 +1102,6 @@ class Repository
fetch_ref(path_to_repo, ref, ref_path) fetch_ref(path_to_repo, ref, ref_path)
end end
def update_branch_with_hooks(current_user, branch)
update_autocrlf_option
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
target_branch = find_branch(branch)
was_empty = empty?
# Make commit
newrev = yield(ref)
unless newrev
raise CommitError.new('Failed to create commit')
end
if rugged.lookup(newrev).parent_ids.empty? || target_branch.nil?
oldrev = Gitlab::Git::BLANK_SHA
else
oldrev = rugged.merge_base(newrev, target_branch.dereferenced_target.sha)
end
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
end
newrev
end
def ls_files(ref) def ls_files(ref)
actual_ref = ref || root_ref actual_ref = ref || root_ref
raw_repository.ls_files(actual_ref) raw_repository.ls_files(actual_ref)
...@@ -1175,8 +1160,76 @@ class Repository ...@@ -1175,8 +1160,76 @@ class Repository
end end
end end
protected
def tree_entry_at(branch_name, path)
branch_exists?(branch_name) &&
# tree_entry is private
raw_repository.send(:tree_entry, commit(branch_name), path)
end
def check_tree_entry_for_dir(branch_name, path)
return unless branch_exists?(branch_name)
entry = tree_entry_at(branch_name, path)
return unless entry
if entry[:type] == :blob
raise Gitlab::Git::Repository::InvalidBlobName.new(
"Directory already exists as a file")
else
raise Gitlab::Git::Repository::InvalidBlobName.new(
"Directory already exists")
end
end
private private
def git_action(index, action)
path = normalize_path(action[:file_path])
if action[:action] == :move
previous_path = normalize_path(action[:previous_path])
end
case action[:action]
when :create, :update, :move
mode =
case action[:action]
when :update
index.get(path)[:mode]
when :move
index.get(previous_path)[:mode]
end
mode ||= 0o100644
index.remove(previous_path) if action[:action] == :move
content = if action[:encoding] == 'base64'
Base64.decode64(action[:content])
else
action[:content]
end
oid = rugged.write(content, :blob)
index.add(path: path, oid: oid, mode: mode)
when :delete
index.remove(path)
end
end
def normalize_path(path)
pathname = Gitlab::Git::PathHelper.normalize_path(path)
if pathname.each_filename.include?('..')
raise Gitlab::Git::Repository::InvalidBlobName.new('Invalid path')
end
pathname.to_s
end
def refs_directory_exists? def refs_directory_exists?
return false unless path_with_namespace return false unless path_with_namespace
......
...@@ -4,7 +4,8 @@ module Commits ...@@ -4,7 +4,8 @@ module Commits
class ChangeError < StandardError; end class ChangeError < StandardError; end
def execute def execute
@source_project = params[:source_project] || @project @start_project = params[:start_project] || @project
@start_branch = params[:start_branch]
@target_branch = params[:target_branch] @target_branch = params[:target_branch]
@commit = params[:commit] @commit = params[:commit]
@create_merge_request = params[:create_merge_request].present? @create_merge_request = params[:create_merge_request].present?
...@@ -25,13 +26,28 @@ module Commits ...@@ -25,13 +26,28 @@ module Commits
def commit_change(action) def commit_change(action)
raise NotImplementedError unless repository.respond_to?(action) raise NotImplementedError unless repository.respond_to?(action)
into = @create_merge_request ? @commit.public_send("#{action}_branch_name") : @target_branch if @create_merge_request
tree_id = repository.public_send("check_#{action}_content", @commit, @target_branch) into = @commit.public_send("#{action}_branch_name")
tree_branch = @start_branch
else
into = tree_branch = @target_branch
end
tree_id = repository.public_send(
"check_#{action}_content", @commit, tree_branch)
if tree_id 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,
start_project: @start_project,
start_branch_name: @start_branch)
repository.public_send(action, current_user, @commit, into, tree_id)
success success
else else
error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically. error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically.
...@@ -50,12 +66,12 @@ module Commits ...@@ -50,12 +66,12 @@ module Commits
true true
end end
def create_target_branch(new_branch) def validate_target_branch(new_branch)
# Temporary branch exists and contains the change commit # 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) result = ValidateNewBranchService.new(@project, current_user)
.execute(new_branch, @target_branch, source_project: @source_project) .execute(new_branch)
if result[:status] == :error if result[:status] == :error
raise ChangeError, "There was an error creating the source branch: #{result[:message]}" raise ChangeError, "There was an error creating the source branch: #{result[:message]}"
......
...@@ -3,23 +3,27 @@ require 'securerandom' ...@@ -3,23 +3,27 @@ require 'securerandom'
# Compare 2 branches for one repo or between repositories # Compare 2 branches for one repo or between repositories
# and return Gitlab::Git::Compare object that responds to commits and diffs # and return Gitlab::Git::Compare object that responds to commits and diffs
class CompareService class CompareService
def execute(source_project, source_branch, target_project, target_branch, straight: false) attr_reader :start_project, :start_branch_name
source_commit = source_project.commit(source_branch)
return unless source_commit
source_sha = source_commit.sha def initialize(new_start_project, new_start_branch_name)
@start_project = new_start_project
@start_branch_name = new_start_branch_name
end
def execute(target_project, target_branch, straight: false)
# If compare with other project we need to fetch ref first # If compare with other project we need to fetch ref first
unless target_project == source_project target_project.repository.with_repo_branch_commit(
random_string = SecureRandom.hex start_project.repository,
start_branch_name) do |commit|
break unless commit
target_project.repository.fetch_ref( compare(commit.sha, target_project, target_branch, straight)
source_project.repository.path_to_repo,
"refs/heads/#{source_branch}",
"refs/tmp/#{random_string}/head"
)
end end
end
private
def compare(source_sha, target_project, target_branch, straight)
raw_compare = Gitlab::Git::Compare.new( raw_compare = Gitlab::Git::Compare.new(
target_project.repository.raw_repository, target_project.repository.raw_repository,
target_branch, target_branch,
......
class CreateBranchService < BaseService class CreateBranchService < BaseService
def execute(branch_name, ref, source_project: @project) def execute(branch_name, ref)
valid_branch = Gitlab::GitRefValidator.validate(branch_name) result = ValidateNewBranchService.new(project, current_user)
.execute(branch_name)
unless valid_branch return result if result[:status] == :error
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
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) new_branch = repository.add_branch(current_user, branch_name, ref)
else
repository.add_branch(current_user, branch_name, ref)
end
if new_branch if new_branch
success(new_branch) success(new_branch)
......
...@@ -7,7 +7,7 @@ class DeleteTagService < BaseService ...@@ -7,7 +7,7 @@ class DeleteTagService < BaseService
return error('No such tag', 404) return error('No such tag', 404)
end end
if repository.rm_tag(tag_name) if repository.rm_tag(current_user, tag_name)
release = project.releases.find_by(tag: tag_name) release = project.releases.find_by(tag: tag_name)
release.destroy if release release.destroy if release
......
...@@ -3,9 +3,9 @@ module Files ...@@ -3,9 +3,9 @@ module Files
class ValidationError < StandardError; end class ValidationError < StandardError; end
def execute def execute
@source_project = params[:source_project] || @project @start_project = params[:start_project] || @project
@source_branch = params[:source_branch] @start_branch = params[:start_branch]
@target_branch = params[:target_branch] @target_branch = params[:target_branch]
@commit_message = params[:commit_message] @commit_message = params[:commit_message]
@file_path = params[:file_path] @file_path = params[:file_path]
...@@ -22,10 +22,8 @@ module Files ...@@ -22,10 +22,8 @@ module Files
# Validate parameters # Validate parameters
validate validate
# Create new branch if it different from source_branch # Create new branch if it different from start_branch
if different_branch? validate_target_branch if different_branch?
create_target_branch
end
result = commit result = commit
if result if result
...@@ -40,7 +38,7 @@ module Files ...@@ -40,7 +38,7 @@ module Files
private private
def different_branch? def different_branch?
@source_branch != @target_branch || @source_project != @project @start_branch != @target_branch || @start_project != @project
end end
def file_has_changed? def file_has_changed?
...@@ -61,22 +59,23 @@ module Files ...@@ -61,22 +59,23 @@ module Files
end end
unless project.empty_repo? unless project.empty_repo?
unless @source_project.repository.branch_names.include?(@source_branch) unless @start_project.repository.branch_exists?(@start_branch)
raise_error('You can only create or edit files when you are on a branch') raise_error('You can only create or edit files when you are on a branch')
end end
if different_branch? if different_branch?
if repository.branch_names.include?(@target_branch) if repository.branch_exists?(@target_branch)
raise_error('Branch with such name already exists. You need to switch to this branch in order to make changes') raise_error('Branch with such name already exists. You need to switch to this branch in order to make changes')
end end
end end
end end
end end
def create_target_branch def validate_target_branch
result = CreateBranchService.new(project, current_user).execute(@target_branch, @source_branch, source_project: @source_project) 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]}") raise_error("Something went wrong when we tried to create #{@target_branch} for you: #{result[:message]}")
end end
end end
......
module Files module Files
class CreateDirService < Files::BaseService class CreateDirService < Files::BaseService
def commit 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,
message: @commit_message,
branch_name: @target_branch,
author_email: @author_email,
author_name: @author_name,
start_project: @start_project,
start_branch_name: @start_branch)
end end
def validate def validate
......
module Files module Files
class CreateService < Files::BaseService class CreateService < Files::BaseService
def commit 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,
message: @commit_message,
branch_name: @target_branch,
update: false,
author_email: @author_email,
author_name: @author_name,
start_project: @start_project,
start_branch_name: @start_branch)
end end
def validate def validate
...@@ -24,7 +34,7 @@ module Files ...@@ -24,7 +34,7 @@ module Files
unless project.empty_repo? unless project.empty_repo?
@file_path.slice!(0) if @file_path.start_with?('/') @file_path.slice!(0) if @file_path.start_with?('/')
blob = repository.blob_at_branch(@source_branch, @file_path) blob = repository.blob_at_branch(@start_branch, @file_path)
if blob if blob
raise_error('Your changes could not be committed because a file with the same name already exists') raise_error('Your changes could not be committed because a file with the same name already exists')
......
module Files module Files
class DeleteService < Files::BaseService class DeleteService < Files::BaseService
def commit 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,
message: @commit_message,
branch_name: @target_branch,
author_email: @author_email,
author_name: @author_name,
start_project: @start_project,
start_branch_name: @start_branch)
end end
end end
end end
...@@ -5,11 +5,13 @@ module Files ...@@ -5,11 +5,13 @@ module Files
def commit def commit
repository.multi_action( repository.multi_action(
user: current_user, user: current_user,
branch: @target_branch,
message: @commit_message, message: @commit_message,
branch_name: @target_branch,
actions: params[:actions], actions: params[:actions],
author_email: @author_email, author_email: @author_email,
author_name: @author_name author_name: @author_name,
start_project: @start_project,
start_branch_name: @start_branch
) )
end end
...@@ -61,7 +63,7 @@ module Files ...@@ -61,7 +63,7 @@ module Files
end end
def last_commit def last_commit
Gitlab::Git::Commit.last_for_path(repository, @source_branch, @file_path) Gitlab::Git::Commit.last_for_path(repository, @start_branch, @file_path)
end end
def regex_check(file) def regex_check(file)
......
...@@ -4,11 +4,13 @@ module Files ...@@ -4,11 +4,13 @@ module Files
def commit def commit
repository.update_file(current_user, @file_path, @file_content, repository.update_file(current_user, @file_path, @file_content,
branch: @target_branch,
previous_path: @previous_path,
message: @commit_message, message: @commit_message,
branch_name: @target_branch,
previous_path: @previous_path,
author_email: @author_email, author_email: @author_email,
author_name: @author_name) author_name: @author_name,
start_project: @start_project,
start_branch_name: @start_branch)
end end
private private
...@@ -23,7 +25,7 @@ module Files ...@@ -23,7 +25,7 @@ module Files
def last_commit def last_commit
@last_commit ||= Gitlab::Git::Commit. @last_commit ||= Gitlab::Git::Commit.
last_for_path(@source_project.repository, @source_branch, @file_path) last_for_path(@start_project.repository, @start_branch, @file_path)
end end
end end
end end
...@@ -18,9 +18,9 @@ class GitHooksService ...@@ -18,9 +18,9 @@ class GitHooksService
end end
end end
yield self yield(self).tap do
run_hook('post-receive')
run_hook('post-receive') end
end end
private private
......
class GitOperationService
attr_reader :user, :repository
def initialize(new_user, new_repository)
@user = new_user
@repository = new_repository
end
def add_branch(branch_name, newrev)
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
oldrev = Gitlab::Git::BLANK_SHA
update_ref_in_hooks(ref, newrev, oldrev)
end
def rm_branch(branch)
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch.name
oldrev = branch.target
newrev = Gitlab::Git::BLANK_SHA
update_ref_in_hooks(ref, newrev, oldrev)
end
def add_tag(tag_name, newrev, options = {})
ref = Gitlab::Git::TAG_REF_PREFIX + tag_name
oldrev = Gitlab::Git::BLANK_SHA
with_hooks(ref, newrev, oldrev) do |service|
# We want to pass the OID of the tag object to the hooks. For an
# annotated tag we don't know that OID until after the tag object
# (raw_tag) is created in the repository. That is why we have to
# update the value after creating the tag object. Only the
# "post-receive" hook will receive the correct value in this case.
raw_tag = repository.rugged.tags.create(tag_name, newrev, options)
service.newrev = raw_tag.target_id
end
end
def rm_tag(tag)
ref = Gitlab::Git::TAG_REF_PREFIX + tag.name
oldrev = tag.target
newrev = Gitlab::Git::BLANK_SHA
update_ref_in_hooks(ref, newrev, oldrev) do
repository.rugged.tags.delete(tag_name)
end
end
# Whenever `start_branch_name` is passed, if `branch_name` doesn't exist,
# it would be created from `start_branch_name`.
# If `start_project` is passed, and the branch doesn't exist,
# it would try to find the commits from it instead of current repository.
def with_branch(
branch_name,
start_branch_name: nil,
start_project: repository.project,
&block)
check_with_branch_arguments!(
branch_name, start_branch_name, start_project)
update_branch_with_hooks(branch_name) do
repository.with_repo_branch_commit(
start_project.repository,
start_branch_name || branch_name,
&block)
end
end
private
def update_branch_with_hooks(branch_name)
update_autocrlf_option
was_empty = repository.empty?
# Make commit
newrev = yield
unless newrev
raise Repository::CommitError.new('Failed to create commit')
end
branch = repository.find_branch(branch_name)
oldrev = find_oldrev_from_branch(newrev, branch)
ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
update_ref_in_hooks(ref, newrev, oldrev)
# If repo was empty expire cache
repository.after_create if was_empty
repository.after_create_branch if
was_empty || Gitlab::Git.blank_ref?(oldrev)
newrev
end
def find_oldrev_from_branch(newrev, branch)
return Gitlab::Git::BLANK_SHA unless branch
oldrev = branch.target
if oldrev == repository.rugged.merge_base(newrev, branch.target)
oldrev
else
raise Repository::CommitError.new('Branch diverged')
end
end
def update_ref_in_hooks(ref, newrev, oldrev)
with_hooks(ref, newrev, oldrev) do
update_ref(ref, newrev, oldrev)
end
end
def with_hooks(ref, newrev, oldrev)
GitHooksService.new.execute(
user,
repository.path_to_repo,
oldrev,
newrev,
ref) do |service|
yield(service)
end
end
def update_ref(ref, newrev, oldrev)
# We use 'git update-ref' because libgit2/rugged currently does not
# offer 'compare and swap' ref updates. Without compare-and-swap we can
# (and have!) accidentally reset the ref to an earlier state, clobbering
# commits. See also https://github.com/libgit2/libgit2/issues/1534.
command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
_, status = Gitlab::Popen.popen(
command,
repository.path_to_repo) do |stdin|
stdin.write("update #{ref}\x00#{newrev}\x00#{oldrev}\x00")
end
unless status.zero?
raise Repository::CommitError.new(
"Could not update branch #{Gitlab::Git.branch_name(ref)}." \
" Please refresh and try again.")
end
end
def update_autocrlf_option
if repository.raw_repository.autocrlf != :input
repository.raw_repository.autocrlf = :input
end
end
def check_with_branch_arguments!(
branch_name, start_branch_name, start_project)
return if repository.branch_exists?(branch_name)
if repository.project != start_project
unless start_branch_name
raise ArgumentError,
'Should also pass :start_branch_name if' +
' :start_project is different from current project'
end
unless start_project.repository.branch_exists?(start_branch_name)
raise ArgumentError,
"Cannot find branch #{branch_name} nor" \
" #{start_branch_name} from" \
" #{start_project.path_with_namespace}"
end
elsif start_branch_name
unless repository.branch_exists?(start_branch_name)
raise ArgumentError,
"Cannot find branch #{branch_name} nor" \
" #{start_branch_name} from" \
" #{repository.project.path_with_namespace}"
end
end
end
end
...@@ -47,9 +47,10 @@ module MergeRequests ...@@ -47,9 +47,10 @@ module MergeRequests
end end
def compare_branches def compare_branches
compare = CompareService.new.execute( compare = CompareService.new(
source_project, source_project,
source_branch, source_branch
).execute(
target_project, target_project,
target_branch target_branch
) )
......
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
...@@ -33,13 +33,15 @@ class EmailsOnPushWorker ...@@ -33,13 +33,15 @@ class EmailsOnPushWorker
reverse_compare = false reverse_compare = false
if action == :push if action == :push
compare = CompareService.new.execute(project, after_sha, project, before_sha) compare = CompareService.new(project, after_sha)
.execute(project, before_sha)
diff_refs = compare.diff_refs diff_refs = compare.diff_refs
return false if compare.same return false if compare.same
if compare.commits.empty? if compare.commits.empty?
compare = CompareService.new.execute(project, before_sha, project, after_sha) compare = CompareService.new(project, before_sha)
.execute(project, after_sha)
diff_refs = compare.diff_refs diff_refs = compare.diff_refs
reverse_compare = true reverse_compare = true
......
...@@ -54,7 +54,7 @@ module API ...@@ -54,7 +54,7 @@ module API
authorize! :push_code, user_project authorize! :push_code, user_project
attrs = declared_params attrs = declared_params
attrs[:source_branch] = attrs[:branch_name] attrs[:start_branch] = attrs[:branch_name]
attrs[:target_branch] = attrs[:branch_name] attrs[:target_branch] = attrs[:branch_name]
attrs[:actions].map! do |action| attrs[:actions].map! do |action|
action[:action] = action[:action].to_sym action[:action] = action[:action].to_sym
...@@ -139,8 +139,6 @@ module API ...@@ -139,8 +139,6 @@ module API
commit_params = { commit_params = {
commit: commit, commit: commit,
create_merge_request: false, create_merge_request: false,
source_project: user_project,
source_branch: commit.cherry_pick_branch_name,
target_branch: params[:branch] target_branch: params[:branch]
} }
......
...@@ -5,7 +5,7 @@ module API ...@@ -5,7 +5,7 @@ module API
def commit_params(attrs) def commit_params(attrs)
{ {
file_path: attrs[:file_path], file_path: attrs[:file_path],
source_branch: attrs[:branch_name], start_branch: attrs[:branch_name],
target_branch: attrs[:branch_name], target_branch: attrs[:branch_name],
commit_message: attrs[:commit_message], commit_message: attrs[:commit_message],
file_content: attrs[:content], file_content: attrs[:content],
......
...@@ -6,7 +6,7 @@ module Gitlab ...@@ -6,7 +6,7 @@ module Gitlab
class << self class << self
def ref_name(ref) def ref_name(ref)
ref.gsub(/\Arefs\/(tags|heads)\//, '') ref.sub(/\Arefs\/(tags|heads)\//, '')
end end
def branch_name(ref) def branch_name(ref)
......
...@@ -14,7 +14,8 @@ describe Projects::TemplatesController do ...@@ -14,7 +14,8 @@ describe Projects::TemplatesController do
before do before do
project.add_user(user, Gitlab::Access::MASTER) project.add_user(user, Gitlab::Access::MASTER)
project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) project.repository.commit_file(user, file_path_1, 'something valid',
message: 'test 3', branch_name: 'master', update: false)
end end
describe '#show' do describe '#show' do
......
...@@ -106,6 +106,42 @@ FactoryGirl.define do ...@@ -106,6 +106,42 @@ FactoryGirl.define do
path { 'gitlabhq' } path { 'gitlabhq' }
test_repo test_repo
transient do
create_template nil
end
after :create do |project, evaluator|
TestEnv.copy_repo(project)
if evaluator.create_template
args = evaluator.create_template
project.add_user(args[:user], args[:access])
project.repository.commit_file(
args[:user],
".gitlab/#{args[:path]}/bug.md",
'something valid',
message: 'test 3',
branch_name: 'master',
update: false)
project.repository.commit_file(
args[:user],
".gitlab/#{args[:path]}/template_test.md",
'template_test',
message: 'test 1',
branch_name: 'master',
update: false)
project.repository.commit_file(
args[:user],
".gitlab/#{args[:path]}/feature_proposal.md",
'feature_proposal',
message: 'test 2',
branch_name: 'master',
update: false)
end
end
end end
factory :forked_project_with_submodules, parent: :empty_project do factory :forked_project_with_submodules, parent: :empty_project do
......
...@@ -7,7 +7,7 @@ feature 'User wants to edit a file', feature: true do ...@@ -7,7 +7,7 @@ feature 'User wants to edit a file', feature: true do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:commit_params) do let(:commit_params) do
{ {
source_branch: project.default_branch, start_branch: project.default_branch,
target_branch: project.default_branch, target_branch: project.default_branch,
commit_message: "Committing First Update", commit_message: "Committing First Update",
file_path: ".gitignore", file_path: ".gitignore",
......
...@@ -6,7 +6,8 @@ feature 'project owner creates a license file', feature: true, js: true do ...@@ -6,7 +6,8 @@ feature 'project owner creates a license file', feature: true, js: true do
let(:project_master) { create(:user) } let(:project_master) { create(:user) }
let(:project) { create(:project) } let(:project) { create(:project) }
background do background do
project.repository.remove_file(project_master, 'LICENSE', 'Remove LICENSE', 'master') project.repository.remove_file(project_master, 'LICENSE',
message: 'Remove LICENSE', branch_name: 'master')
project.team << [project_master, :master] project.team << [project_master, :master]
login_as(project_master) login_as(project_master)
visit namespace_project_path(project.namespace, project) visit namespace_project_path(project.namespace, project)
......
...@@ -18,8 +18,20 @@ feature 'issuable templates', feature: true, js: true do ...@@ -18,8 +18,20 @@ feature 'issuable templates', feature: true, js: true do
let(:description_addition) { ' appending to description' } let(:description_addition) { ' appending to description' }
background do background do
project.repository.commit_file(user, '.gitlab/issue_templates/bug.md', template_content, 'added issue template', 'master', false) project.repository.commit_file(
project.repository.commit_file(user, '.gitlab/issue_templates/test.md', longtemplate_content, 'added issue template', 'master', false) user,
'.gitlab/issue_templates/bug.md',
template_content,
message: 'added issue template',
branch_name: 'master',
update: false)
project.repository.commit_file(
user,
'.gitlab/issue_templates/test.md',
longtemplate_content,
message: 'added issue template',
branch_name: 'master',
update: false)
visit edit_namespace_project_issue_path project.namespace, project, issue visit edit_namespace_project_issue_path project.namespace, project, issue
fill_in :'issue[title]', with: 'test issue title' fill_in :'issue[title]', with: 'test issue title'
end end
...@@ -67,7 +79,13 @@ feature 'issuable templates', feature: true, js: true do ...@@ -67,7 +79,13 @@ feature 'issuable templates', feature: true, js: true do
let(:issue) { create(:issue, author: user, assignee: user, project: project) } let(:issue) { create(:issue, author: user, assignee: user, project: project) }
background do background do
project.repository.commit_file(user, '.gitlab/issue_templates/bug.md', template_content, 'added issue template', 'master', false) project.repository.commit_file(
user,
'.gitlab/issue_templates/bug.md',
template_content,
message: 'added issue template',
branch_name: 'master',
update: false)
visit edit_namespace_project_issue_path project.namespace, project, issue visit edit_namespace_project_issue_path project.namespace, project, issue
fill_in :'issue[title]', with: 'test issue title' fill_in :'issue[title]', with: 'test issue title'
fill_in :'issue[description]', with: prior_description fill_in :'issue[description]', with: prior_description
...@@ -86,7 +104,13 @@ feature 'issuable templates', feature: true, js: true do ...@@ -86,7 +104,13 @@ feature 'issuable templates', feature: true, js: true do
let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) } let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) }
background do background do
project.repository.commit_file(user, '.gitlab/merge_request_templates/feature-proposal.md', template_content, 'added merge request template', 'master', false) project.repository.commit_file(
user,
'.gitlab/merge_request_templates/feature-proposal.md',
template_content,
message: 'added merge request template',
branch_name: 'master',
update: false)
visit edit_namespace_project_merge_request_path project.namespace, project, merge_request visit edit_namespace_project_merge_request_path project.namespace, project, merge_request
fill_in :'merge_request[title]', with: 'test merge request title' fill_in :'merge_request[title]', with: 'test merge request title'
end end
...@@ -111,7 +135,13 @@ feature 'issuable templates', feature: true, js: true do ...@@ -111,7 +135,13 @@ feature 'issuable templates', feature: true, js: true do
fork_project.team << [fork_user, :master] fork_project.team << [fork_user, :master]
create(:forked_project_link, forked_to_project: fork_project, forked_from_project: project) create(:forked_project_link, forked_to_project: fork_project, forked_from_project: project)
login_as fork_user login_as fork_user
project.repository.commit_file(fork_user, '.gitlab/merge_request_templates/feature-proposal.md', template_content, 'added merge request template', 'master', false) project.repository.commit_file(
fork_user,
'.gitlab/merge_request_templates/feature-proposal.md',
template_content,
message: 'added merge request template',
branch_name: 'master',
update: false)
visit edit_namespace_project_merge_request_path project.namespace, project, merge_request visit edit_namespace_project_merge_request_path project.namespace, project, merge_request
fill_in :'merge_request[title]', with: 'test merge request title' fill_in :'merge_request[title]', with: 'test merge request title'
end end
......
...@@ -99,7 +99,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do ...@@ -99,7 +99,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
Files::CreateService.new( Files::CreateService.new(
project, project,
current_user, current_user,
source_branch: branch_name, start_branch: branch_name,
target_branch: branch_name, target_branch: branch_name,
commit_message: "Create file", commit_message: "Create file",
file_path: file_name, file_path: file_name,
...@@ -112,7 +112,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do ...@@ -112,7 +112,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
Files::UpdateService.new( Files::UpdateService.new(
project, project,
current_user, current_user,
source_branch: branch_name, start_branch: branch_name,
target_branch: branch_name, target_branch: branch_name,
commit_message: "Update file", commit_message: "Update file",
file_path: file_name, file_path: file_name,
...@@ -125,7 +125,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do ...@@ -125,7 +125,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
Files::DeleteService.new( Files::DeleteService.new(
project, project,
current_user, current_user,
source_branch: branch_name, start_branch: branch_name,
target_branch: branch_name, target_branch: branch_name,
commit_message: "Delete file", commit_message: "Delete file",
file_path: file_name file_path: file_name
......
...@@ -209,7 +209,13 @@ describe Gitlab::GitAccess, lib: true do ...@@ -209,7 +209,13 @@ describe Gitlab::GitAccess, lib: true do
stub_git_hooks stub_git_hooks
project.repository.add_branch(user, unprotected_branch, 'feature') project.repository.add_branch(user, unprotected_branch, 'feature')
target_branch = project.repository.lookup('feature') target_branch = project.repository.lookup('feature')
source_branch = project.repository.commit_file(user, FFaker::InternetSE.login_user_name, FFaker::HipsterIpsum.paragraph, FFaker::HipsterIpsum.sentence, unprotected_branch, false) source_branch = project.repository.commit_file(
user,
FFaker::InternetSE.login_user_name,
FFaker::HipsterIpsum.paragraph,
message: FFaker::HipsterIpsum.sentence,
branch_name: unprotected_branch,
update: false)
rugged = project.repository.rugged rugged = project.repository.rugged
author = { email: "email@example.com", time: Time.now, name: "Example Git User" } author = { email: "email@example.com", time: Time.now, name: "Example Git User" }
......
...@@ -4,16 +4,14 @@ describe Gitlab::Template::IssueTemplate do ...@@ -4,16 +4,14 @@ describe Gitlab::Template::IssueTemplate do
subject { described_class } subject { described_class }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:file_path_1) { '.gitlab/issue_templates/bug.md' } let(:project) do
let(:file_path_2) { '.gitlab/issue_templates/template_test.md' } create(:project,
let(:file_path_3) { '.gitlab/issue_templates/feature_proposal.md' } :repository,
create_template: {
before do user: user,
project.add_user(user, Gitlab::Access::MASTER) access: Gitlab::Access::MASTER,
project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) path: 'issue_templates' })
project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false)
project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false)
end end
describe '.all' do describe '.all' do
......
...@@ -4,16 +4,14 @@ describe Gitlab::Template::MergeRequestTemplate do ...@@ -4,16 +4,14 @@ describe Gitlab::Template::MergeRequestTemplate do
subject { described_class } subject { described_class }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:file_path_1) { '.gitlab/merge_request_templates/bug.md' } let(:project) do
let(:file_path_2) { '.gitlab/merge_request_templates/template_test.md' } create(:project,
let(:file_path_3) { '.gitlab/merge_request_templates/feature_proposal.md' } :repository,
create_template: {
before do user: user,
project.add_user(user, Gitlab::Access::MASTER) access: Gitlab::Access::MASTER,
project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) path: 'merge_request_templates' })
project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false)
project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false)
end end
describe '.all' do describe '.all' do
......
...@@ -21,7 +21,13 @@ describe 'CycleAnalytics#production', feature: true do ...@@ -21,7 +21,13 @@ describe 'CycleAnalytics#production', feature: true do
["production deploy happens after merge request is merged (along with other changes)", ["production deploy happens after merge request is merged (along with other changes)",
lambda do |context, data| lambda do |context, data|
# Make other changes on master # Make other changes on master
sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false) sha = context.project.repository.commit_file(
context.user,
context.random_git_name,
'content',
message: 'commit message',
branch_name: 'master',
update: false)
context.project.repository.commit(sha) context.project.repository.commit(sha)
context.deploy_master context.deploy_master
......
...@@ -29,10 +29,10 @@ describe 'CycleAnalytics#staging', feature: true do ...@@ -29,10 +29,10 @@ describe 'CycleAnalytics#staging', feature: true do
sha = context.project.repository.commit_file( sha = context.project.repository.commit_file(
context.user, context.user,
context.random_git_name, context.random_git_name,
"content", 'content',
"commit message", message: 'commit message',
'master', branch_name: 'master',
false) update: false)
context.project.repository.commit(sha) context.project.repository.commit(sha)
context.deploy_master context.deploy_master
......
...@@ -289,17 +289,39 @@ describe Repository, models: true do ...@@ -289,17 +289,39 @@ describe Repository, models: true do
describe "#commit_dir" do describe "#commit_dir" do
it "commits a change that creates a new directory" do it "commits a change that creates a new directory" do
expect do expect do
repository.commit_dir(user, 'newdir', 'Create newdir', 'master') repository.commit_dir(user, 'newdir',
message: 'Create newdir', branch_name: 'master')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.commits('master').count }.by(1)
newdir = repository.tree('master', 'newdir') newdir = repository.tree('master', 'newdir')
expect(newdir.path).to eq('newdir') expect(newdir.path).to eq('newdir')
end end
context "when committing to another project" do
let(:forked_project) { create(:project) }
it "creates a fork and commit to the forked project" do
expect do
repository.commit_dir(user, 'newdir',
message: 'Create newdir', branch_name: 'patch',
start_branch_name: 'master', start_project: forked_project)
end.to change { repository.commits('master').count }.by(0)
expect(repository.branch_exists?('patch')).to be_truthy
expect(forked_project.repository.branch_exists?('patch')).to be_falsy
newdir = repository.tree('patch', 'newdir')
expect(newdir.path).to eq('newdir')
end
end
context "when an author is specified" do context "when an author is specified" do
it "uses the given email/name to set the commit's author" do it "uses the given email/name to set the commit's author" do
expect do expect do
repository.commit_dir(user, "newdir", "Add newdir", 'master', author_email: author_email, author_name: author_name) repository.commit_dir(user, 'newdir',
message: 'Add newdir',
branch_name: 'master',
author_email: author_email, author_name: author_name)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.commits('master').count }.by(1)
last_commit = repository.commit last_commit = repository.commit
...@@ -314,8 +336,9 @@ describe Repository, models: true do ...@@ -314,8 +336,9 @@ describe Repository, models: true do
it 'commits change to a file successfully' do it 'commits change to a file successfully' do
expect do expect do
repository.commit_file(user, 'CHANGELOG', 'Changelog!', repository.commit_file(user, 'CHANGELOG', 'Changelog!',
'Updates file content', message: 'Updates file content',
'master', true) branch_name: 'master',
update: true)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.commits('master').count }.by(1)
blob = repository.blob_at('master', 'CHANGELOG') blob = repository.blob_at('master', 'CHANGELOG')
...@@ -326,8 +349,12 @@ describe Repository, models: true do ...@@ -326,8 +349,12 @@ describe Repository, models: true do
context "when an author is specified" do context "when an author is specified" do
it "uses the given email/name to set the commit's author" do it "uses the given email/name to set the commit's author" do
expect do expect do
repository.commit_file(user, "README", 'README!', 'Add README', repository.commit_file(user, 'README', 'README!',
'master', true, author_email: author_email, author_name: author_name) message: 'Add README',
branch_name: 'master',
update: true,
author_email: author_email,
author_name: author_name)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.commits('master').count }.by(1)
last_commit = repository.commit last_commit = repository.commit
...@@ -342,7 +369,7 @@ describe Repository, models: true do ...@@ -342,7 +369,7 @@ describe Repository, models: true do
it 'updates filename successfully' do it 'updates filename successfully' do
expect do expect do
repository.update_file(user, 'NEWLICENSE', 'Copyright!', repository.update_file(user, 'NEWLICENSE', 'Copyright!',
branch: 'master', branch_name: 'master',
previous_path: 'LICENSE', previous_path: 'LICENSE',
message: 'Changes filename') message: 'Changes filename')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.commits('master').count }.by(1)
...@@ -355,15 +382,16 @@ describe Repository, models: true do ...@@ -355,15 +382,16 @@ describe Repository, models: true do
context "when an author is specified" do context "when an author is specified" do
it "uses the given email/name to set the commit's author" do it "uses the given email/name to set the commit's author" do
repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) repository.commit_file(user, 'README', 'README!',
message: 'Add README', branch_name: 'master', update: true)
expect do expect do
repository.update_file(user, 'README', "Updated README!", repository.update_file(user, 'README', 'Updated README!',
branch: 'master', branch_name: 'master',
previous_path: 'README', previous_path: 'README',
message: 'Update README', message: 'Update README',
author_email: author_email, author_email: author_email,
author_name: author_name) author_name: author_name)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.commits('master').count }.by(1)
last_commit = repository.commit last_commit = repository.commit
...@@ -376,10 +404,12 @@ describe Repository, models: true do ...@@ -376,10 +404,12 @@ describe Repository, models: true do
describe "#remove_file" do describe "#remove_file" do
it 'removes file successfully' do it 'removes file successfully' do
repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) repository.commit_file(user, 'README', 'README!',
message: 'Add README', branch_name: 'master', update: true)
expect do expect do
repository.remove_file(user, "README", "Remove README", 'master') repository.remove_file(user, 'README',
message: 'Remove README', branch_name: 'master')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.commits('master').count }.by(1)
expect(repository.blob_at('master', 'README')).to be_nil expect(repository.blob_at('master', 'README')).to be_nil
...@@ -387,10 +417,13 @@ describe Repository, models: true do ...@@ -387,10 +417,13 @@ describe Repository, models: true do
context "when an author is specified" do context "when an author is specified" do
it "uses the given email/name to set the commit's author" do it "uses the given email/name to set the commit's author" do
repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) repository.commit_file(user, 'README', 'README!',
message: 'Add README', branch_name: 'master', update: true)
expect do expect do
repository.remove_file(user, "README", "Remove README", 'master', author_email: author_email, author_name: author_name) repository.remove_file(user, 'README',
message: 'Remove README', branch_name: 'master',
author_email: author_email, author_name: author_name)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.commits('master').count }.by(1)
last_commit = repository.commit last_commit = repository.commit
...@@ -538,11 +571,14 @@ describe Repository, models: true do ...@@ -538,11 +571,14 @@ describe Repository, models: true do
describe "#license_blob", caching: true do describe "#license_blob", caching: true do
before do before do
repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master') repository.remove_file(
user, 'LICENSE', message: 'Remove LICENSE', branch_name: 'master')
end end
it 'handles when HEAD points to non-existent ref' do it 'handles when HEAD points to non-existent ref' do
repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false) repository.commit_file(
user, 'LICENSE', 'Copyright!',
message: 'Add LICENSE', branch_name: 'master', update: false)
allow(repository).to receive(:file_on_head). allow(repository).to receive(:file_on_head).
and_raise(Rugged::ReferenceError) and_raise(Rugged::ReferenceError)
...@@ -551,21 +587,27 @@ describe Repository, models: true do ...@@ -551,21 +587,27 @@ describe Repository, models: true do
end end
it 'looks in the root_ref only' do it 'looks in the root_ref only' do
repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'markdown') repository.remove_file(user, 'LICENSE',
repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'markdown', false) message: 'Remove LICENSE', branch_name: 'markdown')
repository.commit_file(user, 'LICENSE',
Licensee::License.new('mit').content,
message: 'Add LICENSE', branch_name: 'markdown', update: false)
expect(repository.license_blob).to be_nil expect(repository.license_blob).to be_nil
end end
it 'detects license file with no recognizable open-source license content' do it 'detects license file with no recognizable open-source license content' do
repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false) repository.commit_file(user, 'LICENSE', 'Copyright!',
message: 'Add LICENSE', branch_name: 'master', update: false)
expect(repository.license_blob.name).to eq('LICENSE') expect(repository.license_blob.name).to eq('LICENSE')
end end
%w[LICENSE LICENCE LiCensE LICENSE.md LICENSE.foo COPYING COPYING.md].each do |filename| %w[LICENSE LICENCE LiCensE LICENSE.md LICENSE.foo COPYING COPYING.md].each do |filename|
it "detects '#{filename}'" do it "detects '#{filename}'" do
repository.commit_file(user, filename, Licensee::License.new('mit').content, "Add #{filename}", 'master', false) repository.commit_file(user, filename,
Licensee::License.new('mit').content,
message: "Add #{filename}", branch_name: 'master', update: false)
expect(repository.license_blob.name).to eq(filename) expect(repository.license_blob.name).to eq(filename)
end end
...@@ -574,7 +616,8 @@ describe Repository, models: true do ...@@ -574,7 +616,8 @@ describe Repository, models: true do
describe '#license_key', caching: true do describe '#license_key', caching: true do
before do before do
repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master') repository.remove_file(user, 'LICENSE',
message: 'Remove LICENSE', branch_name: 'master')
end end
it 'returns nil when no license is detected' do it 'returns nil when no license is detected' do
...@@ -588,13 +631,16 @@ describe Repository, models: true do ...@@ -588,13 +631,16 @@ describe Repository, models: true do
end end
it 'detects license file with no recognizable open-source license content' do it 'detects license file with no recognizable open-source license content' do
repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false) repository.commit_file(user, 'LICENSE', 'Copyright!',
message: 'Add LICENSE', branch_name: 'master', update: false)
expect(repository.license_key).to be_nil expect(repository.license_key).to be_nil
end end
it 'returns the license key' do it 'returns the license key' do
repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'master', false) repository.commit_file(user, 'LICENSE',
Licensee::License.new('mit').content,
message: 'Add LICENSE', branch_name: 'master', update: false)
expect(repository.license_key).to eq('mit') expect(repository.license_key).to eq('mit')
end end
...@@ -707,7 +753,7 @@ describe Repository, models: true do ...@@ -707,7 +753,7 @@ describe Repository, models: true do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do expect do
repository.rm_branch(user, 'new_feature') repository.rm_branch(user, 'feature')
end.to raise_error(GitHooksService::PreReceiveError) end.to raise_error(GitHooksService::PreReceiveError)
end end
...@@ -728,36 +774,51 @@ describe Repository, models: true do ...@@ -728,36 +774,51 @@ describe Repository, models: true do
context 'when pre hooks were successful' do context 'when pre hooks were successful' do
before do before do
expect_any_instance_of(GitHooksService).to receive(:execute). service = GitHooksService.new
with(user, repository.path_to_repo, old_rev, new_rev, 'refs/heads/feature'). expect(GitHooksService).to receive(:new).and_return(service)
and_yield.and_return(true) expect(service).to receive(:execute).
with(
user,
repository.path_to_repo,
old_rev,
new_rev,
'refs/heads/feature').
and_yield(service).and_return(true)
end end
it 'runs without errors' do it 'runs without errors' do
expect do expect do
repository.update_branch_with_hooks(user, 'feature') { new_rev } GitOperationService.new(user, repository).with_branch('feature') do
new_rev
end
end.not_to raise_error end.not_to raise_error
end end
it 'ensures the autocrlf Git option is set to :input' do it 'ensures the autocrlf Git option is set to :input' do
expect(repository).to receive(:update_autocrlf_option) service = GitOperationService.new(user, repository)
expect(service).to receive(:update_autocrlf_option)
repository.update_branch_with_hooks(user, 'feature') { new_rev } service.with_branch('feature') { new_rev }
end end
context "when the branch wasn't empty" do context "when the branch wasn't empty" do
it 'updates the head' do it 'updates the head' do
expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev) expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev)
repository.update_branch_with_hooks(user, 'feature') { new_rev }
GitOperationService.new(user, repository).with_branch('feature') do
new_rev
end
expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev) expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev)
end end
end end
end end
context 'when the update adds more than one commit' do context 'when the update adds more than one commit' do
it 'runs without errors' do let(:old_rev) { '33f3729a45c02fc67d00adb1b8bca394b0e761d9' }
old_rev = '33f3729a45c02fc67d00adb1b8bca394b0e761d9'
it 'runs without errors' do
# old_rev is an ancestor of new_rev # old_rev is an ancestor of new_rev
expect(repository.rugged.merge_base(old_rev, new_rev)).to eq(old_rev) expect(repository.rugged.merge_base(old_rev, new_rev)).to eq(old_rev)
...@@ -767,22 +828,28 @@ describe Repository, models: true do ...@@ -767,22 +828,28 @@ describe Repository, models: true do
branch = 'feature-ff-target' branch = 'feature-ff-target'
repository.add_branch(user, branch, old_rev) repository.add_branch(user, branch, old_rev)
expect { repository.update_branch_with_hooks(user, branch) { new_rev } }.not_to raise_error expect do
GitOperationService.new(user, repository).with_branch(branch) do
new_rev
end
end.not_to raise_error
end end
end end
context 'when the update would remove commits from the target branch' do context 'when the update would remove commits from the target branch' do
it 'raises an exception' do let(:branch) { 'master' }
branch = 'master' let(:old_rev) { repository.find_branch(branch).dereferenced_target.sha }
old_rev = repository.find_branch(branch).dereferenced_target.sha
it 'raises an exception' do
# The 'master' branch is NOT an ancestor of new_rev. # The 'master' branch is NOT an ancestor of new_rev.
expect(repository.rugged.merge_base(old_rev, new_rev)).not_to eq(old_rev) expect(repository.rugged.merge_base(old_rev, new_rev)).not_to eq(old_rev)
# Updating 'master' to new_rev would lose the commits on 'master' that # Updating 'master' to new_rev would lose the commits on 'master' that
# are not contained in new_rev. This should not be allowed. # are not contained in new_rev. This should not be allowed.
expect do expect do
repository.update_branch_with_hooks(user, branch) { new_rev } GitOperationService.new(user, repository).with_branch(branch) do
new_rev
end
end.to raise_error(Repository::CommitError) end.to raise_error(Repository::CommitError)
end end
end end
...@@ -792,7 +859,9 @@ describe Repository, models: true do ...@@ -792,7 +859,9 @@ describe Repository, models: true do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do expect do
repository.update_branch_with_hooks(user, 'feature') { new_rev } GitOperationService.new(user, repository).with_branch('feature') do
new_rev
end
end.to raise_error(GitHooksService::PreReceiveError) end.to raise_error(GitHooksService::PreReceiveError)
end end
end end
...@@ -800,7 +869,6 @@ describe Repository, models: true do ...@@ -800,7 +869,6 @@ describe Repository, models: true do
context 'when target branch is different from source branch' do context 'when target branch is different from source branch' do
before do before do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, '']) allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, ''])
allow(repository).to receive(:update_ref!)
end end
it 'expires branch cache' do it 'expires branch cache' do
...@@ -809,7 +877,10 @@ describe Repository, models: true do ...@@ -809,7 +877,10 @@ describe Repository, models: true do
expect(repository).not_to receive(:expire_emptiness_caches) expect(repository).not_to receive(:expire_emptiness_caches)
expect(repository).to receive(:expire_branches_cache) expect(repository).to receive(:expire_branches_cache)
repository.update_branch_with_hooks(user, 'new-feature') { new_rev } GitOperationService.new(user, repository).
with_branch('new-feature') do
new_rev
end
end end
end end
...@@ -827,7 +898,9 @@ describe Repository, models: true do ...@@ -827,7 +898,9 @@ describe Repository, models: true do
expect(empty_repository).to receive(:expire_branches_cache) expect(empty_repository).to receive(:expire_branches_cache)
empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!', empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!',
'Updates file content', 'master', false) message: 'Updates file content',
branch_name: 'master',
update: false)
end end
end end
end end
...@@ -877,7 +950,7 @@ describe Repository, models: true do ...@@ -877,7 +950,7 @@ describe Repository, models: true do
end end
it 'sets autocrlf to :input' do it 'sets autocrlf to :input' do
repository.update_autocrlf_option GitOperationService.new(nil, repository).send(:update_autocrlf_option)
expect(repository.raw_repository.autocrlf).to eq(:input) expect(repository.raw_repository.autocrlf).to eq(:input)
end end
...@@ -892,7 +965,7 @@ describe Repository, models: true do ...@@ -892,7 +965,7 @@ describe Repository, models: true do
expect(repository.raw_repository).not_to receive(:autocrlf=). expect(repository.raw_repository).not_to receive(:autocrlf=).
with(:input) with(:input)
repository.update_autocrlf_option GitOperationService.new(nil, repository).send(:update_autocrlf_option)
end end
end end
end end
...@@ -1388,9 +1461,10 @@ describe Repository, models: true do ...@@ -1388,9 +1461,10 @@ describe Repository, models: true do
describe '#rm_tag' do describe '#rm_tag' do
it 'removes a tag' do it 'removes a tag' do
expect(repository).to receive(:before_remove_tag) expect(repository).to receive(:before_remove_tag)
expect(repository.rugged.tags).to receive(:delete).with('v1.1.0')
repository.rm_tag('v1.1.0') repository.rm_tag(create(:user), 'v1.1.0')
expect(repository.find_tag('v1.1.0')).to be_nil
end end
end end
...@@ -1458,16 +1532,16 @@ describe Repository, models: true do ...@@ -1458,16 +1532,16 @@ describe Repository, models: true do
end end
end end
describe '#update_ref!' do describe '#update_ref' do
it 'can create a ref' do it 'can create a ref' do
repository.update_ref!('refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA) GitOperationService.new(nil, repository).send(:update_ref, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
expect(repository.find_branch('foobar')).not_to be_nil expect(repository.find_branch('foobar')).not_to be_nil
end end
it 'raises CommitError when the ref update fails' do it 'raises CommitError when the ref update fails' do
expect do expect do
repository.update_ref!('refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA) GitOperationService.new(nil, repository).send(:update_ref, 'refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
end.to raise_error(Repository::CommitError) end.to raise_error(Repository::CommitError)
end end
end end
......
...@@ -3,17 +3,17 @@ require 'spec_helper' ...@@ -3,17 +3,17 @@ require 'spec_helper'
describe CompareService, services: true do describe CompareService, services: true do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:service) { described_class.new } let(:service) { described_class.new(project, 'feature') }
describe '#execute' do describe '#execute' do
context 'compare with base, like feature...fix' do context 'compare with base, like feature...fix' do
subject { service.execute(project, 'feature', project, 'fix', straight: false) } subject { service.execute(project, 'fix', straight: false) }
it { expect(subject.diffs.size).to eq(1) } it { expect(subject.diffs.size).to eq(1) }
end end
context 'straight compare, like feature..fix' do context 'straight compare, like feature..fix' do
subject { service.execute(project, 'feature', project, 'fix', straight: true) } subject { service.execute(project, 'fix', straight: true) }
it { expect(subject.diffs.size).to eq(3) } it { expect(subject.diffs.size).to eq(3) }
end end
......
...@@ -6,7 +6,10 @@ describe Files::UpdateService do ...@@ -6,7 +6,10 @@ describe Files::UpdateService do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:file_path) { 'files/ruby/popen.rb' } let(:file_path) { 'files/ruby/popen.rb' }
let(:new_contents) { "New Content" } let(:new_contents) { 'New Content' }
let(:target_branch) { project.default_branch }
let(:last_commit_sha) { nil }
let(:commit_params) do let(:commit_params) do
{ {
file_path: file_path, file_path: file_path,
...@@ -14,9 +17,9 @@ describe Files::UpdateService do ...@@ -14,9 +17,9 @@ describe Files::UpdateService do
file_content: new_contents, file_content: new_contents,
file_content_encoding: "text", file_content_encoding: "text",
last_commit_sha: last_commit_sha, last_commit_sha: last_commit_sha,
source_project: project, start_project: project,
source_branch: project.default_branch, start_branch: project.default_branch,
target_branch: project.default_branch, target_branch: target_branch
} }
end end
...@@ -54,18 +57,6 @@ describe Files::UpdateService do ...@@ -54,18 +57,6 @@ describe Files::UpdateService do
end end
context "when the last_commit_sha is not supplied" do context "when the last_commit_sha is not supplied" do
let(:commit_params) do
{
file_path: file_path,
commit_message: "Update File",
file_content: new_contents,
file_content_encoding: "text",
source_project: project,
source_branch: project.default_branch,
target_branch: project.default_branch,
}
end
it "returns a hash with the :success status " do it "returns a hash with the :success status " do
results = subject.execute results = subject.execute
...@@ -80,5 +71,15 @@ describe Files::UpdateService do ...@@ -80,5 +71,15 @@ describe Files::UpdateService do
expect(results.data).to eq(new_contents) expect(results.data).to eq(new_contents)
end end
end end
context 'when target branch is different than source branch' do
let(:target_branch) { "#{project.default_branch}-new" }
it 'fires hooks only once' do
expect(GitHooksService).to receive(:new).once.and_call_original
subject.execute
end
end
end end
end end
...@@ -21,7 +21,7 @@ describe GitHooksService, services: true do ...@@ -21,7 +21,7 @@ describe GitHooksService, services: true do
hook = double(trigger: [true, nil]) hook = double(trigger: [true, nil])
expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
expect(service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }).to eq([true, nil]) service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }
end end
end end
......
...@@ -66,7 +66,13 @@ describe MergeRequests::ResolveService do ...@@ -66,7 +66,13 @@ describe MergeRequests::ResolveService do
context 'when the source project is a fork and does not contain the HEAD of the target branch' do context 'when the source project is a fork and does not contain the HEAD of the target branch' do
let!(:target_head) do let!(:target_head) do
project.repository.commit_file(user, 'new-file-in-target', '', 'Add new file in target', 'conflict-start', false) project.repository.commit_file(
user,
'new-file-in-target',
'',
message: 'Add new file in target',
branch_name: 'conflict-start',
update: false)
end end
before do before do
......
...@@ -35,7 +35,13 @@ module CycleAnalyticsHelpers ...@@ -35,7 +35,13 @@ module CycleAnalyticsHelpers
project.repository.add_branch(user, source_branch, 'master') project.repository.add_branch(user, source_branch, 'master')
end end
sha = project.repository.commit_file(user, random_git_name, "content", "commit message", source_branch, false) sha = project.repository.commit_file(
user,
random_git_name,
'content',
message: 'commit message',
branch_name: source_branch,
update: false)
project.repository.commit(sha) project.repository.commit(sha)
opts = { opts = {
......
...@@ -107,7 +107,8 @@ describe GitGarbageCollectWorker do ...@@ -107,7 +107,8 @@ describe GitGarbageCollectWorker do
tree: old_commit.tree, tree: old_commit.tree,
parents: [old_commit], parents: [old_commit],
) )
project.repository.update_ref!( GitOperationService.new(nil, project.repository).send(
:update_ref,
"refs/heads/#{SecureRandom.hex(6)}", "refs/heads/#{SecureRandom.hex(6)}",
new_commit_sha, new_commit_sha,
Gitlab::Git::BLANK_SHA Gitlab::Git::BLANK_SHA
......
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