diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/controllers/concerns/creates_commit.rb | 53 | ||||
| -rw-r--r-- | app/controllers/projects/commit_controller.rb | 8 | ||||
| -rw-r--r-- | app/controllers/projects/compare_controller.rb | 3 | ||||
| -rw-r--r-- | app/models/merge_request_diff.rb | 3 | ||||
| -rw-r--r-- | app/models/repository.rb | 493 | ||||
| -rw-r--r-- | app/services/commits/change_service.rb | 34 | ||||
| -rw-r--r-- | app/services/compare_service.rb | 26 | ||||
| -rw-r--r-- | app/services/create_branch_service.rb | 30 | ||||
| -rw-r--r-- | app/services/delete_tag_service.rb | 2 | ||||
| -rw-r--r-- | app/services/files/base_service.rb | 25 | ||||
| -rw-r--r-- | app/services/files/create_dir_service.rb | 10 | ||||
| -rw-r--r-- | app/services/files/create_service.rb | 14 | ||||
| -rw-r--r-- | app/services/files/delete_service.rb | 10 | ||||
| -rw-r--r-- | app/services/files/multi_service.rb | 8 | ||||
| -rw-r--r-- | app/services/files/update_service.rb | 10 | ||||
| -rw-r--r-- | app/services/git_hooks_service.rb | 6 | ||||
| -rw-r--r-- | app/services/git_operation_service.rb | 179 | ||||
| -rw-r--r-- | app/services/merge_requests/build_service.rb | 5 | ||||
| -rw-r--r-- | app/services/validate_new_branch_service.rb | 22 | ||||
| -rw-r--r-- | app/workers/emails_on_push_worker.rb | 6 |
20 files changed, 626 insertions, 321 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 |
