From 03440eed20cc36a2f9836dc078d2101849e11319 Mon Sep 17 00:00:00 2001
From: Ahmad Sherif <me@ahmadsherif.com>
Date: Fri, 28 Jul 2017 14:16:26 +0200
Subject: [PATCH] Migrate blame loading to Gitaly
Closes gitaly#421
---
GITALY_SERVER_VERSION | 2 +-
Gemfile | 2 +-
Gemfile.lock | 4 +-
lib/gitlab/git/blame.rb | 24 +++--
lib/gitlab/git/repository.rb | 16 ++--
lib/gitlab/gitaly_client/commit_service.rb | 11 +++
spec/lib/gitlab/git/blame_spec.rb | 102 +++++++++++----------
7 files changed, 97 insertions(+), 64 deletions(-)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 4e8f395fa5e..1b58cc10180 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-0.26.0
+0.27.0
diff --git a/Gemfile b/Gemfile
index 403b104a9d6..34f231a04b8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -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 9f90965a567..a17aa3ee1ed 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)
@@ -978,7 +978,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
--
GitLab