diff options
| author | Hiroshi SHIBATA <hsbt@ruby-lang.org> | 2024-08-22 01:40:11 +0900 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-08-21 09:40:11 -0700 |
| commit | 9ae91eb2aa8a82315026e72fb58d89bc23432335 (patch) | |
| tree | ecda502180b50a9ef0b197cfce6d5372ce778389 | |
| parent | 66312ad913d67bfd3c2c83b174eabf537f5def84 (diff) | |
Backport warning feature for bundled gems from master (#11420)
* Make sure to always use the right `warn`
* lib/bundled_gems.rb: more reliable caller detection
The `2` skipped frames went out of sync and now it should be `3`.
Rather than just update the offset, we can implement a way that
is adaptative as long as all require decorators are also called require.
Also we should compute the corresponding `uplevel` otherwise the
warning will still point decorators.
Co-authored-by: "Hiroshi SHIBATA" <hsbt@ruby-lang.org>
* Warn ostruct for Ruby 3.5
* Warn pstore for Ruby 3.5
* Mark rdoc as bundled gems at Ruby 3.5
* Warn to use win32ole without Gemfile for Ruby 3.5
* EXACT list is mostly same as SINCE list on bundled gems.
* Mark to warn fiddle as bundled gems for Ruby 3.5
* Mark to warn logger as bundled gems for Ruby 3.5
* We should use uplevel:2 in another case.
Like the following scenario with bootsnap, that frames are same or smaller than frame_to_skip(=3).
---
"/Users/hsbt/.local/share/rbenv/versions/3.3-dev/lib/ruby/3.3.0/bundled_gems.rb:69:in `block (2 levels) in replace_require'"
"/Users/hsbt/.local/share/gem/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'"
"test_warn_bootsnap.rb:11:in `<main>'"
---
* Delete unnecessary rubocop disable comment
* Show correct script name with sub-feature case
* Skip to show script name with using ruby -r option
* Don't show script name when bundle exec and call ruby script directly.
* Pick word fix from 34adc07372c10170b8ca36111d216cbd8e4699be
---------
Co-authored-by: David RodrÃguez <deivid.rodriguez@riseup.net>
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Co-authored-by: Kentaro Takeyama <75117116+obregonia1@users.noreply.github.com>
| -rw-r--r-- | lib/bundled_gems.rb | 89 | ||||
| -rwxr-xr-x | tool/test_for_warn_bundled_gems/test.sh | 4 | ||||
| -rw-r--r-- | tool/test_for_warn_bundled_gems/test_warn_redefined.rb | 18 |
3 files changed, 84 insertions, 27 deletions
diff --git a/lib/bundled_gems.rb b/lib/bundled_gems.rb index b1e5470fc7..e3d2617457 100644 --- a/lib/bundled_gems.rb +++ b/lib/bundled_gems.rb @@ -26,23 +26,18 @@ module Gem::BUNDLED_GEMS "resolv-replace" => "3.4.0", "rinda" => "3.4.0", "syslog" => "3.4.0", + "ostruct" => "3.5.0", + "pstore" => "3.5.0", + "rdoc" => "3.5.0", + "win32ole" => "3.5.0", + "fiddle" => "3.5.0", + "logger" => "3.5.0", }.freeze SINCE_FAST_PATH = SINCE.transform_keys { |g| g.sub(/\A.*\-/, "") }.freeze EXACT = { - "abbrev" => true, - "base64" => true, - "bigdecimal" => true, - "csv" => true, - "drb" => true, - "getoptlong" => true, - "mutex_m" => true, - "nkf" => true, "kconv" => "nkf", - "observer" => true, - "resolv-replace" => true, - "rinda" => true, - "syslog" => true, + "kconv" => "nkf", }.freeze PREFIXED = { @@ -70,8 +65,12 @@ module Gem::BUNDLED_GEMS [::Kernel.singleton_class, ::Kernel].each do |kernel_class| kernel_class.send(:alias_method, :no_warning_require, :require) kernel_class.send(:define_method, :require) do |name| - if message = ::Gem::BUNDLED_GEMS.warning?(name, specs: spec_names) # rubocop:disable Style/HashSyntax - warn message, :uplevel => 1 + if message = ::Gem::BUNDLED_GEMS.warning?(name, specs: spec_names) + if ::Gem::BUNDLED_GEMS.uplevel > 0 + Kernel.warn message, uplevel: ::Gem::BUNDLED_GEMS.uplevel + else + Kernel.warn message + end end kernel_class.send(:no_warning_require, name) end @@ -83,6 +82,36 @@ module Gem::BUNDLED_GEMS end end + def self.uplevel + frame_count = 0 + frames_to_skip = 3 + uplevel = 0 + require_found = false + Thread.each_caller_location do |cl| + frame_count += 1 + if frames_to_skip >= 1 + frames_to_skip -= 1 + next + end + uplevel += 1 + if require_found + if cl.base_label != "require" + return uplevel + end + else + if cl.base_label == "require" + require_found = true + end + end + # Don't show script name when bundle exec and call ruby script directly. + if cl.path.end_with?("bundle") + frame_count = 0 + break + end + end + require_found ? 1 : frame_count - 1 + end + def self.find_gem(path) if !path return @@ -93,7 +122,7 @@ module Gem::BUNDLED_GEMS else return end - EXACT[n] or PREFIXED[n = n[%r[\A[^/]+(?=/)]]] && n + (EXACT[n] || !!SINCE[n]) or PREFIXED[n = n[%r[\A[^/]+(?=/)]]] && n end def self.warning?(name, specs: nil) @@ -108,7 +137,7 @@ module Gem::BUNDLED_GEMS # We'll fail to warn requires for files that are not the entry point # of the gem, e.g. require "logger/formatter.rb" won't warn. # But that's acceptable because this warning is best effort, - # and in the overwhelming majority of case logger.rb will end + # and in the overwhelming majority of cases logger.rb will end # up required. return unless SINCE_FAST_PATH[File.basename(feature, ".*")] else @@ -148,29 +177,35 @@ module Gem::BUNDLED_GEMS end def self.build_message(gem) - msg = " #{RUBY_VERSION < SINCE[gem] ? "will no longer be" : "is not"} part of the default gems since Ruby #{SINCE[gem]}." + msg = " #{RUBY_VERSION < SINCE[gem] ? "will no longer be" : "is not"} part of the default gems starting from Ruby #{SINCE[gem]}." if defined?(Bundler) - msg += " Add #{gem} to your Gemfile or gemspec." + msg += "\nYou can add #{gem} to your Gemfile or gemspec to silence this warning." - # We detect the gem name from caller_locations. We need to skip 2 frames like: - # lib/ruby/3.3.0+0/bundled_gems.rb:90:in `warning?'", - # lib/ruby/3.3.0+0/bundler/rubygems_integration.rb:247:in `block (2 levels) in replace_require'", + # We detect the gem name from caller_locations. First we walk until we find `require` + # then take the first frame that's not from `require`. # # Additionally, we need to skip Bootsnap and Zeitwerk if present, these # gems decorate Kernel#require, so they are not really the ones issuing # the require call users should be warned about. Those are upwards. - frames_to_skip = 2 + frames_to_skip = 3 location = nil + require_found = false Thread.each_caller_location do |cl| if frames_to_skip >= 1 frames_to_skip -= 1 next end - if cl.base_label != "require" - location = cl.path - break + if require_found + if cl.base_label != "require" + location = cl.path + break + end + else + if cl.base_label == "require" + require_found = true + end end end @@ -183,7 +218,7 @@ module Gem::BUNDLED_GEMS end end if caller_gem - msg += " Also contact author of #{caller_gem} to add #{gem} into its gemspec." + msg += "\nAlso please contact the author of #{caller_gem} to request adding #{gem} into its gemspec." end end else @@ -204,7 +239,7 @@ class LoadError name = path.tr("/", "-") if !defined?(Bundler) && Gem::BUNDLED_GEMS::SINCE[name] && !Gem::BUNDLED_GEMS::WARNED[name] - warn name + Gem::BUNDLED_GEMS.build_message(name) + warn name + Gem::BUNDLED_GEMS.build_message(name), uplevel: Gem::BUNDLED_GEMS.uplevel end super end diff --git a/tool/test_for_warn_bundled_gems/test.sh b/tool/test_for_warn_bundled_gems/test.sh index a14d5bcedc..0cd65f07e8 100755 --- a/tool/test_for_warn_bundled_gems/test.sh +++ b/tool/test_for_warn_bundled_gems/test.sh @@ -51,3 +51,7 @@ echo echo "* Don't show warning bigdecimal/util when bigdecimal on Gemfile" ruby test_no_warn_sub_feature.rb echo + +echo "* Show warning when warn is not the standard one in the current scope" +ruby test_warn_redefined.rb +echo diff --git a/tool/test_for_warn_bundled_gems/test_warn_redefined.rb b/tool/test_for_warn_bundled_gems/test_warn_redefined.rb new file mode 100644 index 0000000000..a898a8c07c --- /dev/null +++ b/tool/test_for_warn_bundled_gems/test_warn_redefined.rb @@ -0,0 +1,18 @@ +module My + def warn(msg) + end + + def my + require "bundler/inline" + + gemfile do + source "https://rubygems.org" + end + + require "csv" + end + + extend self +end + +My.my |
