Auto merge of #4650 - bundler:aa-required-ruby-versions, r=indirect

Respect required ruby versions

This is an extension of #4645, fixing the version check to actually work, testing that the correct versions get installed as a result, and showing the kind of error message that would actually be useful (as opposed to "couldn't find that gem").

9 files changed, 134 insertions, 108 deletions

diff --git a/Rakefile b/Rakefile
index 05e8e4d96c..10914534da 100644
--- a/Rakefile
+++ b/Rakefile
@@ -4,7 +4,7 @@ $:.unshift File.expand_path("../lib", __FILE__)
 require "shellwords"
 require "benchmark"
 
-RUBYGEMS_REPO = if `cd .. && git remote --verbose` =~ /rubygems/i
+RUBYGEMS_REPO = if `cd .. && git remote --verbose 2>/dev/null` =~ /rubygems/i
   File.expand_path("..")
 else
   File.expand_path("tmp/rubygems")
diff --git a/lib/bundler/fetcher.rb b/lib/bundler/fetcher.rb
index ce9d30c141..8d5a1666ea 100644
--- a/lib/bundler/fetcher.rb
+++ b/lib/bundler/fetcher.rb
@@ -131,10 +131,10 @@ module Bundler
         @use_api = false if fetchers.none?(&:api_fetcher?)
       end
 
-      specs.each do |name, version, platform, dependencies|
+      specs.each do |name, version, platform, dependencies, metadata|
         next if name == "bundler"
         spec = if dependencies
-          EndpointSpecification.new(name, version, platform, dependencies)
+          EndpointSpecification.new(name, version, platform, dependencies, metadata)
         else
           RemoteSpecification.new(name, version, platform, self)
         end
diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb
index e1df04efb1..9d947cc9f9 100644
--- a/lib/bundler/resolver.rb
+++ b/lib/bundler/resolver.rb
@@ -122,14 +122,17 @@ module Bundler
         @source ||= first.source
       end
 
-      def for?(platform, required_ruby_version)
-        if spec = @specs[platform]
-          if required_ruby_version && spec.respond_to?(:required_ruby_version) && spec_required_ruby_version = spec.required_ruby_version
-            spec_required_ruby_version.satisfied_by?(required_ruby_version.to_gem_version_with_patchlevel)
-          else
-            true
-          end
-        end
+      def for?(platform, ruby_version)
+        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)
       end
 
       def to_s
diff --git a/spec/commands/viz_spec.rb b/spec/commands/viz_spec.rb
index 20dc9175d9..ee5736c61d 100644
--- a/spec/commands/viz_spec.rb
+++ b/spec/commands/viz_spec.rb
@@ -19,12 +19,12 @@ describe "bundle viz", :ruby => "1.9.3", :if => Bundler.which("dot") do
   end
 
   it "graphs gems that are prereleases" do
-    update_repo(gem_repo1) do
+    build_repo2 do
       build_gem "rack", "1.3.pre"
     end
 
     install_gemfile <<-G
-      source "file://#{gem_repo1}"
+      source "file://#{gem_repo2}"
       gem "rack", "= 1.3.pre"
       gem "rack-obama"
     G
diff --git a/spec/install/binstubs_spec.rb b/spec/install/binstubs_spec.rb
index 0686b449bb..fed43693c9 100644
--- a/spec/install/binstubs_spec.rb
+++ b/spec/install/binstubs_spec.rb
@@ -23,14 +23,14 @@ describe "bundle install" do
 
   describe "when multiple gems contain the same exe" do
     before do
-      update_repo gem_repo1 do
+      build_repo2 do
         build_gem "fake", "14" do |s|
           s.executables = "rackup"
         end
       end
 
       install_gemfile <<-G, :binstubs => true
-        source "file://#{gem_repo1}"
+        source "file://#{gem_repo2}"
         gem "fake"
         gem "rack"
       G
@@ -44,7 +44,7 @@ describe "bundle install" do
 
     it "loads the correct spec's executable" do
       gembin("rackup")
-      expect(out).to eq("1.0.0")
+      expect(out).to eq("1.2")
     end
   end
 end
diff --git a/spec/install/gems/resolving_spec.rb b/spec/install/gems/resolving_spec.rb
index 74acf9a918..a3ff7394d5 100644
--- a/spec/install/gems/resolving_spec.rb
+++ b/spec/install/gems/resolving_spec.rb
@@ -1,118 +1,137 @@
 # frozen_string_literal: true
 require "spec_helper"
 
-describe "bundle install with gem sources" do
-  describe "install time dependencies" do
-    it "installs gems with implicit rake dependencies" do
+describe "bundle install with install-time dependencies" do
+  it "installs gems with implicit rake dependencies" do
+    install_gemfile <<-G
+      source "file://#{gem_repo1}"
+      gem "with_implicit_rake_dep"
+      gem "another_implicit_rake_dep"
+      gem "rake"
+    G
+
+    run <<-R
+      require 'implicit_rake_dep'
+      require 'another_implicit_rake_dep'
+      puts IMPLICIT_RAKE_DEP
+      puts ANOTHER_IMPLICIT_RAKE_DEP
+    R
+    expect(out).to eq("YES\nYES")
+  end
+
+  it "installs gems with a dependency with no type" do
+    build_repo2
+
+    path = "#{gem_repo2}/#{Gem::MARSHAL_SPEC_DIR}/actionpack-2.3.2.gemspec.rz"
+    spec = Marshal.load(Gem.inflate(File.read(path)))
+    spec.dependencies.each do |d|
+      d.instance_variable_set(:@type, :fail)
+    end
+    File.open(path, "w") do |f|
+      f.write Gem.deflate(Marshal.dump(spec))
+    end
+
+    install_gemfile <<-G
+      source "file://#{gem_repo2}"
+      gem "actionpack", "2.3.2"
+    G
+
+    should_be_installed "actionpack 2.3.2", "activesupport 2.3.2"
+  end
+
+  describe "with crazy rubygem plugin stuff" do
+    it "installs plugins" do
       install_gemfile <<-G
         source "file://#{gem_repo1}"
-        gem "with_implicit_rake_dep"
-        gem "another_implicit_rake_dep"
-        gem "rake"
+        gem "net_b"
       G
 
-      run <<-R
-        require 'implicit_rake_dep'
-        require 'another_implicit_rake_dep'
-        puts IMPLICIT_RAKE_DEP
-        puts ANOTHER_IMPLICIT_RAKE_DEP
-      R
-      expect(out).to eq("YES\nYES")
+      should_be_installed "net_b 1.0"
     end
 
-    it "installs gems with a dependency with no type" do
-      build_repo2
+    it "installs plugins depended on by other plugins" do
+      install_gemfile <<-G
+        source "file://#{gem_repo1}"
+        gem "net_a"
+      G
 
-      path = "#{gem_repo2}/#{Gem::MARSHAL_SPEC_DIR}/actionpack-2.3.2.gemspec.rz"
-      spec = Marshal.load(Gem.inflate(File.read(path)))
-      spec.dependencies.each do |d|
-        d.instance_variable_set(:@type, :fail)
-      end
-      File.open(path, "w") do |f|
-        f.write Gem.deflate(Marshal.dump(spec))
-      end
+      should_be_installed "net_a 1.0", "net_b 1.0"
+    end
 
+    it "installs multiple levels of dependencies" do
       install_gemfile <<-G
-        source "file://#{gem_repo2}"
-        gem "actionpack", "2.3.2"
+        source "file://#{gem_repo1}"
+        gem "net_c"
+        gem "net_e"
       G
 
-      should_be_installed "actionpack 2.3.2", "activesupport 2.3.2"
+      should_be_installed "net_a 1.0", "net_b 1.0", "net_c 1.0", "net_d 1.0", "net_e 1.0"
     end
 
-    describe "with crazy rubygem plugin stuff" do
-      it "installs plugins" do
-        install_gemfile <<-G
+    context "with ENV['DEBUG_RESOLVER'] set" do
+      it "produces debug output" do
+        gemfile <<-G
           source "file://#{gem_repo1}"
-          gem "net_b"
+          gem "net_c"
+          gem "net_e"
         G
 
-        should_be_installed "net_b 1.0"
-      end
-
-      it "installs plugins depended on by other plugins" do
-        install_gemfile <<-G
-          source "file://#{gem_repo1}"
-          gem "net_a"
-        G
+        resolve_output = capture(:stdout) do
+          bundle :install, :env => { "DEBUG_RESOLVER" => "1" }
+        end
 
-        should_be_installed "net_a 1.0", "net_b 1.0"
+        expect(resolve_output).to include("Creating possibility state for net_c")
       end
+    end
 
-      it "installs multiple levels of dependencies" do
-        install_gemfile <<-G
+    context "with ENV['DEBUG_RESOLVER_TREE'] set" do
+      it "produces debug output" do
+        gemfile <<-G
           source "file://#{gem_repo1}"
           gem "net_c"
           gem "net_e"
         G
 
-        should_be_installed "net_a 1.0", "net_b 1.0", "net_c 1.0", "net_d 1.0", "net_e 1.0"
-      end
+        resolve_output = capture(:stdout) do
+          bundle :install, :env => { "DEBUG_RESOLVER_TREE" => "1" }
+        end
 
-      context "with ENV['DEBUG_RESOLVER'] set" do
-        it "produces debug output" do
-          gemfile <<-G
-            source "file://#{gem_repo1}"
-            gem "net_c"
-            gem "net_e"
-          G
+        expect(resolve_output).to include(" net_b")
+        expect(resolve_output).to include(" net_build_extensions (1.0)")
+      end
+    end
+  end
 
-          resolve_output = capture(:stdout) do
-            bundle :install, :env => { "DEBUG_RESOLVER" => "1" }
+  describe "when a required ruby version" do
+    context "allows only an older version" do
+      it "installs the older version" do
+        build_repo2 do
+          build_gem "rack", "9001.0.0" do |s|
+            s.required_ruby_version = "> 9000"
           end
-
-          expect(resolve_output).to include("Creating possibility state for net_c")
         end
-      end
 
-      context "with ENV['DEBUG_RESOLVER_TREE'] set" do
-        it "produces debug output" do
-          gemfile <<-G
-            source "file://#{gem_repo1}"
-            gem "net_c"
-            gem "net_e"
-          G
-
-          resolve_output = capture(:stdout) do
-            bundle :install, :env => { "DEBUG_RESOLVER_TREE" => "1" }
-          end
+        install_gemfile <<-G, :artifice => "compact_index", :env => { "BUNDLER_SPEC_GEM_REPO" => gem_repo2 }
+          ruby "#{RUBY_VERSION}"
+          source "http://localgemserver.test/"
+          gem 'rack'
+        G
 
-          expect(resolve_output).to include(" net_b")
-          expect(resolve_output).to include(" net_build_extensions (1.0)")
-        end
+        expect(out).to_not include("rack-9001.0.0 requires ruby version > 9000")
+        should_be_installed("rack 1.2")
       end
     end
 
-    describe "when some gems require a different version of ruby" do
+    context "allows no gems" do
       it "does not try to install those gems" do
-        update_repo gem_repo1 do
+        build_repo2 do
           build_gem "require_ruby" do |s|
             s.required_ruby_version = "> 9000"
           end
         end
 
-        install_gemfile <<-G, :artifice => "compact_index"
-          source "file://#{gem_repo1}"
+        install_gemfile <<-G
+          source "file://#{gem_repo2}"
           gem 'require_ruby'
         G
 
@@ -120,23 +139,23 @@ describe "bundle install with gem sources" do
         expect(out).to include("require_ruby-1.0 requires ruby version > 9000, which is incompatible with the current version, #{Bundler::RubyVersion.system}")
       end
     end
+  end
 
-    describe "when some gems require a different version of rubygems" do
-      it "does not try to install those gems" do
-        update_repo gem_repo1 do
-          build_gem "require_rubygems" do |s|
-            s.required_rubygems_version = "> 9000"
-          end
+  describe "when a required rubygems version disallows a gem" do
+    it "does not try to install those gems" do
+      build_repo2 do
+        build_gem "require_rubygems" do |s|
+          s.required_rubygems_version = "> 9000"
         end
+      end
 
-        install_gemfile <<-G, :artifice => "compact_index"
-          source "file://#{gem_repo1}"
-          gem 'require_rubygems'
-        G
+      install_gemfile <<-G
+        source "file://#{gem_repo2}"
+        gem 'require_rubygems'
+      G
 
-        expect(out).to_not include("Gem::InstallError: require_rubygems requires RubyGems version > 9000")
-        expect(out).to include("require_rubygems-1.0 requires rubygems version > 9000, which is incompatible with the current version, #{Gem::VERSION}")
-      end
+      expect(out).to_not include("Gem::InstallError: require_rubygems requires RubyGems version > 9000")
+      expect(out).to include("require_rubygems-1.0 requires rubygems version > 9000, which is incompatible with the current version, #{Gem::VERSION}")
     end
   end
 end
diff --git a/spec/support/artifice/compact_index.rb b/spec/support/artifice/compact_index.rb
index a9fe7112b0..233c192a67 100644
--- a/spec/support/artifice/compact_index.rb
+++ b/spec/support/artifice/compact_index.rb
@@ -63,7 +63,7 @@ class CompactIndexAPI < Endpoint
       end
     end
 
-    def gems(gem_repo = gem_repo1)
+    def gems(gem_repo = GEM_REPO)
       @gems ||= {}
       @gems[gem_repo] ||= begin
         specs = Bundler::Deprecate.skip_during do
diff --git a/spec/support/artifice/endpoint.rb b/spec/support/artifice/endpoint.rb
index b0e9bd0f26..2955889a86 100644
--- a/spec/support/artifice/endpoint.rb
+++ b/spec/support/artifice/endpoint.rb
@@ -13,11 +13,12 @@ require "artifice"
 require "sinatra/base"
 
 class Endpoint < Sinatra::Base
+  GEM_REPO = Pathname.new(ENV["BUNDLER_SPEC_GEM_REPO"] || Spec::Path.gem_repo1)
   set :raise_errors, true
   set :show_exceptions, false
 
   helpers do
-    def dependencies_for(gem_names, gem_repo = gem_repo1)
+    def dependencies_for(gem_names, gem_repo = GEM_REPO)
       return [] if gem_names.nil? || gem_names.empty?
 
       require "rubygems"
@@ -50,11 +51,11 @@ class Endpoint < Sinatra::Base
   end
 
   get "/fetch/actual/gem/:id" do
-    File.read("#{gem_repo1}/quick/Marshal.4.8/#{params[:id]}")
+    File.read("#{GEM_REPO}/quick/Marshal.4.8/#{params[:id]}")
   end
 
   get "/gems/:id" do
-    File.read("#{gem_repo1}/gems/#{params[:id]}")
+    File.read("#{GEM_REPO}/gems/#{params[:id]}")
   end
 
   get "/api/v1/dependencies" do
@@ -62,11 +63,11 @@ class Endpoint < Sinatra::Base
   end
 
   get "/specs.4.8.gz" do
-    File.read("#{gem_repo1}/specs.4.8.gz")
+    File.read("#{GEM_REPO}/specs.4.8.gz")
   end
 
   get "/prerelease_specs.4.8.gz" do
-    File.read("#{gem_repo1}/prerelease_specs.4.8.gz")
+    File.read("#{GEM_REPO}/prerelease_specs.4.8.gz")
   end
 end
 
diff --git a/spec/support/builders.rb b/spec/support/builders.rb
index d091ff69a9..00024db7cf 100644
--- a/spec/support/builders.rb
+++ b/spec/support/builders.rb
@@ -368,6 +368,9 @@ module Spec
     end
 
     def update_repo(path)
+      if path == gem_repo1 && caller.first.split(" ").last == "`build_repo`"
+        raise "Updating gem_repo1 is unsupported -- use gem_repo2 instead"
+      end
       return unless block_given?
       @_build_path = "#{path}/gems"
       yield