Auto merge of #5995 - bundler:seg-gvp-major, r=indirect

[2.0] [Resolver] Use the GVP for major updates in 2.0

Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.

The problem was @chrismo wants to start delegating all sorting to the GVP.

My diagnosis was to start doing that in 2.0.

Closes https://github.com/bundler/bundler/issues/5993

My fix adds a feature flag.

(cherry picked from commit a325b6d74f9d8db306f264c43bbe580ec0f1acc8)

6 files changed, 23 insertions, 19 deletions

diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 6b176c2ae4..8e56d4a9bc 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -9,7 +9,6 @@ module Bundler
 
     attr_reader(
       :dependencies,
-      :gem_version_promoter,
       :locked_deps,
       :locked_gems,
       :platforms,
@@ -125,25 +124,25 @@ module Bundler
         @unlock[:gems] = @locked_specs.for(eager_unlock, [], false, false, false).map(&:name)
       end
 
-      @gem_version_promoter = create_gem_version_promoter
-
       @dependency_changes = converge_dependencies
       @local_changes = converge_locals
 
       @requires = compute_requires
     end
 
-    def create_gem_version_promoter
-      locked_specs =
-        if unlocking? && @locked_specs.empty? && !@lockfile_contents.empty?
-          # Definition uses an empty set of locked_specs to indicate all gems
-          # are unlocked, but GemVersionPromoter needs the locked_specs
-          # for conservative comparison.
-          Bundler::SpecSet.new(@locked_gems.specs)
-        else
-          @locked_specs
-        end
-      GemVersionPromoter.new(locked_specs, @unlock[:gems])
+    def gem_version_promoter
+      @gem_version_promoter ||= begin
+        locked_specs =
+          if unlocking? && @locked_specs.empty? && !@lockfile_contents.empty?
+            # Definition uses an empty set of locked_specs to indicate all gems
+            # are unlocked, but GemVersionPromoter needs the locked_specs
+            # for conservative comparison.
+            Bundler::SpecSet.new(@locked_gems.specs)
+          else
+            @locked_specs
+          end
+        GemVersionPromoter.new(locked_specs, @unlock[:gems])
+      end
     end
 
     def resolve_with_cache!
@@ -214,7 +213,7 @@ module Bundler
       @index = nil
       @resolve = nil
       @specs = nil
-      @gem_version_promoter = create_gem_version_promoter
+      @gem_version_promoter = nil
 
       Bundler.ui.debug "The definition is missing dependencies, failed to resolve & materialize locally (#{e})"
       true
diff --git a/lib/bundler/feature_flag.rb b/lib/bundler/feature_flag.rb
index 04ad42d73d..d3cb32e720 100644
--- a/lib/bundler/feature_flag.rb
+++ b/lib/bundler/feature_flag.rb
@@ -54,6 +54,7 @@ module Bundler
     settings_flag(:suppress_install_using_messages) { bundler_2_mode? }
     settings_flag(:unlock_source_unlocks_spec) { !bundler_2_mode? }
     settings_flag(:update_requires_all_flag) { bundler_2_mode? }
+    settings_flag(:use_gem_version_promoter_for_major_updates) { bundler_2_mode? }
 
     settings_option(:default_cli_command) { bundler_2_mode? ? :cli_help : :install }
 
diff --git a/lib/bundler/gem_version_promoter.rb b/lib/bundler/gem_version_promoter.rb
index 2751e70929..adb951a7a0 100644
--- a/lib/bundler/gem_version_promoter.rb
+++ b/lib/bundler/gem_version_promoter.rb
@@ -7,6 +7,8 @@ module Bundler
   # available dependency versions as found in its index, before returning it to
   # to the resolution engine to select the best version.
   class GemVersionPromoter
+    DEBUG = ENV["DEBUG_RESOLVER"]
+
     attr_reader :level, :locked_specs, :unlock_gems
 
     # By default, strict is false, meaning every available version of a gem
@@ -64,7 +66,7 @@ module Bundler
     # @return [SpecGroup] A new instance of the SpecGroup Array sorted and
     #    possibly filtered.
     def sort_versions(dep, spec_groups)
-      before_result = "before sort_versions: #{debug_format_result(dep, spec_groups).inspect}" if ENV["DEBUG_RESOLVER"]
+      before_result = "before sort_versions: #{debug_format_result(dep, spec_groups).inspect}" if DEBUG
 
       @sort_versions[dep] ||= begin
         gem_name = dep.name
@@ -78,7 +80,7 @@ module Bundler
         else
           sort_dep_specs(spec_groups, locked_spec)
         end.tap do |specs|
-          if ENV["DEBUG_RESOLVER"]
+          if DEBUG
             STDERR.puts before_result
             STDERR.puts " after sort_versions: #{debug_format_result(dep, specs).inspect}"
           end
diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb
index 872dee074b..545b4cc88a 100644
--- a/lib/bundler/resolver.rb
+++ b/lib/bundler/resolver.rb
@@ -39,7 +39,7 @@ module Bundler
       @gem_version_promoter = gem_version_promoter
       @allow_bundler_dependency_conflicts = Bundler.feature_flag.allow_bundler_dependency_conflicts?
       @lockfile_uses_separate_rubygems_sources = Bundler.feature_flag.lockfile_uses_separate_rubygems_sources?
-      @use_gvp = !@gem_version_promoter.major?
+      @use_gvp = Bundler.feature_flag.use_gem_version_promoter_for_major_updates? || !@gem_version_promoter.major?
     end
 
     def start(requirements)
@@ -137,7 +137,7 @@ module Bundler
         end
         # GVP handles major itself, but it's still a bit risky to trust it with it
         # until we get it settled with new behavior. For 2.x it can take over all cases.
-        if @gem_version_promoter.major?
+        if !@use_gvp
           spec_groups
         else
           @gem_version_promoter.sort_versions(dependency, spec_groups)
diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb
index b365d08c2b..8574855f0c 100644
--- a/lib/bundler/settings.rb
+++ b/lib/bundler/settings.rb
@@ -56,6 +56,7 @@ module Bundler
       suppress_install_using_messages
       unlock_source_unlocks_spec
       update_requires_all_flag
+      use_gem_version_promoter_for_major_updates
     ].freeze
 
     NUMBER_KEYS = %w[
diff --git a/spec/quality_spec.rb b/spec/quality_spec.rb
index 067e9c4901..4f95757ebb 100644
--- a/spec/quality_spec.rb
+++ b/spec/quality_spec.rb
@@ -178,6 +178,7 @@ RSpec.describe "The library itself" do
       gem.mit
       inline
       lockfile_uses_separate_rubygems_sources
+      use_gem_version_promoter_for_major_updates
       warned_version
     ]