ruby-changes:61542
From: David <ko1@a...>
Date: Fri, 5 Jun 2020 07:34:23 +0900 (JST)
Subject: [ruby-changes:61542] f0f138aa5d (master): [rubygems/rubygems] Fix `$LOADED_FEATURES` cache sometimes not respected
https://git.ruby-lang.org/ruby.git/commit/?id=f0f138aa5d From f0f138aa5d5857fb435bfaef95201d530f6d35da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@r...> Date: Thu, 21 May 2020 15:20:57 +0200 Subject: [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 diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb index 6a7faaf..81e37b9 100644 --- a/lib/rubygems/core_ext/kernel_require.rb +++ b/lib/rubygems/core_ext/kernel_require.rb @@ -39,46 +39,41 @@ module Kernel https://github.com/ruby/ruby/blob/trunk/lib/rubygems/core_ext/kernel_require.rb#L39 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 c6be26a..af95320 100644 --- a/test/rubygems/test_require.rb +++ b/test/rubygems/test_require.rb @@ -45,6 +45,35 @@ class TestGemRequire < Gem::TestCase https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_require.rb#L45 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" -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/