From 26653eb0359002e9991bf5089c5505b566125f26 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 26 Dec 2018 10:24:16 -0800 Subject: Don't create a temp reference for branch comparisons within project A temp reference is only needed to fetch a branch from another project, as in the case for forked repositories. For branch comparisons within the same project, we can just use the existing branch names to do the comparison. Relates to https://gitlab.com/gitlab-org/gitlab-ce/issues/38689#note_126107862 --- .../sh-avoid-fetching-temp-refs-within-project.yml | 5 +++ lib/gitlab/git/repository.rb | 8 ++++- spec/lib/gitlab/git/repository_spec.rb | 36 ++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml diff --git a/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml b/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml new file mode 100644 index 00000000000..9b747f5d83d --- /dev/null +++ b/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml @@ -0,0 +1,5 @@ +--- +title: Don't create a temp reference for branch comparisons within project +merge_request: +author: +type: fixed diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index a22e3c4b9dd..6cebc9ac9eb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -732,6 +732,12 @@ module Gitlab end def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:) + if source_repository == self + return unless commit(source_branch_name).present? + + return Gitlab::Git::Compare.new(self, target_branch_name, source_branch_name, straight: straight) + end + tmp_ref = "refs/tmp/#{SecureRandom.hex}" return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref) @@ -743,7 +749,7 @@ module Gitlab straight: straight ) ensure - delete_refs(tmp_ref) + delete_refs(tmp_ref) if tmp_ref end def write_ref(ref_path, ref, old_ref: nil) diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 778950c95e4..85c45ea4183 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1966,6 +1966,42 @@ describe Gitlab::Git::Repository, :seed_helper do end end + describe '#compare_source_branch' do + context 'within same repository' do + 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', repository, 'feature', straight: false) + expect(compare).to be_a(Gitlab::Git::Compare) + expect(compare.commits.count).to be > 0 + end + + it 'returns nil when source ref does not exist' do + compare = repository.compare_source_branch('master', repository, 'non-existent-branch', straight: false) + + expect(compare).to be_nil + end + end + + context 'with different repositories' do + it 'creates a temp ref' do + 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, 'feature', straight: false) + expect(compare).to be_a(Gitlab::Git::Compare) + expect(compare.commits.count).to be > 0 + end + + it 'returns nil when source ref does not exist' do + compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false) + + expect(compare).to be_nil + end + end + end + describe '#checksum' do it 'calculates the checksum for non-empty repo' do expect(repository.checksum).to eq '51d0a9662681f93e1fee547a6b7ba2bcaf716059' -- cgit v1.2.1