diff options
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 4 | ||||
| -rw-r--r-- | lib/gitlab/git/blame.rb | 24 | ||||
| -rw-r--r-- | lib/gitlab/git/repository.rb | 16 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client/commit_service.rb | 11 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/blame_spec.rb | 102 |
6 files changed, 96 insertions, 63 deletions
@@ -391,7 +391,7 @@ gem 'vmstat', '~> 2.3.0' gem 'sys-filesystem', '~> 1.1.6' # Gitaly GRPC client -gem 'gitaly', '~> 0.23.0' +gem 'gitaly', '~> 0.24.0' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 73f45f3ccb6..57cfe0a9de0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -269,7 +269,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly (0.23.0) + gitaly (0.24.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -974,7 +974,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly (~> 0.23.0) + gitaly (~> 0.24.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.5.1) diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb index 0deaab01b5b..8dbe25e55f6 100644 --- a/lib/gitlab/git/blame.rb +++ b/lib/gitlab/git/blame.rb @@ -1,5 +1,3 @@ -# Gitaly note: JV: needs 1 RPC for #load_blame. - module Gitlab module Git class Blame @@ -26,15 +24,29 @@ module Gitlab private - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/376 def load_blame - cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{@repo.path} blame -p #{@sha} -- #{@path}) - # Read in binary mode to ensure ASCII-8BIT - raw_output = IO.popen(cmd, 'rb') {|io| io.read } + raw_output = @repo.gitaly_migrate(:blame) do |is_enabled| + if is_enabled + load_blame_by_gitaly + else + load_blame_by_shelling_out + end + end + output = encode_utf8(raw_output) process_raw_blame output end + def load_blame_by_gitaly + @repo.gitaly_commit_client.raw_blame(@sha, @path) + end + + def load_blame_by_shelling_out + cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{@repo.path} blame -p #{@sha} -- #{@path}) + # Read in binary mode to ensure ASCII-8BIT + IO.popen(cmd, 'rb') {|io| io.read } + end + def process_raw_blame(output) lines, final = [], [] info, commits = {}, {} diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index ffe2c8b91bb..1c3beb5e834 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -683,6 +683,14 @@ module Gitlab @gitaly_repository_client ||= Gitlab::GitalyClient::RepositoryService.new(self) end + def gitaly_migrate(method, &block) + Gitlab::GitalyClient.migrate(method, &block) + rescue GRPC::NotFound => e + raise NoRepository.new(e) + rescue GRPC::BadStatus => e + raise CommandError.new(e) + end + private # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'. @@ -1017,14 +1025,6 @@ module Gitlab raw_output.to_i end - - def gitaly_migrate(method, &block) - Gitlab::GitalyClient.migrate(method, &block) - rescue GRPC::NotFound => e - raise NoRepository.new(e) - rescue GRPC::BadStatus => e - raise CommandError.new(e) - end end end end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index b1424a458e9..1ae13677b42 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -128,6 +128,17 @@ module Gitlab response.languages.map { |l| { value: l.share.round(2), label: l.name, color: l.color, highlight: l.color } } end + def raw_blame(revision, path) + request = Gitaly::RawBlameRequest.new( + repository: @gitaly_repo, + revision: revision, + path: path + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :raw_blame, request) + response.reduce("") { |memo, msg| memo << msg.data } + end + private def commit_diff_request_params(commit, options = {}) diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index 66c016d14b3..800c245b130 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -7,63 +7,73 @@ describe Gitlab::Git::Blame, seed_helper: true do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end - context "each count" do - it do - data = [] - blame.each do |commit, line| - data << { - commit: commit, - line: line - } - end - - expect(data.size).to eq(95) - expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) - expect(data.first[:line]).to eq("# Contribute to GitLab") - expect(data.first[:line]).to be_utf8 - end - end + shared_examples 'blaming a file' do + context "each count" do + it do + data = [] + blame.each do |commit, line| + data << { + commit: commit, + line: line + } + end - context "ISO-8859 encoding" do - let(:blame) do - Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt") + expect(data.size).to eq(95) + expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) + expect(data.first[:line]).to eq("# Contribute to GitLab") + expect(data.first[:line]).to be_utf8 + end end - it 'converts to UTF-8' do - data = [] - blame.each do |commit, line| - data << { - commit: commit, - line: line - } + context "ISO-8859 encoding" do + let(:blame) do + Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt") end - expect(data.size).to eq(1) - expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) - expect(data.first[:line]).to eq("Ä ü") - expect(data.first[:line]).to be_utf8 - end - end + it 'converts to UTF-8' do + data = [] + blame.each do |commit, line| + data << { + commit: commit, + line: line + } + end - context "unknown encoding" do - let(:blame) do - Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt") + expect(data.size).to eq(1) + expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) + expect(data.first[:line]).to eq("Ä ü") + expect(data.first[:line]).to be_utf8 + end end - it 'converts to UTF-8' do - expect(CharlockHolmes::EncodingDetector).to receive(:detect).and_return(nil) - data = [] - blame.each do |commit, line| - data << { + context "unknown encoding" do + let(:blame) do + Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt") + end + + it 'converts to UTF-8' do + expect(CharlockHolmes::EncodingDetector).to receive(:detect).and_return(nil) + data = [] + blame.each do |commit, line| + data << { commit: commit, line: line - } - end + } + end - expect(data.size).to eq(1) - expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) - expect(data.first[:line]).to eq(" ") - expect(data.first[:line]).to be_utf8 + expect(data.size).to eq(1) + expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) + expect(data.first[:line]).to eq(" ") + expect(data.first[:line]).to be_utf8 + end end end + + context 'when Gitaly blame feature is enabled' do + it_behaves_like 'blaming a file' + end + + context 'when Gitaly blame feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'blaming a file' + end end |