Skip to content

Commit

Permalink
[rubygems/rubygems] Improve sources representation
Browse files Browse the repository at this point in the history
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.

rubygems/rubygems@b5f2b88957
  • Loading branch information
deivid-rodriguez authored and matzbot committed Dec 3, 2021
1 parent 7d974cc commit 248fae0
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 21 deletions.
4 changes: 2 additions & 2 deletions lib/bundler/cli/update.rb
Expand Up @@ -66,7 +66,7 @@ def run

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
Expand Down Expand Up @@ -95,7 +95,7 @@ def run
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
Expand Down
1 change: 1 addition & 0 deletions lib/bundler/plugin/api/source.rb
Expand Up @@ -283,6 +283,7 @@ def to_lock
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)
Expand Down
4 changes: 2 additions & 2 deletions lib/bundler/resolver.rb
Expand Up @@ -270,7 +270,7 @@ def verify_gemfile_dependencies_are_found!(requirements)
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
Expand Down Expand Up @@ -369,7 +369,7 @@ def version_conflict_message(e)
o << if metadata_requirement
"is not available in #{relevant_source}"
else
"in #{relevant_source.to_err}.\n"
"in #{relevant_source}.\n"
end
end
end,
Expand Down
2 changes: 1 addition & 1 deletion lib/bundler/source.rb
Expand Up @@ -67,7 +67,7 @@ def inspect
"#<#{self.class}:0x#{object_id} #{self}>"
end

def to_err
def identifier
to_s
end

Expand Down
18 changes: 11 additions & 7 deletions lib/bundler/source/rubygems.rb
Expand Up @@ -98,26 +98,30 @@ def to_lock
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
Expand Down
2 changes: 1 addition & 1 deletion lib/bundler/source/rubygems_aggregate.rb
Expand Up @@ -16,7 +16,7 @@ def specs
@index
end

def to_err
def identifier
to_s
end

Expand Down
6 changes: 3 additions & 3 deletions lib/bundler/source_list.rb
Expand Up @@ -106,14 +106,14 @@ def lock_sources
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

Expand Down Expand Up @@ -211,7 +211,7 @@ def warn_on_git_protocol(source)
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)
Expand Down
8 changes: 4 additions & 4 deletions spec/bundler/install/gemfile/sources_spec.rb
Expand Up @@ -1336,8 +1336,8 @@
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
Expand Down Expand Up @@ -1366,8 +1366,8 @@
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
Expand Down
2 changes: 1 addition & 1 deletion spec/bundler/support/indexes.rb
Expand Up @@ -17,7 +17,7 @@ def platform(*args)
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
Expand Down

0 comments on commit 248fae0

Please sign in to comment.