diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 540fe010127..d6c0980255f 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -490,7 +490,11 @@ module Gitlab return [] end - log_by_shell(sha, options) + if log_using_shell?(options) + log_by_shell(sha, options) + else + log_by_walk(sha, options) + end end def count_commits(options) @@ -1529,6 +1533,27 @@ module Gitlab end end + def log_using_shell?(options) + options[:path].present? || + options[:disable_walk] || + options[:skip_merges] || + options[:after] || + options[:before] + end + + def log_by_walk(sha, options) + walk_options = { + show: sha, + sort: Rugged::SORT_NONE, + limit: options[:limit], + offset: options[:offset] + } + Rugged::Walker.walk(rugged, walk_options).to_a + end + + # Gitaly note: JV: although #log_by_shell shells out to Git I think the + # complexity is such that we should migrate it as Ruby before trying to + # do it in Go. def log_by_shell(sha, options) limit = options[:limit].to_i offset = options[:offset].to_i diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 36ca3980de9..18b2e55e659 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -899,6 +899,44 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + context "compare results between log_by_walk and log_by_shell" do + let(:options) { { ref: "master" } } + let(:commits_by_walk) { repository.log(options).map(&:id) } + let(:commits_by_shell) { repository.log(options.merge({ disable_walk: true })).map(&:id) } + + it { expect(commits_by_walk).to eq(commits_by_shell) } + + context "with limit" do + let(:options) { { ref: "master", limit: 1 } } + + it { expect(commits_by_walk).to eq(commits_by_shell) } + end + + context "with offset" do + let(:options) { { ref: "master", offset: 1 } } + + it { expect(commits_by_walk).to eq(commits_by_shell) } + end + + context "with skip_merges" do + let(:options) { { ref: "master", skip_merges: true } } + + it { expect(commits_by_walk).to eq(commits_by_shell) } + end + + context "with path" do + let(:options) { { ref: "master", path: "encoding" } } + + it { expect(commits_by_walk).to eq(commits_by_shell) } + + context "with follow" do + let(:options) { { ref: "master", path: "encoding", follow: true } } + + it { expect(commits_by_walk).to eq(commits_by_shell) } + end + end + end + context "where provides 'after' timestamp" do options = { after: Time.iso8601('2014-03-03T20:15:01+00:00') }