Moved GemVersionPromoter inside search_for cache.

In the process I was able to simplify some of the code inside
GemVersionPromoter dealing with SpecGroups.

I also attempted to implement the :major behavior into
GemVersionPromoter as that would eliminate the logic to skip it in
`#search_for`, however I ran into some test failures that I need to
investigate further, though unit specs are working so far.

5 files changed, 59 insertions, 44 deletions

diff --git a/lib/bundler/gem_version_promoter.rb b/lib/bundler/gem_version_promoter.rb
index 20a3a2b225..50848ca580 100644
--- a/lib/bundler/gem_version_promoter.rb
+++ b/lib/bundler/gem_version_promoter.rb
@@ -1,9 +1,11 @@
 # frozen_string_literal: true
 module Bundler
+  # MODO: docs
   class GemVersionPromoter
     attr_reader :level, :locked_specs, :unlock_gems
     attr_accessor :strict
 
+    # MODO: docs
     def initialize(locked_specs = SpecSet.new([]), unlock_gems = [])
       @level_default = :major
       @level = @level_default
@@ -13,6 +15,7 @@ module Bundler
       @sort_versions = {}
     end
 
+    # MODO: docs
     def level=(value)
       v = begin
         case value
@@ -25,6 +28,7 @@ module Bundler
       @level = v
     end
 
+    # MODO: docs
     def sort_versions(dep, dep_specs)
       before_result = "before sort_versions: #{debug_format_result(dep, dep_specs).inspect}" if ENV["DEBUG_RESOLVER"]
 
@@ -53,13 +57,9 @@ module Bundler
   private
 
     def filter_dep_specs(specs, locked_spec)
-      res = specs.select do |sg|
-        # SpecGroup is grouped by name/version, multiple entries for multiple platforms.
-        # We only need the name, which will be the same, so hard coding to first is ok.
-        gem_spec = sg.first
-
-        if locked_spec
-          gsv = gem_spec.version
+      res = specs.select do |spec_group|
+        if locked_spec && !(level == :major)
+          gsv = spec_group.version
           lsv = locked_spec.version
 
           must_match = level == :minor ? [0] : [0, 1]
@@ -74,20 +74,19 @@ module Bundler
       sort_dep_specs(res, locked_spec)
     end
 
-    # reminder: sort still filters anything older than locked version
-    # :major bundle update behavior can move a gem to an older version
-    # in order to satisfy the dependency tree.
     def sort_dep_specs(specs, locked_spec)
       return specs unless locked_spec
       gem_name = locked_spec.name
       locked_version = locked_spec.version
 
-      filtered = specs.select {|s| s.first.version >= locked_version }
+      filtered = specs.select {|s| s.version >= locked_version }
 
       filtered.sort do |a, b|
-        a_ver = a.first.version
-        b_ver = b.first.version
+        a_ver = a.version
+        b_ver = b.version
         case
+        when level == :major
+          a_ver <=> b_ver
         when a_ver.segments[0] != b_ver.segments[0]
           b_ver <=> a_ver
         when !(level == :minor) && (a_ver.segments[1] != b_ver.segments[1])
@@ -107,15 +106,16 @@ module Bundler
     end
 
     def move_version_to_end(specs, version, result)
-      spec_group = specs.detect {|s| s.first.version.to_s == version.to_s }
+      spec_group = specs.detect {|s| s.version.to_s == version.to_s }
       return unless spec_group
-      result.reject! {|s| s.first.version.to_s == version.to_s }
+      result.reject! {|s| s.version.to_s == version.to_s }
       result << spec_group
     end
 
     def debug_format_result(dep, res)
       a = [dep.to_s,
            res.map {|sg| [sg.version, sg.dependencies_for_activated_platforms.map {|dp| [dp.name, dp.requirement.to_s] }] }]
+      last_map = a.last.map {|sg_data| [sg_data.first.version, sg_data.last.map {|aa| aa.join(" ") }] }
       [a.first, last_map, level, strict ? :strict : :not_strict]
     end
   end
diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb
index 1b4e5a1eb3..eb564f5857 100644
--- a/lib/bundler/resolver.rb
+++ b/lib/bundler/resolver.rb
@@ -250,28 +250,34 @@ module Bundler
         if vertex = @base_dg.vertex_named(dependency.name)
           locked_requirement = vertex.payload.requirement
         end
-        if results.any?
-          nested = []
-          results.each do |spec|
-            version, specs = nested.last
-            if version == spec.version
-              specs << spec
-            else
-              nested << [spec.version, [spec]]
+        spec_groups = begin
+          if results.any?
+            nested = []
+            results.each do |spec|
+              version, specs = nested.last
+              if version == spec.version
+                specs << spec
+              else
+                nested << [spec.version, [spec]]
+              end
             end
+            nested.reduce([]) do |groups, (version, specs)|
+              next groups if locked_requirement && !locked_requirement.satisfied_by?(version)
+              groups << SpecGroup.new(specs)
+            end
+          else
+            []
           end
-          nested.reduce([]) do |groups, (version, specs)|
-            next groups if locked_requirement && !locked_requirement.satisfied_by?(version)
-            groups << SpecGroup.new(specs)
-          end
-        else
-          []
         end
+        # MODO: ideally trying to get GVP to sort by major. but some test cases aren't showing consistent order
+        # by default. (so we don't have to have this conditional here)
+        if @gem_version_promoter.level != :major
+          @gem_version_promoter.sort_versions(dependency, spec_groups)
+        else
+          spec_groups
+        end # .tap {|sg| STDERR.puts "sg>> #{sg.inspect}" }
       end
-      platform_results = search.select {|sg| sg.for?(platform, @ruby_version) }.each {|sg| sg.activate_platform!(platform) }
-      return platform_results if @gem_version_promoter.level == :major # default behavior
-      # MODO: put this inside the cache
-      @gem_version_promoter.sort_versions(dependency, platform_results)
+      search.select {|sg| sg.for?(platform, @ruby_version) }.each {|sg| sg.activate_platform!(platform) }
     end
 
     def index_for(dependency)
diff --git a/spec/bundler/gem_version_promoter_spec.rb b/spec/bundler/gem_version_promoter_spec.rb
index 876f6c0c98..8c9660e2ac 100644
--- a/spec/bundler/gem_version_promoter_spec.rb
+++ b/spec/bundler/gem_version_promoter_spec.rb
@@ -13,14 +13,14 @@ describe Bundler::GemVersionPromoter do
       end
     end
 
-    def unlocking(options = {})
+    def unlocking(options)
       make_instance(Bundler::SpecSet.new([]), ["foo"]).tap do |p|
         p.level = options[:level] if options[:level]
         p.strict = options[:strict] if options[:strict]
       end
     end
 
-    def keep_locked(options = {})
+    def keep_locked(options)
       make_instance(Bundler::SpecSet.new([]), ["bar"]).tap do |p|
         p.level = options[:level] if options[:level]
         p.strict = options[:strict] if options[:strict]
@@ -28,7 +28,7 @@ describe Bundler::GemVersionPromoter do
     end
 
     def build_spec_group(name, version)
-      build_spec(name, version).map {|s| Array(s) }
+      Bundler::Resolver::SpecGroup.new(build_spec(name, version))
     end
 
     # Rightmost (highest array index) in result is most preferred.
@@ -50,7 +50,7 @@ describe Bundler::GemVersionPromoter do
       end
 
       it "when unlocking prefer next release first" do
-        unlocking
+        unlocking(:level => :patch)
         res = @gvp.filter_dep_specs(
           build_spec_group("foo", %w(1.7.8 1.7.9 1.8.0)),
           build_spec("foo", "1.7.8").first)
@@ -58,7 +58,7 @@ describe Bundler::GemVersionPromoter do
       end
 
       it "when unlocking keep current when already at latest release" do
-        unlocking
+        unlocking(:level => :patch)
         res = @gvp.filter_dep_specs(
           build_spec_group("foo", %w(1.7.9 1.8.0 2.0.0)),
           build_spec("foo", "1.7.9").first)
@@ -72,7 +72,7 @@ describe Bundler::GemVersionPromoter do
 
     context "sort specs (not strict) (minor not allowed)" do
       it "when not unlocking, same order but make sure build_spec version is most preferred to stay put" do
-        keep_locked
+        keep_locked(:level => :patch)
         res = @gvp.sort_dep_specs(
           build_spec_group("foo", %w(1.7.6 1.7.7 1.7.8 1.7.9 1.8.0 1.8.1 2.0.0 2.0.1)),
           build_spec("foo", "1.7.7").first)
@@ -80,7 +80,7 @@ describe Bundler::GemVersionPromoter do
       end
 
       it "when unlocking favor next release, then current over minor increase" do
-        unlocking
+        unlocking(:level => :patch)
         res = @gvp.sort_dep_specs(
           build_spec_group("foo", %w(1.7.7 1.7.8 1.7.9 1.8.0)),
           build_spec("foo", "1.7.8").first)
@@ -88,7 +88,7 @@ describe Bundler::GemVersionPromoter do
       end
 
       it "when unlocking do proper integer comparison, not string" do
-        unlocking
+        unlocking(:level => :patch)
         res = @gvp.sort_dep_specs(
           build_spec_group("foo", %w(1.7.7 1.7.8 1.7.9 1.7.15 1.8.0)),
           build_spec("foo", "1.7.8").first)
@@ -96,7 +96,7 @@ describe Bundler::GemVersionPromoter do
       end
 
       it "leave current when unlocking but already at latest release" do
-        unlocking
+        unlocking(:level => :patch)
         res = @gvp.sort_dep_specs(
           build_spec_group("foo", %w(1.7.9 1.8.0 2.0.0)),
           build_spec("foo", "1.7.9").first)
diff --git a/spec/commands/update_spec.rb b/spec/commands/update_spec.rb
index cd23c4891d..5db8878ac1 100644
--- a/spec/commands/update_spec.rb
+++ b/spec/commands/update_spec.rb
@@ -449,7 +449,7 @@ describe "bundle update conservative" do
       should_be_installed "foo 1.0.1"
     end
 
-    it "single gem without dependencies update all" do
+    it "single gem without dependencies update all with debugging" do
       build_repo4 do
         build_gem "foo", %w(1.0.0 1.0.1 1.1.0 2.0.0)
       end
@@ -464,7 +464,7 @@ describe "bundle update conservative" do
         gem 'foo'
       G
 
-      bundle "update --patch"
+      bundle "update --patch", :env => { "DEBUG_RESOLVER" => true }
 
       should_be_installed "foo 1.0.1"
     end
diff --git a/spec/resolver/basic_spec.rb b/spec/resolver/basic_spec.rb
index 90233d0389..0bae4464c7 100644
--- a/spec/resolver/basic_spec.rb
+++ b/spec/resolver/basic_spec.rb
@@ -113,6 +113,7 @@ describe "Resolving" do
         gem("foo", "1.4.5") { dep "bar", "~> 2.1" }
         gem("foo", "1.5.0") { dep "bar", "~> 2.1" }
         gem("foo", "1.5.1") { dep "bar", "~> 3.0" }
+        gem("foo", "2.0.0") { dep "bar", "~> 3.0" }
         gem "bar", %w(2.0.3 2.0.4 2.0.5 2.1.0 2.1.1 3.0.0)
       end
       dep "foo"
@@ -179,6 +180,14 @@ describe "Resolving" do
       should_conservative_resolve_and_include [:minor, :strict], [], %w(foo-1.5.0 bar-2.1.1)
     end
 
+    it "resolves all gems to latest major" do
+      should_conservative_resolve_and_include :major, [], %w(foo-2.0.0 bar-3.0.0)
+    end
+
+    it "resolves all gems to latest major strict" do
+      should_conservative_resolve_and_include [:major, :strict], [], %w(foo-2.0.0 bar-3.0.0)
+    end
+
     it "could revert to a previous version"
 
     it "will not revert to a previous version in strict mode"