diff options
author | The Bundler Bot <bot@bundler.io> | 2017-09-10 16:34:04 +0000 |
---|---|---|
committer | The Bundler Bot <bot@bundler.io> | 2017-09-10 16:34:04 +0000 |
commit | 5df439c0f811661a51c1683ec243b7cc0efca4bf (patch) | |
tree | 1447080c41a8f9422b0345afa90a144dd4e5fabf | |
parent | b3cbf4a7538ca1d506a81706171b81c173bd048c (diff) | |
parent | 789974c1d3e08830b49d7517b943d33d24ed2192 (diff) | |
download | bundler-5df439c0f811661a51c1683ec243b7cc0efca4bf.tar.gz |
Auto merge of #5985 - bundler:seg-multisource-error, r=indirect
[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 https://github.com/bundler/bundler/issues/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.
-rw-r--r-- | lib/bundler/resolver.rb | 31 | ||||
-rw-r--r-- | spec/install/gemfile/sources_spec.rb | 29 |
2 files changed, 57 insertions, 3 deletions
diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb index cc87bcb0cb..78152d71cd 100644 --- a/lib/bundler/resolver.rb +++ b/lib/bundler/resolver.rb @@ -44,9 +44,12 @@ module Bundler 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) @@ -354,5 +357,29 @@ module Bundler :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 diff --git a/spec/install/gemfile/sources_spec.rb b/spec/install/gemfile/sources_spec.rb index 0b837f87a1..8d69a663de 100644 --- a/spec/install/gemfile/sources_spec.rb +++ b/spec/install/gemfile/sources_spec.rb @@ -545,7 +545,7 @@ RSpec.describe "bundle install with gems on multiple sources" do 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 @@ -617,4 +617,31 @@ RSpec.describe "bundle install with gems on multiple sources" do 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 |