[rubygems/rubygems] Fix `$LOADED_FEATURES` cache sometimes not respected

In the cases where the initial manually `-I` path resolution succeeded,
we were passing a full path to the original require effectively skipping
the `$LOADED_FEATURES` cache. With this change, we _only_ do the
resolution when a matching requirable path is found in a default gem. In
that case, we skip activation of the default gem if we detect that the
required file will be picked up for a `-I` path.

https://github.com/rubygems/rubygems/commit/22ad5717c3

2 files changed, 53 insertions, 29 deletions

diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
index 6a7faaf2d1..81e37b98bf 100644
--- a/lib/rubygems/core_ext/kernel_require.rb
+++ b/lib/rubygems/core_ext/kernel_require.rb
@@ -39,46 +39,41 @@ module Kernel
 
     path = path.to_path if path.respond_to? :to_path
 
-    # Ensure -I beats a default gem
-    # https://github.com/rubygems/rubygems/pull/1868
-    resolved_path = begin
-      rp = nil
-      load_path_check_index = Gem.load_path_insert_index - Gem.activated_gem_paths
-      Gem.suffixes.each do |s|
-        $LOAD_PATH[0...load_path_check_index].each do |lp|
-          safe_lp = lp.dup.tap(&Gem::UNTAINT)
-          begin
-            if File.symlink? safe_lp # for backward compatibility
-              next
+    if spec = Gem.find_unresolved_default_spec(path)
+      # Ensure -I beats a default gem
+      # https://github.com/rubygems/rubygems/pull/1868
+      resolved_path = begin
+        rp = nil
+        load_path_check_index = Gem.load_path_insert_index - Gem.activated_gem_paths
+        Gem.suffixes.each do |s|
+          $LOAD_PATH[0...load_path_check_index].each do |lp|
+            safe_lp = lp.dup.tap(&Gem::UNTAINT)
+            begin
+              if File.symlink? safe_lp # for backward compatibility
+                next
+              end
+            rescue SecurityError
+              RUBYGEMS_ACTIVATION_MONITOR.exit
+              raise
             end
-          rescue SecurityError
-            RUBYGEMS_ACTIVATION_MONITOR.exit
-            raise
-          end
 
-          full_path = File.expand_path(File.join(safe_lp, "#{path}#{s}"))
-          if File.file?(full_path)
-            rp = full_path
-            break
+            full_path = File.expand_path(File.join(safe_lp, "#{path}#{s}"))
+            if File.file?(full_path)
+              rp = full_path
+              break
+            end
           end
+          break if rp
         end
-        break if rp
+        rp
       end
-      rp
-    end
 
-    if resolved_path
-      RUBYGEMS_ACTIVATION_MONITOR.exit
-      return gem_original_require(resolved_path)
-    end
-
-    if spec = Gem.find_unresolved_default_spec(path)
       begin
         Kernel.send(:gem, spec.name, Gem::Requirement.default_prerelease)
       rescue Exception
         RUBYGEMS_ACTIVATION_MONITOR.exit
         raise
-      end
+      end unless resolved_path
     end
 
     # If there are no unresolved deps, then we can use just try
diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb
index c6be26a658..af95320ce3 100644
--- a/test/rubygems/test_require.rb
+++ b/test/rubygems/test_require.rb
@@ -45,6 +45,35 @@ class TestGemRequire < Gem::TestCase
     refute require(path), "'#{path}' was not yet required"
   end
 
+  def test_respect_loaded_features_caching_like_standard_require
+    dir = Dir.mktmpdir("test_require", @tempdir)
+
+    lp1 = File.join dir, 'foo1'
+    foo1 = File.join lp1, 'foo.rb'
+
+    FileUtils.mkdir_p lp1
+    File.open(foo1, 'w') { |f| f.write "class Object; HELLO = 'foo1' end" }
+
+    lp = $LOAD_PATH.dup
+
+    $LOAD_PATH.unshift lp1
+    assert_require 'foo'
+    assert_equal "foo1", ::Object::HELLO
+
+    lp2 = File.join dir, 'foo2'
+    foo2 = File.join lp2, 'foo.rb'
+
+    FileUtils.mkdir_p lp2
+    File.open(foo2, 'w') { |f| f.write "class Object; HELLO = 'foo2' end" }
+
+    $LOAD_PATH.unshift lp2
+    refute_require 'foo'
+    assert_equal "foo1", ::Object::HELLO
+  ensure
+    $LOAD_PATH.replace lp
+    Object.send :remove_const, :HELLO if Object.const_defined? :HELLO
+  end
+
   # Providing -I on the commandline should always beat gems
   def test_dash_i_beats_gems
     a1 = util_spec "a", "1", {"b" => "= 1"}, "lib/test_gem_require_a.rb"