Merge pull request #7673 from rubygems/deivid-rodriguez/treat-default…

…-gems-as-regular-gems

Improve default gem handling by treating default gems as any other gem

bundler/lib/bundler/installer/gem_installer.rb

@@ -54,7 +54,6 @@ def install
       spec.source.install(
         spec,
         force: force,
-        ensure_builtin_gems_cached: standalone,
         build_args: Array(spec_settings),
         previous_spec: previous_spec,
       )

bundler/lib/bundler/rubygems_integration.rb

@@ -481,11 +481,25 @@ def path_separator
     end
 
     def all_specs
+      SharedHelpers.major_deprecation 2, "Bundler.rubygems.all_specs has been removed in favor of Bundler.rubygems.installed_specs"
+
       Gem::Specification.stubs.map do |stub|
         StubSpecification.from_stub(stub)
       end
     end
 
+    def installed_specs
+      Gem::Specification.stubs.reject(&:default_gem?).map do |stub|
+        StubSpecification.from_stub(stub)
+      end
+    end
+
+    def default_specs
+      Gem::Specification.default_stubs.map do |stub|
+        StubSpecification.from_stub(stub)
+      end
+    end
+
     def find_bundler(version)
       find_name("bundler").find {|s| s.version.to_s == version }
     end

bundler/lib/bundler/source/rubygems.rb

@@ -136,20 +136,17 @@ def specs
           index = @allow_remote ? remote_specs.dup : Index.new
           index.merge!(cached_specs) if @allow_cached
           index.merge!(installed_specs) if @allow_local
+
+          # complete with default specs, only if not already available in the
+          # index through remote, cached, or installed specs
+          index.use(default_specs) if @allow_local
+
           index
         end
       end
 
       def install(spec, options = {})
-        force = options[:force]
-        ensure_builtin_gems_cached = options[:ensure_builtin_gems_cached]
-
-        if ensure_builtin_gems_cached && spec.default_gem? && !cached_path(spec)
-          cached_built_in_gem(spec) unless spec.remote
-          force = true
-        end
-
-        if installed?(spec) && !force
+        if (spec.default_gem? && !cached_built_in_gem(spec)) || (installed?(spec) && !options[:force])
           print_using_message "Using #{version_message(spec, options[:previous_spec])}"
           return nil # no post-install message
         end
@@ -362,7 +359,7 @@ def remove_auth(remote)
 
       def installed_specs
         @installed_specs ||= Index.build do |idx|
-          Bundler.rubygems.all_specs.reverse_each do |spec|
+          Bundler.rubygems.installed_specs.reverse_each do |spec|
             spec.source = self
             if Bundler.rubygems.spec_missing_extensions?(spec, false)
               Bundler.ui.debug "Source #{self} is ignoring #{spec} because it is missing extensions"
@@ -373,6 +370,15 @@ def installed_specs
         end
       end
 
+      def default_specs
+        @default_specs ||= Index.build do |idx|
+          Bundler.rubygems.default_specs.each do |spec|
+            spec.source = self
+            idx << spec
+          end
+        end
+      end
+
       def cached_specs
         @cached_specs ||= begin
           idx = Index.new

bundler/spec/bundler/installer/gem_installer_spec.rb

@@ -14,7 +14,7 @@
     it "invokes install method with empty build_args" do
       allow(spec_source).to receive(:install).with(
         spec,
-        { force: false, ensure_builtin_gems_cached: false, build_args: [], previous_spec: nil }
+        { force: false, build_args: [], previous_spec: nil }
       )
       subject.install_from_spec
     end
@@ -28,7 +28,7 @@
       allow(Bundler.settings).to receive(:[]).with("build.dummy").and_return("--with-dummy-config=dummy")
       expect(spec_source).to receive(:install).with(
         spec,
-        { force: false, ensure_builtin_gems_cached: false, build_args: ["--with-dummy-config=dummy"], previous_spec: nil }
+        { force: false, build_args: ["--with-dummy-config=dummy"], previous_spec: nil }
       )
       subject.install_from_spec
     end
@@ -42,7 +42,7 @@
       allow(Bundler.settings).to receive(:[]).with("build.dummy").and_return("--with-dummy-config=dummy --with-another-dummy-config")
       expect(spec_source).to receive(:install).with(
         spec,
-        { force: false, ensure_builtin_gems_cached: false, build_args: ["--with-dummy-config=dummy", "--with-another-dummy-config"], previous_spec: nil }
+        { force: false, build_args: ["--with-dummy-config=dummy", "--with-another-dummy-config"], previous_spec: nil }
       )
       subject.install_from_spec
     end

bundler/spec/cache/gems_spec.rb

@@ -93,68 +93,80 @@
     let(:default_json_version) { ruby "gem 'json'; require 'json'; puts JSON::VERSION" }
 
     before :each do
-      build_repo2 do
-        build_gem "json", default_json_version
-      end
-
       build_gem "json", default_json_version, to_system: true, default: true
     end
 
-    it "uses builtin gems when installing to system gems" do
-      bundle "config set path.system true"
-      install_gemfile %(source "#{file_uri_for(gem_repo1)}"; gem 'json', '#{default_json_version}'), verbose: true
-      expect(out).to include("Using json #{default_json_version}")
-    end
+    context "when a remote gem is available for caching" do
+      before do
+        build_repo2 do
+          build_gem "json", default_json_version
+        end
+      end
 
-    it "caches remote and builtin gems" do
-      install_gemfile <<-G
-        source "#{file_uri_for(gem_repo2)}"
-        gem 'json', '#{default_json_version}'
-        gem 'rack', '1.0.0'
-      G
+      it "uses remote gems when installing to system gems" do
+        bundle "config set path.system true"
+        install_gemfile %(source "#{file_uri_for(gem_repo2)}"; gem 'json', '#{default_json_version}'), verbose: true
+        expect(out).to include("Installing json #{default_json_version}")
+      end
 
-      bundle :cache
-      expect(bundled_app("vendor/cache/rack-1.0.0.gem")).to exist
-      expect(bundled_app("vendor/cache/json-#{default_json_version}.gem")).to exist
-    end
+      it "caches remote and builtin gems" do
+        install_gemfile <<-G
+          source "#{file_uri_for(gem_repo2)}"
+          gem 'json', '#{default_json_version}'
+          gem 'rack', '1.0.0'
+        G
 
-    it "caches builtin gems when cache_all_platforms is set" do
-      gemfile <<-G
-        source "#{file_uri_for(gem_repo2)}"
-        gem "json"
-      G
+        bundle :cache
+        expect(bundled_app("vendor/cache/rack-1.0.0.gem")).to exist
+        expect(bundled_app("vendor/cache/json-#{default_json_version}.gem")).to exist
+      end
 
-      bundle "config set cache_all_platforms true"
+      it "caches builtin gems when cache_all_platforms is set" do
+        gemfile <<-G
+          source "#{file_uri_for(gem_repo2)}"
+          gem "json"
+        G
 
-      bundle :cache
-      expect(bundled_app("vendor/cache/json-#{default_json_version}.gem")).to exist
-    end
+        bundle "config set cache_all_platforms true"
 
-    it "doesn't make remote request after caching the gem" do
-      build_gem "builtin_gem_2", "1.0.2", path: bundled_app("vendor/cache") do |s|
-        s.summary = "This builtin_gem is bundled with Ruby"
+        bundle :cache
+        expect(bundled_app("vendor/cache/json-#{default_json_version}.gem")).to exist
       end
 
-      install_gemfile <<-G
-        source "#{file_uri_for(gem_repo2)}"
-        gem 'builtin_gem_2', '1.0.2'
-      G
+      it "doesn't make remote request after caching the gem" do
+        build_gem "builtin_gem_2", "1.0.2", path: bundled_app("vendor/cache") do |s|
+          s.summary = "This builtin_gem is bundled with Ruby"
+        end
 
-      bundle "install --local"
-      expect(the_bundle).to include_gems("builtin_gem_2 1.0.2")
+        install_gemfile <<-G
+          source "#{file_uri_for(gem_repo2)}"
+          gem 'builtin_gem_2', '1.0.2'
+        G
+
+        bundle "install --local"
+        expect(the_bundle).to include_gems("builtin_gem_2 1.0.2")
+      end
     end
 
-    it "errors if the builtin gem isn't available to cache" do
-      bundle "config set path.system true"
+    context "when a remote gem is not available for caching" do
+      it "uses builtin gems when installing to system gems" do
+        bundle "config set path.system true"
+        install_gemfile %(source "#{file_uri_for(gem_repo1)}"; gem 'json', '#{default_json_version}'), verbose: true
+        expect(out).to include("Using json #{default_json_version}")
+      end
 
-      install_gemfile <<-G
-        source "#{file_uri_for(gem_repo1)}"
-        gem 'json', '#{default_json_version}'
-      G
+      it "errors when explicitly caching" do
+        bundle "config set path.system true"
 
-      bundle :cache, raise_on_error: false
-      expect(exitstatus).to_not eq(0)
-      expect(err).to include("json-#{default_json_version} is built in to Ruby, and can't be cached")
+        install_gemfile <<-G
+          source "#{file_uri_for(gem_repo1)}"
+          gem 'json', '#{default_json_version}'
+        G
+
+        bundle :cache, raise_on_error: false
+        expect(exitstatus).to_not eq(0)
+        expect(err).to include("json-#{default_json_version} is built in to Ruby, and can't be cached")
+      end
     end
   end
 

bundler/spec/commands/open_spec.rb

@@ -164,7 +164,6 @@
 
       install_gemfile <<-G
         source "#{file_uri_for(gem_repo1)}"
-        gem "json"
       G
     end
 

bundler/spec/runtime/setup_spec.rb

@@ -942,9 +942,9 @@ def clean_load_path(lp)
     G
 
     run <<-R
-      puts Bundler.rubygems.all_specs.map(&:name)
+      puts Bundler.rubygems.installed_specs.map(&:name)
       Gem.refresh
-      puts Bundler.rubygems.all_specs.map(&:name)
+      puts Bundler.rubygems.installed_specs.map(&:name)
     R
 
     expect(out).to eq("activesupport\nbundler\nactivesupport\nbundler")
@@ -1376,6 +1376,24 @@ def lock_with(ruby_version = nil)
       expect(out).to eq("undefined\nconstant")
     end
 
+    it "activates default gems when they are part of the bundle, but not installed explicitly", :ruby_repo do
+      default_json_version = ruby "gem 'json'; require 'json'; puts JSON::VERSION"
+
+      build_repo2 do
+        build_gem "json", default_json_version
+      end
+
+      gemfile "source \"#{file_uri_for(gem_repo2)}\"; gem 'json'"
+
+      ruby <<-RUBY
+        require "bundler/setup"
+        require "json"
+        puts defined?(::JSON) ? "JSON defined" : "JSON undefined"
+      RUBY
+
+      expect(err).to be_empty
+    end
+
     describe "default gem activation" do
       let(:exemptions) do
         exempts = %w[did_you_mean bundler uri pathname]