summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-02-02 23:26:35 +0000
committerDouwe Maan <douwe@gitlab.com>2017-02-02 23:26:35 +0000
commite7d530a624fbb8854047c3983eaa0972a19f36c8 (patch)
tree92f77913a359ccf4078cf010add02d8b8f765490
parentdce77b1d6d5e2b82441a685d385ee809c6b35b76 (diff)
parent54fca95160389fe7993df5d82635b83804833fee (diff)
downloadgitlab-ce-e7d530a624fbb8854047c3983eaa0972a19f36c8.tar.gz
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
-rw-r--r--app/controllers/concerns/creates_commit.rb53
-rw-r--r--app/controllers/projects/commit_controller.rb8
-rw-r--r--app/controllers/projects/compare_controller.rb3
-rw-r--r--app/models/merge_request_diff.rb3
-rw-r--r--app/models/repository.rb493
-rw-r--r--app/services/commits/change_service.rb34
-rw-r--r--app/services/compare_service.rb26
-rw-r--r--app/services/create_branch_service.rb30
-rw-r--r--app/services/delete_tag_service.rb2
-rw-r--r--app/services/files/base_service.rb25
-rw-r--r--app/services/files/create_dir_service.rb10
-rw-r--r--app/services/files/create_service.rb14
-rw-r--r--app/services/files/delete_service.rb10
-rw-r--r--app/services/files/multi_service.rb8
-rw-r--r--app/services/files/update_service.rb10
-rw-r--r--app/services/git_hooks_service.rb6
-rw-r--r--app/services/git_operation_service.rb179
-rw-r--r--app/services/merge_requests/build_service.rb5
-rw-r--r--app/services/validate_new_branch_service.rb22
-rw-r--r--app/workers/emails_on_push_worker.rb6
-rw-r--r--lib/api/commits.rb4
-rw-r--r--lib/api/files.rb2
-rw-r--r--lib/gitlab/git.rb2
-rw-r--r--spec/controllers/projects/templates_controller_spec.rb3
-rw-r--r--spec/factories/projects.rb36
-rw-r--r--spec/features/projects/files/editing_a_file_spec.rb2
-rw-r--r--spec/features/projects/files/project_owner_creates_license_file_spec.rb3
-rw-r--r--spec/features/projects/issuable_templates_spec.rb40
-rw-r--r--spec/lib/gitlab/diff/position_tracer_spec.rb6
-rw-r--r--spec/lib/gitlab/git_access_spec.rb8
-rw-r--r--spec/lib/gitlab/template/issue_template_spec.rb18
-rw-r--r--spec/lib/gitlab/template/merge_request_template_spec.rb18
-rw-r--r--spec/models/cycle_analytics/production_spec.rb8
-rw-r--r--spec/models/cycle_analytics/staging_spec.rb8
-rw-r--r--spec/models/repository_spec.rb180
-rw-r--r--spec/services/compare_service_spec.rb6
-rw-r--r--spec/services/files/update_service_spec.rb33
-rw-r--r--spec/services/git_hooks_service_spec.rb2
-rw-r--r--spec/services/merge_requests/resolve_service_spec.rb8
-rw-r--r--spec/support/cycle_analytics_helpers.rb8
-rw-r--r--spec/workers/git_garbage_collect_worker_spec.rb3
41 files changed, 906 insertions, 439 deletions
diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb
index 6f43ce5226d..6286d67d30c 100644
--- a/app/controllers/concerns/creates_commit.rb
+++ b/app/controllers/concerns/creates_commit.rb
@@ -4,13 +4,15 @@ module CreatesCommit
def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil)
set_commit_variables
+ start_branch = @mr_target_branch unless initial_commit?
commit_params = @commit_params.merge(
- source_project: @project,
- source_branch: @ref,
- target_branch: @target_branch
+ start_project: @mr_target_project,
+ start_branch: start_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
update_flash_notice(success_notice)
@@ -89,20 +91,18 @@ module CreatesCommit
@mr_source_project != @mr_target_project
end
- def different_branch?
- @mr_source_branch != @mr_target_branch || different_project?
- end
-
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
+ # TODO: We should really clean this up
def set_commit_variables
- @mr_source_branch ||= @target_branch
-
if can?(current_user, :push_code, @project)
# Edit file in this project
- @tree_edit_project = @project
@mr_source_project = @project
if @project.forked?
@@ -112,15 +112,34 @@ module CreatesCommit
else
# Merge request to this project
@mr_target_project = @project
- @mr_target_branch ||= @ref
+ @mr_target_branch = @ref || @target_branch
end
else
- # Edit file in fork
- @tree_edit_project = current_user.fork_of(@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_branch ||= @ref
+ @mr_target_branch = @ref || @target_branch
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
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index c871043efbd..b5a7078a3a1 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -50,7 +50,7 @@ class Projects::CommitController < Projects::ApplicationController
end
def revert
- assign_change_commit_vars(@commit.revert_branch_name)
+ assign_change_commit_vars
return render_404 if @target_branch.blank?
@@ -59,7 +59,7 @@ class Projects::CommitController < Projects::ApplicationController
end
def cherry_pick
- assign_change_commit_vars(@commit.cherry_pick_branch_name)
+ assign_change_commit_vars
return render_404 if @target_branch.blank?
@@ -116,11 +116,9 @@ class Projects::CommitController < Projects::ApplicationController
}
end
- def assign_change_commit_vars(mr_source_branch)
+ def assign_change_commit_vars
@commit = project.commit(params[:id])
@target_branch = params[:target_branch]
- @mr_source_branch = mr_source_branch
- @mr_target_branch = @target_branch
@commit_params = {
commit: @commit,
create_merge_request: params[:create_merge_request].present? || different_project?
diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index d32966645c8..321cde255c3 100644
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@@ -46,7 +46,8 @@ class Projects::CompareController < Projects::ApplicationController
end
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
@commits = @compare.commits
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index dadb81f9b6e..70bad2a4396 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -169,7 +169,8 @@ class MergeRequestDiff < ActiveRecord::Base
# 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.
# 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
def commits_count
diff --git a/app/models/repository.rb b/app/models/repository.rb
index d77b7692d75..a54d75f7019 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -5,7 +5,7 @@ class Repository
attr_accessor :path_with_namespace, :project
- class CommitError < StandardError; end
+ CommitError = Class.new(StandardError)
# Methods that cache data from the Git repository.
#
@@ -64,10 +64,6 @@ class Repository
@raw_repository ||= Gitlab::Git::Repository.new(path_to_repo)
end
- def update_autocrlf_option
- raw_repository.autocrlf = :input if raw_repository.autocrlf != :input
- end
-
# Return absolute path to repository
def path_to_repo
@path_to_repo ||= File.expand_path(
@@ -168,63 +164,46 @@ class Repository
tags.find { |tag| tag.name == name }
end
- def add_branch(user, branch_name, target)
- oldrev = Gitlab::Git::BLANK_SHA
- ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
- target = commit(target).try(:id)
+ def add_branch(user, branch_name, ref)
+ newrev = commit(ref).try(:sha)
- return false unless target
+ return false unless newrev
- GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do
- update_ref!(ref, target, oldrev)
- end
+ GitOperationService.new(user, self).add_branch(branch_name, newrev)
after_create_branch
find_branch(branch_name)
end
def add_tag(user, tag_name, target, message = nil)
- oldrev = Gitlab::Git::BLANK_SHA
- ref = Gitlab::Git::TAG_REF_PREFIX + tag_name
- target = commit(target).try(:id)
-
- return false unless target
-
+ newrev = commit(target).try(:id)
options = { message: message, tagger: user_to_committer(user) } if message
- GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do |service|
- raw_tag = rugged.tags.create(tag_name, target, options)
- service.newrev = raw_tag.target_id
- end
+ return false unless newrev
+
+ GitOperationService.new(user, self).add_tag(tag_name, newrev, options)
find_tag(tag_name)
end
def rm_branch(user, branch_name)
before_remove_branch
-
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
- update_ref!(ref, newrev, oldrev)
- end
+ GitOperationService.new(user, self).rm_branch(branch)
after_remove_branch
true
end
- def rm_tag(tag_name)
+ def rm_tag(user, tag_name)
before_remove_tag
+ tag = find_tag(tag_name)
- begin
- rugged.tags.delete(tag_name)
- true
- rescue Rugged::ReferenceError
- false
- end
+ GitOperationService.new(user, self).rm_tag(tag)
+
+ after_remove_tag
+ true
end
def ref_names
@@ -241,21 +220,6 @@ class Repository
false
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.
# 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
@@ -435,6 +399,11 @@ class Repository
repository_event(:remove_tag)
end
+ # Runs code after removing a tag.
+ def after_remove_tag
+ expire_tags_cache
+ end
+
def before_import
expire_content_cache
end
@@ -779,121 +748,132 @@ class Repository
@tags ||= raw_repository.tags
end
- def commit_dir(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
- }
- }
-
- 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))
+ # rubocop:disable Metrics/ParameterLists
+ def commit_dir(
+ user, path,
+ message:, branch_name:,
+ author_email: nil, author_name: nil,
+ start_branch_name: nil, start_project: project)
+ check_tree_entry_for_dir(branch_name, path)
- 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
-
- 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
- options[:file][:previous_path] = previous_path
- Gitlab::Git::Blob.rename(raw_repository, options)
- else
- Gitlab::Git::Blob.commit(raw_repository, options)
+ commit_file(
+ user,
+ "#{path}/.gitkeep",
+ '',
+ 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
-
- 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
- def multi_action(user:, branch:, message:, actions:, author_email: nil, author_name: nil)
- update_branch_with_hooks(user, branch) do |ref|
+ 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: :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
- 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
+ parents = if start_commit
+ index.read_tree(start_commit.raw_commit.tree)
+ [start_commit.sha]
+ else
+ []
+ end
- actions.each do |action|
- case action[:action]
- 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
+ actions.each do |act|
+ git_action(index, act)
end
options = {
@@ -906,6 +886,7 @@ class Repository
Rugged::Commit.create(rugged, options)
end
end
+ # rubocop:enable Metrics/ParameterLists
def get_committer_and_author(user, email: nil, name: nil)
committer = user_to_committer(user)
@@ -918,7 +899,7 @@ class Repository
end
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
def can_be_merged?(source_sha, target_branch)
@@ -933,16 +914,17 @@ class Repository
end
def merge(user, merge_request, options = {})
- our_commit = rugged.branches[merge_request.target_branch].target
- their_commit = rugged.lookup(merge_request.diff_head_sha)
+ GitOperationService.new(user, self).with_branch(
+ 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 source" if their_commit.nil?
+ raise 'Invalid merge target' unless our_commit
+ raise 'Invalid merge source' unless their_commit
- merge_index = rugged.merge_commits(our_commit, their_commit)
- return false if merge_index.conflicts?
+ merge_index = rugged.merge_commits(our_commit, their_commit)
+ break if merge_index.conflicts?
- update_branch_with_hooks(user, merge_request.target_branch) do
actual_options = options.merge(
parents: [our_commit, their_commit],
tree: merge_index.write_tree(rugged),
@@ -952,34 +934,48 @@ class Repository
merge_request.update(in_progress_merge_commit_sha: commit_id)
commit_id
end
+ rescue Repository::CommitError # when merge_index.conflicts?
+ false
end
- def revert(user, commit, base_branch, revert_tree_id = nil)
- source_sha = find_branch(base_branch).dereferenced_target.sha
- revert_tree_id ||= check_revert_content(commit, base_branch)
+ def revert(
+ user, commit, branch_name, revert_tree_id = nil,
+ start_branch_name: nil, start_project: project)
+ revert_tree_id ||= check_revert_content(commit, branch_name)
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)
- source_sha = Rugged::Commit.create(rugged,
+
+ Rugged::Commit.create(rugged,
message: commit.revert_message(user),
author: committer,
committer: committer,
tree: revert_tree_id,
- parents: [rugged.lookup(source_sha)])
+ parents: [start_commit.sha])
end
end
- def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil)
- source_sha = find_branch(base_branch).dereferenced_target.sha
- cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch)
+ def cherry_pick(
+ user, commit, branch_name, cherry_pick_tree_id = nil,
+ 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
- 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)
- source_sha = Rugged::Commit.create(rugged,
+
+ Rugged::Commit.create(rugged,
message: commit.message,
author: {
email: commit.author_email,
@@ -988,22 +984,22 @@ class Repository
},
committer: committer,
tree: cherry_pick_tree_id,
- parents: [rugged.lookup(source_sha)])
+ parents: [start_commit.sha])
end
end
- def resolve_conflicts(user, branch, params)
- update_branch_with_hooks(user, branch) do
+ def resolve_conflicts(user, branch_name, params)
+ GitOperationService.new(user, self).with_branch(branch_name) do
committer = user_to_committer(user)
Rugged::Commit.create(rugged, params.merge(author: committer, committer: committer))
end
end
- def check_revert_content(commit, base_branch)
- source_sha = find_branch(base_branch).dereferenced_target.sha
- args = [commit.id, source_sha]
- args << { mainline: 1 } if commit.merge_commit?
+ def check_revert_content(target_commit, branch_name)
+ source_sha = commit(branch_name).sha
+ args = [target_commit.sha, source_sha]
+ args << { mainline: 1 } if target_commit.merge_commit?
revert_index = rugged.revert_commit(*args)
return false if revert_index.conflicts?
@@ -1014,10 +1010,10 @@ class Repository
tree_id
end
- def check_cherry_pick_content(commit, base_branch)
- source_sha = find_branch(base_branch).dereferenced_target.sha
- args = [commit.id, source_sha]
- args << 1 if commit.merge_commit?
+ def check_cherry_pick_content(target_commit, branch_name)
+ source_sha = commit(branch_name).sha
+ args = [target_commit.sha, source_sha]
+ args << 1 if target_commit.merge_commit?
cherry_pick_index = rugged.cherrypick_commit(*args)
return false if cherry_pick_index.conflicts?
@@ -1075,6 +1071,28 @@ class Repository
Gitlab::Popen.popen(args, path_to_repo).first.lines.map(&:strip)
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)
args = %W(#{Gitlab.config.git.bin_path} fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
Gitlab::Popen.popen(args, path_to_repo)
@@ -1084,39 +1102,6 @@ class Repository
fetch_ref(path_to_repo, ref, ref_path)
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)
actual_ref = ref || root_ref
raw_repository.ls_files(actual_ref)
@@ -1175,8 +1160,76 @@ class Repository
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
+ 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?
return false unless path_with_namespace
diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index 4d410f66c55..25e22f14e60 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -4,7 +4,8 @@ module Commits
class ChangeError < StandardError; end
def execute
- @source_project = params[:source_project] || @project
+ @start_project = params[:start_project] || @project
+ @start_branch = params[:start_branch]
@target_branch = params[:target_branch]
@commit = params[:commit]
@create_merge_request = params[:create_merge_request].present?
@@ -25,13 +26,28 @@ module Commits
def commit_change(action)
raise NotImplementedError unless repository.respond_to?(action)
- into = @create_merge_request ? @commit.public_send("#{action}_branch_name") : @target_branch
- tree_id = repository.public_send("check_#{action}_content", @commit, @target_branch)
+ if @create_merge_request
+ 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
- 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
else
error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically.
@@ -50,12 +66,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]}"
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
index 5e8fafca98c..ab4c02a97a0 100644
--- a/app/services/compare_service.rb
+++ b/app/services/compare_service.rb
@@ -3,23 +3,27 @@ require 'securerandom'
# Compare 2 branches for one repo or between repositories
# and return Gitlab::Git::Compare object that responds to commits and diffs
class CompareService
- def execute(source_project, source_branch, target_project, target_branch, straight: false)
- source_commit = source_project.commit(source_branch)
- return unless source_commit
+ attr_reader :start_project, :start_branch_name
- 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
- unless target_project == source_project
- random_string = SecureRandom.hex
+ target_project.repository.with_repo_branch_commit(
+ start_project.repository,
+ start_branch_name) do |commit|
+ break unless commit
- target_project.repository.fetch_ref(
- source_project.repository.path_to_repo,
- "refs/heads/#{source_branch}",
- "refs/tmp/#{random_string}/head"
- )
+ compare(commit.sha, target_project, target_branch, straight)
end
+ end
+
+ private
+ def compare(source_sha, target_project, target_branch, straight)
raw_compare = Gitlab::Git::Compare.new(
target_project.repository.raw_repository,
target_branch,
diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb
index e004a303496..77459d8779d 100644
--- a/app/services/create_branch_service.rb
+++ b/app/services/create_branch_service.rb
@@ -1,31 +1,11 @@
class CreateBranchService < BaseService
- def execute(branch_name, ref, source_project: @project)
- valid_branch = Gitlab::GitRefValidator.validate(branch_name)
+ def execute(branch_name, ref)
+ result = ValidateNewBranchService.new(project, current_user)
+ .execute(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
-
- 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
+ return result if result[:status] == :error
- repository.find_branch(branch_name)
- else
- repository.add_branch(current_user, branch_name, ref)
- end
+ new_branch = repository.add_branch(current_user, branch_name, ref)
if new_branch
success(new_branch)
diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb
index a44dee14a0f..9d4bffb93e9 100644
--- a/app/services/delete_tag_service.rb
+++ b/app/services/delete_tag_service.rb
@@ -7,7 +7,7 @@ class DeleteTagService < BaseService
return error('No such tag', 404)
end
- if repository.rm_tag(tag_name)
+ if repository.rm_tag(current_user, tag_name)
release = project.releases.find_by(tag: tag_name)
release.destroy if release
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index 9bd4bd464f7..0a25f56d24c 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -3,9 +3,9 @@ module Files
class ValidationError < StandardError; end
def execute
- @source_project = params[:source_project] || @project
- @source_branch = params[:source_branch]
- @target_branch = params[:target_branch]
+ @start_project = params[:start_project] || @project
+ @start_branch = params[:start_branch]
+ @target_branch = params[:target_branch]
@commit_message = params[:commit_message]
@file_path = params[:file_path]
@@ -22,10 +22,8 @@ module Files
# Validate parameters
validate
- # Create new branch if it different from source_branch
- if different_branch?
- create_target_branch
- end
+ # Create new branch if it different from start_branch
+ validate_target_branch if different_branch?
result = commit
if result
@@ -40,7 +38,7 @@ module Files
private
def different_branch?
- @source_branch != @target_branch || @source_project != @project
+ @start_branch != @target_branch || @start_project != @project
end
def file_has_changed?
@@ -61,22 +59,23 @@ module Files
end
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')
end
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')
end
end
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
diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb
index e5b4d60e467..858de5f0538 100644
--- a/app/services/files/create_dir_service.rb
+++ b/app/services/files/create_dir_service.rb
@@ -1,7 +1,15 @@
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,
+ 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
def validate
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index b23576b9a28..88dd7bbaedb 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -1,7 +1,17 @@
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,
+ 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
def validate
@@ -24,7 +34,7 @@ module Files
unless project.empty_repo?
@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
raise_error('Your changes could not be committed because a file with the same name already exists')
diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb
index 4f7e7a5baaa..50f0ffcac9f 100644
--- a/app/services/files/delete_service.rb
+++ b/app/services/files/delete_service.rb
@@ -1,7 +1,15 @@
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,
+ 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
diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb
index 54446e90007..6ba868df04d 100644
--- a/app/services/files/multi_service.rb
+++ b/app/services/files/multi_service.rb
@@ -5,11 +5,13 @@ module Files
def commit
repository.multi_action(
user: current_user,
- branch: @target_branch,
message: @commit_message,
+ branch_name: @target_branch,
actions: params[:actions],
author_email: @author_email,
- author_name: @author_name
+ author_name: @author_name,
+ start_project: @start_project,
+ start_branch_name: @start_branch
)
end
@@ -61,7 +63,7 @@ module Files
end
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
def regex_check(file)
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index 47a18e3e132..a71fe61a4b6 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -4,11 +4,13 @@ module Files
def commit
repository.update_file(current_user, @file_path, @file_content,
- branch: @target_branch,
- previous_path: @previous_path,
message: @commit_message,
+ branch_name: @target_branch,
+ previous_path: @previous_path,
author_email: @author_email,
- author_name: @author_name)
+ author_name: @author_name,
+ start_project: @start_project,
+ start_branch_name: @start_branch)
end
private
@@ -23,7 +25,7 @@ module Files
def last_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
diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb
index 6cd3908d43a..d222d1e63aa 100644
--- a/app/services/git_hooks_service.rb
+++ b/app/services/git_hooks_service.rb
@@ -18,9 +18,9 @@ class GitHooksService
end
end
- yield self
-
- run_hook('post-receive')
+ yield(self).tap do
+ run_hook('post-receive')
+ end
end
private
diff --git a/app/services/git_operation_service.rb b/app/services/git_operation_service.rb
new file mode 100644
index 00000000000..27bcc047601
--- /dev/null
+++ b/app/services/git_operation_service.rb
@@ -0,0 +1,179 @@
+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
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 1d6d2754559..f4d52e3ebbd 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -47,9 +47,10 @@ module MergeRequests
end
def compare_branches
- compare = CompareService.new.execute(
+ compare = CompareService.new(
source_project,
- source_branch,
+ source_branch
+ ).execute(
target_project,
target_branch
)
diff --git a/app/services/validate_new_branch_service.rb b/app/services/validate_new_branch_service.rb
new file mode 100644
index 00000000000..2f61be184ce
--- /dev/null
+++ b/app/services/validate_new_branch_service.rb
@@ -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
diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb
index b9cd49985dc..f5ccc84c160 100644
--- a/app/workers/emails_on_push_worker.rb
+++ b/app/workers/emails_on_push_worker.rb
@@ -33,13 +33,15 @@ class EmailsOnPushWorker
reverse_compare = false
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
return false if compare.same
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
reverse_compare = true
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index e6d707f3c3d..2fefe760d24 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -54,7 +54,7 @@ module API
authorize! :push_code, user_project
attrs = declared_params
- attrs[:source_branch] = attrs[:branch_name]
+ attrs[:start_branch] = attrs[:branch_name]
attrs[:target_branch] = attrs[:branch_name]
attrs[:actions].map! do |action|
action[:action] = action[:action].to_sym
@@ -139,8 +139,6 @@ module API
commit_params = {
commit: commit,
create_merge_request: false,
- source_project: user_project,
- source_branch: commit.cherry_pick_branch_name,
target_branch: params[:branch]
}
diff --git a/lib/api/files.rb b/lib/api/files.rb
index 2e79e22e649..c58472de578 100644
--- a/lib/api/files.rb
+++ b/lib/api/files.rb
@@ -5,7 +5,7 @@ module API
def commit_params(attrs)
{
file_path: attrs[:file_path],
- source_branch: attrs[:branch_name],
+ start_branch: attrs[:branch_name],
target_branch: attrs[:branch_name],
commit_message: attrs[:commit_message],
file_content: attrs[:content],
diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb
index 3cd515e4a3a..d3df3f1bca1 100644
--- a/lib/gitlab/git.rb
+++ b/lib/gitlab/git.rb
@@ -6,7 +6,7 @@ module Gitlab
class << self
def ref_name(ref)
- ref.gsub(/\Arefs\/(tags|heads)\//, '')
+ ref.sub(/\Arefs\/(tags|heads)\//, '')
end
def branch_name(ref)
diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb
index 99d0bcfa8d1..80f84a388ce 100644
--- a/spec/controllers/projects/templates_controller_spec.rb
+++ b/spec/controllers/projects/templates_controller_spec.rb
@@ -14,7 +14,8 @@ describe Projects::TemplatesController do
before do
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
describe '#show' do
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 992580a6b34..715b2a27b30 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -106,6 +106,42 @@ FactoryGirl.define do
path { 'gitlabhq' }
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
factory :forked_project_with_submodules, parent: :empty_project do
diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb
index fe047e00409..36a80d7575d 100644
--- a/spec/features/projects/files/editing_a_file_spec.rb
+++ b/spec/features/projects/files/editing_a_file_spec.rb
@@ -7,7 +7,7 @@ feature 'User wants to edit a file', feature: true do
let(:user) { create(:user) }
let(:commit_params) do
{
- source_branch: project.default_branch,
+ start_branch: project.default_branch,
target_branch: project.default_branch,
commit_message: "Committing First Update",
file_path: ".gitignore",
diff --git a/spec/features/projects/files/project_owner_creates_license_file_spec.rb b/spec/features/projects/files/project_owner_creates_license_file_spec.rb
index a521ce50f35..64094af29c0 100644
--- a/spec/features/projects/files/project_owner_creates_license_file_spec.rb
+++ b/spec/features/projects/files/project_owner_creates_license_file_spec.rb
@@ -6,7 +6,8 @@ feature 'project owner creates a license file', feature: true, js: true do
let(:project_master) { create(:user) }
let(:project) { create(:project) }
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]
login_as(project_master)
visit namespace_project_path(project.namespace, project)
diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb
index 6dae5c64b30..e90a033b8c4 100644
--- a/spec/features/projects/issuable_templates_spec.rb
+++ b/spec/features/projects/issuable_templates_spec.rb
@@ -18,8 +18,20 @@ feature 'issuable templates', feature: true, js: true do
let(:description_addition) { ' appending to description' }
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/test.md', longtemplate_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)
+ 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
fill_in :'issue[title]', with: 'test issue title'
end
@@ -67,7 +79,13 @@ feature 'issuable templates', feature: true, js: true do
let(:issue) { create(:issue, author: user, assignee: user, project: project) }
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
fill_in :'issue[title]', with: 'test issue title'
fill_in :'issue[description]', with: prior_description
@@ -86,7 +104,13 @@ feature 'issuable templates', feature: true, js: true do
let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) }
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
fill_in :'merge_request[title]', with: 'test merge request title'
end
@@ -111,7 +135,13 @@ feature 'issuable templates', feature: true, js: true do
fork_project.team << [fork_user, :master]
create(:forked_project_link, forked_to_project: fork_project, forked_from_project: project)
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
fill_in :'merge_request[title]', with: 'test merge request title'
end
diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb
index f5822fed37c..ffb9ba86fbb 100644
--- a/spec/lib/gitlab/diff/position_tracer_spec.rb
+++ b/spec/lib/gitlab/diff/position_tracer_spec.rb
@@ -99,7 +99,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
Files::CreateService.new(
project,
current_user,
- source_branch: branch_name,
+ start_branch: branch_name,
target_branch: branch_name,
commit_message: "Create file",
file_path: file_name,
@@ -112,7 +112,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
Files::UpdateService.new(
project,
current_user,
- source_branch: branch_name,
+ start_branch: branch_name,
target_branch: branch_name,
commit_message: "Update file",
file_path: file_name,
@@ -125,7 +125,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
Files::DeleteService.new(
project,
current_user,
- source_branch: branch_name,
+ start_branch: branch_name,
target_branch: branch_name,
commit_message: "Delete file",
file_path: file_name
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index b080be62b34..116ab16ae74 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -209,7 +209,13 @@ describe Gitlab::GitAccess, lib: true do
stub_git_hooks
project.repository.add_branch(user, unprotected_branch, '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
author = { email: "email@example.com", time: Time.now, name: "Example Git User" }
diff --git a/spec/lib/gitlab/template/issue_template_spec.rb b/spec/lib/gitlab/template/issue_template_spec.rb
index 45cec65a284..1335a2b8f35 100644
--- a/spec/lib/gitlab/template/issue_template_spec.rb
+++ b/spec/lib/gitlab/template/issue_template_spec.rb
@@ -4,16 +4,14 @@ describe Gitlab::Template::IssueTemplate do
subject { described_class }
let(:user) { create(:user) }
- let(:project) { create(:project, :repository) }
- let(:file_path_1) { '.gitlab/issue_templates/bug.md' }
- let(:file_path_2) { '.gitlab/issue_templates/template_test.md' }
- let(:file_path_3) { '.gitlab/issue_templates/feature_proposal.md' }
-
- before do
- 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_2, "template_test", "test 1", "master", false)
- project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false)
+
+ let(:project) do
+ create(:project,
+ :repository,
+ create_template: {
+ user: user,
+ access: Gitlab::Access::MASTER,
+ path: 'issue_templates' })
end
describe '.all' do
diff --git a/spec/lib/gitlab/template/merge_request_template_spec.rb b/spec/lib/gitlab/template/merge_request_template_spec.rb
index ae51b79be22..320b870309a 100644
--- a/spec/lib/gitlab/template/merge_request_template_spec.rb
+++ b/spec/lib/gitlab/template/merge_request_template_spec.rb
@@ -4,16 +4,14 @@ describe Gitlab::Template::MergeRequestTemplate do
subject { described_class }
let(:user) { create(:user) }
- let(:project) { create(:project, :repository) }
- let(:file_path_1) { '.gitlab/merge_request_templates/bug.md' }
- let(:file_path_2) { '.gitlab/merge_request_templates/template_test.md' }
- let(:file_path_3) { '.gitlab/merge_request_templates/feature_proposal.md' }
-
- before do
- 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_2, "template_test", "test 1", "master", false)
- project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false)
+
+ let(:project) do
+ create(:project,
+ :repository,
+ create_template: {
+ user: user,
+ access: Gitlab::Access::MASTER,
+ path: 'merge_request_templates' })
end
describe '.all' do
diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb
index 2cbee741fb0..b9fe492fe2c 100644
--- a/spec/models/cycle_analytics/production_spec.rb
+++ b/spec/models/cycle_analytics/production_spec.rb
@@ -21,7 +21,13 @@ describe 'CycleAnalytics#production', feature: true do
["production deploy happens after merge request is merged (along with other changes)",
lambda do |context, data|
# 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.deploy_master
diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb
index 104e65335dd..9a024d533a1 100644
--- a/spec/models/cycle_analytics/staging_spec.rb
+++ b/spec/models/cycle_analytics/staging_spec.rb
@@ -29,10 +29,10 @@ describe 'CycleAnalytics#staging', feature: true do
sha = context.project.repository.commit_file(
context.user,
context.random_git_name,
- "content",
- "commit message",
- 'master',
- false)
+ 'content',
+ message: 'commit message',
+ branch_name: 'master',
+ update: false)
context.project.repository.commit(sha)
context.deploy_master
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 829b69093c9..901cfb907f2 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -289,17 +289,39 @@ describe Repository, models: true do
describe "#commit_dir" do
it "commits a change that creates a new directory" 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)
newdir = repository.tree('master', 'newdir')
expect(newdir.path).to eq('newdir')
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
it "uses the given email/name to set the commit's author" 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)
last_commit = repository.commit
@@ -314,8 +336,9 @@ describe Repository, models: true do
it 'commits change to a file successfully' do
expect do
repository.commit_file(user, 'CHANGELOG', 'Changelog!',
- 'Updates file content',
- 'master', true)
+ message: 'Updates file content',
+ branch_name: 'master',
+ update: true)
end.to change { repository.commits('master').count }.by(1)
blob = repository.blob_at('master', 'CHANGELOG')
@@ -326,8 +349,12 @@ describe Repository, models: true do
context "when an author is specified" do
it "uses the given email/name to set the commit's author" do
expect do
- repository.commit_file(user, "README", 'README!', 'Add README',
- 'master', true, author_email: author_email, author_name: author_name)
+ repository.commit_file(user, 'README', 'README!',
+ 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)
last_commit = repository.commit
@@ -342,7 +369,7 @@ describe Repository, models: true do
it 'updates filename successfully' do
expect do
repository.update_file(user, 'NEWLICENSE', 'Copyright!',
- branch: 'master',
+ branch_name: 'master',
previous_path: 'LICENSE',
message: 'Changes filename')
end.to change { repository.commits('master').count }.by(1)
@@ -355,15 +382,16 @@ describe Repository, models: true do
context "when an author is specified" 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
- repository.update_file(user, 'README', "Updated README!",
- branch: 'master',
- previous_path: 'README',
- message: 'Update README',
- author_email: author_email,
- author_name: author_name)
+ repository.update_file(user, 'README', 'Updated README!',
+ branch_name: 'master',
+ previous_path: 'README',
+ message: 'Update README',
+ author_email: author_email,
+ author_name: author_name)
end.to change { repository.commits('master').count }.by(1)
last_commit = repository.commit
@@ -376,10 +404,12 @@ describe Repository, models: true do
describe "#remove_file" 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
- 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)
expect(repository.blob_at('master', 'README')).to be_nil
@@ -387,10 +417,13 @@ describe Repository, models: true do
context "when an author is specified" 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
- 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)
last_commit = repository.commit
@@ -538,11 +571,14 @@ describe Repository, models: true do
describe "#license_blob", caching: true do
before do
- repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master')
+ repository.remove_file(
+ user, 'LICENSE', message: 'Remove LICENSE', branch_name: 'master')
end
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).
and_raise(Rugged::ReferenceError)
@@ -551,21 +587,27 @@ describe Repository, models: true do
end
it 'looks in the root_ref only' do
- repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'markdown')
- repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'markdown', false)
+ repository.remove_file(user, 'LICENSE',
+ 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
end
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')
end
%w[LICENSE LICENCE LiCensE LICENSE.md LICENSE.foo COPYING COPYING.md].each do |filename|
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)
end
@@ -574,7 +616,8 @@ describe Repository, models: true do
describe '#license_key', caching: true do
before do
- repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master')
+ repository.remove_file(user, 'LICENSE',
+ message: 'Remove LICENSE', branch_name: 'master')
end
it 'returns nil when no license is detected' do
@@ -588,13 +631,16 @@ describe Repository, models: true do
end
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
end
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')
end
@@ -707,7 +753,7 @@ describe Repository, models: true do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
expect do
- repository.rm_branch(user, 'new_feature')
+ repository.rm_branch(user, 'feature')
end.to raise_error(GitHooksService::PreReceiveError)
end
@@ -728,36 +774,51 @@ describe Repository, models: true do
context 'when pre hooks were successful' do
before do
- expect_any_instance_of(GitHooksService).to receive(:execute).
- with(user, repository.path_to_repo, old_rev, new_rev, 'refs/heads/feature').
- and_yield.and_return(true)
+ service = GitHooksService.new
+ expect(GitHooksService).to receive(:new).and_return(service)
+ 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
it 'runs without errors' 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
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
context "when the branch wasn't empty" do
it 'updates the head' do
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)
end
end
end
context 'when the update adds more than one commit' do
- it 'runs without errors' do
- old_rev = '33f3729a45c02fc67d00adb1b8bca394b0e761d9'
+ let(:old_rev) { '33f3729a45c02fc67d00adb1b8bca394b0e761d9' }
+ it 'runs without errors' do
# old_rev is an ancestor of new_rev
expect(repository.rugged.merge_base(old_rev, new_rev)).to eq(old_rev)
@@ -767,22 +828,28 @@ describe Repository, models: true do
branch = 'feature-ff-target'
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
context 'when the update would remove commits from the target branch' do
- it 'raises an exception' do
- branch = 'master'
- old_rev = repository.find_branch(branch).dereferenced_target.sha
+ let(:branch) { 'master' }
+ let(:old_rev) { repository.find_branch(branch).dereferenced_target.sha }
+ it 'raises an exception' do
# The 'master' branch is NOT an ancestor of new_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
# are not contained in new_rev. This should not be allowed.
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
end
@@ -792,7 +859,9 @@ describe Repository, models: true do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
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
end
@@ -800,7 +869,6 @@ describe Repository, models: true do
context 'when target branch is different from source branch' do
before do
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, ''])
- allow(repository).to receive(:update_ref!)
end
it 'expires branch cache' do
@@ -809,7 +877,10 @@ describe Repository, models: true do
expect(repository).not_to receive(:expire_emptiness_caches)
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
@@ -827,7 +898,9 @@ describe Repository, models: true do
expect(empty_repository).to receive(:expire_branches_cache)
empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!',
- 'Updates file content', 'master', false)
+ message: 'Updates file content',
+ branch_name: 'master',
+ update: false)
end
end
end
@@ -877,7 +950,7 @@ describe Repository, models: true do
end
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)
end
@@ -892,7 +965,7 @@ describe Repository, models: true do
expect(repository.raw_repository).not_to receive(:autocrlf=).
with(:input)
- repository.update_autocrlf_option
+ GitOperationService.new(nil, repository).send(:update_autocrlf_option)
end
end
end
@@ -1388,9 +1461,10 @@ describe Repository, models: true do
describe '#rm_tag' do
it 'removes a tag' do
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
@@ -1458,16 +1532,16 @@ describe Repository, models: true do
end
end
- describe '#update_ref!' do
+ describe '#update_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
end
it 'raises CommitError when the ref update fails' 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
end
diff --git a/spec/services/compare_service_spec.rb b/spec/services/compare_service_spec.rb
index 3760f19aaa2..0a7fc58523f 100644
--- a/spec/services/compare_service_spec.rb
+++ b/spec/services/compare_service_spec.rb
@@ -3,17 +3,17 @@ require 'spec_helper'
describe CompareService, services: true do
let(:project) { create(:project) }
let(:user) { create(:user) }
- let(:service) { described_class.new }
+ let(:service) { described_class.new(project, 'feature') }
describe '#execute' 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) }
end
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) }
end
diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb
index d3c37c7820f..35e6e139238 100644
--- a/spec/services/files/update_service_spec.rb
+++ b/spec/services/files/update_service_spec.rb
@@ -6,7 +6,10 @@ describe Files::UpdateService do
let(:project) { create(:project) }
let(:user) { create(:user) }
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
{
file_path: file_path,
@@ -14,9 +17,9 @@ describe Files::UpdateService do
file_content: new_contents,
file_content_encoding: "text",
last_commit_sha: last_commit_sha,
- source_project: project,
- source_branch: project.default_branch,
- target_branch: project.default_branch,
+ start_project: project,
+ start_branch: project.default_branch,
+ target_branch: target_branch
}
end
@@ -54,18 +57,6 @@ describe Files::UpdateService do
end
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
results = subject.execute
@@ -80,5 +71,15 @@ describe Files::UpdateService do
expect(results.data).to eq(new_contents)
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
diff --git a/spec/services/git_hooks_service_spec.rb b/spec/services/git_hooks_service_spec.rb
index 41b0968b8b4..3318dfb22b6 100644
--- a/spec/services/git_hooks_service_spec.rb
+++ b/spec/services/git_hooks_service_spec.rb
@@ -21,7 +21,7 @@ describe GitHooksService, services: true do
hook = double(trigger: [true, nil])
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
diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb
index 388abb6a0df..a0e51681725 100644
--- a/spec/services/merge_requests/resolve_service_spec.rb
+++ b/spec/services/merge_requests/resolve_service_spec.rb
@@ -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
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
before do
diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb
index 75c95d70951..6ed55289ed9 100644
--- a/spec/support/cycle_analytics_helpers.rb
+++ b/spec/support/cycle_analytics_helpers.rb
@@ -35,7 +35,13 @@ module CycleAnalyticsHelpers
project.repository.add_branch(user, source_branch, 'master')
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)
opts = {
diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb
index e471a68a49a..5ef8cf1105b 100644
--- a/spec/workers/git_garbage_collect_worker_spec.rb
+++ b/spec/workers/git_garbage_collect_worker_spec.rb
@@ -107,7 +107,8 @@ describe GitGarbageCollectWorker do
tree: old_commit.tree,
parents: [old_commit],
)
- project.repository.update_ref!(
+ GitOperationService.new(nil, project.repository).send(
+ :update_ref,
"refs/heads/#{SecureRandom.hex(6)}",
new_commit_sha,
Gitlab::Git::BLANK_SHA