Conservative updates

lib/bundler.rb

@@ -30,6 +30,7 @@ module Bundler
   autoload :Fetcher,                "bundler/fetcher"
   autoload :GemHelper,              "bundler/gem_helper"
   autoload :GemHelpers,             "bundler/gem_helpers"
+  autoload :GemVersionPromoter,     "bundler/gem_version_promoter"
   autoload :Graph,                  "bundler/graph"
   autoload :Index,                  "bundler/index"
   autoload :Installer,              "bundler/installer"

lib/bundler/cli.rb

@@ -208,6 +208,14 @@ def install
       "Update ruby specified in Gemfile.lock"
     method_option "bundler", :type => :string, :lazy_default => "> 0.a", :banner =>
       "Update the locked version of bundler"
+    method_option "patch", :type => :boolean, :hide => true, :banner =>
+      "Prefer updating only to next patch version"
+    method_option "minor", :type => :boolean, :hide => true, :banner =>
+      "Prefer updating only to next minor version"
+    method_option "major", :type => :boolean, :hide => true, :banner =>
+      "Prefer updating to next major version (default)"
+    method_option "strict", :type => :boolean, :hide => true, :banner =>
+      "Do not allow any gem to be updated past latest --patch/--minor/--major"
     def update(*gems)
       require "bundler/cli/update"
       Update.new(options, gems).run

lib/bundler/cli/update.rb

@@ -39,6 +39,13 @@ def run
         Bundler.definition(:gems => gems, :sources => sources, :ruby => options[:ruby])
       end
 
+      patch_level = [:major, :minor, :patch].select {|v| options.keys.include?(v.to_s) }
+      raise ProductionError, "Provide only one of the following options: #{patch_level.join(", ")}" unless patch_level.length <= 1
+      Bundler.definition.gem_version_promoter.tap do |gvp|
+        gvp.level = patch_level.first || :major
+        gvp.strict = options[:strict]
+      end
+
       Bundler::Fetcher.disable_endpoint = options["full-index"]
 
       opts = options.dup

lib/bundler/definition.rb

@@ -7,7 +7,7 @@ module Bundler
   class Definition
     include GemHelpers
 
-    attr_reader :dependencies, :platforms, :ruby_version, :locked_deps
+    attr_reader :dependencies, :platforms, :ruby_version, :locked_deps, :gem_version_promoter
 
     # Given a gemfile and lockfile creates a Bundler definition
     #
@@ -94,6 +94,8 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
       end
       @unlocking ||= @unlock[:ruby] ||= (!@locked_ruby_version ^ !@ruby_version)
 
+      @gem_version_promoter = create_gem_version_promoter
+
       current_platform = Bundler.rubygems.platforms.map {|p| generic(p) }.compact.last
       add_platform(current_platform)
 
@@ -123,6 +125,21 @@ def fixup_dependency_types!
       end
     end
 
+    def create_gem_version_promoter
+      locked_specs = begin
+        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.
+          locked = Bundler::LockfileParser.new(@lockfile_contents)
+          Bundler::SpecSet.new(locked.specs)
+        else
+          @locked_specs
+        end
+      end
+      GemVersionPromoter.new(locked_specs, @unlock[:gems])
+    end
+
     def resolve_with_cache!
       raise "Specs already loaded" if @specs
       sources.cached!
@@ -221,7 +238,7 @@ def resolve
         else
           # Run a resolve against the locally available gems
           Bundler.ui.debug("Found changes from the lockfile, re-resolving dependencies because #{change_reason}")
-          last_resolve.merge Resolver.resolve(expanded_dependencies, index, source_requirements, last_resolve, ruby_version)
+          last_resolve.merge Resolver.resolve(expanded_dependencies, index, source_requirements, last_resolve, ruby_version, gem_version_promoter)
         end
       end
     end

lib/bundler/gem_version_promoter.rb

@@ -0,0 +1,172 @@
+# frozen_string_literal: true
+module Bundler
+  # This class contains all of the logic for determining the next version of a
+  # Gem to update to based on the requested level (patch, minor, major).
+  # Primarily designed to work with Resolver which will provide it the list of
+  # available dependency versions as found in its index, before returning it to
+  # to the resolution engine to select the best version.
+  class GemVersionPromoter
+    attr_reader :level, :locked_specs, :unlock_gems
+
+    # By default, strict is false, meaning every available version of a gem
+    # is returned from sort_versions. The order gives preference to the
+    # requested level (:patch, :minor, :major) but in complicated requirement
+    # cases some gems will by necessity by promoted past the requested level,
+    # or even reverted to older versions.
+    #
+    # If strict is set to true, the results from sort_versions will be
+    # truncated, eliminating any version outside the current level scope.
+    # This can lead to unexpected outcomes or even VersionConflict exceptions
+    # that report a version of a gem not existing for versions that indeed do
+    # existing in the referenced source.
+    attr_accessor :strict
+
+    # Given a list of locked_specs and a list of gems to unlock creates a
+    # GemVersionPromoter instance.
+    #
+    # @param locked_specs [SpecSet] All current locked specs. Unlike Definition
+    #   where this list is empty if all gems are being updated, this should
+    #   always be populated for all gems so this class can properly function.
+    # @param unlock_gems [String] List of gem names being unlocked. If empty,
+    #   all gems will be considered unlocked.
+    # @return [GemVersionPromoter]
+    def initialize(locked_specs = SpecSet.new([]), unlock_gems = [])
+      @level = :major
+      @strict = false
+      @locked_specs = locked_specs
+      @unlock_gems = unlock_gems
+      @sort_versions = {}
+    end
+
+    # @param value [Symbol] One of three Symbols: :major, :minor or :patch.
+    def level=(value)
+      v = case value
+          when String, Symbol
+            value.to_sym
+      end
+
+      raise ArgumentError, "Unexpected level #{v}. Must be :major, :minor or :patch" unless [:major, :minor, :patch].include?(v)
+      @level = v
+    end
+
+    # Given a Dependency and an Array of SpecGroups of available versions for a
+    # gem, this method will return the Array of SpecGroups sorted (and possibly
+    # truncated if strict is true) in an order to give preference to the current
+    # level (:major, :minor or :patch) when resolution is deciding what versions
+    # best resolve all dependencies in the bundle.
+    # @param dep [Dependency] The Dependency of the gem.
+    # @param spec_groups [SpecGroup] An array of SpecGroups for the same gem
+    #    named in the @dep param.
+    # @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"]
+
+      @sort_versions[dep] ||= begin
+        gem_name = dep.name
+
+        # An Array per version returned, different entries for different platforms.
+        # We only need the version here so it's ok to hard code this to the first instance.
+        locked_spec = locked_specs[gem_name].first
+
+        if strict
+          filter_dep_specs(spec_groups, locked_spec)
+        else
+          sort_dep_specs(spec_groups, locked_spec)
+        end.tap do |specs|
+          if ENV["DEBUG_RESOLVER"]
+            STDERR.puts before_result
+            STDERR.puts " after sort_versions: #{debug_format_result(dep, specs).inspect}"
+          end
+        end
+      end
+    end
+
+    # @return [bool] Convenience method for testing value of level variable.
+    def major?
+      level == :major
+    end
+
+    # @return [bool] Convenience method for testing value of level variable.
+    def minor?
+      level == :minor
+    end
+
+  private
+
+    def filter_dep_specs(spec_groups, locked_spec)
+      res = spec_groups.select do |spec_group|
+        if locked_spec && !major?
+          gsv = spec_group.version
+          lsv = locked_spec.version
+
+          must_match = minor? ? [0] : [0, 1]
+
+          matches = must_match.map {|idx| gsv.segments[idx] == lsv.segments[idx] }
+          (matches.uniq == [true]) ? (gsv >= lsv) : false
+        else
+          true
+        end
+      end
+
+      sort_dep_specs(res, locked_spec)
+    end
+
+    def sort_dep_specs(spec_groups, locked_spec)
+      return spec_groups unless locked_spec
+      gem_name = locked_spec.name
+      locked_version = locked_spec.version
+
+      spec_groups.sort do |a, b|
+        a_ver = a.version
+        b_ver = b.version
+        case
+        when major?
+          a_ver <=> b_ver
+        when either_version_older_than_locked(locked_version, a_ver, b_ver)
+          a_ver <=> b_ver
+        when segments_do_not_match(:major, a_ver, b_ver)
+          b_ver <=> a_ver
+        when !minor? && segments_do_not_match(:minor, a_ver, b_ver)
+          b_ver <=> a_ver
+        else
+          a_ver <=> b_ver
+        end
+      end.tap do |result|
+        # default :major behavior in Bundler does not do this
+        unless major?
+          unless unlocking_gem?(gem_name)
+            move_version_to_end(spec_groups, locked_version, result)
+          end
+        end
+      end
+    end
+
+    def either_version_older_than_locked(locked_version, a_ver, b_ver)
+      a_ver < locked_version || b_ver < locked_version
+    end
+
+    def segments_do_not_match(level, a_ver, b_ver)
+      index = [:major, :minor].index(level)
+      a_ver.segments[index] != b_ver.segments[index]
+    end
+
+    def unlocking_gem?(gem_name)
+      unlock_gems.empty? || unlock_gems.include?(gem_name)
+    end
+
+    def move_version_to_end(spec_groups, version, result)
+      spec_group = spec_groups.detect {|s| s.version.to_s == version.to_s }
+      return unless spec_group
+      result.reject! {|s| s.version.to_s == version.to_s }
+      result << spec_group
+    end
+
+    def debug_format_result(dep, spec_groups)
+      a = [dep.to_s,
+           spec_groups.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
+end

lib/bundler/resolver.rb

@@ -172,14 +172,14 @@ def __dependencies
     # ==== Returns
     # <GemBundle>,nil:: If the list of dependencies can be resolved, a
     #   collection of gemspecs is returned. Otherwise, nil is returned.
-    def self.resolve(requirements, index, source_requirements = {}, base = [], ruby_version = nil)
+    def self.resolve(requirements, index, source_requirements = {}, base = [], ruby_version = nil, gem_version_promoter = GemVersionPromoter.new)
       base = SpecSet.new(base) unless base.is_a?(SpecSet)
-      resolver = new(index, source_requirements, base, ruby_version)
+      resolver = new(index, source_requirements, base, ruby_version, gem_version_promoter)
       result = resolver.start(requirements)
       SpecSet.new(result)
     end
 
-    def initialize(index, source_requirements, base, ruby_version)
+    def initialize(index, source_requirements, base, ruby_version, gem_version_promoter)
       @index = index
       @source_requirements = source_requirements
       @base = base
@@ -188,6 +188,7 @@ def initialize(index, source_requirements, base, ruby_version)
       @base_dg = Molinillo::DependencyGraph.new
       @base.each {|ls| @base_dg.add_vertex(ls.name, Dependency.new(ls.name, ls.version), true) }
       @ruby_version = ruby_version
+      @gem_version_promoter = gem_version_promoter
     end
 
     def start(requirements)
@@ -249,7 +250,7 @@ def search_for(dependency)
         if vertex = @base_dg.vertex_named(dependency.name)
           locked_requirement = vertex.payload.requirement
         end
-        if results.any?
+        spec_groups = if results.any?
           nested = []
           results.each do |spec|
             version, specs = nested.last
@@ -266,6 +267,13 @@ def search_for(dependency)
         else
           []
         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?
+          spec_groups
+        else
+          @gem_version_promoter.sort_versions(dependency, spec_groups)
+        end
       end
       search.select {|sg| sg.for?(platform, @ruby_version) }.each {|sg| sg.activate_platform!(platform) }
     end

lib/bundler/spec_set.rb

@@ -7,7 +7,7 @@ class SpecSet
     extend Forwardable
     include TSort, Enumerable
 
-    def_delegators :@specs, :<<, :length, :add, :remove, :size
+    def_delegators :@specs, :<<, :length, :add, :remove, :size, :empty?
     def_delegators :sorted, :each
 
     def initialize(specs)

spec/bundler/definition_spec.rb

@@ -133,4 +133,51 @@
       G
     end
   end
+
+  describe "initialize" do
+    context "gem version promoter" do
+      context "with lockfile" do
+        before :each do
+          install_gemfile <<-G
+          source "file://#{gem_repo1}"
+          gem "foo"
+          G
+        end
+
+        it "should get a locked specs list when updating all" do
+          definition = Bundler::Definition.new(bundled_app("Gemfile.lock"), [], Bundler::SourceList.new, true)
+          locked_specs = definition.gem_version_promoter.locked_specs
+          expect(locked_specs.to_a.map(&:name)).to eq ["foo"]
+          expect(definition.instance_variable_get("@locked_specs").empty?).to eq true
+        end
+      end
+
+      context "without gemfile or lockfile" do
+        it "should not attempt to parse empty lockfile contents" do
+          definition = Bundler::Definition.new(nil, [], mock_source_list, true)
+          expect(definition.gem_version_promoter.locked_specs.to_a).to eq []
+        end
+      end
+
+      def mock_source_list
+        Class.new do
+          def all_sources
+            []
+          end
+
+          def path_sources
+            []
+          end
+
+          def rubygems_remotes
+            []
+          end
+
+          def replace_sources!(arg)
+            nil
+          end
+        end.new
+      end
+    end
+  end
 end