PR#64: Fix `require` behavior allowing to load libraries multiple times. - rpms/ruby

rpms / ruby

#64 Fix `require` behavior allowing to load libraries multiple times.

Merged 3 years ago by vondruch. Opened 3 years ago by vondruch.

rpms/ vondruch/ruby rhbz1835836 into master

Fix `require` behavior allowing to load libraries multiple times.

Vít Ondruch • 3 years ago

bc5cb60

ruby.spec

file modified

+21 -1

		`@@ -22,7 +22,7 @@`
		`%endif`


		`- %global release 131`
		`+ %global release 132`
		`%{!?release_string:%define release_string %{?development_release:0.}%{release}%{?development_release:.%{development_release}}%{?dist}}`

		`# The RubyGems library has to stay out of Ruby directory tree, since the`
		`@@ -157,6 +157,18 @@`
		`# Fix compatibility with libyaml 0.2.5`
		`# https://bugs.ruby-lang.org/issues/16949`
		`Patch14: ruby-2.7.2-psych-fix-yaml-tests.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.`
		`# https://github.com/ruby/openssl/pull/216`
		`@@ -571,6 +583,10 @@`
		`%patch12 -p1`
		`%patch13 -p1`
		`%patch14 -p1`
		`+ %patch15 -p1`
		`+ %patch16 -p1`
		`+ %patch17 -p1`
		`+ %patch18 -p1`
		`%patch22 -p1`

		`# Provide an example of usage of the tapset:`
		`@@ -1274,6 +1290,10 @@`


		`%changelog`
		`+ * Wed Jun 24 2020 Vít Ondruch <vondruch@redhat.com> - 2.7.1-132`
		+ - Fix `require` behavior allowing to load libraries multiple times.
		`+ Resolves: rhbz#1835836`
		`+`
		`* Fri May 15 2020 Vít Ondruch <vondruch@redhat.com> - 2.7.1-131`
		`- Relax rubygems-devel dependency on rubygems.`

rubygems-3.1.3-Fix-I-require-priority.patch

file added

+167

		`@@ -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`

rubygems-3.1.3-Fix-correctness-and-performance-regression-in-require.patch

file added

+324

		`@@ -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`

rubygems-3.1.3-Improve-require.patch

file added

+92

		`@@ -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`
		`+`

rubygems-3.1.3-Revert-Exclude-empty-suffix-from-I-require-loop.patch

file added

+29

		`@@ -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`
		`+`

vondruch commented 3 years ago

Resolves: rhbz#1835836

vondruch commented 3 years ago

@jaruga Could you please give it test run (I hope that the simple koji CI will provide test build soonish)

vondruch commented 3 years ago

The x86_64 build is ready.

jaruga commented 3 years ago

Sure. I like this issue will be also fixed too.
https://bugzilla.redhat.com/show_bug.cgi?id=1850541

jaruga commented 3 years ago

I tested with the test.rb which I used on https://bugzilla.redhat.com/show_bug.cgi?id=1835836#c19 .
There is no error. Same warnings with rpms/rubygems's one.

$ ruby test.rb 
== gem: irb ==
/usr/share/ruby/irb/input-method.rb:18: warning: already initialized constant IRB::STDIN_FILE_NAME
/usr/share/gems/gems/irb-1.2.3/lib/irb/input-method.rb:18: warning: previous definition of STDIN_FILE_NAME was here
== gem: rdoc ==
== gem: bigdecimal ==
== gem: io-console ==
== gem: json ==
== gem: openssl ==
== gem: psych ==
/usr/share/gems/gems/psych-3.1.0/lib/psych.rb:233: warning: already initialized constant Psych::LIBYAML_VERSION
/usr/share/ruby/psych.rb:233: warning: previous definition of LIBYAML_VERSION was here
/usr/share/gems/gems/psych-3.1.0/lib/psych.rb:235: warning: already initialized constant Psych::NOT_GIVEN
/usr/share/ruby/psych.rb:235: warning: previous definition of NOT_GIVEN was here
== gem: bundler ==

I am also running the scratch build here.
https://koji.fedoraproject.org/koji/taskinfo?taskID=46087741

jaruga commented 3 years ago

It looks good to me.