Commit 976d373a authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Make use of local ref if it is reachable

parent 1ac4b24d
--- ---
title: Don't create a temp reference for branch comparisons within project title: Don't create a temp reference for branch comparisons within project
merge_request: merge_request: 24038
author: author:
type: fixed type: fixed
...@@ -732,20 +732,27 @@ module Gitlab ...@@ -732,20 +732,27 @@ module Gitlab
end end
def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:) def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
reachable_ref =
if source_repository == self if source_repository == self
return Gitlab::Git::Compare.new(self, target_branch_name, source_branch_name, straight: straight) source_branch_name
else
# If a tmp ref was created before for a separate repo comparison (forks),
# we're able to short-circuit the tmp ref re-creation:
# 1. Take the SHA from the source repo
# 2. Read that in the current "target" repo
# 3. If that SHA is still known (readable), it means GC hasn't
# cleaned it up yet, so we can use it instead re-writing the tmp ref.
source_commit_id = source_repository.commit(source_branch_name)&.sha
commit(source_commit_id)&.sha if source_commit_id
end end
return compare(target_branch_name, reachable_ref, straight: straight) if reachable_ref
tmp_ref = "refs/tmp/#{SecureRandom.hex}" tmp_ref = "refs/tmp/#{SecureRandom.hex}"
return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref) return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref)
Gitlab::Git::Compare.new( compare(target_branch_name, tmp_ref, straight: straight)
self,
target_branch_name,
tmp_ref,
straight: straight
)
ensure ensure
delete_refs(tmp_ref) if tmp_ref delete_refs(tmp_ref) if tmp_ref
end end
...@@ -1003,6 +1010,13 @@ module Gitlab ...@@ -1003,6 +1010,13 @@ module Gitlab
private private
def compare(base_ref, head_ref, straight:)
Gitlab::Git::Compare.new(self,
base_ref,
head_ref,
straight: straight)
end
def empty_diff_stats def empty_diff_stats
Gitlab::Git::DiffStatsCollection.new([]) Gitlab::Git::DiffStatsCollection.new([])
end end
......
...@@ -1967,6 +1967,8 @@ describe Gitlab::Git::Repository, :seed_helper do ...@@ -1967,6 +1967,8 @@ describe Gitlab::Git::Repository, :seed_helper do
end end
describe '#compare_source_branch' do describe '#compare_source_branch' do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') }
context 'within same repository' do context 'within same repository' do
it 'does not create a temp ref' do it 'does not create a temp ref' do
expect(repository).not_to receive(:fetch_source_branch!) expect(repository).not_to receive(:fetch_source_branch!)
...@@ -1985,22 +1987,48 @@ describe Gitlab::Git::Repository, :seed_helper do ...@@ -1985,22 +1987,48 @@ describe Gitlab::Git::Repository, :seed_helper do
end end
context 'with different repositories' do context 'with different repositories' do
it 'creates a temp ref' do context 'when ref is known by source repo, but not by target' do
expect(repository).to receive(:fetch_source_branch!).and_call_original before do
expect(repository).to receive(:delete_refs).and_call_original mutable_repository.write_ref('another-branch', 'feature')
end
it 'creates temp ref' do
expect(repository).not_to receive(:fetch_source_branch!)
expect(repository).not_to receive(:delete_refs)
compare = repository.compare_source_branch('master', mutable_repository, 'feature', straight: false) compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false)
expect(compare).to be_a(Gitlab::Git::Compare) expect(compare).to be_a(Gitlab::Git::Compare)
expect(compare.commits.count).to be > 0 expect(compare.commits.count).to be > 0
end end
end
context 'when ref is known by source and target repos' do
before do
mutable_repository.write_ref('another-branch', 'feature')
repository.write_ref('another-branch', 'feature')
end
it 'does not create a temp ref' do
expect(repository).not_to receive(:fetch_source_branch!)
expect(repository).not_to receive(:delete_refs)
compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false)
expect(compare).to be_a(Gitlab::Git::Compare)
expect(compare.commits.count).to be > 0
end
end
context 'when ref is unknown by source repo' do
it 'returns nil when source ref does not exist' do it 'returns nil when source ref does not exist' do
compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false) expect(repository).to receive(:fetch_source_branch!).and_call_original
expect(repository).to receive(:delete_refs).and_call_original
compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false)
expect(compare).to be_nil expect(compare).to be_nil
end end
end end
end end
end
describe '#checksum' do describe '#checksum' do
it 'calculates the checksum for non-empty repo' do it 'calculates the checksum for non-empty repo' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment