Auto merge of #4654 - bundler:aa-ruby-version-conflict-message, r=indirect

Add resolver-style error messages for required Ruby conflicts

*Proposal:* When a gem in the Gemfile requires a Ruby version different than the Gemfile's `ruby`, print a conflict error message that looks more or less exactly like any other resolution conflict error.

*Current behavior:* On `master` today, Bundler will ignore the gem's required_ruby_version until install-time, and raise an exception like "require_ruby-1.0 requires ruby version > 9000, which is incompatible with the current version`.

After #4650 has been merged, Bundler will correctly reject gems that require a different version of Ruby, and instead print the error "Could not find `required_ruby` in any of the sources in your Gemfile".

*Rationale for change:* I believe that error message is surprising and misleading. The source _does_ contain `required ruby`, and I think we'll get bug reports about how our error message is wrong.

8 files changed, 146 insertions, 40 deletions

diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index e1826746ff..dc3b4690d7 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -62,6 +62,7 @@ module Bundler
       @specs           = nil
       @ruby_version    = ruby_version
 
+      @lockfile               = lockfile
       @lockfile_contents      = String.new
       @locked_bundler_version = nil
       @locked_ruby_version    = nil
@@ -94,11 +95,7 @@ module Bundler
 
       @unlock[:gems] ||= []
       @unlock[:sources] ||= []
-      @unlock[:ruby] ||= if @ruby_version && @locked_ruby_version
-        unless locked_ruby_version_object = RubyVersion.from_string(@locked_ruby_version)
-          raise LockfileError, "Failed to create a `RubyVersion` object from " \
-            "`#{@locked_ruby_version}` found in #{lockfile} -- try running `bundle update --ruby`."
-        end
+      @unlock[:ruby] ||= if @ruby_version && locked_ruby_version_object
         @ruby_version.diff(locked_ruby_version_object)
       end
       @unlocking ||= @unlock[:ruby] ||= (!@locked_ruby_version ^ !@ruby_version)
@@ -249,7 +246,7 @@ module Bundler
         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, gem_version_promoter, additional_base_requirements_for_resolve)
+          last_resolve.merge Resolver.resolve(expanded_dependencies, index, source_requirements, last_resolve, gem_version_promoter, additional_base_requirements_for_resolve)
         end
       end
     end
@@ -264,6 +261,8 @@ module Bundler
           dependency_names -= pinned_spec_names(source.specs)
           dependency_names.concat(source.unmet_deps).uniq!
         end
+        idx << Gem::Specification.new("ruby\0", RubyVersion.system.to_gem_version_with_patchlevel)
+        idx << Gem::Specification.new("rubygems\0", Gem::VERSION)
       end
     end
 
@@ -340,6 +339,18 @@ module Bundler
       end
     end
 
+    def locked_ruby_version_object
+      return unless @locked_ruby_version
+      @locked_ruby_version_object ||= begin
+        unless version = RubyVersion.from_string(@locked_ruby_version)
+          raise LockfileError, "The Ruby version #{@locked_ruby_version} from " \
+            "#{@lockfile} could not be parsed. " \
+            "Try running bundle update --ruby to resolve this."
+        end
+        version
+      end
+    end
+
     def to_lock
       out = String.new
 
@@ -728,8 +739,38 @@ module Bundler
       @locked_specs.any? {|s| s.satisfies?(dep) && (!dep.source || s.source.include?(dep.source)) }
     end
 
+    # This list of dependencies is only used in #resolve, so it's OK to add
+    # the metadata dependencies here
     def expanded_dependencies
-      @expanded_dependencies ||= expand_dependencies(dependencies, @remote)
+      @expanded_dependencies ||= begin
+        ruby_versions = concat_ruby_version_requirements(@ruby_version)
+        if ruby_versions.empty? || !@ruby_version.exact?
+          concat_ruby_version_requirements(RubyVersion.system)
+          concat_ruby_version_requirements(locked_ruby_version_object) unless @unlock[:ruby]
+        end
+
+        metadata_dependencies = [
+          Dependency.new("ruby\0", ruby_versions),
+          Dependency.new("rubygems\0", Gem::VERSION),
+        ]
+        expand_dependencies(dependencies + metadata_dependencies, @remote)
+      end
+    end
+
+    def concat_ruby_version_requirements(ruby_version, ruby_versions = [])
+      return ruby_versions unless ruby_version
+      if ruby_version.patchlevel
+        ruby_versions << ruby_version.to_gem_version_with_patchlevel
+      else
+        ruby_versions.concat(ruby_version.versions.map do |version|
+          requirement = Gem::Requirement.new(version)
+          if requirement.exact?
+            "~> #{version}.0"
+          else
+            requirement
+          end
+        end)
+      end
     end
 
     def expand_dependencies(dependencies, remote = false)
diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb
index e1d993831f..3ab4ae6f38 100644
--- a/lib/bundler/resolver.rb
+++ b/lib/bundler/resolver.rb
@@ -75,8 +75,8 @@ module Bundler
 
       def initialize(a)
         super
-        @required_by  = []
-        @activated    = []
+        @required_by = []
+        @activated_platforms = []
         @dependencies = nil
         @specs        = {}
 
@@ -88,13 +88,13 @@ module Bundler
       def initialize_copy(o)
         super
         @required_by = o.required_by.dup
-        @activated   = o.activated.dup
+        @activated_platforms = o.activated.dup
       end
 
       def to_specs
         specs = {}
 
-        @activated.each do |p|
+        @activated_platforms.each do |p|
           next unless s = @specs[p]
           platform = generic(Gem::Platform.new(s.platform))
           next if specs[platform]
@@ -107,7 +107,9 @@ module Bundler
       end
 
       def activate_platform!(platform)
-        @activated << platform if !@activated.include?(platform) && for?(platform, nil)
+        return unless for?(platform)
+        return if @activated_platforms.include?(platform)
+        @activated_platforms << platform
       end
 
       def name
@@ -122,17 +124,9 @@ module Bundler
         @source ||= first.source
       end
 
-      def for?(platform, ruby_version)
+      def for?(platform)
         spec = @specs[platform]
-        return false unless spec
-
-        return true if ruby_version.nil?
-        # Only allow endpoint specifications since they won't hit the network to
-        # fetch the full gemspec when calling required_ruby_version
-        return true if !spec.is_a?(EndpointSpecification) && !spec.is_a?(Gem::Specification)
-        return true if spec.required_ruby_version.nil?
-
-        spec.required_ruby_version.satisfied_by?(ruby_version.to_gem_version_with_patchlevel)
+        !spec.nil?
       end
 
       def to_s
@@ -140,7 +134,11 @@ module Bundler
       end
 
       def dependencies_for_activated_platforms
-        @activated.map {|p| __dependencies[p] }.flatten
+        dependencies = @activated_platforms.map {|p| __dependencies[p] }
+        metadata_dependencies = @activated_platforms.map do |platform|
+          metadata_dependencies(@specs[platform], platform)
+        end
+        dependencies.concat(metadata_dependencies).flatten
       end
 
       def platforms_for_dependency_named(dependency)
@@ -163,6 +161,21 @@ module Bundler
           dependencies
         end
       end
+
+      def metadata_dependencies(spec, platform)
+        return [] unless spec
+        # Only allow endpoint specifications since they won't hit the network to
+        # fetch the full gemspec when calling required_ruby_version
+        return [] if !spec.is_a?(EndpointSpecification) && !spec.is_a?(Gem::Specification)
+        dependencies = []
+        if !spec.required_ruby_version.nil? && !spec.required_ruby_version.none?
+          dependencies << DepProxy.new(Gem::Dependency.new("ruby\0", spec.required_ruby_version), platform)
+        end
+        if !spec.required_rubygems_version.nil? && !spec.required_rubygems_version.none?
+          dependencies << DepProxy.new(Gem::Dependency.new("rubygems\0", spec.required_rubygems_version), platform)
+        end
+        dependencies
+      end
     end
 
     # Figures out the best possible configuration of gems that satisfies
@@ -175,14 +188,14 @@ module Bundler
     # ==== 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, gem_version_promoter = GemVersionPromoter.new, additional_base_requirements = [])
+    def self.resolve(requirements, index, source_requirements = {}, base = [], gem_version_promoter = GemVersionPromoter.new, additional_base_requirements = [])
       base = SpecSet.new(base) unless base.is_a?(SpecSet)
-      resolver = new(index, source_requirements, base, ruby_version, gem_version_promoter, additional_base_requirements)
+      resolver = new(index, source_requirements, base, gem_version_promoter, additional_base_requirements)
       result = resolver.start(requirements)
       SpecSet.new(result)
     end
 
-    def initialize(index, source_requirements, base, ruby_version, gem_version_promoter, additional_base_requirements)
+    def initialize(index, source_requirements, base, gem_version_promoter, additional_base_requirements)
       @index = index
       @source_requirements = source_requirements
       @base = base
@@ -194,14 +207,15 @@ module Bundler
         @base_dg.add_vertex(ls.name, DepProxy.new(dep, ls.platform), true)
       end
       additional_base_requirements.each {|d| @base_dg.add_vertex(d.name, d) }
-      @ruby_version = ruby_version
       @gem_version_promoter = gem_version_promoter
     end
 
     def start(requirements)
       verify_gemfile_dependencies_are_found!(requirements)
       dg = @resolver.resolve(requirements, @base_dg)
-      dg.map(&:payload).map(&:to_specs).flatten
+      dg.map(&:payload).
+        reject {|sg| sg.name.end_with?("\0") }.
+        map(&:to_specs).flatten
     rescue Molinillo::VersionConflict => e
       raise VersionConflict.new(e.conflicts.keys.uniq, e.message)
     rescue Molinillo::CircularDependencyError => e
@@ -282,7 +296,7 @@ module Bundler
           @gem_version_promoter.sort_versions(dependency, spec_groups)
         end
       end
-      search.select {|sg| sg.for?(platform, @ruby_version) }.each {|sg| sg.activate_platform!(platform) }
+      search.select {|sg| sg.for?(platform) }.each {|sg| sg.activate_platform!(platform) }
     end
 
     def index_for(dependency)
@@ -307,7 +321,7 @@ module Bundler
 
     def requirement_satisfied_by?(requirement, activated, spec)
       return false unless requirement.matches_spec?(spec) || spec.source.is_a?(Source::Gemspec)
-      spec.activate_platform!(requirement.__platform) || spec.for?(requirement.__platform, nil)
+      spec.activate_platform!(requirement.__platform) || spec.for?(requirement.__platform)
     end
 
     def sort_dependencies(dependencies, activated, conflicts)
diff --git a/lib/bundler/ruby_version.rb b/lib/bundler/ruby_version.rb
index 9321f94c23..8c050c6d31 100644
--- a/lib/bundler/ruby_version.rb
+++ b/lib/bundler/ruby_version.rb
@@ -128,6 +128,11 @@ module Bundler
       end
     end
 
+    def exact?
+      return @exact if defined?(@exact)
+      @exact = versions.all? {|v| Gem::Requirement.create(v).exact? }
+    end
+
   private
 
     def matches?(requirements, version)
diff --git a/lib/bundler/rubygems_ext.rb b/lib/bundler/rubygems_ext.rb
index fc8eadd186..7cd83e631e 100644
--- a/lib/bundler/rubygems_ext.rb
+++ b/lib/bundler/rubygems_ext.rb
@@ -159,6 +159,11 @@ module Gem
     def none?
       @none ||= (to_s == ">= 0")
     end unless allocate.respond_to?(:none?)
+
+    def exact?
+      return false unless @requirements.size == 1
+      @requirements[0][0] == "="
+    end unless allocate.respond_to?(:exact?)
   end
 
   class Platform
diff --git a/spec/install/gems/resolving_spec.rb b/spec/install/gems/resolving_spec.rb
index 816799c0f8..0204a222f9 100644
--- a/spec/install/gems/resolving_spec.rb
+++ b/spec/install/gems/resolving_spec.rb
@@ -119,20 +119,58 @@ describe "bundle install with install-time dependencies" do
     end
 
     context "allows no gems" do
-      it "does not try to install those gems" do
+      before do
         build_repo2 do
           build_gem "require_ruby" do |s|
             s.required_ruby_version = "> 9000"
           end
         end
+      end
 
-        install_gemfile <<-G
-          source "file://#{gem_repo2}"
-          gem 'require_ruby'
-        G
+      let(:ruby_requirement) { %("#{RUBY_VERSION}") }
+      let(:error_message_requirement) { "~> #{RUBY_VERSION}.0" }
+
+      shared_examples_for "ruby version conflicts" do
+        it "raises an error during resolution" do
+          install_gemfile <<-G, :artifice => "compact_index", :env => { "BUNDLER_SPEC_GEM_REPO" => gem_repo2 }
+            source "http://localgemserver.test/"
+            ruby #{ruby_requirement}
+            gem 'require_ruby'
+          G
+
+          expect(out).to_not include("Gem::InstallError: require_ruby requires Ruby version > 9000")
+
+          nice_error = strip_whitespace(<<-E).strip
+            Fetching gem metadata from http://localgemserver.test/.
+            Fetching version metadata from http://localgemserver.test/
+            Resolving dependencies...
+            Bundler could not find compatible versions for gem "ruby\0":
+              In Gemfile:
+                ruby\0 (#{error_message_requirement})
+
+                require_ruby was resolved to 1.0, which depends on
+                  ruby\0 (> 9000)
+
+            Could not find gem 'ruby\0 (> 9000)', which is required by gem 'require_ruby', in any of the sources.
+          E
+          expect(out).to eq(nice_error)
+        end
+      end
+
+      it_behaves_like "ruby version conflicts"
+
+      describe "with a < requirement" do
+        let(:ruby_requirement) { %("< 5000") }
+        let(:error_message_requirement) { "< 5000" }
+
+        it_behaves_like "ruby version conflicts"
+      end
+
+      describe "with a compound requirement" do
+        let(:ruby_requirement) { %("< 5000", "> 0.1") }
+        let(:error_message_requirement) { "< 5000, > 0.1" }
 
-        expect(out).to_not include("Gem::InstallError: require_ruby requires Ruby version > 9000")
-        expect(out).to include("require_ruby-1.0 requires ruby version > 9000, which is incompatible with the current version, #{Bundler::RubyVersion.system}")
+        it_behaves_like "ruby version conflicts"
       end
     end
   end
diff --git a/spec/install/gemspecs_spec.rb b/spec/install/gemspecs_spec.rb
index 3e6021b7e2..8f719bf601 100644
--- a/spec/install/gemspecs_spec.rb
+++ b/spec/install/gemspecs_spec.rb
@@ -50,7 +50,7 @@ describe "bundle install" do
   context "when ruby version is specified in gemspec and gemfile" do
     it "installs when patch level is not specified and the version matches" do
       build_lib("foo", :path => bundled_app) do |s|
-        s.required_ruby_version = RUBY_VERSION
+        s.required_ruby_version = "~> #{RUBY_VERSION}.0"
       end
 
       install_gemfile <<-G
diff --git a/spec/resolver/basic_spec.rb b/spec/resolver/basic_spec.rb
index b7b8b4c3b8..3e8883d1d4 100644
--- a/spec/resolver/basic_spec.rb
+++ b/spec/resolver/basic_spec.rb
@@ -92,15 +92,18 @@ describe "Resolving" do
       gem "bar", "2.0.0" do |s|
         s.required_ruby_version = "~> 2.0.0"
       end
+
+      gem "ruby\0", "1.8.7"
     end
     dep "foo"
+    dep "ruby\0", "1.8.7"
 
     deps = []
     @deps.each do |d|
       deps << Bundler::DepProxy.new(d, "ruby")
     end
 
-    should_resolve_and_include %w(foo-1.0.0 bar-1.0.0), [{}, [], Bundler::RubyVersion.new("1.8.7", nil, nil, nil)]
+    should_resolve_and_include %w(foo-1.0.0 bar-1.0.0), [{}, []]
   end
 
   context "conservative" do
diff --git a/spec/support/indexes.rb b/spec/support/indexes.rb
index 9a7879bc74..29780014fc 100644
--- a/spec/support/indexes.rb
+++ b/spec/support/indexes.rb
@@ -62,7 +62,7 @@ module Spec
         s.level = opts.first
         s.strict = opts.include?(:strict)
       end
-      should_resolve_and_include specs, [{}, @base, nil, search]
+      should_resolve_and_include specs, [{}, @base, search]
     end
 
     def an_awesome_index