diff --git a/Gemfile b/Gemfile
index 893299fb635bb1998ce4d7c2abc21fdc02a40302..f5d19f32d65ffb99d05ebbb4faaef55687d1eb1b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -386,7 +386,7 @@ gem 'vmstat', '~> 2.3.0'
gem 'sys-filesystem', '~> 1.1.6'
# Gitaly GRPC client
-gem 'gitaly', '~> 0.17.0'
+gem 'gitaly', '~> 0.18.0'
gem 'toml-rb', '~> 0.3.15', require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 0c4fc1fee0ced848c15b18891516f45e79fe3857..b0b9c179479f148c2b55ee8935859bfb7728cca8 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.17.0)
+ gitaly (0.18.0)
google-protobuf (~> 3.1)
grpc (~> 1.0)
github-linguist (4.7.6)
@@ -972,7 +972,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.2.0)
- gitaly (~> 0.17.0)
+ gitaly (~> 0.18.0)
github-linguist (~> 4.7.0)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-markup (~> 1.5.1)
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index 76a562f356e8adbedeedb8dd20811aa98c8f1d59..09511cc6504a0c30da0670ddd41d7a8e5ce3539c 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -98,17 +98,13 @@ module Gitlab
# Commit.between(repo, '29eda46b', 'master')
#
def between(repo, base, head)
- commits = Gitlab::GitalyClient.migrate(:commits_between) do |is_enabled|
+ Gitlab::GitalyClient.migrate(:commits_between) do |is_enabled|
if is_enabled
repo.gitaly_commit_client.between(base, head)
else
- repo.commits_between(base, head)
+ repo.commits_between(base, head).map { |c| decorate(c) }
end
end
-
- commits.map do |commit|
- decorate(commit)
- end
rescue Rugged::ReferenceError
[]
end
@@ -135,6 +131,16 @@ module Gitlab
#
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/326
def find_all(repo, options = {})
+ Gitlab::GitalyClient.migrate(:find_all_commits) do |is_enabled|
+ if is_enabled
+ find_all_by_gitaly(repo, options)
+ else
+ find_all_by_rugged(repo, options)
+ end
+ end
+ end
+
+ def find_all_by_rugged(repo, options = {})
actual_options = options.dup
allowed_options = [:ref, :max_count, :skip, :order]
@@ -173,6 +179,10 @@ module Gitlab
[]
end
+ def find_all_by_gitaly(repo, options = {})
+ Gitlab::GitalyClient::CommitService.new(repo).find_all_commits(options)
+ end
+
def decorate(commit, ref = nil)
Gitlab::Git::Commit.new(commit, ref)
end
@@ -214,11 +224,12 @@ module Gitlab
def initialize(raw_commit, head = nil)
raise "Nil as raw commit passed" unless raw_commit
- if raw_commit.is_a?(Hash)
+ case raw_commit
+ when Hash
init_from_hash(raw_commit)
- elsif raw_commit.is_a?(Rugged::Commit)
+ when Rugged::Commit
init_from_rugged(raw_commit)
- elsif raw_commit.is_a?(Gitaly::GitCommit)
+ when Gitlab::GitalyClient::Commit
init_from_gitaly(raw_commit)
else
raise "Invalid raw commit type: #{raw_commit.class}"
@@ -298,7 +309,14 @@ module Gitlab
end
def parents
- raw_commit.parents.map { |c| Gitlab::Git::Commit.new(c) }
+ case raw_commit
+ when Rugged::Commit
+ raw_commit.parents.map { |c| Gitlab::Git::Commit.new(c) }
+ when Gitlab::GitalyClient::Commit
+ parent_ids.map { |oid| self.class.find(raw_commit.repository, oid) }.compact
+ else
+ raise NotImplementedError, "commit source doesn't support #parents"
+ end
end
def stats
diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb
new file mode 100644
index 0000000000000000000000000000000000000000..61fe462d7624af0a871bf90ca6e162bad5f09c92
--- /dev/null
+++ b/lib/gitlab/gitaly_client/commit.rb
@@ -0,0 +1,14 @@
+module Gitlab
+ module GitalyClient
+ class Commit
+ attr_reader :repository, :gitaly_commit
+
+ delegate :id, :subject, :body, :author, :committer, :parent_ids, to: :gitaly_commit
+
+ def initialize(repository, gitaly_commit)
+ @repository = repository
+ @gitaly_commit = gitaly_commit
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index b749955cddcff9fa620ac8ccea0c16e5716e714e..5b3a7659b848383b93278f25fa89bd5c40830734 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -80,6 +80,19 @@ module Gitlab
consume_commits_response(response)
end
+ def find_all_commits(opts = {})
+ request = Gitaly::FindAllCommitsRequest.new(
+ repository: @gitaly_repo,
+ revision: opts[:ref].to_s,
+ max_count: opts[:max_count].to_i,
+ skip: opts[:skip].to_i
+ )
+ request.order = opts[:order].upcase if opts[:order].present?
+
+ response = GitalyClient.call(@repository.storage, :commit_service, :find_all_commits, request)
+ consume_commits_response(response)
+ end
+
private
def commit_diff_request_params(commit, options = {})
@@ -94,7 +107,12 @@ module Gitlab
end
def consume_commits_response(response)
- response.flat_map { |r| r.commits }
+ response.flat_map do |message|
+ message.commits.map do |gitaly_commit|
+ commit = GitalyClient::Commit.new(@repository, gitaly_commit)
+ Gitlab::Git::Commit.new(commit)
+ end
+ end
end
end
end
diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb
index 60de91324f042ec98733cf26dba925f153add6c4..730fdb112d9e61b18aeffe6556473b5675ec880c 100644
--- a/spec/lib/gitlab/git/commit_spec.rb
+++ b/spec/lib/gitlab/git/commit_spec.rb
@@ -91,7 +91,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
committer: committer
)
end
- let(:commit) { described_class.new(gitaly_commit) }
+ let(:commit) { described_class.new(Gitlab::GitalyClient::Commit.new(repository, gitaly_commit)) }
it { expect(commit.short_id).to eq(id[0..10]) }
it { expect(commit.id).to eq(id) }
@@ -290,69 +290,85 @@ describe Gitlab::Git::Commit, seed_helper: true do
end
describe '.find_all' do
- it 'should return a return a collection of commits' do
- commits = described_class.find_all(repository)
+ shared_examples 'finding all commits' do
+ it 'should return a return a collection of commits' do
+ commits = described_class.find_all(repository)
- expect(commits).not_to be_empty
- expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) )
- end
-
- context 'while applying a sort order based on the `order` option' do
- it "allows ordering topologically (no parents shown before their children)" do
- expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_TOPO)
-
- described_class.find_all(repository, order: :topo)
+ expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) )
end
- it "allows ordering by date" do
- expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_DATE | Rugged::SORT_TOPO)
-
- described_class.find_all(repository, order: :date)
+ context 'max_count' do
+ subject do
+ commits = Gitlab::Git::Commit.find_all(
+ repository,
+ max_count: 50
+ )
+
+ commits.map(&:id)
+ end
+
+ it 'has 33 elements' do
+ expect(subject.size).to eq(33)
+ end
+
+ it 'includes the expected commits' do
+ expect(subject).to include(
+ SeedRepo::Commit::ID,
+ SeedRepo::Commit::PARENT_ID,
+ SeedRepo::FirstCommit::ID
+ )
+ end
end
- it "applies no sorting by default" do
- expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_NONE)
-
- described_class.find_all(repository)
+ context 'ref + max_count + skip' do
+ subject do
+ commits = Gitlab::Git::Commit.find_all(
+ repository,
+ ref: 'master',
+ max_count: 50,
+ skip: 1
+ )
+
+ commits.map(&:id)
+ end
+
+ it 'has 24 elements' do
+ expect(subject.size).to eq(24)
+ end
+
+ it 'includes the expected commits' do
+ expect(subject).to include(SeedRepo::Commit::ID, SeedRepo::FirstCommit::ID)
+ expect(subject).not_to include(SeedRepo::LastCommit::ID)
+ end
end
end
- context 'max_count' do
- subject do
- commits = Gitlab::Git::Commit.find_all(
- repository,
- max_count: 50
- )
+ context 'when Gitaly find_all_commits feature is enabled' do
+ it_behaves_like 'finding all commits'
+ end
- commits.map { |c| c.id }
- end
+ context 'when Gitaly find_all_commits feature is disabled', skip_gitaly_mock: true do
+ it_behaves_like 'finding all commits'
- it 'has 31 elements' do
- expect(subject.size).to eq(33)
- end
- it { is_expected.to include(SeedRepo::Commit::ID) }
- it { is_expected.to include(SeedRepo::Commit::PARENT_ID) }
- it { is_expected.to include(SeedRepo::FirstCommit::ID) }
- end
+ context 'while applying a sort order based on the `order` option' do
+ it "allows ordering topologically (no parents shown before their children)" do
+ expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_TOPO)
- context 'ref + max_count + skip' do
- subject do
- commits = Gitlab::Git::Commit.find_all(
- repository,
- ref: 'master',
- max_count: 50,
- skip: 1
- )
+ described_class.find_all(repository, order: :topo)
+ end
- commits.map { |c| c.id }
- end
+ it "allows ordering by date" do
+ expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_DATE | Rugged::SORT_TOPO)
+
+ described_class.find_all(repository, order: :date)
+ end
+
+ it "applies no sorting by default" do
+ expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_NONE)
- it 'has 23 elements' do
- expect(subject.size).to eq(24)
+ described_class.find_all(repository)
+ end
end
- it { is_expected.to include(SeedRepo::Commit::ID) }
- it { is_expected.to include(SeedRepo::FirstCommit::ID) }
- it { is_expected.not_to include(SeedRepo::LastCommit::ID) }
end
end
end