Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Auto merge of #5985 - bundler:seg-multisource-error, r=indirect
Browse files Browse the repository at this point in the history
[2.0] [Resolver] Error when it is ambigous which transitive source a gem should come from

### What was the end-user problem that led to this PR?

The problem was the "source priority" in ambiguous source situations was ... ambiguous.

### What was your diagnosis of the problem?

My diagnosis was we should error and require a user explicitly pin the dependency to a source in those situations, rather than leaving the source used up to an implementation detail.

### What is your fix for the problem, implemented in this PR?

My fix attempts to implement the priority described in the conversation in #4629.

### Why did you choose this fix out of the possible options?

I chose this fix because it still allows using the default source as a backup, while only taking the "relevant" sources into account, so that the error/warning is not overzealous.
  • Loading branch information
bundlerbot committed Sep 10, 2017
2 parents b3cbf4a + 789974c commit 5df439c
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 3 deletions.
31 changes: 29 additions & 2 deletions lib/bundler/resolver.rb
Expand Up @@ -44,9 +44,12 @@ def initialize(index, source_requirements, base, gem_version_promoter, additiona
def start(requirements)
verify_gemfile_dependencies_are_found!(requirements)
dg = @resolver.resolve(requirements, @base_dg)
dg.map(&:payload).
dg.
tap {|resolved| validate_resolved_specs!(resolved) }.
map(&:payload).
reject {|sg| sg.name.end_with?("\0") }.
map(&:to_specs).flatten
map(&:to_specs).
flatten
rescue Molinillo::VersionConflict => e
message = version_conflict_message(e)
raise VersionConflict.new(e.conflicts.keys.uniq, message)
Expand Down Expand Up @@ -354,5 +357,29 @@ def version_conflict_message(e)
:version_for_spec => lambda {|spec| spec.version }
)
end

def validate_resolved_specs!(resolved_specs)
resolved_specs.each do |v|
name = v.name
next unless sources = relevant_sources_for_vertex(v)
sources.compact!
if default_index = sources.index(@source_requirements[:default])
sources.delete_at(default_index)
end
sources.reject! {|s| s.specs[name].empty? }
sources.uniq!
next if sources.size <= 1

multisource_disabled = Bundler.feature_flag.disable_multisource?

msg = ["The gem '#{name}' was found in multiple relevant sources."]
msg.concat sources.map {|s| " * #{s}" }.sort
msg << "You #{multisource_disabled ? :must : :should} add this gem to the source block for the source you wish it to be installed from."
msg = msg.join("\n")

raise SecurityError, msg if multisource_disabled
Bundler.ui.error "Warning: #{msg}"
end
end
end
end
29 changes: 28 additions & 1 deletion spec/install/gemfile/sources_spec.rb
Expand Up @@ -545,7 +545,7 @@
end

it "does not re-resolve" do
bundle :install, :verbose => true
bundle! :install, :verbose => true
expect(out).to include("using resolution from the lockfile")
expect(out).not_to include("re-resolving dependencies")
end
Expand Down Expand Up @@ -617,4 +617,31 @@
end
end
end

context "when a gem is available from multiple ambiguous sources", :bundler => "2" do
it "raises, suggesting a source block" do
build_repo4 do
build_gem "depends_on_rack" do |s|
s.add_dependency "rack"
end
build_gem "rack"
end

install_gemfile <<-G
source "file:#{gem_repo4}"
source "file:#{gem_repo1}" do
gem "thin"
end
gem "depends_on_rack"
G
expect(last_command).to be_failure
expect(last_command.stderr).to eq strip_whitespace(<<-EOS).strip
The gem 'rack' was found in multiple relevant sources.
* rubygems repository file:#{gem_repo1}/ or installed locally
* rubygems repository file:#{gem_repo4}/ or installed locally
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
end
end
end

0 comments on commit 5df439c

Please sign in to comment.