Support for reverting to older versions.

This is the first behavior change from bundler-patch. Used to be older
versions would never be an option, but Bundler proper has always
supported this (if necessary to resolve the dependency tree) and there
can be some legit cases for doing this.

The `--strict` flag could be used to override this behavior, but I'm
running into a Molinillo behavior that I'm not sure is correct, so the
specs involving the strict option are failing right now. I'm going to
push so @segiddins and I can discuss.

3 files changed, 56 insertions, 10 deletions

diff --git a/lib/bundler/gem_version_promoter.rb b/lib/bundler/gem_version_promoter.rb
index 84c971f329..6276c926ce 100644
--- a/lib/bundler/gem_version_promoter.rb
+++ b/lib/bundler/gem_version_promoter.rb
@@ -82,17 +82,17 @@ module Bundler
       gem_name = locked_spec.name
       locked_version = locked_spec.version
 
-      specs = specs.select {|s| s.version >= locked_version } unless major?
-
       specs.sort do |a, b|
         a_ver = a.version
         b_ver = b.version
         case
         when major?
           a_ver <=> b_ver
-        when a_ver.segments[0] != b_ver.segments[0]
+        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? && (a_ver.segments[1] != b_ver.segments[1])
+        when !minor? && segments_do_not_match(:minor, a_ver, b_ver)
           b_ver <=> a_ver
         else
           a_ver <=> b_ver
@@ -107,6 +107,15 @@ module Bundler
       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
diff --git a/spec/bundler/gem_version_promoter_spec.rb b/spec/bundler/gem_version_promoter_spec.rb
index 740928a641..5cbc74bcd9 100644
--- a/spec/bundler/gem_version_promoter_spec.rb
+++ b/spec/bundler/gem_version_promoter_spec.rb
@@ -88,9 +88,9 @@ describe Bundler::GemVersionPromoter do
       it "when not unlocking, same order but make sure build_spec version is most preferred to stay put" do
         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_group("foo", %w(1.5.4 1.6.5 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)
-        expect(versions(res)).to eq %w(2.0.0 2.0.1 1.8.0 1.8.1 1.7.8 1.7.9 1.7.7)
+        expect(versions(res)).to eq %w(1.5.4 1.6.5 1.7.6 2.0.0 2.0.1 1.8.0 1.8.1 1.7.8 1.7.9 1.7.7)
       end
 
       it "when unlocking favor next release, then current over minor increase" do
@@ -98,7 +98,7 @@ describe Bundler::GemVersionPromoter do
         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)
-        expect(versions(res)).to eq %w(1.8.0 1.7.8 1.7.9)
+        expect(versions(res)).to eq %w(1.7.7 1.8.0 1.7.8 1.7.9)
       end
 
       it "when unlocking do proper integer comparison, not string" do
@@ -106,7 +106,7 @@ describe Bundler::GemVersionPromoter do
         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)
-        expect(versions(res)).to eq %w(1.8.0 1.7.8 1.7.9 1.7.15)
+        expect(versions(res)).to eq %w(1.7.7 1.8.0 1.7.8 1.7.9 1.7.15)
       end
 
       it "leave current when unlocking but already at latest release" do
diff --git a/spec/resolver/basic_spec.rb b/spec/resolver/basic_spec.rb
index 6613e34b78..2542baac51 100644
--- a/spec/resolver/basic_spec.rb
+++ b/spec/resolver/basic_spec.rb
@@ -188,8 +188,45 @@ describe "Resolving" 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"
+    # Why would this happen in real life? If bar 2.2 has a bug that the author of foo wants to bypass
+    # by reverting the dependency, the author of foo could release a new gem with an older requirement.
+    context "revert to previous" do
+      before :each do
+        @index = build_index do
+          gem("foo", "1.4.3") { dep "bar", "~> 2.2" }
+          gem("foo", "1.4.4") { dep "bar", "~> 2.1.0" }
+          gem("foo", "1.5.0") { dep "bar", "~> 2.0.0" }
+          gem "bar", %w(2.0.5 2.1.1 2.2.3)
+        end
+        dep "foo"
+
+        # base represents declared dependencies in the Gemfile that are still satisfied by the lockfile
+        @base = Bundler::SpecSet.new([])
+
+        # locked represents versions in lockfile
+        @locked = locked(%w(foo 1.4.3), %w(bar 2.2.3))
+      end
+
+      after :each do
+        ENV["DEBUG_RESOLVER"] = nil
+      end
 
-    it "will not revert to a previous version in strict mode"
+      it "could revert to a previous version level patch" do
+        should_conservative_resolve_and_include :patch, [], %w(foo-1.4.4 bar-2.1.1)
+      end
+
+      it "will not revert to a previous version in strict mode level patch" do
+        ENV["DEBUG_RESOLVER"] = "true"
+        should_conservative_resolve_and_include [:patch, :strict], [], %w(foo-1.4.3 bar-2.1.1)
+      end
+
+      it "could revert to a previous version level minor" do
+        should_conservative_resolve_and_include :minor, [], %w(foo-1.5.0 bar-2.0.5)
+      end
+
+      it "will not revert to a previous version in strict mode level minor" do
+        should_conservative_resolve_and_include [:minor, :strict], [], %w(foo-1.4.3 bar-2.1.1)
+      end
+    end
   end
 end