Fix require behavior allowing to load libraries multiple times.
				
					
				
			Resolves: rhbz#1842989
This commit is contained in:
		
							parent
							
								
									9b65bf6795
								
							
						
					
					
						commit
						e7b80e8e7c
					
				
							
								
								
									
										20
									
								
								ruby.spec
									
									
									
									
									
								
							
							
						
						
									
										20
									
								
								ruby.spec
									
									
									
									
									
								
							| @ -154,6 +154,18 @@ Patch12: ruby-2.8.0-Moved-not-implemented-method-tests.patch | |||||||
| # Prevent issues with openssl loading when RubyGems are disabled. | # Prevent issues with openssl loading when RubyGems are disabled. | ||||||
| # https://github.com/ruby/openssl/pull/242 | # https://github.com/ruby/openssl/pull/242 | ||||||
| Patch13: ruby-2.8.0-remove-unneeded-gem-require-for-ipaddr.patch | Patch13: ruby-2.8.0-remove-unneeded-gem-require-for-ipaddr.patch | ||||||
|  | # Fix `require` behavior allowing to load libraries multiple times. | ||||||
|  | # https://github.com/rubygems/rubygems/issues/3647 | ||||||
|  | # Because there were multiple fixes in `Kernel.require` in recent months, | ||||||
|  | # pickup all the changes one by one instead of squashing them. | ||||||
|  | # https://github.com/rubygems/rubygems/pull/3124 | ||||||
|  | Patch15: rubygems-3.1.3-Fix-I-require-priority.patch | ||||||
|  | # https://github.com/rubygems/rubygems/pull/3133 | ||||||
|  | Patch16: rubygems-3.1.3-Improve-require.patch | ||||||
|  | # https://github.com/rubygems/rubygems/pull/3153 | ||||||
|  | Patch17: rubygems-3.1.3-Revert-Exclude-empty-suffix-from-I-require-loop.patch | ||||||
|  | # https://github.com/rubygems/rubygems/pull/3639 | ||||||
|  | Patch18: rubygems-3.1.3-Fix-correctness-and-performance-regression-in-require.patch | ||||||
| 
 | 
 | ||||||
| # Add support for .include directive used by OpenSSL config files. | # Add support for .include directive used by OpenSSL config files. | ||||||
| # https://github.com/ruby/openssl/pull/216 | # https://github.com/ruby/openssl/pull/216 | ||||||
| @ -567,6 +579,10 @@ rm -rf ext/fiddle/libffi* | |||||||
| %patch11 -p1 | %patch11 -p1 | ||||||
| %patch12 -p1 | %patch12 -p1 | ||||||
| %patch13 -p1 | %patch13 -p1 | ||||||
|  | %patch15 -p1 | ||||||
|  | %patch16 -p1 | ||||||
|  | %patch17 -p1 | ||||||
|  | %patch18 -p1 | ||||||
| %patch22 -p1 | %patch22 -p1 | ||||||
| 
 | 
 | ||||||
| # Provide an example of usage of the tapset: | # Provide an example of usage of the tapset: | ||||||
| @ -1289,6 +1305,10 @@ make check TESTS="-v $DISABLE_TESTS" MSPECOPT="-fs $MSPECOPTS" | |||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| %changelog | %changelog | ||||||
|  | * Fri Jun 26 2020 Vít Ondruch <vondruch@redhat.com> - 2.7.1-133 | ||||||
|  | - Fix `require` behavior allowing to load libraries multiple times. | ||||||
|  |   Resolves: rhbz#1842989 | ||||||
|  | 
 | ||||||
| * Fri Jun 26 2020 Jun Aruga <jaruga@redhat.com> - 2.7.1-133 | * Fri Jun 26 2020 Jun Aruga <jaruga@redhat.com> - 2.7.1-133 | ||||||
| - Ship racc binary. | - Ship racc binary. | ||||||
|   Resolves: rhbz#1851388 |   Resolves: rhbz#1851388 | ||||||
|  | |||||||
							
								
								
									
										167
									
								
								rubygems-3.1.3-Fix-I-require-priority.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										167
									
								
								rubygems-3.1.3-Fix-I-require-priority.patch
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,167 @@ | |||||||
|  | From 912d141a351053d0f6d915b5e7807f6a8f4c0631 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Tue, 4 Feb 2020 17:25:56 +0100 | ||||||
|  | Subject: [PATCH 1/2] Make non "test_" method private | ||||||
|  | 
 | ||||||
|  | ---
 | ||||||
|  |  test/rubygems/test_require.rb | 2 ++ | ||||||
|  |  1 file changed, 2 insertions(+) | ||||||
|  | 
 | ||||||
|  | diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb
 | ||||||
|  | index aa2675af5d..d618a93473 100644
 | ||||||
|  | --- a/test/rubygems/test_require.rb
 | ||||||
|  | +++ b/test/rubygems/test_require.rb
 | ||||||
|  | @@ -532,6 +532,8 @@ def test_require_bundler_with_bundler_version
 | ||||||
|  |      end | ||||||
|  |    end | ||||||
|  |   | ||||||
|  | +  private
 | ||||||
|  | +
 | ||||||
|  |    def silence_warnings | ||||||
|  |      old_verbose, $VERBOSE = $VERBOSE, false | ||||||
|  |      yield | ||||||
|  | 
 | ||||||
|  | From b3944384f44b869985051863d8b05b545d09a585 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Tue, 4 Feb 2020 17:26:28 +0100 | ||||||
|  | Subject: [PATCH 2/2] Fix require issue with file extension priority | ||||||
|  | 
 | ||||||
|  | If `require "a"` is run when two folders have been specified in the -I | ||||||
|  | option including a "a.rb" file and a "a.so" file respectively, the ruby | ||||||
|  | spec says that the ".rb" file should always be preferred. However, the | ||||||
|  | logic we added in https://github.com/rubygems/rubygems/commit/6b81076d9 | ||||||
|  | to make the -I option always beat default gems does not respect this | ||||||
|  | spec, creating a difference from the original ruby-core's require. | ||||||
|  | 
 | ||||||
|  | [the ruby spec says]: https://github.com/ruby/spec/blob/d80a6e2b221d4f17a8cadcac75ef950c59cba901/core/kernel/shared/require.rb#L234-L246 | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb | 28 +++++------ | ||||||
|  |  test/rubygems/test_require.rb           | 62 +++++++++++++++++++++++++ | ||||||
|  |  2 files changed, 74 insertions(+), 16 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | index 60f4d18712..369f2c743e 100644
 | ||||||
|  | --- a/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | +++ b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | @@ -43,18 +43,18 @@ def require(path)
 | ||||||
|  |      # https://github.com/rubygems/rubygems/pull/1868 | ||||||
|  |      resolved_path = begin | ||||||
|  |        rp = nil | ||||||
|  | -      $LOAD_PATH[0...Gem.load_path_insert_index || -1].each do |lp|
 | ||||||
|  | -        safe_lp = lp.dup.tap(&Gem::UNTAINT)
 | ||||||
|  | -        begin
 | ||||||
|  | -          if File.symlink? safe_lp # for backward compatibility
 | ||||||
|  | -            next
 | ||||||
|  | +      Gem.suffixes.each do |s|
 | ||||||
|  | +        $LOAD_PATH[0...Gem.load_path_insert_index || -1].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
 | ||||||
|  |   | ||||||
|  | -        Gem.suffixes.each do |s|
 | ||||||
|  |            full_path = File.expand_path(File.join(safe_lp, "#{path}#{s}")) | ||||||
|  |            if File.file?(full_path) | ||||||
|  |              rp = full_path | ||||||
|  | @@ -67,12 +67,8 @@ def require(path)
 | ||||||
|  |      end | ||||||
|  |   | ||||||
|  |      if resolved_path | ||||||
|  | -      begin
 | ||||||
|  | -        RUBYGEMS_ACTIVATION_MONITOR.exit
 | ||||||
|  | -        return gem_original_require(resolved_path)
 | ||||||
|  | -      rescue LoadError
 | ||||||
|  | -        RUBYGEMS_ACTIVATION_MONITOR.enter
 | ||||||
|  | -      end
 | ||||||
|  | +      RUBYGEMS_ACTIVATION_MONITOR.exit
 | ||||||
|  | +      return gem_original_require(resolved_path)
 | ||||||
|  |      end | ||||||
|  |   | ||||||
|  |      if spec = Gem.find_unresolved_default_spec(path) | ||||||
|  | diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb
 | ||||||
|  | index d618a93473..7cffbfa7fe 100644
 | ||||||
|  | --- a/test/rubygems/test_require.rb
 | ||||||
|  | +++ b/test/rubygems/test_require.rb
 | ||||||
|  | @@ -120,6 +120,24 @@ def test_dash_i_beats_default_gems
 | ||||||
|  |      Object.send :remove_const, :HELLO if Object.const_defined? :HELLO | ||||||
|  |    end | ||||||
|  |   | ||||||
|  | +  def test_dash_i_respects_default_library_extension_priority
 | ||||||
|  | +    skip "extensions don't quite work on jruby" if Gem.java_platform?
 | ||||||
|  | +
 | ||||||
|  | +    dash_i_ext_arg = util_install_extension_file('a')
 | ||||||
|  | +    dash_i_lib_arg = util_install_ruby_file('a')
 | ||||||
|  | +
 | ||||||
|  | +    lp = $LOAD_PATH.dup
 | ||||||
|  | +
 | ||||||
|  | +    begin
 | ||||||
|  | +      $LOAD_PATH.unshift dash_i_lib_arg
 | ||||||
|  | +      $LOAD_PATH.unshift dash_i_ext_arg
 | ||||||
|  | +      assert_require 'a'
 | ||||||
|  | +      assert_match(/a\.rb$/, $LOADED_FEATURES.last)
 | ||||||
|  | +    ensure
 | ||||||
|  | +      $LOAD_PATH.replace lp
 | ||||||
|  | +    end
 | ||||||
|  | +  end
 | ||||||
|  | +
 | ||||||
|  |    def test_concurrent_require | ||||||
|  |      Object.const_set :FILE_ENTERED_LATCH, Latch.new(2) | ||||||
|  |      Object.const_set :FILE_EXIT_LATCH, Latch.new(1) | ||||||
|  | @@ -541,4 +559,48 @@ def silence_warnings
 | ||||||
|  |      $VERBOSE = old_verbose | ||||||
|  |    end | ||||||
|  |   | ||||||
|  | +  def util_install_extension_file(name)
 | ||||||
|  | +    spec = quick_gem name
 | ||||||
|  | +    util_build_gem spec
 | ||||||
|  | +
 | ||||||
|  | +    spec.extensions << "extconf.rb"
 | ||||||
|  | +    write_file File.join(@tempdir, "extconf.rb") do |io|
 | ||||||
|  | +      io.write <<-RUBY
 | ||||||
|  | +        require "mkmf"
 | ||||||
|  | +        create_makefile("#{name}")
 | ||||||
|  | +      RUBY
 | ||||||
|  | +    end
 | ||||||
|  | +
 | ||||||
|  | +    write_file File.join(@tempdir, "#{name}.c") do |io|
 | ||||||
|  | +      io.write <<-C
 | ||||||
|  | +        #include <ruby.h>
 | ||||||
|  | +        void Init_#{name}() { }
 | ||||||
|  | +      C
 | ||||||
|  | +    end
 | ||||||
|  | +
 | ||||||
|  | +    spec.files += ["extconf.rb", "#{name}.c"]
 | ||||||
|  | +
 | ||||||
|  | +    so = File.join(spec.gem_dir, "#{name}.#{RbConfig::CONFIG["DLEXT"]}")
 | ||||||
|  | +    refute_path_exists so
 | ||||||
|  | +
 | ||||||
|  | +    path = Gem::Package.build spec
 | ||||||
|  | +    installer = Gem::Installer.at path
 | ||||||
|  | +    installer.install
 | ||||||
|  | +    assert_path_exists so
 | ||||||
|  | +
 | ||||||
|  | +    spec.gem_dir
 | ||||||
|  | +  end
 | ||||||
|  | +
 | ||||||
|  | +  def util_install_ruby_file(name)
 | ||||||
|  | +    dir_lib = Dir.mktmpdir("test_require_lib", @tempdir)
 | ||||||
|  | +    dash_i_lib_arg = File.join dir_lib
 | ||||||
|  | +
 | ||||||
|  | +    a_rb = File.join dash_i_lib_arg, "#{name}.rb"
 | ||||||
|  | +
 | ||||||
|  | +    FileUtils.mkdir_p File.dirname a_rb
 | ||||||
|  | +    File.open(a_rb, 'w') { |f| f.write "# #{name}.rb" }
 | ||||||
|  | +
 | ||||||
|  | +    dash_i_lib_arg
 | ||||||
|  | +  end
 | ||||||
|  | +
 | ||||||
|  |  end | ||||||
| @ -0,0 +1,324 @@ | |||||||
|  | From 00d98eb8a3245fb93a475ecbbbc4c7ec7e6704cd Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Tue, 19 May 2020 14:00:00 +0200 | ||||||
|  | Subject: [PATCH 1/5] Fix performance regression in `require` | ||||||
|  | 
 | ||||||
|  | Our check for `-I` paths should not go through all activated gems. | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems.rb                         | 10 ++++++++++ | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb |  2 +- | ||||||
|  |  lib/rubygems/test_case.rb               |  1 + | ||||||
|  |  test/rubygems/test_require.rb           | 11 +++++++++++ | ||||||
|  |  4 files changed, 23 insertions(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/rubygems.rb b/lib/rubygems.rb
 | ||||||
|  | index 843cb49e4a..d1a9a1c7e1 100644
 | ||||||
|  | --- a/lib/rubygems.rb
 | ||||||
|  | +++ b/lib/rubygems.rb
 | ||||||
|  | @@ -660,10 +660,20 @@ def self.load_path_insert_index
 | ||||||
|  |      index | ||||||
|  |    end | ||||||
|  |   | ||||||
|  | +  ##
 | ||||||
|  | +  # The number of paths in the `$LOAD_PATH` from activated gems. Used to
 | ||||||
|  | +  # prioritize `-I` and `ENV['RUBYLIB`]` entries during `require`.
 | ||||||
|  | +
 | ||||||
|  | +  def self.activated_gem_paths
 | ||||||
|  | +    @activated_gem_paths ||= 0
 | ||||||
|  | +  end
 | ||||||
|  | +
 | ||||||
|  |    ## | ||||||
|  |    # Add a list of paths to the $LOAD_PATH at the proper place. | ||||||
|  |   | ||||||
|  |    def self.add_to_load_path(*paths) | ||||||
|  | +    @activated_gem_paths = activated_gem_paths + paths.size
 | ||||||
|  | +
 | ||||||
|  |      insert_index = load_path_insert_index | ||||||
|  |   | ||||||
|  |      if insert_index | ||||||
|  | diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | index ed24111bd5..7625ce1bee 100644
 | ||||||
|  | --- a/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | +++ b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | @@ -47,7 +47,7 @@ def require(path)
 | ||||||
|  |          load_path_insert_index = Gem.load_path_insert_index | ||||||
|  |          break unless load_path_insert_index | ||||||
|  |   | ||||||
|  | -        $LOAD_PATH[0...load_path_insert_index].each do |lp|
 | ||||||
|  | +        $LOAD_PATH[0...load_path_insert_index - Gem.activated_gem_paths].each do |lp|
 | ||||||
|  |            safe_lp = lp.dup.tap(&Gem::UNTAINT) | ||||||
|  |            begin | ||||||
|  |              if File.symlink? safe_lp # for backward compatibility | ||||||
|  | diff --git a/lib/rubygems/test_case.rb b/lib/rubygems/test_case.rb
 | ||||||
|  | index a05a2898d1..53dd495aef 100644
 | ||||||
|  | --- a/lib/rubygems/test_case.rb
 | ||||||
|  | +++ b/lib/rubygems/test_case.rb
 | ||||||
|  | @@ -379,6 +379,7 @@ def setup
 | ||||||
|  |      Gem::Security.reset | ||||||
|  |   | ||||||
|  |      Gem.loaded_specs.clear | ||||||
|  | +    Gem.instance_variable_set(:@activated_gem_paths, 0)
 | ||||||
|  |      Gem.clear_default_specs | ||||||
|  |      Bundler.reset! | ||||||
|  |   | ||||||
|  | diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb
 | ||||||
|  | index f36892f8cc..9f2fe3439a 100644
 | ||||||
|  | --- a/test/rubygems/test_require.rb
 | ||||||
|  | +++ b/test/rubygems/test_require.rb
 | ||||||
|  | @@ -382,6 +382,17 @@ def test_default_gem_require_activates_just_once
 | ||||||
|  |      assert_equal 0, times_called | ||||||
|  |    end | ||||||
|  |   | ||||||
|  | +  def test_second_gem_require_does_not_resolve_path_manually_before_going_through_standard_require
 | ||||||
|  | +    a1 = util_spec "a", "1", nil, "lib/test_gem_require_a.rb"
 | ||||||
|  | +    install_gem a1
 | ||||||
|  | +
 | ||||||
|  | +    assert_require "test_gem_require_a"
 | ||||||
|  | +
 | ||||||
|  | +    stub(:gem_original_require, ->(path) { assert_equal "test_gem_require_a", path }) do
 | ||||||
|  | +      require "test_gem_require_a"
 | ||||||
|  | +    end
 | ||||||
|  | +  end
 | ||||||
|  | +
 | ||||||
|  |    def test_realworld_default_gem | ||||||
|  |      testing_ruby_repo = !ENV["GEM_COMMAND"].nil? | ||||||
|  |      skip "this test can't work under ruby-core setup" if testing_ruby_repo || java_platform? | ||||||
|  | 
 | ||||||
|  | From ae95885dff6189c5ac59bbdf685cb4ec4751fdef Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Tue, 19 May 2020 14:08:19 +0200 | ||||||
|  | Subject: [PATCH 2/5] Refactor `Gem.load_path_insert_index` | ||||||
|  | 
 | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems.rb                         | 13 +++---------- | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb |  5 +---- | ||||||
|  |  2 files changed, 4 insertions(+), 14 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/rubygems.rb b/lib/rubygems.rb
 | ||||||
|  | index d1a9a1c7e1..ca80326459 100644
 | ||||||
|  | --- a/lib/rubygems.rb
 | ||||||
|  | +++ b/lib/rubygems.rb
 | ||||||
|  | @@ -657,7 +657,7 @@ def self.load_path_insert_index
 | ||||||
|  |   | ||||||
|  |      index = $LOAD_PATH.index RbConfig::CONFIG['sitelibdir'] | ||||||
|  |   | ||||||
|  | -    index
 | ||||||
|  | +    index || 0
 | ||||||
|  |    end | ||||||
|  |   | ||||||
|  |    ## | ||||||
|  | @@ -674,15 +674,8 @@ def self.activated_gem_paths
 | ||||||
|  |    def self.add_to_load_path(*paths) | ||||||
|  |      @activated_gem_paths = activated_gem_paths + paths.size | ||||||
|  |   | ||||||
|  | -    insert_index = load_path_insert_index
 | ||||||
|  | -
 | ||||||
|  | -    if insert_index
 | ||||||
|  | -      # gem directories must come after -I and ENV['RUBYLIB']
 | ||||||
|  | -      $LOAD_PATH.insert(insert_index, *paths)
 | ||||||
|  | -    else
 | ||||||
|  | -      # we are probably testing in core, -I and RUBYLIB don't apply
 | ||||||
|  | -      $LOAD_PATH.unshift(*paths)
 | ||||||
|  | -    end
 | ||||||
|  | +    # gem directories must come after -I and ENV['RUBYLIB']
 | ||||||
|  | +    $LOAD_PATH.insert(Gem.load_path_insert_index, *paths)
 | ||||||
|  |    end | ||||||
|  |   | ||||||
|  |    @yaml_loaded = false | ||||||
|  | diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | index 7625ce1bee..decf4829f1 100644
 | ||||||
|  | --- a/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | +++ b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | @@ -44,10 +44,7 @@ def require(path)
 | ||||||
|  |      resolved_path = begin | ||||||
|  |        rp = nil | ||||||
|  |        Gem.suffixes.each do |s| | ||||||
|  | -        load_path_insert_index = Gem.load_path_insert_index
 | ||||||
|  | -        break unless load_path_insert_index
 | ||||||
|  | -
 | ||||||
|  | -        $LOAD_PATH[0...load_path_insert_index - Gem.activated_gem_paths].each do |lp|
 | ||||||
|  | +        $LOAD_PATH[0...Gem.load_path_insert_index - Gem.activated_gem_paths].each do |lp|
 | ||||||
|  |            safe_lp = lp.dup.tap(&Gem::UNTAINT) | ||||||
|  |            begin | ||||||
|  |              if File.symlink? safe_lp # for backward compatibility | ||||||
|  | 
 | ||||||
|  | From da1492e9d7b28d068fbfbb0ba1cafcc516681567 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Tue, 19 May 2020 14:32:12 +0200 | ||||||
|  | Subject: [PATCH 3/5] Extract a local outside the loop | ||||||
|  | 
 | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb | 3 ++- | ||||||
|  |  1 file changed, 2 insertions(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | index decf4829f1..6a7faaf2d1 100644
 | ||||||
|  | --- a/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | +++ b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | @@ -43,8 +43,9 @@ def require(path)
 | ||||||
|  |      # 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...Gem.load_path_insert_index - Gem.activated_gem_paths].each do |lp|
 | ||||||
|  | +        $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 | ||||||
|  | 
 | ||||||
|  | From 22ad5717c38feda2375b53628d15ae3db2195684 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Thu, 21 May 2020 15:20:57 +0200 | ||||||
|  | Subject: [PATCH 4/5] 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. | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb | 53 +++++++++++-------------- | ||||||
|  |  test/rubygems/test_require.rb           | 29 ++++++++++++++ | ||||||
|  |  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 @@ 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 | ||||||
|  | diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb
 | ||||||
|  | index 9f2fe3439a..2b11e26dfe 100644
 | ||||||
|  | --- a/test/rubygems/test_require.rb
 | ||||||
|  | +++ b/test/rubygems/test_require.rb
 | ||||||
|  | @@ -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" | ||||||
|  | 
 | ||||||
|  | From db872c7a18d616f4447bdcca3130be6db9e5cb03 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Sat, 23 May 2020 20:18:41 +0200 | ||||||
|  | Subject: [PATCH 5/5] Remove direct reference to PR | ||||||
|  | 
 | ||||||
|  | The code is quite different now, so I think the link might be even | ||||||
|  | confusing. If you want to know more, use git history. | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb | 1 - | ||||||
|  |  1 file changed, 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | index 81e37b98bf..115ae0cb50 100644
 | ||||||
|  | --- a/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | +++ b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | @@ -41,7 +41,6 @@ def require(path)
 | ||||||
|  |   | ||||||
|  |      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 | ||||||
							
								
								
									
										92
									
								
								rubygems-3.1.3-Improve-require.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										92
									
								
								rubygems-3.1.3-Improve-require.patch
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,92 @@ | |||||||
|  | From c5197b2ab35ba389f48918e0c773b43b6dca2fa5 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Fri, 7 Feb 2020 17:16:05 +0100 | ||||||
|  | Subject: [PATCH 1/3] Tweaks to get test passing more reliably | ||||||
|  | 
 | ||||||
|  | ---
 | ||||||
|  |  test/rubygems/test_require.rb | 6 ++++-- | ||||||
|  |  1 file changed, 4 insertions(+), 2 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb
 | ||||||
|  | index 7cffbfa7fe..67c55416d4 100644
 | ||||||
|  | --- a/test/rubygems/test_require.rb
 | ||||||
|  | +++ b/test/rubygems/test_require.rb
 | ||||||
|  | @@ -567,18 +567,20 @@ def util_install_extension_file(name)
 | ||||||
|  |      write_file File.join(@tempdir, "extconf.rb") do |io| | ||||||
|  |        io.write <<-RUBY | ||||||
|  |          require "mkmf" | ||||||
|  | +        CONFIG['LDSHARED'] = '$(TOUCH) $@ ||'
 | ||||||
|  |          create_makefile("#{name}") | ||||||
|  |        RUBY | ||||||
|  |      end | ||||||
|  |   | ||||||
|  |      write_file File.join(@tempdir, "#{name}.c") do |io| | ||||||
|  |        io.write <<-C | ||||||
|  | -        #include <ruby.h>
 | ||||||
|  |          void Init_#{name}() { } | ||||||
|  |        C | ||||||
|  |      end | ||||||
|  |   | ||||||
|  | -    spec.files += ["extconf.rb", "#{name}.c"]
 | ||||||
|  | +    write_file File.join(@tempdir, "depend")
 | ||||||
|  | +
 | ||||||
|  | +    spec.files += ["extconf.rb", "depend", "#{name}.c"]
 | ||||||
|  |   | ||||||
|  |      so = File.join(spec.gem_dir, "#{name}.#{RbConfig::CONFIG["DLEXT"]}") | ||||||
|  |      refute_path_exists so | ||||||
|  | 
 | ||||||
|  | From 7bfd7319cd751837c3ccaf1d97b02846eaaf39d5 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Tue, 11 Feb 2020 11:56:06 +0100 | ||||||
|  | Subject: [PATCH 2/3] Fix bug bug calculating $LOAD_PATH's to check in | ||||||
|  |  `require` | ||||||
|  | 
 | ||||||
|  | In `Gem.load_path_insert_index` is not set, we end up having | ||||||
|  | `$LOAD_PATH[0...-1]`, unintentionally skipping the last $LOAD_PATH entry | ||||||
|  | from the check. | ||||||
|  | 
 | ||||||
|  | The correct thing to do in that case is to not even try since we have no | ||||||
|  | way of distinguisng default LOAD_PATH entries from those added with -I. | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb | 5 ++++- | ||||||
|  |  1 file changed, 4 insertions(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | index 369f2c743e..a8d170f13a 100644
 | ||||||
|  | --- a/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | +++ b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | @@ -44,7 +44,10 @@ def require(path)
 | ||||||
|  |      resolved_path = begin | ||||||
|  |        rp = nil | ||||||
|  |        Gem.suffixes.each do |s| | ||||||
|  | -        $LOAD_PATH[0...Gem.load_path_insert_index || -1].each do |lp|
 | ||||||
|  | +        load_path_insert_index = Gem.load_path_insert_index
 | ||||||
|  | +        break unless load_path_insert_index
 | ||||||
|  | +
 | ||||||
|  | +        $LOAD_PATH[0...load_path_insert_index].each do |lp|
 | ||||||
|  |            safe_lp = lp.dup.tap(&Gem::UNTAINT) | ||||||
|  |            begin | ||||||
|  |              if File.symlink? safe_lp # for backward compatibility | ||||||
|  | 
 | ||||||
|  | From 4fc0ab21c0f7713829abb522ce3b6d8e24c126b3 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Fri, 14 Feb 2020 02:03:04 +0100 | ||||||
|  | Subject: [PATCH 3/3] Exclude empty suffix from `-I` require loop | ||||||
|  | 
 | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb | 2 +- | ||||||
|  |  1 file changed, 1 insertion(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | index a8d170f13a..9712fb6ac0 100644
 | ||||||
|  | --- a/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | +++ b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | @@ -43,7 +43,7 @@ def require(path)
 | ||||||
|  |      # https://github.com/rubygems/rubygems/pull/1868 | ||||||
|  |      resolved_path = begin | ||||||
|  |        rp = nil | ||||||
|  | -      Gem.suffixes.each do |s|
 | ||||||
|  | +      Gem.suffixes[1..-1].each do |s|
 | ||||||
|  |          load_path_insert_index = Gem.load_path_insert_index | ||||||
|  |          break unless load_path_insert_index | ||||||
|  |   | ||||||
| @ -0,0 +1,29 @@ | |||||||
|  | From 301e30bf97dd603ca81d52b90186908575c4ddf8 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net> | ||||||
|  | Date: Tue, 25 Feb 2020 15:01:44 +0100 | ||||||
|  | Subject: [PATCH] Revert "Exclude empty suffix from `-I` require loop" | ||||||
|  | 
 | ||||||
|  | This reverts commit 4fc0ab21c0f7713829abb522ce3b6d8e24c126b3. | ||||||
|  | 
 | ||||||
|  | Technically, extensionless ruby files are valid ruby files that can be | ||||||
|  | required. For example, `bin/bundle` is sometimes required from other | ||||||
|  | binstubs even if it's also runnable directly. | ||||||
|  | 
 | ||||||
|  | So, we should technically consider this kind of files too. | ||||||
|  | ---
 | ||||||
|  |  lib/rubygems/core_ext/kernel_require.rb | 2 +- | ||||||
|  |  1 file changed, 1 insertion(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | index 9712fb6ac0..a8d170f13a 100644
 | ||||||
|  | --- a/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | +++ b/lib/rubygems/core_ext/kernel_require.rb
 | ||||||
|  | @@ -43,7 +43,7 @@ def require(path)
 | ||||||
|  |      # https://github.com/rubygems/rubygems/pull/1868 | ||||||
|  |      resolved_path = begin | ||||||
|  |        rp = nil | ||||||
|  | -      Gem.suffixes[1..-1].each do |s|
 | ||||||
|  | +      Gem.suffixes.each do |s|
 | ||||||
|  |          load_path_insert_index = Gem.load_path_insert_index | ||||||
|  |          break unless load_path_insert_index | ||||||
|  |   | ||||||
		Loading…
	
		Reference in New Issue
	
	Block a user