From 0c0d40d4173be6ce06c7cb3186f70aeafb5222ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Sat, 23 Jul 2022 08:44:47 +0200 Subject: [PATCH] Don't discard candidates matching ruby metadata Do dependency filtering and materialization in one step. Before, dependency filtering would not consider ruby metadata so it would discard variants that end up not being materializable in the end. Co-authored-by: Ian Ker-Seymer --- bundler/lib/bundler/gem_helpers.rb | 8 +++- bundler/lib/bundler/lazy_specification.rb | 6 ++- bundler/lib/bundler/spec_set.rb | 7 +-- .../install/gemfile/specific_platform_spec.rb | 47 +++++++++++++++++++ bundler/spec/install/gems/resolving_spec.rb | 22 +++++++-- 5 files changed, 78 insertions(+), 12 deletions(-) diff --git a/bundler/lib/bundler/gem_helpers.rb b/bundler/lib/bundler/gem_helpers.rb index 6bc4fb4991fb..0d50d8687b90 100644 --- a/bundler/lib/bundler/gem_helpers.rb +++ b/bundler/lib/bundler/gem_helpers.rb @@ -44,6 +44,12 @@ def platform_specificity_match(spec_platform, user_platform) def select_best_platform_match(specs, platform) matching = specs.select {|spec| spec.match_platform(platform) } + + sort_best_platform_match(matching, platform) + end + module_function :select_best_platform_match + + def sort_best_platform_match(matching, platform) exact = matching.select {|spec| spec.platform == platform } return exact if exact.any? @@ -52,7 +58,7 @@ def select_best_platform_match(specs, platform) sorted_matching.take_while {|spec| same_specificity(platform, spec, exemplary_spec) && same_deps(spec, exemplary_spec) } end - module_function :select_best_platform_match + module_function :sort_best_platform_match class PlatformMatch def self.specificity_score(spec_platform, user_platform) diff --git a/bundler/lib/bundler/lazy_specification.rb b/bundler/lib/bundler/lazy_specification.rb index 805afba51fb1..a88172d96b85 100644 --- a/bundler/lib/bundler/lazy_specification.rb +++ b/bundler/lib/bundler/lazy_specification.rb @@ -88,6 +88,8 @@ def materialize_for_installation source.specs.search(self) end + return self if candidates.empty? + __materialize__(candidates) end @@ -105,8 +107,8 @@ def __materialize__(candidates) spec.is_a?(StubSpecification) || (spec.required_ruby_version.satisfied_by?(Gem.ruby_version) && spec.required_rubygems_version.satisfied_by?(Gem.rubygems_version)) - end || candidates.last - search.dependencies = dependencies if search && (search.is_a?(RemoteSpecification) || search.is_a?(EndpointSpecification)) + end + search.dependencies = dependencies if search && search.full_name == full_name && (search.is_a?(RemoteSpecification) || search.is_a?(EndpointSpecification)) search end end diff --git a/bundler/lib/bundler/spec_set.rb b/bundler/lib/bundler/spec_set.rb index 6a3813eef8a2..e21a3ea6f5b0 100644 --- a/bundler/lib/bundler/spec_set.rb +++ b/bundler/lib/bundler/spec_set.rb @@ -66,10 +66,6 @@ def to_hash def materialize(deps) materialized = self.for(deps, true).uniq - materialized.map! do |s| - next s unless s.is_a?(LazySpecification) - s.materialize_for_installation || s - end SpecSet.new(materialized) end @@ -180,7 +176,8 @@ def tsort_each_node def specs_for_dependency(dep, platform) specs_for_name = lookup[dep.name] if platform.nil? - GemHelpers.select_best_platform_match(specs_for_name.select {|s| Gem::Platform.match_spec?(s) }, Bundler.local_platform) + matching_specs = specs_for_name.map {|s| s.materialize_for_installation if Gem::Platform.match_spec?(s) }.compact + GemHelpers.sort_best_platform_match(matching_specs, Bundler.local_platform) else GemHelpers.select_best_platform_match(specs_for_name, dep.force_ruby_platform ? Gem::Platform::RUBY : platform) end diff --git a/bundler/spec/install/gemfile/specific_platform_spec.rb b/bundler/spec/install/gemfile/specific_platform_spec.rb index 6994cb2af158..48349aaef435 100644 --- a/bundler/spec/install/gemfile/specific_platform_spec.rb +++ b/bundler/spec/install/gemfile/specific_platform_spec.rb @@ -374,6 +374,41 @@ ERROR end + it "can fallback to a source gem when platform gems are incompatible with current ruby version" do + setup_multiplatform_gem_with_source_gem + + source = file_uri_for(gem_repo2) + + gemfile <<~G + source "#{source}" + + gem "my-precompiled-gem" + G + + # simulate lockfile which includes both a precompiled gem with: + # - Gem the current platform (with imcompatible ruby version) + # - A source gem with compatible ruby version + lockfile <<-L + GEM + remote: #{source}/ + specs: + my-precompiled-gem (3.0.0) + my-precompiled-gem (3.0.0-#{Bundler.local_platform}) + + PLATFORMS + ruby + #{Bundler.local_platform} + + DEPENDENCIES + my-precompiled-gem + + BUNDLED WITH + #{Bundler::VERSION} + L + + bundle :install + end + private def setup_multiplatform_gem @@ -404,4 +439,16 @@ def setup_multiplatform_gem_with_different_dependencies_per_platform build_gem("CFPropertyList") end end + + def setup_multiplatform_gem_with_source_gem + build_repo2 do + build_gem("my-precompiled-gem", "3.0.0") + build_gem("my-precompiled-gem", "3.0.0") do |s| + s.platform = Bundler.local_platform + + # purposely unresolvable + s.required_ruby_version = ">= 1000.0.0" + end + end + end end diff --git a/bundler/spec/install/gems/resolving_spec.rb b/bundler/spec/install/gems/resolving_spec.rb index 1679ad446049..a55db83dc47f 100644 --- a/bundler/spec/install/gems/resolving_spec.rb +++ b/bundler/spec/install/gems/resolving_spec.rb @@ -241,7 +241,7 @@ expect(the_bundle).to include_gems("rack 1.2") end - it "gives a meaningful error if there's a lockfile using the newer incompatible version" do + it "automatically updates lockfile to use the older version" do build_repo2 do build_gem "parallel_tests", "3.7.0" do |s| s.required_ruby_version = ">= #{current_ruby_minor}" @@ -273,9 +273,23 @@ #{Bundler::VERSION} L - bundle "install --verbose", :artifice => "compact_index", :env => { "BUNDLER_SPEC_GEM_REPO" => gem_repo2.to_s }, :raise_on_error => false - expect(err).to include("parallel_tests-3.8.0 requires ruby version >= #{next_ruby_minor}") - expect(err).not_to include("That means the author of parallel_tests (3.8.0) has removed it.") + bundle "install --verbose", :artifice => "compact_index", :env => { "BUNDLER_SPEC_GEM_REPO" => gem_repo2.to_s } + + expect(lockfile).to eq <<~L + GEM + remote: http://localgemserver.test/ + specs: + parallel_tests (3.7.0) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + parallel_tests + + BUNDLED WITH + #{Bundler::VERSION} + L end it "gives a meaningful error on ruby version mismatches between dependencies" do