From 248fae0ec43c2df6d9b545acc7f56bdfd5f89dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Thu, 11 Nov 2021 23:58:40 +0100 Subject: [PATCH] [rubygems/rubygems] Improve sources representation We have two representations of a source. Once used for sorting, which should not depend on the source's state, but solely on its static information, like remotes. Another one used for error and informational messages, which should properly inform about the exact state of the source when the message is printed. This commit makes the latter be the default implementation of `to_s`, so that error and informational messages are more accurate by default. https://github.com/rubygems/rubygems/commit/b5f2b88957 --- lib/bundler/cli/update.rb | 4 ++-- lib/bundler/plugin/api/source.rb | 1 + lib/bundler/resolver.rb | 4 ++-- lib/bundler/source.rb | 2 +- lib/bundler/source/rubygems.rb | 18 +++++++++++------- lib/bundler/source/rubygems_aggregate.rb | 2 +- lib/bundler/source_list.rb | 6 +++--- spec/bundler/install/gemfile/sources_spec.rb | 8 ++++---- spec/bundler/support/indexes.rb | 2 +- 9 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/bundler/cli/update.rb b/lib/bundler/cli/update.rb index 1adcaef67c..95a8886ea5 100644 --- a/lib/bundler/cli/update.rb +++ b/lib/bundler/cli/update.rb @@ -66,7 +66,7 @@ module Bundler if locked_gems = Bundler.definition.locked_gems previous_locked_info = locked_gems.specs.reduce({}) do |h, s| - h[s.name] = { :spec => s, :version => s.version, :source => s.source.to_s } + h[s.name] = { :spec => s, :version => s.version, :source => s.source.identifier } h end end @@ -95,7 +95,7 @@ module Bundler end locked_source = locked_info[:source] - new_source = new_spec.source.to_s + new_source = new_spec.source.identifier next if locked_source != new_source new_version = new_spec.version diff --git a/lib/bundler/plugin/api/source.rb b/lib/bundler/plugin/api/source.rb index f6f4ac4f0a..32b1d0ee38 100644 --- a/lib/bundler/plugin/api/source.rb +++ b/lib/bundler/plugin/api/source.rb @@ -283,6 +283,7 @@ module Bundler def to_s "plugin source for #{@type} with uri #{@uri}" end + alias_method :identifier, :to_s # Note: Do not override if you don't know what you are doing. def include?(other) diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb index 56002ace60..5eb17a3921 100644 --- a/lib/bundler/resolver.rb +++ b/lib/bundler/resolver.rb @@ -270,7 +270,7 @@ module Bundler rescue GemfileNotFound nil end - message = String.new("Could not find gem '#{SharedHelpers.pretty_dependency(requirement)}' in #{source.to_err}#{cache_message}.\n") + message = String.new("Could not find gem '#{SharedHelpers.pretty_dependency(requirement)}' in #{source}#{cache_message}.\n") message << "The source contains the following versions of '#{name}': #{formatted_versions_with_platforms(versions_with_platforms)}" if versions_with_platforms.any? end raise GemNotFound, message @@ -369,7 +369,7 @@ module Bundler o << if metadata_requirement "is not available in #{relevant_source}" else - "in #{relevant_source.to_err}.\n" + "in #{relevant_source}.\n" end end end, diff --git a/lib/bundler/source.rb b/lib/bundler/source.rb index 434112ac8a..2a2b332cff 100644 --- a/lib/bundler/source.rb +++ b/lib/bundler/source.rb @@ -67,7 +67,7 @@ module Bundler "#<#{self.class}:0x#{object_id} #{self}>" end - def to_err + def identifier to_s end diff --git a/lib/bundler/source/rubygems.rb b/lib/bundler/source/rubygems.rb index 2f9d1f5a12..8bc3aa17e9 100644 --- a/lib/bundler/source/rubygems.rb +++ b/lib/bundler/source/rubygems.rb @@ -98,26 +98,30 @@ module Bundler out << " specs:\n" end - def to_err + def to_s if remotes.empty? "locally installed gems" - elsif @allow_remote + elsif @allow_remote && @allow_cached && @allow_local + "rubygems repository #{remote_names}, cached gems or installed locally" + elsif @allow_remote && @allow_local "rubygems repository #{remote_names} or installed locally" - elsif @allow_cached - "cached gems from rubygems repository #{remote_names} or installed locally" + elsif @allow_remote + "rubygems repository #{remote_names}" + elsif @allow_cached && @allow_local + "cached gems or installed locally" else "locally installed gems" end end - def to_s + def identifier if remotes.empty? "locally installed gems" else - "rubygems repository #{remote_names} or installed locally" + "rubygems repository #{remote_names}" end end - alias_method :name, :to_s + alias_method :name, :identifier def specs @specs ||= begin diff --git a/lib/bundler/source/rubygems_aggregate.rb b/lib/bundler/source/rubygems_aggregate.rb index 09cf4002ea..99ef81ad54 100644 --- a/lib/bundler/source/rubygems_aggregate.rb +++ b/lib/bundler/source/rubygems_aggregate.rb @@ -16,7 +16,7 @@ module Bundler @index end - def to_err + def identifier to_s end diff --git a/lib/bundler/source_list.rb b/lib/bundler/source_list.rb index ba356d40ad..a4773397c7 100644 --- a/lib/bundler/source_list.rb +++ b/lib/bundler/source_list.rb @@ -106,14 +106,14 @@ module Bundler end def lock_other_sources - (path_sources + git_sources + plugin_sources).sort_by(&:to_s) + (path_sources + git_sources + plugin_sources).sort_by(&:identifier) end def lock_rubygems_sources if merged_gem_lockfile_sections? [combine_rubygems_sources] else - rubygems_sources.sort_by(&:to_s) + rubygems_sources.sort_by(&:identifier) end end @@ -211,7 +211,7 @@ module Bundler end def equivalent_sources?(lock_sources, replacement_sources) - lock_sources.sort_by(&:to_s) == replacement_sources.sort_by(&:to_s) + lock_sources.sort_by(&:identifier) == replacement_sources.sort_by(&:identifier) end def equivalent_source?(source, other_source) diff --git a/spec/bundler/install/gemfile/sources_spec.rb b/spec/bundler/install/gemfile/sources_spec.rb index 7a0c148254..5456e95f33 100644 --- a/spec/bundler/install/gemfile/sources_spec.rb +++ b/spec/bundler/install/gemfile/sources_spec.rb @@ -1336,8 +1336,8 @@ RSpec.describe "bundle install with gems on multiple sources" do G expect(err).to eq strip_whitespace(<<-EOS).strip Warning: The gem 'rack' was found in multiple relevant sources. - * rubygems repository https://gem.repo1/ or installed locally - * rubygems repository https://gem.repo4/ or installed locally + * rubygems repository https://gem.repo1/ + * rubygems repository https://gem.repo4/ You should add this gem to the source block for the source you wish it to be installed from. EOS expect(last_command).to be_success @@ -1366,8 +1366,8 @@ RSpec.describe "bundle install with gems on multiple sources" do expect(last_command).to be_failure expect(err).to eq strip_whitespace(<<-EOS).strip The gem 'rack' was found in multiple relevant sources. - * rubygems repository https://gem.repo1/ or installed locally - * rubygems repository https://gem.repo4/ or installed locally + * rubygems repository https://gem.repo1/ + * rubygems repository https://gem.repo4/ You must add this gem to the source block for the source you wish it to be installed from. EOS expect(the_bundle).not_to be_locked diff --git a/spec/bundler/support/indexes.rb b/spec/bundler/support/indexes.rb index 91dd699b5f..638f394e76 100644 --- a/spec/bundler/support/indexes.rb +++ b/spec/bundler/support/indexes.rb @@ -17,7 +17,7 @@ module Spec def resolve(args = []) @platforms ||= ["ruby"] deps = [] - default_source = instance_double("Bundler::Source::Rubygems", :specs => @index, :to_err => "locally install gems") + default_source = instance_double("Bundler::Source::Rubygems", :specs => @index, :to_s => "locally install gems") source_requirements = { :default => default_source } @deps.each do |d| source_requirements[d.name] = d.source = default_source