Skip to content

Commit 9ae91eb

Browse files
hsbtdeivid-rodriguezbyrootobregonia1
authored
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 34adc07 --------- 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>
1 parent 66312ad commit 9ae91eb

File tree

3 files changed

+84
-27
lines changed

3 files changed

+84
-27
lines changed

lib/bundled_gems.rb

Lines changed: 62 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,18 @@ module Gem::BUNDLED_GEMS
2626
"resolv-replace" => "3.4.0",
2727
"rinda" => "3.4.0",
2828
"syslog" => "3.4.0",
29+
"ostruct" => "3.5.0",
30+
"pstore" => "3.5.0",
31+
"rdoc" => "3.5.0",
32+
"win32ole" => "3.5.0",
33+
"fiddle" => "3.5.0",
34+
"logger" => "3.5.0",
2935
}.freeze
3036

3137
SINCE_FAST_PATH = SINCE.transform_keys { |g| g.sub(/\A.*\-/, "") }.freeze
3238

3339
EXACT = {
34-
"abbrev" => true,
35-
"base64" => true,
36-
"bigdecimal" => true,
37-
"csv" => true,
38-
"drb" => true,
39-
"getoptlong" => true,
40-
"mutex_m" => true,
41-
"nkf" => true, "kconv" => "nkf",
42-
"observer" => true,
43-
"resolv-replace" => true,
44-
"rinda" => true,
45-
"syslog" => true,
40+
"kconv" => "nkf",
4641
}.freeze
4742

4843
PREFIXED = {
@@ -70,8 +65,12 @@ def self.replace_require(specs)
7065
[::Kernel.singleton_class, ::Kernel].each do |kernel_class|
7166
kernel_class.send(:alias_method, :no_warning_require, :require)
7267
kernel_class.send(:define_method, :require) do |name|
73-
if message = ::Gem::BUNDLED_GEMS.warning?(name, specs: spec_names) # rubocop:disable Style/HashSyntax
74-
warn message, :uplevel => 1
68+
if message = ::Gem::BUNDLED_GEMS.warning?(name, specs: spec_names)
69+
if ::Gem::BUNDLED_GEMS.uplevel > 0
70+
Kernel.warn message, uplevel: ::Gem::BUNDLED_GEMS.uplevel
71+
else
72+
Kernel.warn message
73+
end
7574
end
7675
kernel_class.send(:no_warning_require, name)
7776
end
@@ -83,6 +82,36 @@ def self.replace_require(specs)
8382
end
8483
end
8584

85+
def self.uplevel
86+
frame_count = 0
87+
frames_to_skip = 3
88+
uplevel = 0
89+
require_found = false
90+
Thread.each_caller_location do |cl|
91+
frame_count += 1
92+
if frames_to_skip >= 1
93+
frames_to_skip -= 1
94+
next
95+
end
96+
uplevel += 1
97+
if require_found
98+
if cl.base_label != "require"
99+
return uplevel
100+
end
101+
else
102+
if cl.base_label == "require"
103+
require_found = true
104+
end
105+
end
106+
# Don't show script name when bundle exec and call ruby script directly.
107+
if cl.path.end_with?("bundle")
108+
frame_count = 0
109+
break
110+
end
111+
end
112+
require_found ? 1 : frame_count - 1
113+
end
114+
86115
def self.find_gem(path)
87116
if !path
88117
return
@@ -93,7 +122,7 @@ def self.find_gem(path)
93122
else
94123
return
95124
end
96-
EXACT[n] or PREFIXED[n = n[%r[\A[^/]+(?=/)]]] && n
125+
(EXACT[n] || !!SINCE[n]) or PREFIXED[n = n[%r[\A[^/]+(?=/)]]] && n
97126
end
98127

99128
def self.warning?(name, specs: nil)
@@ -108,7 +137,7 @@ def self.warning?(name, specs: nil)
108137
# We'll fail to warn requires for files that are not the entry point
109138
# of the gem, e.g. require "logger/formatter.rb" won't warn.
110139
# But that's acceptable because this warning is best effort,
111-
# and in the overwhelming majority of case logger.rb will end
140+
# and in the overwhelming majority of cases logger.rb will end
112141
# up required.
113142
return unless SINCE_FAST_PATH[File.basename(feature, ".*")]
114143
else
@@ -148,29 +177,35 @@ def self.warning?(name, specs: nil)
148177
end
149178

150179
def self.build_message(gem)
151-
msg = " #{RUBY_VERSION < SINCE[gem] ? "will no longer be" : "is not"} part of the default gems since Ruby #{SINCE[gem]}."
180+
msg = " #{RUBY_VERSION < SINCE[gem] ? "will no longer be" : "is not"} part of the default gems starting from Ruby #{SINCE[gem]}."
152181

153182
if defined?(Bundler)
154-
msg += " Add #{gem} to your Gemfile or gemspec."
183+
msg += "\nYou can add #{gem} to your Gemfile or gemspec to silence this warning."
155184

156-
# We detect the gem name from caller_locations. We need to skip 2 frames like:
157-
# lib/ruby/3.3.0+0/bundled_gems.rb:90:in `warning?'",
158-
# lib/ruby/3.3.0+0/bundler/rubygems_integration.rb:247:in `block (2 levels) in replace_require'",
185+
# We detect the gem name from caller_locations. First we walk until we find `require`
186+
# then take the first frame that's not from `require`.
159187
#
160188
# Additionally, we need to skip Bootsnap and Zeitwerk if present, these
161189
# gems decorate Kernel#require, so they are not really the ones issuing
162190
# the require call users should be warned about. Those are upwards.
163-
frames_to_skip = 2
191+
frames_to_skip = 3
164192
location = nil
193+
require_found = false
165194
Thread.each_caller_location do |cl|
166195
if frames_to_skip >= 1
167196
frames_to_skip -= 1
168197
next
169198
end
170199

171-
if cl.base_label != "require"
172-
location = cl.path
173-
break
200+
if require_found
201+
if cl.base_label != "require"
202+
location = cl.path
203+
break
204+
end
205+
else
206+
if cl.base_label == "require"
207+
require_found = true
208+
end
174209
end
175210
end
176211

@@ -183,7 +218,7 @@ def self.build_message(gem)
183218
end
184219
end
185220
if caller_gem
186-
msg += " Also contact author of #{caller_gem} to add #{gem} into its gemspec."
221+
msg += "\nAlso please contact the author of #{caller_gem} to request adding #{gem} into its gemspec."
187222
end
188223
end
189224
else
@@ -204,7 +239,7 @@ def message
204239

205240
name = path.tr("/", "-")
206241
if !defined?(Bundler) && Gem::BUNDLED_GEMS::SINCE[name] && !Gem::BUNDLED_GEMS::WARNED[name]
207-
warn name + Gem::BUNDLED_GEMS.build_message(name)
242+
warn name + Gem::BUNDLED_GEMS.build_message(name), uplevel: Gem::BUNDLED_GEMS.uplevel
208243
end
209244
super
210245
end

tool/test_for_warn_bundled_gems/test.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,7 @@ echo
5151
echo "* Don't show warning bigdecimal/util when bigdecimal on Gemfile"
5252
ruby test_no_warn_sub_feature.rb
5353
echo
54+
55+
echo "* Show warning when warn is not the standard one in the current scope"
56+
ruby test_warn_redefined.rb
57+
echo
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
module My
2+
def warn(msg)
3+
end
4+
5+
def my
6+
require "bundler/inline"
7+
8+
gemfile do
9+
source "https://rubygems.org"
10+
end
11+
12+
require "csv"
13+
end
14+
15+
extend self
16+
end
17+
18+
My.my

0 commit comments

Comments
 (0)