Skip to content

Commit

Permalink
Backport warning feature for bundled gems from master (#11420)
Browse files Browse the repository at this point in the history
* 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" <[email protected]>

* 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 34adc07

---------

Co-authored-by: David Rodríguez <[email protected]>
Co-authored-by: Jean Boussier <[email protected]>
Co-authored-by: Kentaro Takeyama <[email protected]>
  • Loading branch information
4 people authored Aug 21, 2024
1 parent 66312ad commit 9ae91eb
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 27 deletions.
89 changes: 62 additions & 27 deletions lib/bundled_gems.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -70,8 +65,12 @@ def self.replace_require(specs)
[::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
Expand All @@ -83,6 +82,36 @@ def self.replace_require(specs)
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
Expand All @@ -93,7 +122,7 @@ def self.find_gem(path)
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)
Expand All @@ -108,7 +137,7 @@ def self.warning?(name, specs: nil)
# 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
Expand Down Expand Up @@ -148,29 +177,35 @@ def self.warning?(name, specs: nil)
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

Expand All @@ -183,7 +218,7 @@ def self.build_message(gem)
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
Expand All @@ -204,7 +239,7 @@ def message

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
Expand Down
4 changes: 4 additions & 0 deletions tool/test_for_warn_bundled_gems/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 18 additions & 0 deletions tool/test_for_warn_bundled_gems/test_warn_redefined.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 9ae91eb

Please sign in to comment.