diff options
| -rw-r--r-- | app/models/repository.rb | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/23062-allow-git-log-to-accept-follow-and-skip.yml | 4 | ||||
| -rw-r--r-- | lib/gitlab/git/repository.rb | 24 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 96 |
4 files changed, 105 insertions, 23 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb index cd2568ad445..d7668cbd131 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -109,9 +109,7 @@ class Repository offset: offset, after: after, before: before, - # --follow doesn't play well with --skip. See: - # https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520 - follow: false, + follow: path.present?, skip_merges: skip_merges } diff --git a/changelogs/unreleased/23062-allow-git-log-to-accept-follow-and-skip.yml b/changelogs/unreleased/23062-allow-git-log-to-accept-follow-and-skip.yml new file mode 100644 index 00000000000..f7c856040e0 --- /dev/null +++ b/changelogs/unreleased/23062-allow-git-log-to-accept-follow-and-skip.yml @@ -0,0 +1,4 @@ +--- +title: Make Git history follow renames again by performing the --skip in Ruby +merge_request: +author: diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 4b6ad8037ce..9c68060064a 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -324,24 +324,30 @@ module Gitlab end def log_by_shell(sha, options) + limit = options[:limit].to_i + offset = options[:offset].to_i + use_follow_flag = options[:follow] && options[:path].present? + + # We will perform the offset in Ruby because --follow doesn't play well with --skip. + # See: https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520 + offset_in_ruby = use_follow_flag && options[:offset].present? + limit += offset if offset_in_ruby + cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path} log) - cmd += %W(-n #{options[:limit].to_i}) + cmd += %W(-n #{limit}) cmd += %w(--format=%H) - cmd += %W(--skip=#{options[:offset].to_i}) - cmd += %w(--follow) if options[:follow] + cmd += %W(--skip=#{offset}) unless offset_in_ruby + cmd += %w(--follow) if use_follow_flag cmd += %w(--no-merges) if options[:skip_merges] cmd += %W(--after=#{options[:after].iso8601}) if options[:after] cmd += %W(--before=#{options[:before].iso8601}) if options[:before] cmd += [sha] cmd += %W(-- #{options[:path]}) if options[:path].present? - raw_output = IO.popen(cmd) {|io| io.read } - - log = raw_output.lines.map do |c| - Rugged::Commit.new(rugged, c.strip) - end + raw_output = IO.popen(cmd) { |io| io.read } + lines = offset_in_ruby ? raw_output.lines.drop(offset) : raw_output.lines - log.is_a?(Array) ? log : [] + lines.map { |c| Rugged::Commit.new(rugged, c.strip) } end def sha_from_ref(ref) diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 2a915bf426f..05594089db3 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -546,7 +546,7 @@ describe Gitlab::Git::Repository, seed_helper: true do repository.log(options.merge(path: "encoding")) end - it "should not follow renames" do + it "does not follow renames" do expect(log_commits).to include(commit_with_new_name) expect(log_commits).to include(rename_commit) expect(log_commits).not_to include(commit_with_old_name) @@ -554,14 +554,88 @@ describe Gitlab::Git::Repository, seed_helper: true do end context "and 'path' is a file that matches the new filename" do - let(:log_commits) do - repository.log(options.merge(path: "encoding/CHANGELOG")) + context 'without offset' do + let(:log_commits) do + repository.log(options.merge(path: "encoding/CHANGELOG")) + end + + it "follows renames" do + expect(log_commits).to include(commit_with_new_name) + expect(log_commits).to include(rename_commit) + expect(log_commits).to include(commit_with_old_name) + end end - it "should follow renames" do - expect(log_commits).to include(commit_with_new_name) - expect(log_commits).to include(rename_commit) - expect(log_commits).to include(commit_with_old_name) + context 'with offset=1' do + let(:log_commits) do + repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1)) + end + + it "follows renames and skip the latest commit" do + expect(log_commits).not_to include(commit_with_new_name) + expect(log_commits).to include(rename_commit) + expect(log_commits).to include(commit_with_old_name) + end + end + + context 'with offset=1', 'and limit=1' do + let(:log_commits) do + repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1, limit: 1)) + end + + it "follows renames, skip the latest commit and return only one commit" do + expect(log_commits).not_to include(commit_with_new_name) + expect(log_commits).to include(rename_commit) + expect(log_commits).not_to include(commit_with_old_name) + end + end + + context 'with offset=1', 'and limit=2' do + let(:log_commits) do + repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1, limit: 2)) + end + + it "follows renames, skip the latest commit and return only two commits" do + expect(log_commits).not_to include(commit_with_new_name) + expect(log_commits).to include(rename_commit) + expect(log_commits).to include(commit_with_old_name) + end + end + + context 'with offset=2' do + let(:log_commits) do + repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2)) + end + + it "follows renames and skip the latest commit" do + expect(log_commits).not_to include(commit_with_new_name) + expect(log_commits).not_to include(rename_commit) + expect(log_commits).to include(commit_with_old_name) + end + end + + context 'with offset=2', 'and limit=1' do + let(:log_commits) do + repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2, limit: 1)) + end + + it "follows renames, skip the two latest commit and return only one commit" do + expect(log_commits).not_to include(commit_with_new_name) + expect(log_commits).not_to include(rename_commit) + expect(log_commits).to include(commit_with_old_name) + end + end + + context 'with offset=2', 'and limit=2' do + let(:log_commits) do + repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2, limit: 2)) + end + + it "follows renames, skip the two latest commit and return only one commit" do + expect(log_commits).not_to include(commit_with_new_name) + expect(log_commits).not_to include(rename_commit) + expect(log_commits).to include(commit_with_old_name) + end end end @@ -570,17 +644,17 @@ describe Gitlab::Git::Repository, seed_helper: true do repository.log(options.merge(path: "CHANGELOG")) end - it "should not follow renames" do - expect(log_commits).to include(commit_with_old_name) - expect(log_commits).to include(rename_commit) + it "does not follow renames" do expect(log_commits).not_to include(commit_with_new_name) + expect(log_commits).to include(rename_commit) + expect(log_commits).to include(commit_with_old_name) end end context "unknown ref" do let(:log_commits) { repository.log(options.merge(ref: 'unknown')) } - it "should return empty" do + it "return an empty array" do expect(log_commits).to eq([]) end end |