From eb36fa17a6ae5cda8950904b5a52e6aa365ae591 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Mon, 11 Sep 2017 14:52:27 +0200 Subject: Migrate Gitlab::Git::Repository#diff to Gitaly Closes gitaly#524 --- GITALY_SERVER_VERSION | 2 +- lib/gitlab/diff/file.rb | 4 +-- lib/gitlab/git/diff.rb | 4 +++ lib/gitlab/git/repository.rb | 10 +++++- lib/gitlab/gitaly_client/commit_service.rb | 57 +++++++++++++++++++++++------- 5 files changed, 61 insertions(+), 16 deletions(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index ca75280b09b..9b0025a7850 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.38.0 +0.40.0 diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 1dabd4ebdd0..fcac85ff892 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -5,7 +5,7 @@ module Gitlab delegate :new_file?, :deleted_file?, :renamed_file?, :old_path, :new_path, :a_mode, :b_mode, :mode_changed?, - :submodule?, :expanded?, :too_large?, :collapsed?, :line_count, to: :diff, prefix: false + :submodule?, :expanded?, :too_large?, :collapsed?, :line_count, :has_binary_notice?, to: :diff, prefix: false # Finding a viewer for a diff file happens based only on extension and whether the # diff file blobs are binary or text, which means 1 diff file should only be matched by 1 viewer, @@ -166,7 +166,7 @@ module Gitlab end def binary? - old_blob&.binary? || new_blob&.binary? + has_binary_notice? || old_blob&.binary? || new_blob&.binary? end def text? diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index a23c8cf0dd1..096301d300f 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -206,6 +206,10 @@ module Gitlab Diff.binary_message(@old_path, @new_path) end + def has_binary_notice? + @diff.start_with?('Binary') + end + private def init_from_rugged(rugged) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 4b000bd31e2..6d9f742a2d3 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -475,7 +475,15 @@ 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) - Gitlab::Git::DiffCollection.new(diff_patches(from, to, options, *paths), options) + 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) end # Returns a RefName for a given SHA diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index b536eb1868c..cf3a3554552 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -32,20 +32,38 @@ module Gitlab GitalyClient.call(@repository.storage, :commit_service, :commit_is_ancestor, request).value end + def diff(from, to, options = {}) + from_id = case from + when NilClass + EMPTY_TREE_ID + when Rugged::Commit + from.oid + else + from + end + + to_id = case to + when NilClass + EMPTY_TREE_ID + when Rugged::Commit + to.oid + else + to + end + + request_params = diff_between_commits_request_params(from_id, to_id, options) + + call_commit_diff(request_params, options) + end + def diff_from_parent(commit, options = {}) - request_params = commit_diff_request_params(commit, options) - request_params[:ignore_whitespace_change] = options.fetch(:ignore_whitespace_change, false) - request_params[:enforce_limits] = options.fetch(:limits, true) - request_params[:collapse_diffs] = request_params[:enforce_limits] || !options.fetch(:expanded, true) - request_params.merge!(Gitlab::Git::DiffCollection.collection_limits(options).to_h) + request_params = diff_from_parent_request_params(commit, options) - request = Gitaly::CommitDiffRequest.new(request_params) - response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request) - GitalyClient::DiffStitcher.new(response) + call_commit_diff(request_params, options) end def commit_deltas(commit) - request = Gitaly::CommitDeltaRequest.new(commit_diff_request_params(commit)) + request = Gitaly::CommitDeltaRequest.new(diff_from_parent_request_params(commit)) response = GitalyClient.call(@repository.storage, :diff_service, :commit_delta, request) response.flat_map { |msg| msg.deltas } @@ -214,13 +232,28 @@ module Gitlab private - def commit_diff_request_params(commit, options = {}) + def call_commit_diff(request_params, options = {}) + request_params[:ignore_whitespace_change] = options.fetch(:ignore_whitespace_change, false) + request_params[:enforce_limits] = options.fetch(:limits, true) + request_params[:collapse_diffs] = request_params[:enforce_limits] || !options.fetch(:expanded, true) + request_params.merge!(Gitlab::Git::DiffCollection.collection_limits(options).to_h) + + request = Gitaly::CommitDiffRequest.new(request_params) + response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request) + GitalyClient::DiffStitcher.new(response) + end + + def diff_from_parent_request_params(commit, options = {}) parent_id = commit.parent_ids.first || EMPTY_TREE_ID + diff_between_commits_request_params(parent_id, commit.id, options) + end + + def diff_between_commits_request_params(from_id, to_id, options) { repository: @gitaly_repo, - left_commit_id: parent_id, - right_commit_id: commit.id, + left_commit_id: from_id, + right_commit_id: to_id, paths: options.fetch(:paths, []).map { |path| GitalyClient.encode(path) } } end -- cgit v1.2.1