From 64dfe2cba1594b67e56233230be8137fa0afacf9 Mon Sep 17 00:00:00 2001 From: "Jacob Vosmaer (GitLab)" Date: Thu, 5 Jul 2018 10:22:04 +0000 Subject: [PATCH] Revert "Merge branch 'gitaly-mandatory-20180703-jv' into 'master'" This reverts merge request !20339 --- lib/gitlab/git/commit.rb | 14 +++- lib/gitlab/git/conflict/resolver.rb | 69 +++++++++++++++++-- lib/gitlab/git/repository.rb | 19 ++++- .../conflicts/list_service_spec.rb | 18 +++++ .../conflicts/resolve_service_spec.rb | 11 +++ 5 files changed, 123 insertions(+), 8 deletions(-) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 36d56e411d8..c67826da1d2 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -63,8 +63,12 @@ module Gitlab # This saves us an RPC round trip. return nil if commit_id.include?(':') - commit = repo.wrapped_gitaly_errors do - repo.gitaly_commit_client.find_commit(commit_id) + commit = repo.gitaly_migrate(:find_commit) do |is_enabled| + if is_enabled + repo.gitaly_commit_client.find_commit(commit_id) + else + rugged_find(repo, commit_id) + end end decorate(repo, commit) if commit @@ -74,6 +78,12 @@ module Gitlab nil end + def rugged_find(repo, commit_id) + obj = repo.rev_parse_target(commit_id) + + obj.is_a?(Rugged::Commit) ? obj : nil + end + # Get last commit for HEAD # # Ex. diff --git a/lib/gitlab/git/conflict/resolver.rb b/lib/gitlab/git/conflict/resolver.rb index 0e4a973301f..c3cb0264112 100644 --- a/lib/gitlab/git/conflict/resolver.rb +++ b/lib/gitlab/git/conflict/resolver.rb @@ -12,8 +12,14 @@ module Gitlab end def conflicts - @conflicts ||= @target_repository.wrapped_gitaly_errors do - gitaly_conflicts_client(@target_repository).list_conflict_files.to_a + @conflicts ||= begin + @target_repository.gitaly_migrate(:conflicts_list_conflict_files) do |is_enabled| + if is_enabled + gitaly_conflicts_client(@target_repository).list_conflict_files.to_a + else + rugged_list_conflict_files + end + end end rescue GRPC::FailedPrecondition => e raise Gitlab::Git::Conflict::Resolver::ConflictSideMissing.new(e.message) @@ -22,8 +28,12 @@ module Gitlab end def resolve_conflicts(source_repository, resolution, source_branch:, target_branch:) - source_repository.wrapped_gitaly_errors do - gitaly_conflicts_client(source_repository).resolve_conflicts(@target_repository, resolution, source_branch, target_branch) + source_repository.gitaly_migrate(:conflicts_resolve_conflicts) do |is_enabled| + if is_enabled + gitaly_conflicts_client(source_repository).resolve_conflicts(@target_repository, resolution, source_branch, target_branch) + else + rugged_resolve_conflicts(source_repository, resolution, source_branch, target_branch) + end end end @@ -51,6 +61,57 @@ module Gitlab def gitaly_conflicts_client(repository) repository.gitaly_conflicts_client(@our_commit_oid, @their_commit_oid) end + + def write_resolved_file_to_index(repository, index, file, params) + if params[:sections] + resolved_lines = file.resolve_lines(params[:sections]) + new_file = resolved_lines.map { |line| line[:full_line] }.join("\n") + + new_file << "\n" if file.our_blob.data.end_with?("\n") + elsif params[:content] + new_file = file.resolve_content(params[:content]) + end + + our_path = file.our_path + + oid = repository.rugged.write(new_file, :blob) + index.add(path: our_path, oid: oid, mode: file.our_mode) + index.conflict_remove(our_path) + end + + def rugged_list_conflict_files + target_index = @target_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid) + + # We don't need to do `with_repo_branch_commit` here, because the target + # project always fetches source refs when creating merge request diffs. + conflict_files(@target_repository, target_index) + end + + def rugged_resolve_conflicts(source_repository, resolution, source_branch, target_branch) + source_repository.with_repo_branch_commit(@target_repository, target_branch) do + index = source_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid) + conflicts = conflict_files(source_repository, index) + + resolution.files.each do |file_params| + conflict_file = conflict_for_path(conflicts, file_params[:old_path], file_params[:new_path]) + + write_resolved_file_to_index(source_repository, index, conflict_file, file_params) + end + + unless index.conflicts.empty? + missing_files = index.conflicts.map { |file| file[:ours][:path] } + + raise ResolutionError, "Missing resolutions for the following files: #{missing_files.join(', ')}" + end + + commit_params = { + message: resolution.commit_message, + parents: [@our_commit_oid, @their_commit_oid] + } + + source_repository.commit_index(resolution.user, source_branch, index, commit_params) + end + end end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 647f54d5b84..bbfe6ab1d95 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -555,8 +555,12 @@ module Gitlab # diff options. The +options+ hash can also include :break_rewrites to # split larger rewrites into delete/add pairs. def diff(from, to, options = {}, *paths) - iterator = wrapped_gitaly_errors do - gitaly_commit_client.diff(from, to, options.merge(paths: paths)) + iterator = gitaly_migrate(:diff_between) do |is_enabled| + if is_enabled + gitaly_commit_client.diff(from, to, options.merge(paths: paths)) + else + diff_patches(from, to, options, *paths) + end end Gitlab::Git::DiffCollection.new(iterator, options) @@ -1587,6 +1591,17 @@ module Gitlab tmp_entry end + # Return the Rugged patches for the diff between +from+ and +to+. + def diff_patches(from, to, options = {}, *paths) + options ||= {} + break_rewrites = options[:break_rewrites] + actual_options = Gitlab::Git::Diff.filter_diff_options(options.merge(paths: paths)) + + diff = rugged.diff(from, to, actual_options) + diff.find_similar!(break_rewrites: break_rewrites) + diff.each_patch + end + def sort_branches(branches, sort_by) case sort_by when 'name' diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index 97da8a88660..d57852615d9 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -84,5 +84,23 @@ describe MergeRequests::Conflicts::ListService do expect(service.can_be_resolved_in_ui?).to be_falsey end + + context 'with gitaly disabled', :skip_gitaly_mock do + it 'returns a falsey value when the MR has a missing ref after a force push' do + merge_request = create_merge_request('conflict-resolvable') + service = conflicts_service(merge_request) + allow_any_instance_of(Rugged::Repository).to receive(:merge_commits).and_raise(Rugged::OdbError) + + expect(service.can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a falsey value when the MR has a missing revision after a force push' do + merge_request = create_merge_request('conflict-resolvable') + service = conflicts_service(merge_request) + allow(merge_request).to receive_message_chain(:target_branch_head, :raw, :id).and_return(Gitlab::Git::BLANK_SHA) + + expect(service.can_be_resolved_in_ui?).to be_falsey + end + end end end diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb index 7edf8a96c94..cff09237005 100644 --- a/spec/services/merge_requests/conflicts/resolve_service_spec.rb +++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb @@ -123,6 +123,17 @@ describe MergeRequests::Conflicts::ResolveService do expect(merge_request_from_fork.source_branch_head.parents.map(&:id)) .to eq(['404fa3fc7c2c9b5dacff102f353bdf55b1be2813', target_head]) end + + context 'when gitaly is disabled', :skip_gitaly_mock do + it 'gets conflicts from the source project' do + # REFACTOR NOTE: We used to test that `project.repository.rugged` wasn't + # used in this case, but since the refactor, for simplification, + # we always use that repository for read only operations. + expect(forked_project.repository.rugged).to receive(:merge_commits).and_call_original + + subject + end + end end end -- GitLab