From c83370671be81809e027476293151bd2c67e8beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Wed, 29 Jan 2025 20:17:17 +0100 Subject: [PATCH] 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. --- lib/bundled_gems.rb | 29 ++++++++++++++++++----------- spec/bundled_gems_spec.rb | 26 +++++++++++++------------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/lib/bundled_gems.rb b/lib/bundled_gems.rb index 590a55fd64..4e96c555d3 100644 --- a/lib/bundled_gems.rb +++ b/lib/bundled_gems.rb @@ -101,7 +101,7 @@ module Gem::BUNDLED_GEMS # :nodoc: def self.warning?(name, specs: nil) # 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 # 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?("/") # bootsnap expands `require "csv"` to `require "#{LIBDIR}/csv.rb"`, # and `require "syslog"` to `require "#{ARCHDIR}/syslog.so"`. - name = feature.delete_prefix(ARCHDIR).delete_prefix(LIBDIR).sub(LIBEXT, "") - segments = name.split("/") + feature.delete_prefix!(ARCHDIR) + feature.delete_prefix!(LIBDIR) + segments = feature.split("/") name = segments.shift name = EXACT[name] || name if !SINCE[name] @@ -119,8 +120,7 @@ module Gem::BUNDLED_GEMS # :nodoc: end segments.any? else - name = feature.sub(LIBEXT, "") - name = EXACT[name] || name + name = EXACT[feature] || feature return unless SINCE[name] false end @@ -130,18 +130,25 @@ module Gem::BUNDLED_GEMS # :nodoc: return if WARNED[name] WARNED[name] = true + level = RUBY_VERSION < SINCE[name] ? "warning" : "error" + if subfeature "#{feature} is found in #{name}, which" else - "#{feature} was loaded from the standard library, but" - end + build_message(name) + "#{feature} #{level == "warning" ? "was loaded" : "used to be loaded"} from the standard library, but" + end + build_message(name, level) end - def self.build_message(name) - msg = " #{RUBY_VERSION < SINCE[name] ? "will no longer be" : "is not"} part of the default gems starting from Ruby #{SINCE[name]}." + def self.build_message(name, level) + 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) - 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` # then take the first frame that's not from `require`. @@ -230,7 +237,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), uplevel: Gem::BUNDLED_GEMS.uplevel + warn name + Gem::BUNDLED_GEMS.build_message(name, "error"), uplevel: Gem::BUNDLED_GEMS.uplevel end super end diff --git a/spec/bundled_gems_spec.rb b/spec/bundled_gems_spec.rb index 8d4d42bd32..0eb142b7ab 100644 --- a/spec/bundled_gems_spec.rb +++ b/spec/bundled_gems_spec.rb @@ -90,9 +90,9 @@ RSpec.describe "bundled_gems.rb" do require "openssl" 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(/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/) end @@ -112,7 +112,7 @@ RSpec.describe "bundled_gems.rb" do require "active_support/all" 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/) end @@ -128,7 +128,7 @@ RSpec.describe "bundled_gems.rb" do end 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("You can add net-smtp") end @@ -144,7 +144,7 @@ RSpec.describe "bundled_gems.rb" do require "openssl/bn" 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/) end @@ -158,7 +158,7 @@ RSpec.describe "bundled_gems.rb" do 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/) end @@ -176,7 +176,7 @@ RSpec.describe "bundled_gems.rb" do 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/) end @@ -185,7 +185,7 @@ RSpec.describe "bundled_gems.rb" do create_file("Gemfile", "source 'https://rubygems.org'") 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 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 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/) end @@ -250,7 +250,7 @@ RSpec.describe "bundled_gems.rb" do require Gem::BUNDLED_GEMS::ARCHDIR + 'openssl' 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: # test_warn_bootsnap.rb:14: warning: ... end @@ -270,7 +270,7 @@ RSpec.describe "bundled_gems.rb" do require Gem::BUNDLED_GEMS::ARCHDIR + "openssl" 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: # test_warn_bootsnap_rubyarchdir_gem.rb:14: warning: ... end @@ -299,7 +299,7 @@ RSpec.describe "bundled_gems.rb" do require Gem.loaded_specs["fileutils2"].full_gem_path + '/lib/fileutils2' 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: # $GEM_HOME/gems/childprocess-5.0.0/lib/childprocess.rb:7: warning: end @@ -319,7 +319,7 @@ RSpec.describe "bundled_gems.rb" do create_file("Gemfile", "source 'https://rubygems.org'") 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/) end