Merge branch 'pks-checks-commits-for-combinational-explosion' into 'master'

checks: Fix combinatorial explosion in `#commits_for()`

See merge request gitlab-org/gitlab!67827

lib/gitlab/checks/changes_access.rb

@@ -48,16 +48,28 @@ module Gitlab
         commits_by_id = commits.index_by(&:id)
 
         result = []
-        pending = [newrev]
+        pending = Set[newrev]
 
         # We go up the parent chain of our newrev and collect all commits which
         # are new. In case a commit's ID cannot be found in the set of new
         # commits, then it must already be a preexisting commit.
-        pending.each do |rev|
-          commit = commits_by_id[rev]
+        while pending.any?
+          rev = pending.first
+          pending.delete(rev)
+
+          # Remove the revision from commit candidates such that we don't walk
+          # it multiple times. If the hash doesn't contain the revision, then
+          # we have either already walked the commit or it's not new.
+          commit = commits_by_id.delete(rev)
           next if commit.nil?
 
-          pending.push(*commit.parent_ids)
+          # Only add the parent ID to the pending set if we actually know its
+          # commit to guards us against readding an ID which we have already
+          # queued up before.
+          commit.parent_ids.each do |parent_id|
+            pending.add(parent_id) if commits_by_id.has_key?(parent_id)
+          end
+
           result << commit
         end
 

spec/lib/gitlab/checks/changes_access_spec.rb

@@ -160,6 +160,36 @@ RSpec.describe Gitlab::Checks::ChangesAccess do
 
       it_behaves_like 'a listing of new commits'
     end
+
+    context 'with criss-cross merges' do
+      let(:new_commits) do
+        [
+          create_commit(newrev, %w[a1 b1]),
+          create_commit('a1', %w[a2 b2]),
+          create_commit('a2', %w[a3 b3]),
+          create_commit('a3', %w[c]),
+          create_commit('b1', %w[b2 a2]),
+          create_commit('b2', %w[b3 a3]),
+          create_commit('b3', %w[c]),
+          create_commit('c', [])
+        ]
+      end
+
+      let(:expected_commits) do
+        [
+          create_commit(newrev, %w[a1 b1]),
+          create_commit('a1', %w[a2 b2]),
+          create_commit('b1', %w[b2 a2]),
+          create_commit('a2', %w[a3 b3]),
+          create_commit('b2', %w[b3 a3]),
+          create_commit('a3', %w[c]),
+          create_commit('b3', %w[c]),
+          create_commit('c', [])
+        ]
+      end
+
+      it_behaves_like 'a listing of new commits'
+    end
   end
 
   def create_commit(id, parent_ids)