Add resolver-style error messages for required Ruby conflicts

[indirect]

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.

[segiddins]

Depending on how high a priority this is, I can try and get to it by the start of next week?

[segiddins]

diff --git a/lib/bundler/dep_proxy.rb b/lib/bundler/dep_proxy.rb
index 998975b..05bcd38 100644
--- a/lib/bundler/dep_proxy.rb
+++ b/lib/bundler/dep_proxy.rb
@@ -13,6 +13,7 @@ module Bundler
     end

     def ==(other)
+      return dep == other if other.is_a?(Gem::Dependency)
       dep == other.dep && __platform == other.__platform
     end

diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb
index 918b3c9..d9bafca 100644
--- a/lib/bundler/resolver.rb
+++ b/lib/bundler/resolver.rb
@@ -140,6 +140,10 @@ module Bundler
         @activated.map {|p| __dependencies[p] }.flatten
       end

+      def ruby_version_dependencies_for_activated_platforms
+        @activated.map {|p| Gem::Dependency.new(Gem::Platform::RUBY, @specs[p].required_ruby_version) }.compact
+      end
+
       def platforms_for_dependency_named(dependency)
         __dependencies.select {|_, deps| deps.map(&:name).include? dependency }.keys
       end
@@ -175,6 +179,7 @@ module Bundler
     def self.resolve(requirements, index, source_requirements = {}, base = [], ruby_version = nil)
       base = SpecSet.new(base) unless base.is_a?(SpecSet)
       resolver = new(index, source_requirements, base, ruby_version)
+      requirements += [Gem::Dependency.new("ruby", @ruby_version.gem_version)] if @ruby_version
       result = resolver.start(requirements)
       SpecSet.new(result)
     end
@@ -239,10 +244,21 @@ module Bundler
     include Molinillo::SpecificationProvider

     def dependencies_for(specification)
-      specification.dependencies_for_activated_platforms
+      deps = specification.dependencies_for_activated_platforms
+      deps.concat(specification.ruby_version_dependencies_for_activated_platforms)
+      deps
+    end
+
+    def ruby_specs_for(dependency)
+      return [] unless dependency.requirement.satisfied_by?(@ruby_version.gem_version)
+      [SpecGroup.new([Gem::Specification.new do |s|
+        s.name = dependency.name
+        s.version = @ruby_version.gem_version
+      end])]
     end

     def search_for(dependency)
+      return ruby_specs_for(dependency) if dependency.name == Gem::Platform::RUBY
       platform = dependency.__platform
       dependency = dependency.dep unless dependency.is_a? Gem::Dependency
       search = @search_for[dependency] ||= begin

[indirect]

lol this isn't the start of next week! I like the idea, and would like to ship it in 1.13 if we can finish it in, say, less than 10-15 hours of work?

[segiddins]

@indirect in your opinion, should we resolve for rubygems versions in addition to ruby versions?

[indirect]

If we have it, I guess so? it seems much less critical, since (iirc) almost no gems in existence have required rubygems versions.

[indirect]

So, having said that, let's punt on required rubygems version and wait to see if it's needed in real life before we do the work of implementing it, testing it, and resigning ourselves to supporting it. We have the data in the index if we need it.

[homu]

☔ The latest upstream changes (presumably #4650) made this pull request unmergeable. Please resolve the merge conflicts.

[indirect]

@segiddins rebase? I'd still like to ship this.

[segiddins]

This requires more than just a rebase, it requires someone to actually implement it. So far, the branch is just your test case, and my very hacky patch is merely a comment :P

[segiddins]

@indirect this is awful. please talk me out of this approach.

[segiddins]

This might work now?

[segiddins]

Hopefully green now

[segiddins]

@indirect this is green on travis, please review?

[indirect]

I think the RubyVersion object information isn't helpful to end users reading this message—maybe something like

The Ruby version #{@locked_ruby_version} from #{@lockfile} could not be parsed. Try running bundle update --ruby to resolve this.

[segiddins]

👍 looks way better. in reality this exception should never be hit and I think I copied it from above

[indirect]

I am super impressed that you managed to make the existing infrastructure do so much of the work for this. 💯 r+ once you feel the comments have been addressed.

[segiddins]

@indirect r?

[indirect]

@homu r+

[homu]

📌 Commit 2b3e175 has been approved by indirect

[homu]

⌛ Testing commit 2b3e175 with merge 8513ed1...

[homu]

☀️ Test successful - status