Improve bundled gems warning messages

Currently evenn if the require actually fails, they suggest that the
file was actually loaded, which is confusing. I reworded them to reduce
this confusion.
This commit is contained in:
David Rodríguez 2025-01-29 20:17:17 +01:00 committed by Hiroshi SHIBATA
parent 03a0c4e079
commit c83370671b
2 changed files with 31 additions and 24 deletions

View File

@ -101,7 +101,7 @@ module Gem::BUNDLED_GEMS # :nodoc:
def self.warning?(name, specs: nil) def self.warning?(name, specs: nil)
# name can be a feature name or a file path with String or Pathname # name can be a feature name or a file path with String or Pathname
feature = File.path(name) feature = File.path(name).sub(LIBEXT, "")
# The actual checks needed to properly identify the gem being required # The actual checks needed to properly identify the gem being required
# are costly (see [Bug #20641]), so we first do a much cheaper check # are costly (see [Bug #20641]), so we first do a much cheaper check
@ -109,8 +109,9 @@ module Gem::BUNDLED_GEMS # :nodoc:
subfeature = if feature.include?("/") subfeature = if feature.include?("/")
# bootsnap expands `require "csv"` to `require "#{LIBDIR}/csv.rb"`, # bootsnap expands `require "csv"` to `require "#{LIBDIR}/csv.rb"`,
# and `require "syslog"` to `require "#{ARCHDIR}/syslog.so"`. # and `require "syslog"` to `require "#{ARCHDIR}/syslog.so"`.
name = feature.delete_prefix(ARCHDIR).delete_prefix(LIBDIR).sub(LIBEXT, "") feature.delete_prefix!(ARCHDIR)
segments = name.split("/") feature.delete_prefix!(LIBDIR)
segments = feature.split("/")
name = segments.shift name = segments.shift
name = EXACT[name] || name name = EXACT[name] || name
if !SINCE[name] if !SINCE[name]
@ -119,8 +120,7 @@ module Gem::BUNDLED_GEMS # :nodoc:
end end
segments.any? segments.any?
else else
name = feature.sub(LIBEXT, "") name = EXACT[feature] || feature
name = EXACT[name] || name
return unless SINCE[name] return unless SINCE[name]
false false
end end
@ -130,18 +130,25 @@ module Gem::BUNDLED_GEMS # :nodoc:
return if WARNED[name] return if WARNED[name]
WARNED[name] = true WARNED[name] = true
level = RUBY_VERSION < SINCE[name] ? "warning" : "error"
if subfeature if subfeature
"#{feature} is found in #{name}, which" "#{feature} is found in #{name}, which"
else else
"#{feature} was loaded from the standard library, but" "#{feature} #{level == "warning" ? "was loaded" : "used to be loaded"} from the standard library, but"
end + build_message(name) end + build_message(name, level)
end end
def self.build_message(name) def self.build_message(name, level)
msg = " #{RUBY_VERSION < SINCE[name] ? "will no longer be" : "is not"} part of the default gems starting from Ruby #{SINCE[name]}." msg = if level == "warning"
" will no longer be part of the default gems starting from Ruby #{SINCE[name]}"
else
" is not part of the default gems since Ruby #{SINCE[name]}."
end
if defined?(Bundler) if defined?(Bundler)
msg += "\nYou can add #{name} to your Gemfile or gemspec to silence this warning." motivation = level == "warning" ? "silence this warning" : "fix this error"
msg += "\nYou can add #{name} to your Gemfile or gemspec to #{motivation}."
# We detect the gem name from caller_locations. First we walk until we find `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`. # then take the first frame that's not from `require`.
@ -230,7 +237,7 @@ class LoadError
name = path.tr("/", "-") name = path.tr("/", "-")
if !defined?(Bundler) && Gem::BUNDLED_GEMS::SINCE[name] && !Gem::BUNDLED_GEMS::WARNED[name] if !defined?(Bundler) && Gem::BUNDLED_GEMS::SINCE[name] && !Gem::BUNDLED_GEMS::WARNED[name]
warn name + Gem::BUNDLED_GEMS.build_message(name), uplevel: Gem::BUNDLED_GEMS.uplevel warn name + Gem::BUNDLED_GEMS.build_message(name, "error"), uplevel: Gem::BUNDLED_GEMS.uplevel
end end
super super
end end

View File

@ -90,9 +90,9 @@ RSpec.describe "bundled_gems.rb" do
require "openssl" require "openssl"
RUBY RUBY
expect(err).to include(/csv was loaded from (.*) from Ruby 3.4.0/) expect(err).to include(/csv used to be loaded from (.*) since Ruby 3.4.0/)
expect(err).to include(/-e:15/) expect(err).to include(/-e:15/)
expect(err).to include(/openssl was loaded from (.*) from Ruby #{RUBY_VERSION}/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby #{RUBY_VERSION}/)
expect(err).to include(/-e:18/) expect(err).to include(/-e:18/)
end end
@ -112,7 +112,7 @@ RSpec.describe "bundled_gems.rb" do
require "active_support/all" require "active_support/all"
RUBY RUBY
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
expect(err).to include(/lib\/active_support\/all\.rb:1/) expect(err).to include(/lib\/active_support\/all\.rb:1/)
end end
@ -128,7 +128,7 @@ RSpec.describe "bundled_gems.rb" do
end end
RUBY RUBY
expect(err).to include(/net\/smtp was loaded from (.*) from Ruby 3.1.0/) expect(err).to include(/net\/smtp used to be loaded from (.*) since Ruby 3.1.0/)
expect(err).to include(/-e:15/) expect(err).to include(/-e:15/)
expect(err).to include("You can add net-smtp") expect(err).to include("You can add net-smtp")
end end
@ -144,7 +144,7 @@ RSpec.describe "bundled_gems.rb" do
require "openssl/bn" require "openssl/bn"
RUBY RUBY
expect(err).to include(/openssl\/bn is found in openssl, (.*) part of the default gems starting from Ruby #{RUBY_VERSION}/) expect(err).to include(/openssl\/bn is found in openssl, (.*) part of the default gems since Ruby #{RUBY_VERSION}/)
expect(err).to include(/-e:14/) expect(err).to include(/-e:14/)
end end
@ -158,7 +158,7 @@ RSpec.describe "bundled_gems.rb" do
bundle "exec ruby script.rb" bundle "exec ruby script.rb"
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
expect(err).to include(/script\.rb:8/) expect(err).to include(/script\.rb:8/)
end end
@ -176,7 +176,7 @@ RSpec.describe "bundled_gems.rb" do
bundle "exec ./script.rb" bundle "exec ./script.rb"
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
expect(err).to include(/script\.rb:9/) expect(err).to include(/script\.rb:9/)
end end
@ -185,7 +185,7 @@ RSpec.describe "bundled_gems.rb" do
create_file("Gemfile", "source 'https://rubygems.org'") create_file("Gemfile", "source 'https://rubygems.org'")
bundle "exec ruby -r./stub -ropenssl -e ''" bundle "exec ruby -r./stub -ropenssl -e ''"
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
end end
it "Show warning when warn is not the standard one in the current scope" do it "Show warning when warn is not the standard one in the current scope" do
@ -208,7 +208,7 @@ RSpec.describe "bundled_gems.rb" do
My.my My.my
RUBY RUBY
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
expect(err).to include(/-e:19/) expect(err).to include(/-e:19/)
end end
@ -250,7 +250,7 @@ RSpec.describe "bundled_gems.rb" do
require Gem::BUNDLED_GEMS::ARCHDIR + 'openssl' require Gem::BUNDLED_GEMS::ARCHDIR + 'openssl'
RUBY RUBY
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
# TODO: We should assert caller location like below: # TODO: We should assert caller location like below:
# test_warn_bootsnap.rb:14: warning: ... # test_warn_bootsnap.rb:14: warning: ...
end end
@ -270,7 +270,7 @@ RSpec.describe "bundled_gems.rb" do
require Gem::BUNDLED_GEMS::ARCHDIR + "openssl" require Gem::BUNDLED_GEMS::ARCHDIR + "openssl"
RUBY RUBY
expect(err).to include(/openssl was loaded from (.*) from Ruby #{RUBY_VERSION}/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby #{RUBY_VERSION}/)
# TODO: We should assert caller location like below: # TODO: We should assert caller location like below:
# test_warn_bootsnap_rubyarchdir_gem.rb:14: warning: ... # test_warn_bootsnap_rubyarchdir_gem.rb:14: warning: ...
end end
@ -299,7 +299,7 @@ RSpec.describe "bundled_gems.rb" do
require Gem.loaded_specs["fileutils2"].full_gem_path + '/lib/fileutils2' require Gem.loaded_specs["fileutils2"].full_gem_path + '/lib/fileutils2'
RUBY RUBY
expect(err).to include(/fileutils was loaded from (.*) from Ruby #{RUBY_VERSION}/) expect(err).to include(/fileutils used to be loaded from (.*) since Ruby #{RUBY_VERSION}/)
# TODO: We should assert caller location like below: # TODO: We should assert caller location like below:
# $GEM_HOME/gems/childprocess-5.0.0/lib/childprocess.rb:7: warning: # $GEM_HOME/gems/childprocess-5.0.0/lib/childprocess.rb:7: warning:
end end
@ -319,7 +319,7 @@ RSpec.describe "bundled_gems.rb" do
create_file("Gemfile", "source 'https://rubygems.org'") create_file("Gemfile", "source 'https://rubygems.org'")
bundle "exec ruby script.rb" bundle "exec ruby script.rb"
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/) expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
expect(err).to include(/script\.rb:13/) expect(err).to include(/script\.rb:13/)
end end