Skip to content

Commit

Permalink
Fix $LOADED_FEATURES cache sometimes not respected
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
deivid-rodriguez committed May 23, 2020
1 parent da1492e commit 22ad571
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 29 deletions.
53 changes: 24 additions & 29 deletions lib/rubygems/core_ext/kernel_require.rb
Expand Up @@ -39,46 +39,41 @@ def require(path)

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
Expand Down
29 changes: 29 additions & 0 deletions test/rubygems/test_require.rb
Expand Up @@ -45,6 +45,35 @@ def refute_require(path)
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"
Expand Down

0 comments on commit 22ad571

Please sign in to comment.