diff options
author | Bundlerbot <bot@bundler.io> | 2020-01-16 09:53:03 +0000 |
---|---|---|
committer | Bundlerbot <bot@bundler.io> | 2020-01-16 09:53:03 +0000 |
commit | d852b30b66165dbb88682f72a8929568af7e7c57 (patch) | |
tree | 7283342f31444b20580aedfbc64679bd1ab00a2e | |
parent | 14fb40d464562e2f12cf78a8713f6597cc682b00 (diff) | |
parent | e96b6b36146c3fab3a956dddb79c79822ebbb5ab (diff) | |
download | bundler-d852b30b66165dbb88682f72a8929568af7e7c57.tar.gz |
Merge #7522
7522: Improve platform specific gem resolution r=deivid-rodriguez a=kou
### What was the end-user problem that led to this PR?
Platform specific gems aren't resolved when platform specific gems are conflicted.
For example, in the following situation, foo-1.0.0-x64-mingw32 that requires bar<1 is conflicted because there is no bar<1. Without this change, Bundler raises a conflict error. But users can use foo-1.0.0 (no x64-mingw32) in this situation. With this change, Bundler resolves to foo-1.0.0 (no x64-mingw32).
```ruby
@index = build_index do
gem "bar", "1.0.0"
gem "foo", "1.0.0"
gem "foo", "1.0.0", "x64-mingw32" do
dep "bar", "< 1"
end
end
dep "foo"
platforms "x64-mingw32"
````
See also #6247. This change includes the specs that were added in #6247.
### What was your diagnosis of the problem?
Not platform specific gem (foo-1.0.0 in the above case) isn't tried to be resolved when platform specific gem (foo-1.0.0-x64-mingw32 in the above case) is available.
### What is your fix for the problem, implemented in this PR?
In this PR, not platform specific gem (foo-1.0.0 in the above case) is also tried to be resolved even when platform specific gem (foo-1.0.0-x64-mingw32 in the above case) is available. Priority is "platform specific gem" -> "not platform specific gem". So platform specific gem is usable, platform specific gem is used. Not platform specific gem is used as fallback.
`search_for` represents this. Here is the `search_for` specification:
https://github.com/bundler/bundler/blob/master/lib/bundler/vendor/molinillo/lib/molinillo/modules/specification_provider.rb#L11-L13
> Search for the specifications that match the given dependency. The specifications in the returned array will be considered in reverse order, so the latest version ought to be last.
## Why did you choose this fix out of the possible options?
I choose this fix because this is based on Molinillo's specification.
Co-authored-by: Lars Kanis <lars@greiz-reinsdorf.de>
Co-authored-by: Samuel Giddins <segiddins@segiddins.me>
Co-authored-by: Sutou Kouhei <kou@clear-code.com>
-rw-r--r-- | lib/bundler/definition.rb | 24 | ||||
-rw-r--r-- | lib/bundler/dependency.rb | 9 | ||||
-rw-r--r-- | lib/bundler/dsl.rb | 3 | ||||
-rw-r--r-- | lib/bundler/lazy_specification.rb | 8 | ||||
-rw-r--r-- | lib/bundler/resolver.rb | 30 | ||||
-rw-r--r-- | lib/bundler/resolver/spec_group.rb | 31 | ||||
-rw-r--r-- | spec/bundler/dsl_spec.rb | 35 | ||||
-rw-r--r-- | spec/install/gemfile/platform_spec.rb | 4 | ||||
-rw-r--r-- | spec/install/gems/resolving_spec.rb | 11 | ||||
-rw-r--r-- | spec/resolver/platform_spec.rb | 96 | ||||
-rw-r--r-- | spec/runtime/platform_spec.rb | 1 | ||||
-rw-r--r-- | spec/support/indexes.rb | 4 |
12 files changed, 183 insertions, 73 deletions
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb index d6fbb0b5b7..3e5186039a 100644 --- a/lib/bundler/definition.rb +++ b/lib/bundler/definition.rb @@ -82,7 +82,11 @@ module Bundler @lockfile_contents = Bundler.read_file(lockfile) @locked_gems = LockfileParser.new(@lockfile_contents) @locked_platforms = @locked_gems.platforms - @platforms = @locked_platforms.dup + if Bundler.settings[:force_ruby_platform] + @platforms = [Gem::Platform::RUBY] + else + @platforms = @locked_platforms.dup + end @locked_bundler_version = @locked_gems.bundler_version @locked_ruby_version = @locked_gems.ruby_version @@ -228,12 +232,13 @@ module Bundler end def current_dependencies - dependencies.select(&:should_include?) + dependencies.select do |d| + d.should_include? && !d.gem_platforms(@platforms).empty? + end end def specs_for(groups) - deps = dependencies.select {|d| (d.groups & groups).any? } - deps.delete_if {|d| !d.should_include? } + deps = dependencies_for(groups) specs.for(expand_dependencies(deps)) end @@ -706,9 +711,6 @@ module Bundler elsif dep.source dep.source = sources.get(dep.source) end - if dep.source.is_a?(Source::Gemspec) - dep.platforms.concat(@platforms.map {|p| Dependency::REVERSE_PLATFORM_MAP[p] }.flatten(1)).uniq! - end end changes = false @@ -907,10 +909,16 @@ module Bundler deps end + def dependencies_for(groups) + current_dependencies.reject do |d| + (d.groups & groups).empty? + end + end + def requested_dependencies groups = requested_groups groups.map!(&:to_sym) - dependencies.reject {|d| !d.should_include? || (d.groups & groups).empty? } + dependencies_for(groups) end def source_requirements diff --git a/lib/bundler/dependency.rb b/lib/bundler/dependency.rb index 6c2642163e..26e5f3d1a5 100644 --- a/lib/bundler/dependency.rb +++ b/lib/bundler/dependency.rb @@ -74,15 +74,6 @@ module Bundler :x64_mingw_26 => Gem::Platform::X64_MINGW, }.freeze - REVERSE_PLATFORM_MAP = {}.tap do |reverse_platform_map| - PLATFORM_MAP.each do |key, value| - reverse_platform_map[value] ||= [] - reverse_platform_map[value] << key - end - - reverse_platform_map.each {|_, platforms| platforms.freeze } - end.freeze - def initialize(name, version, options = {}, &blk) type = options["type"] || :runtime super(name, version, type) diff --git a/lib/bundler/dsl.rb b/lib/bundler/dsl.rb index 99a369281a..bb92a28381 100644 --- a/lib/bundler/dsl.rb +++ b/lib/bundler/dsl.rb @@ -75,8 +75,7 @@ module Bundler @gemspecs << spec - gem_platforms = Bundler::Dependency::REVERSE_PLATFORM_MAP[Bundler::GemHelpers.generic_local_platform] - gem spec.name, :name => spec.name, :path => path, :glob => glob, :platforms => gem_platforms + gem spec.name, :name => spec.name, :path => path, :glob => glob group(development_group) do spec.development_dependencies.each do |dep| diff --git a/lib/bundler/lazy_specification.rb b/lib/bundler/lazy_specification.rb index 32c8bb9557..7a6f5614ef 100644 --- a/lib/bundler/lazy_specification.rb +++ b/lib/bundler/lazy_specification.rb @@ -46,6 +46,14 @@ module Bundler identifier == other.identifier end + def eql?(other) + identifier.eql?(other.identifier) + end + + def hash + identifier.hash + end + def satisfies?(dependency) @name == dependency.name && dependency.requirement.satisfied_by?(Gem::Version.new(@version)) end diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb index c7caf01c7d..2374ed3e5a 100644 --- a/lib/bundler/resolver.rb +++ b/lib/bundler/resolver.rb @@ -146,7 +146,26 @@ module Bundler @gem_version_promoter.sort_versions(dependency, spec_groups) end end - search.select {|sg| sg.for?(platform) }.each {|sg| sg.activate_platform!(platform) } + selected_sgs = [] + search.each do |sg| + next unless sg.for?(platform) + # Add a spec group for "non platform specific spec" as the fallback + # spec group. + sg_ruby = sg.copy_for(Gem::Platform::RUBY) + selected_sgs << sg_ruby if sg_ruby + sg_all_platforms = nil + all_platforms = @platforms + [platform] + sorted_all_platforms = self.class.sort_platforms(all_platforms) + sorted_all_platforms.reverse_each do |other_platform| + if sg_all_platforms.nil? + sg_all_platforms = sg.copy_for(other_platform) + else + sg_all_platforms.activate_platform!(other_platform) + end + end + selected_sgs << sg_all_platforms + end + selected_sgs end def index_for(dependency) @@ -183,9 +202,7 @@ module Bundler end def requirement_satisfied_by?(requirement, activated, spec) - return false unless requirement.matches_spec?(spec) || spec.source.is_a?(Source::Gemspec) - spec.activate_platform!(requirement.__platform) if !@platforms || @platforms.include?(requirement.__platform) - true + requirement.matches_spec?(spec) || spec.source.is_a?(Source::Gemspec) end def relevant_sources_for_vertex(vertex) @@ -223,8 +240,9 @@ module Bundler end def self.platform_sort_key(platform) - return ["", "", ""] if Gem::Platform::RUBY == platform - platform.to_a.map {|part| part || "" } + # Prefer specific platform to not specific platform + return ["99-LAST", "", "", ""] if Gem::Platform::RUBY == platform + ["00", *platform.to_a.map {|part| part || "" }] end private diff --git a/lib/bundler/resolver/spec_group.rb b/lib/bundler/resolver/spec_group.rb index e5772eed81..d5d12f7a2d 100644 --- a/lib/bundler/resolver/spec_group.rb +++ b/lib/bundler/resolver/spec_group.rb @@ -9,6 +9,7 @@ module Bundler attr_accessor :ignores_bundler_dependencies def initialize(all_specs) + @all_specs = all_specs raise ArgumentError, "cannot initialize with an empty value" unless exemplary_spec = all_specs.first @name = exemplary_spec.name @version = exemplary_spec.version @@ -28,7 +29,7 @@ module Bundler lazy_spec = LazySpecification.new(name, version, s.platform, source) lazy_spec.dependencies.replace s.dependencies lazy_spec - end.compact + end.compact.uniq end def activate_platform!(platform) @@ -37,13 +38,25 @@ module Bundler @activated_platforms << platform end + def copy_for(platform) + copied_sg = self.class.new(@all_specs) + copied_sg.ignores_bundler_dependencies = @ignores_bundler_dependencies + return nil unless copied_sg.for?(platform) + copied_sg.activate_platform!(platform) + copied_sg + end + + def spec_for(platform) + @specs[platform] + end + def for?(platform) - spec = @specs[platform] - !spec.nil? + !spec_for(platform).nil? end def to_s - @to_s ||= "#{name} (#{version})" + activated_platforms_string = sorted_activated_platforms.join(", ") + "#{name} (#{version}) (#{activated_platforms_string})" end def dependencies_for_activated_platforms @@ -58,6 +71,7 @@ module Bundler return unless other.is_a?(SpecGroup) name == other.name && version == other.version && + sorted_activated_platforms == other.sorted_activated_platforms && source == other.source end @@ -65,11 +79,18 @@ module Bundler return unless other.is_a?(SpecGroup) name.eql?(other.name) && version.eql?(other.version) && + sorted_activated_platforms.eql?(other.sorted_activated_platforms) && source.eql?(other.source) end def hash - to_s.hash ^ source.hash + name.hash ^ version.hash ^ sorted_activated_platforms.hash ^ source.hash + end + + protected + + def sorted_activated_platforms + @activated_platforms.sort_by(&:to_s) end private diff --git a/spec/bundler/dsl_spec.rb b/spec/bundler/dsl_spec.rb index 319472d4b0..9299c014af 100644 --- a/spec/bundler/dsl_spec.rb +++ b/spec/bundler/dsl_spec.rb @@ -174,41 +174,6 @@ RSpec.describe Bundler::Dsl do end end - describe "#gemspec" do - let(:spec) do - Gem::Specification.new do |gem| - gem.name = "example" - gem.platform = platform - end - end - - before do - allow(Dir).to receive(:[]).and_return(["spec_path"]) - allow(Bundler).to receive(:load_gemspec).with("spec_path").and_return(spec) - allow(Bundler).to receive(:default_gemfile).and_return(Pathname.new("./Gemfile")) - end - - context "with a ruby platform" do - let(:platform) { "ruby" } - - it "keeps track of the ruby platforms in the dependency" do - allow(Gem::Platform).to receive(:local).and_return(rb) - subject.gemspec - expect(subject.dependencies.last.platforms).to eq(Bundler::Dependency::REVERSE_PLATFORM_MAP[Gem::Platform::RUBY]) - end - end - - context "with a jruby platform" do - let(:platform) { "java" } - - it "keeps track of the jruby platforms in the dependency" do - allow(Gem::Platform).to receive(:local).and_return(java) - subject.gemspec - expect(subject.dependencies.last.platforms).to eq(Bundler::Dependency::REVERSE_PLATFORM_MAP[Gem::Platform::JAVA]) - end - end - end - context "can bundle groups of gems with" do # git "https://github.com/rails/rails.git" do # gem "railties" diff --git a/spec/install/gemfile/platform_spec.rb b/spec/install/gemfile/platform_spec.rb index eab5e7f00c..52e1cf86fa 100644 --- a/spec/install/gemfile/platform_spec.rb +++ b/spec/install/gemfile/platform_spec.rb @@ -379,8 +379,6 @@ RSpec.describe "bundle install with platform conditionals" do end it "prints a helpful warning when a dependency is unused on any platform" do - skip "prints warning but bundle install fails" if Gem.win_platform? - simulate_platform "ruby" simulate_ruby_engine "ruby" @@ -401,8 +399,6 @@ The dependency #{Gem::Dependency.new("rack", ">= 0")} will be unused by any of t before { bundle! "config set disable_platform_warnings true" } it "does not print the warning when a dependency is unused on any platform" do - skip "skips warning but bundle install fails" if Gem.win_platform? - simulate_platform "ruby" simulate_ruby_engine "ruby" diff --git a/spec/install/gems/resolving_spec.rb b/spec/install/gems/resolving_spec.rb index 523f9eb151..547d13134f 100644 --- a/spec/install/gems/resolving_spec.rb +++ b/spec/install/gems/resolving_spec.rb @@ -156,6 +156,13 @@ RSpec.describe "bundle install with install-time dependencies" do let(:ruby_requirement) { %("#{RUBY_VERSION}") } let(:error_message_requirement) { "~> #{RUBY_VERSION}.0" } + let(:error_message_platform) do + if Bundler.feature_flag.specific_platform? + " #{Bundler.local_platform}" + else + "" + end + end shared_examples_for "ruby version conflicts" do it "raises an error during resolution" do @@ -172,9 +179,9 @@ RSpec.describe "bundle install with install-time dependencies" do nice_error = strip_whitespace(<<-E).strip Bundler found conflicting requirements for the Ruby\0 version: In Gemfile: - Ruby\0 (#{error_message_requirement}) + Ruby\0 (#{error_message_requirement})#{error_message_platform} - require_ruby was resolved to 1.0, which depends on + require_ruby#{error_message_platform} was resolved to 1.0, which depends on Ruby\0 (> 9000) Ruby\0 (> 9000), which is required by gem 'require_ruby', is not available in the local ruby installation diff --git a/spec/resolver/platform_spec.rb b/spec/resolver/platform_spec.rb index fee0cf1f1c..415c5458df 100644 --- a/spec/resolver/platform_spec.rb +++ b/spec/resolver/platform_spec.rb @@ -28,6 +28,98 @@ RSpec.describe "Resolving platform craziness" do end end + it "takes the latest ruby gem, even if an older platform specific version is available" do + @index = build_index do + gem "foo", "1.0.0" + gem "foo", "1.0.0", "x64-mingw32" + gem "foo", "1.1.0" + end + dep "foo" + platforms "x64-mingw32" + + should_resolve_as %w[foo-1.1.0] + end + + it "takes the ruby version if the platform version is incompatible" do + @index = build_index do + gem "bar", "1.0.0" + gem "foo", "1.0.0" + gem "foo", "1.0.0", "x64-mingw32" do + dep "bar", "< 1" + end + end + dep "foo" + platforms "x64-mingw32" + + should_resolve_as %w[foo-1.0.0] + end + + it "prefers the platform specific gem to the ruby version" do + @index = build_index do + gem "foo", "1.0.0" + gem "foo", "1.0.0", "x64-mingw32" + end + dep "foo" + platforms "x64-mingw32" + + should_resolve_as %w[foo-1.0.0-x64-mingw32] + end + + it "takes the latest ruby gem if the platform specific gem doesn't match the required_ruby_version" do + @index = build_index do + gem "foo", "1.0.0" + gem "foo", "1.0.0", "x64-mingw32" + gem "foo", "1.1.0" + gem "foo", "1.1.0", "x64-mingw32" do |s| + s.required_ruby_version = [">= 2.0", "< 2.4"] + end + gem "Ruby\0", "2.5.1" + end + dep "foo" + dep "Ruby\0", "2.5.1" + platforms "x64-mingw32" + + should_resolve_as %w[foo-1.1.0] + end + + it "takes the latest ruby gem with required_ruby_version if the platform specific gem doesn't match the required_ruby_version" do + @index = build_index do + gem "foo", "1.0.0" + gem "foo", "1.0.0", "x64-mingw32" + gem "foo", "1.1.0" do |s| + s.required_ruby_version = [">= 2.0"] + end + gem "foo", "1.1.0", "x64-mingw32" do |s| + s.required_ruby_version = [">= 2.0", "< 2.4"] + end + gem "Ruby\0", "2.5.1" + end + dep "foo" + dep "Ruby\0", "2.5.1" + platforms "x64-mingw32" + + should_resolve_as %w[foo-1.1.0] + end + + it "takes the latest ruby gem if the platform specific gem doesn't match the required_ruby_version with multiple platforms" do + @index = build_index do + gem "foo", "1.0.0" + gem "foo", "1.0.0", "x64-mingw32" + gem "foo", "1.1.0" do |s| + s.required_ruby_version = [">= 2.0"] + end + gem "foo", "1.1.0", "x64-mingw32" do |s| + s.required_ruby_version = [">= 2.0", "< 2.4"] + end + gem "Ruby\0", "2.5.1" + end + dep "foo" + dep "Ruby\0", "2.5.1" + platforms "x86_64-linux", "x64-mingw32" + + should_resolve_as %w[foo-1.1.0] + end + describe "with mingw32" do before :each do @index = build_index do @@ -90,11 +182,11 @@ RSpec.describe "Resolving platform craziness" do end end - it "reports on the conflict" do + it "takes the ruby version as fallback" do platforms "ruby", "java" dep "foo" - should_conflict_on "baz" + should_resolve_as %w[bar-1.0.0 baz-1.0.0 foo-1.0.0] end end end diff --git a/spec/runtime/platform_spec.rb b/spec/runtime/platform_spec.rb index ad9c56d14e..70c7594395 100644 --- a/spec/runtime/platform_spec.rb +++ b/spec/runtime/platform_spec.rb @@ -113,6 +113,7 @@ RSpec.describe "Bundler.setup with multi platform stuff" do bundle! "install" expect(the_bundle).to include_gems "platform_specific 1.0 RUBY" + expect(the_bundle).to not_include_gems "nokogiri" end end diff --git a/spec/support/indexes.rb b/spec/support/indexes.rb index dc6e0bd1e9..7440523fc9 100644 --- a/spec/support/indexes.rb +++ b/spec/support/indexes.rb @@ -26,6 +26,10 @@ module Spec end end source_requirements ||= {} + args[0] ||= [] # base + args[1] ||= Bundler::GemVersionPromoter.new # gem_version_promoter + args[2] ||= [] # additional_base_requirements + args[3] ||= @platforms # platforms Bundler::Resolver.resolve(deps, @index, source_requirements, *args) end |