Auto merge of #4257 - bundler:seg-ruby-version-fixes, r=indirect

Ruby version fixes

I believe these bugs should be blockers for 1.12, but I don't have it in me to write specs yet.

\c @indirect

8 files changed, 292 insertions, 46 deletions

diff --git a/.rubocop.yml b/.rubocop.yml
index c2ce653729..8ad27fbe1f 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -16,6 +16,7 @@ Lint/AssignmentInCondition:
 
 Lint/EndAlignment:
   AlignWith: variable
+  AutoCorrect: true
 
 Lint/UnusedMethodArgument:
   Enabled: false
diff --git a/lib/bundler/cli/platform.rb b/lib/bundler/cli/platform.rb
index aec08789a9..b5f906bfd9 100644
--- a/lib/bundler/cli/platform.rb
+++ b/lib/bundler/cli/platform.rb
@@ -8,8 +8,10 @@ module Bundler
 
     def run
       platforms, ruby_version = Bundler.ui.silence do
+        locked_ruby_version = Bundler.locked_gems && Bundler.locked_gems.ruby_version
+        gemfile_ruby_version = Bundler.definition.ruby_version && Bundler.definition.ruby_version.single_version_string
         [Bundler.definition.platforms.map {|p| "* #{p}" },
-         Bundler.definition.ruby_version]
+         locked_ruby_version || gemfile_ruby_version]
       end
       output = []
 
diff --git a/lib/bundler/fetcher.rb b/lib/bundler/fetcher.rb
index 348c29dcbe..afbecdd553 100644
--- a/lib/bundler/fetcher.rb
+++ b/lib/bundler/fetcher.rb
@@ -166,18 +166,18 @@ module Bundler
 
         agent = String.new("bundler/#{Bundler::VERSION}")
         agent << " rubygems/#{Gem::VERSION}"
-        agent << " ruby/#{ruby.version}"
+        agent << " ruby/#{ruby.versions_string(ruby.versions)}"
         agent << " (#{ruby.host})"
         agent << " command/#{ARGV.first}"
 
         if ruby.engine != "ruby"
           # engine_version raises on unknown engines
           engine_version = begin
-                             ruby.engine_version
+                             ruby.engine_versions
                            rescue
                              "???"
                            end
-          agent << " #{ruby.engine}/#{engine_version}"
+          agent << " #{ruby.engine}/#{ruby.versions_string(engine_version)}"
         end
 
         agent << " options/#{Bundler.settings.all.join(",")}"
diff --git a/lib/bundler/ruby_dsl.rb b/lib/bundler/ruby_dsl.rb
index 47f2bcc337..a410b7f3d7 100644
--- a/lib/bundler/ruby_dsl.rb
+++ b/lib/bundler/ruby_dsl.rb
@@ -1,11 +1,16 @@
 # frozen_string_literal: true
 module Bundler
   module RubyDsl
-    def ruby(ruby_version, options = {})
+    def ruby(*ruby_version)
+      options = ruby_version.last.is_a?(Hash) ? ruby_version.pop : {}
+      ruby_version.flatten!
       raise GemfileError, "Please define :engine_version" if options[:engine] && options[:engine_version].nil?
       raise GemfileError, "Please define :engine" if options[:engine_version] && options[:engine].nil?
 
-      raise GemfileEvalError, "ruby_version must match the :engine_version for MRI" if options[:engine] == "ruby" && options[:engine_version] && ruby_version != options[:engine_version]
+      if options[:engine] == "ruby" && options[:engine_version] &&
+          ruby_version != Array(options[:engine_version])
+        raise GemfileEvalError, "ruby_version must match the :engine_version for MRI"
+      end
       @ruby_version = RubyVersion.new(ruby_version, options[:patchlevel], options[:engine], options[:engine_version])
     end
   end
diff --git a/lib/bundler/ruby_version.rb b/lib/bundler/ruby_version.rb
index b8f3b8a863..800a4433f2 100644
--- a/lib/bundler/ruby_version.rb
+++ b/lib/bundler/ruby_version.rb
@@ -1,9 +1,14 @@
 # frozen_string_literal: true
 module Bundler
   class RubyVersion
-    attr_reader :version, :patchlevel, :engine, :engine_version
+    attr_reader :versions,
+      :patchlevel,
+      :engine,
+      :engine_versions,
+      :gem_version,
+      :engine_gem_version
 
-    def initialize(version, patchlevel, engine, engine_version)
+    def initialize(versions, patchlevel, engine, engine_version)
       # The parameters to this method must satisfy the
       # following constraints, which are verified in
       # the DSL:
@@ -16,25 +21,31 @@ module Bundler
       #   must not be specified, or the engine version
       #   specified must match the version.
 
-      @version        = version
-      @input_engine   = engine
-      @engine         = engine || "ruby"
-      @engine_version = engine_version || version
-      @patchlevel     = patchlevel
+      @versions           = Array(versions)
+      @gem_version        = Gem::Requirement.create(@versions.first).requirements.first.last
+      @input_engine       = engine
+      @engine             = engine || "ruby"
+      @engine_versions    = (engine_version && Array(engine_version)) || @versions
+      @engine_gem_version = Gem::Requirement.create(@engine_versions.first).requirements.first.last
+      @patchlevel         = patchlevel
     end
 
-    def to_s
-      output = String.new("ruby #{version}")
+    def to_s(versions = self.versions)
+      output = String.new("ruby #{versions_string(versions)}")
       output << "p#{patchlevel}" if patchlevel
-      output << " (#{engine} #{engine_version})" unless engine == "ruby"
+      output << " (#{engine} #{versions_string(engine_versions)})" unless engine == "ruby"
 
       output
     end
 
+    def single_version_string
+      to_s(gem_version)
+    end
+
     def ==(other)
-      version == other.version &&
+      versions == other.versions &&
         engine == other.engine &&
-        engine_version == other.engine_version &&
+        engine_versions == other.engine_versions &&
         patchlevel == other.patchlevel
     end
 
@@ -46,10 +57,6 @@ module Bundler
       ].join("-")
     end
 
-    def gem_version
-      Gem::Version.new(version)
-    end
-
     # Returns a tuple of these things:
     #   [diff, this, other]
     #   The priority of attributes are
@@ -57,17 +64,22 @@ module Bundler
     #   2. ruby_version
     #   3. engine_version
     def diff(other)
+      raise ArgumentError, "Can only diff with a RubyVersion" unless other.is_a?(RubyVersion)
       if engine != other.engine && @input_engine
         [:engine, engine, other.engine]
-      elsif !version || !matches?(version, other.version)
-        [:version, version, other.version]
-      elsif @input_engine && !matches?(engine_version, other.engine_version)
-        [:engine_version, engine_version, other.engine_version]
+      elsif versions.empty? || !matches?(versions, other.gem_version)
+        [:version, versions_string(versions), versions_string(other.versions)]
+      elsif @input_engine && !matches?(engine_versions, other.engine_gem_version)
+        [:engine_version, versions_string(engine_versions), versions_string(other.engine_versions)]
       elsif patchlevel && (!patchlevel.is_a?(String) || !other.patchlevel.is_a?(String) || !matches?(patchlevel, other.patchlevel))
         [:patchlevel, patchlevel, other.patchlevel]
       end
     end
 
+    def versions_string(versions)
+      Array(versions).join(", ")
+    end
+
     def self.system
       ruby_engine = if defined?(RUBY_ENGINE) && !RUBY_ENGINE.nil?
         RUBY_ENGINE.dup
@@ -90,8 +102,10 @@ module Bundler
 
   private
 
-    def matches?(requirement, version)
-      Gem::Requirement.create(requirement).satisfied_by?(Gem::Version.new(version))
+    def matches?(requirements, version)
+      Array(requirements).all? do |requirement|
+        Gem::Requirement.create(requirement).satisfied_by?(Gem::Version.create(version))
+      end
     end
   end
 end
diff --git a/spec/bundler/ruby_dsl_spec.rb b/spec/bundler/ruby_dsl_spec.rb
new file mode 100644
index 0000000000..ac73747ac7
--- /dev/null
+++ b/spec/bundler/ruby_dsl_spec.rb
@@ -0,0 +1,95 @@
+# frozen_string_literal: true
+require "spec_helper"
+require "bundler/ruby_dsl"
+
+describe Bundler::RubyDsl do
+  class MockDSL
+    include Bundler::RubyDsl
+
+    attr_reader :ruby_version
+  end
+
+  let(:dsl) { MockDSL.new }
+  let(:ruby_version) { "2.0.0" }
+  let(:version) { "2.0.0" }
+  let(:engine) { "jruby" }
+  let(:engine_version) { "9000" }
+  let(:patchlevel) { "100" }
+  let(:options) do
+    { :patchlevel => patchlevel,
+      :engine => engine,
+      :engine_version => engine_version }
+  end
+
+  let(:invoke) do
+    proc do
+      args = Array(ruby_version) + [options]
+      dsl.ruby(*args)
+    end
+  end
+
+  subject do
+    invoke.call
+    dsl.ruby_version
+  end
+
+  describe "#ruby_version" do
+    shared_examples_for "it stores the ruby version" do
+      it "stores the version" do
+        expect(subject.versions).to eq(Array(ruby_version))
+        expect(subject.gem_version.version).to eq(version)
+      end
+
+      it "stores the engine details" do
+        expect(subject.engine).to eq(engine)
+        expect(subject.engine_versions).to eq(Array(engine_version))
+      end
+
+      it "stores the patchlevel" do
+        expect(subject.patchlevel).to eq(patchlevel)
+      end
+    end
+
+    context "with a plain version" do
+      it_behaves_like "it stores the ruby version"
+    end
+
+    context "with a single requirement" do
+      let(:ruby_version) { ">= 2.0.0" }
+      it_behaves_like "it stores the ruby version"
+    end
+
+    context "with two requirements in the same string" do
+      let(:ruby_version) { ">= 2.0.0, < 3.0" }
+      it "raises an error" do
+        expect { subject }.to raise_error(ArgumentError)
+      end
+    end
+
+    context "with two requirements" do
+      let(:ruby_version) { ["~> 2.0.0", "> 2.0.1"] }
+      it_behaves_like "it stores the ruby version"
+    end
+
+    context "with multiple engine versions" do
+      let(:engine_version) { ["> 200", "< 300"] }
+      it_behaves_like "it stores the ruby version"
+    end
+
+    context "with no options hash" do
+      let(:invoke) { proc { dsl.ruby(ruby_version) } }
+
+      let(:patchlevel) { nil }
+      let(:engine) { "ruby" }
+      let(:engine_version) { version }
+
+      it_behaves_like "it stores the ruby version"
+
+      context "and with multiple requirements" do
+        let(:ruby_version) { ["~> 2.0.0", "> 2.0.1"] }
+        let(:engine_version) { ruby_version }
+        it_behaves_like "it stores the ruby version"
+      end
+    end
+  end
+end
diff --git a/spec/bundler/ruby_version_spec.rb b/spec/bundler/ruby_version_spec.rb
index 9c9eda04dc..ce68463029 100644
--- a/spec/bundler/ruby_version_spec.rb
+++ b/spec/bundler/ruby_version_spec.rb
@@ -31,7 +31,28 @@ describe "Bundler::RubyVersion and its subclasses" do
         let(:engine_version) { nil }
 
         it "should set engine version as the passed version" do
-          expect(subject.engine_version).to eq("2.0.0")
+          expect(subject.engine_versions).to eq(["2.0.0"])
+        end
+      end
+
+      context "is called with multiple requirements" do
+        let(:version) { ["<= 2.0.0", "> 1.9.3"] }
+        let(:engine_version) { nil }
+
+        it "sets the versions" do
+          expect(subject.versions).to eq(version)
+        end
+
+        it "sets the engine versions" do
+          expect(subject.engine_versions).to eq(version)
+        end
+      end
+
+      context "is called with multiple engine requirements" do
+        let(:engine_version) { [">= 2.0", "< 2.3"] }
+
+        it "sets the engine versions" do
+          expect(subject.engine_versions).to eq(engine_version)
         end
       end
     end
@@ -55,6 +76,16 @@ describe "Bundler::RubyVersion and its subclasses" do
           expect(subject.to_s).to eq("ruby 2.0.0p645")
         end
       end
+
+      context "with multiple requirements" do
+        let(:engine_version) { ["> 9", "< 11"] }
+        let(:version) { ["> 8", "< 10"] }
+        let(:patchlevel) { nil }
+
+        it "should return info string with all requirements" do
+          expect(subject.to_s).to eq("ruby > 8, < 10 (jruby > 9, < 11)")
+        end
+      end
     end
 
     describe "#==" do
@@ -115,12 +146,23 @@ describe "Bundler::RubyVersion and its subclasses" do
     end
 
     describe "#gem_version" do
-      let(:gem_version_obj) { Gem::Version.new(version) }
-
-      it "should return a Gem::Version instance with the correct version" do
-        expect(ruby_version.gem_version).to eq(gem_version_obj)
-        expect(ruby_version.gem_version.version).to eq("2.0.0")
+      let(:gem_version) { "2.0.0" }
+      let(:gem_version_obj) { Gem::Version.new(gem_version) }
+
+      shared_examples_for "it parses the version from the requirement string" do |version|
+        let(:version) { version }
+        it "should return the underlying version" do
+          expect(ruby_version.gem_version).to eq(gem_version_obj)
+          expect(ruby_version.gem_version.version).to eq(gem_version)
+        end
       end
+
+      it_behaves_like "it parses the version from the requirement string", "2.0.0"
+      it_behaves_like "it parses the version from the requirement string", ">= 2.0.0"
+      it_behaves_like "it parses the version from the requirement string", "~> 2.0.0"
+      it_behaves_like "it parses the version from the requirement string", "< 2.0.0"
+      it_behaves_like "it parses the version from the requirement string", "= 2.0.0"
+      it_behaves_like "it parses the version from the requirement string", ["> 2.0.0", "< 2.4.5"]
     end
 
     describe "#diff" do
@@ -134,13 +176,13 @@ describe "Bundler::RubyVersion and its subclasses" do
 
       shared_examples_for "there is a difference in the versions" do
         it "should return a tuple with :version and the two different versions" do
-          expect(ruby_version.diff(other_ruby_version)).to eq([:version, version, other_version])
+          expect(ruby_version.diff(other_ruby_version)).to eq([:version, Array(version).join(", "), Array(other_version).join(", ")])
         end
       end
 
       shared_examples_for "there is a difference in the engine versions" do
         it "should return a tuple with :engine_version and the two different engine versions" do
-          expect(ruby_version.diff(other_ruby_version)).to eq([:engine_version, engine_version, other_engine_version])
+          expect(ruby_version.diff(other_ruby_version)).to eq([:engine_version, Array(engine_version).join(", "), Array(other_engine_version).join(", ")])
         end
       end
 
@@ -177,6 +219,16 @@ describe "Bundler::RubyVersion and its subclasses" do
         it_behaves_like "there is a difference in the versions"
       end
 
+      context "detects version discrepancies with multiple requirements second" do
+        let(:other_version)        { "2.0.1" }
+        let(:other_patchlevel)     { "643" }
+        let(:other_engine_version) { "2.0.0" }
+
+        let(:version) { ["> 2.0.0", "< 1.0.0"] }
+
+        it_behaves_like "there is a difference in the versions"
+      end
+
       context "detects engine version discrepancies third" do
         let(:other_patchlevel)     { "643" }
         let(:other_engine_version) { "2.0.0" }
@@ -184,6 +236,15 @@ describe "Bundler::RubyVersion and its subclasses" do
         it_behaves_like "there is a difference in the engine versions"
       end
 
+      context "detects engine version discrepancies with multiple requirements third" do
+        let(:other_patchlevel)     { "643" }
+        let(:other_engine_version) { "2.0.0" }
+
+        let(:engine_version) { ["> 2.0.0", "< 1.0.0"] }
+
+        it_behaves_like "there is a difference in the engine versions"
+      end
+
       context "detects patchlevel discrepancies last" do
         let(:other_patchlevel) { "643" }
 
@@ -203,6 +264,32 @@ describe "Bundler::RubyVersion and its subclasses" do
         it_behaves_like "there are no differences"
       end
 
+      context "successfully matches multiple gem requirements" do
+        let(:version)              { [">= 2.0.0", "< 2.4.5"] }
+        let(:patchlevel)           { "< 643" }
+        let(:engine)               { "ruby" }
+        let(:engine_version)       { ["~> 2.0.1", "< 2.4.5"] }
+        let(:other_version)        { "2.0.0" }
+        let(:other_patchlevel)     { "642" }
+        let(:other_engine)         { "ruby" }
+        let(:other_engine_version) { "2.0.5" }
+
+        it_behaves_like "there are no differences"
+      end
+
+      context "successfully detects bad gem requirements with versions with multiple requirements" do
+        let(:version)              { ["~> 2.0.0", "< 2.0.5"] }
+        let(:patchlevel)           { "< 643" }
+        let(:engine)               { "ruby" }
+        let(:engine_version)       { "~> 2.0.1" }
+        let(:other_version)        { "2.0.5" }
+        let(:other_patchlevel)     { "642" }
+        let(:other_engine)         { "ruby" }
+        let(:other_engine_version) { "2.0.5" }
+
+        it_behaves_like "there is a difference in the versions"
+      end
+
       context "successfully detects bad gem requirements with versions" do
         let(:version)              { "~> 2.0.0" }
         let(:patchlevel)           { "< 643" }
@@ -263,8 +350,8 @@ describe "Bundler::RubyVersion and its subclasses" do
 
       describe "#version" do
         it "should return a copy of the value of RUBY_VERSION" do
-          expect(subject.version).to eq(RUBY_VERSION)
-          expect(subject.version).to_not be(RUBY_VERSION)
+          expect(subject.versions).to eq([RUBY_VERSION])
+          expect(subject.versions.first).to_not be(RUBY_VERSION)
         end
       end
 
@@ -296,8 +383,8 @@ describe "Bundler::RubyVersion and its subclasses" do
           end
 
           it "should return a copy of the value of RUBY_VERSION" do
-            expect(bundler_system_ruby_version.engine_version).to eq("2.2.4")
-            expect(bundler_system_ruby_version.engine_version).to_not be(RUBY_VERSION)
+            expect(bundler_system_ruby_version.engine_versions).to eq(["2.2.4"])
+            expect(bundler_system_ruby_version.engine_versions.first).to_not be(RUBY_VERSION)
           end
         end
 
@@ -308,8 +395,8 @@ describe "Bundler::RubyVersion and its subclasses" do
           end
 
           it "should return a copy of the value of Rubinius::VERSION" do
-            expect(bundler_system_ruby_version.engine_version).to eq("2.0.0")
-            expect(bundler_system_ruby_version.engine_version).to_not be(Rubinius::VERSION)
+            expect(bundler_system_ruby_version.engine_versions).to eq(["2.0.0"])
+            expect(bundler_system_ruby_version.engine_versions.first).to_not be(Rubinius::VERSION)
           end
         end
 
@@ -320,8 +407,8 @@ describe "Bundler::RubyVersion and its subclasses" do
           end
 
           it "should return a copy of the value of JRUBY_VERSION" do
-            expect(subject.engine_version).to eq("2.1.1")
-            expect(bundler_system_ruby_version.engine_version).to_not be(JRUBY_VERSION)
+            expect(subject.engine_versions).to eq(["2.1.1"])
+            expect(bundler_system_ruby_version.engine_versions.first).to_not be(JRUBY_VERSION)
           end
         end
 
@@ -332,7 +419,7 @@ describe "Bundler::RubyVersion and its subclasses" do
           end
 
           it "should raise a BundlerError with a 'not recognized' message" do
-            expect { bundler_system_ruby_version.engine_version }.to raise_error(Bundler::BundlerError, "RUBY_ENGINE value not_supported_ruby_engine is not recognized")
+            expect { bundler_system_ruby_version.engine_versions }.to raise_error(Bundler::BundlerError, "RUBY_ENGINE value not_supported_ruby_engine is not recognized")
           end
         end
       end
diff --git a/spec/other/platform_spec.rb b/spec/other/platform_spec.rb
index 7942e604a5..8d546fca57 100644
--- a/spec/other/platform_spec.rb
+++ b/spec/other/platform_spec.rb
@@ -191,10 +191,52 @@ G
       G
 
       bundle "platform --ruby"
-      puts err
 
       expect(out).to eq("No ruby version specified")
     end
+
+    it "handles when there is a locked requirement" do
+      gemfile <<-G
+        ruby "< 1.8.7"
+      G
+
+      lockfile <<-L
+        GEM
+          specs:
+
+        PLATFORMS
+          ruby
+
+        DEPENDENCIES
+
+        RUBY VERSION
+           ruby 1.0.0p127
+
+        BUNDLED WITH
+           #{Bundler::VERSION}
+      L
+
+      bundle! "platform --ruby"
+      expect(out).to eq("ruby 1.0.0p127")
+    end
+
+    it "handles when there is a requirement in the gemfile" do
+      gemfile <<-G
+        ruby ">= 1.8.7"
+      G
+
+      bundle! "platform --ruby"
+      expect(out).to eq("ruby 1.8.7")
+    end
+
+    it "handles when there are multiple requirements in the gemfile" do
+      gemfile <<-G
+        ruby ">= 1.8.7", "< 2.0.0"
+      G
+
+      bundle! "platform --ruby"
+      expect(out).to eq("ruby 1.8.7")
+    end
   end
 
   let(:ruby_version_correct) { "ruby \"#{RUBY_VERSION}\", :engine => \"#{local_ruby_engine}\", :engine_version => \"#{local_engine_version}\"" }