Make use of local ref if it is reachablesh-avoid-fetching-temp-refs-within-project

3 files changed, 61 insertions, 19 deletions

diff --git a/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml b/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml
index 9b747f5d83d..7511543f7f6 100644
--- a/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml
+++ b/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml
@@ -1,5 +1,5 @@
 ---
 title: Don't create a temp reference for branch comparisons within project
-merge_request:
+merge_request: 24038
 author:
 type: fixed
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index d7f5720c795..c12cb6a6434 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -732,20 +732,27 @@ module Gitlab
       end
 
       def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
-        if source_repository == self
-          return Gitlab::Git::Compare.new(self, target_branch_name, source_branch_name, straight: straight)
-        end
+        reachable_ref =
+          if source_repository == self
+            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
+
+        return compare(target_branch_name, reachable_ref, straight: straight) if reachable_ref
 
         tmp_ref = "refs/tmp/#{SecureRandom.hex}"
 
         return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref)
 
-        Gitlab::Git::Compare.new(
-          self,
-          target_branch_name,
-          tmp_ref,
-          straight: straight
-        )
+        compare(target_branch_name, tmp_ref, straight: straight)
       ensure
         delete_refs(tmp_ref) if tmp_ref
       end
@@ -1003,6 +1010,13 @@ module Gitlab
 
       private
 
+      def compare(base_ref, head_ref, straight:)
+        Gitlab::Git::Compare.new(self,
+                                 base_ref,
+                                 head_ref,
+                                 straight: straight)
+      end
+
       def empty_diff_stats
         Gitlab::Git::DiffStatsCollection.new([])
       end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index aa800db4f16..45fe5d72937 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1967,6 +1967,8 @@ describe Gitlab::Git::Repository, :seed_helper do
   end
 
   describe '#compare_source_branch' do
+    let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') }
+
     context 'within same repository' do
       it 'does not create a temp ref' do
         expect(repository).not_to receive(:fetch_source_branch!)
@@ -1985,19 +1987,45 @@ describe Gitlab::Git::Repository, :seed_helper do
     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
+      context 'when ref is known by source repo, but not by target' do
+        before do
+          mutable_repository.write_ref('another-branch', 'feature')
+        end
 
-        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
+        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, 'another-branch', straight: false)
+          expect(compare).to be_a(Gitlab::Git::Compare)
+          expect(compare.commits.count).to be > 0
+        end
       end
 
-      it 'returns nil when source ref does not exist' do
-        compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false)
+      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
 
-        expect(compare).to be_nil
+      context 'when ref is unknown by source repo' do
+        it 'returns nil when source ref does not exist' 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, 'non-existent-branch', straight: false)
+          expect(compare).to be_nil
+        end
       end
     end
   end