Merge #7246

7246: No need to make gem refresh a noop r=indirect a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that rubygems/rubygems#2711 fails because the test in bundler that checks that `Gem.refresh` reveals no system gems fails.

### What was your diagnosis of the problem?

My diagnosis was that we don't really need to make `Gem.refresh` a noop at all, because we already setup a `post_reset` hook to automatically go back to the set of specs bundler knows about, everytime `Gem::Specification.reset` is called. See https://github.com/bundler/bundler/blob/847db56c6b5d2156d27990d73a9949507693985a/lib/bundler/rubygems_integration.rb#L551-L554.

And `Gem::Specification.reset` is what `Gem.refresh` does: https://github.com/rubygems/rubygems/blob/564bb0a5bb310ccceaf6cd6391b3e67e0712edd5/lib/rubygems.rb#L861-L867.

### What is your fix for the problem, implemented in this PR?

My fix is to remove the redefinition of `Gem.refresh`. This makes sense to me not only because it's unnecessary, but because the monkeypatch could still be very easily workarounded by calling `Gem::Specification.reset` directly.

After undoing the `Gem.refresh` redefinition, I changed the test to still make sure that the (not overriden) `Gem.refresh` still does not change the set of specs bundler knows about.

This is a nice simplification, but doesn't really fix the spec for https://github.com/rubygems/rubygems/pull/2711, because after the first call to `Bundler.rubygems.find_name("rack")`, the `@stubs_by_name` array in rubygems gets populated and thus is no longer empty.

We can test the same thing without using `@stubs_by_name` under the hood, so I changed the test to do that, so that the rubygems PR is not affected.

### Why did you choose this fix out of the possible options?

I chose this fix because it simplifies the integration between bundler & rubygems, and it unblocks https://github.com/rubygems/rubygems/pull/2711.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

2 files changed, 4 insertions, 12 deletions

diff --git a/lib/bundler/rubygems_integration.rb b/lib/bundler/rubygems_integration.rb
index 734ac91bd9..15771e6c0a 100644
--- a/lib/bundler/rubygems_integration.rb
+++ b/lib/bundler/rubygems_integration.rb
@@ -441,13 +441,6 @@ module Bundler
       end
     end
 
-    # Because Bundler has a static view of what specs are available,
-    # we don't #refresh, so stub it out.
-    def replace_refresh
-      gem_class = (class << Gem; self; end)
-      redefine_method(gem_class, :refresh) {}
-    end
-
     # Replace or hook into RubyGems to provide a bundlerized view
     # of the world.
     def replace_entrypoints(specs)
@@ -468,7 +461,6 @@ module Bundler
       replace_gem(specs, specs_by_name)
       stub_rubygems(specs)
       replace_bin_path(specs_by_name)
-      replace_refresh
 
       Gem.clear_paths
     end
diff --git a/spec/runtime/setup_spec.rb b/spec/runtime/setup_spec.rb
index 3866bebb44..995a269018 100644
--- a/spec/runtime/setup_spec.rb
+++ b/spec/runtime/setup_spec.rb
@@ -845,7 +845,7 @@ end
     end
   end
 
-  it "stubs out Gem.refresh so it does not reveal system gems" do
+  it "does not reveal system gems even when Gem.refresh is called" do
     system_gems "rack-1.0.0"
 
     install_gemfile <<-G
@@ -854,12 +854,12 @@ end
     G
 
     run <<-R
-      puts Bundler.rubygems.find_name("rack").inspect
+      puts Bundler.rubygems.all_specs.map(&:name)
       Gem.refresh
-      puts Bundler.rubygems.find_name("rack").inspect
+      puts Bundler.rubygems.all_specs.map(&:name)
     R
 
-    expect(out).to eq("[]\n[]")
+    expect(out).to eq("activesupport\nbundler\nactivesupport\nbundler")
   end
 
   describe "when a vendored gem specification uses the :path option" do