Skip to content

Commit

Permalink
[rubygems/rubygems] Don't discard candidates matching ruby metadata
Browse files Browse the repository at this point in the history
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.

rubygems/rubygems@0c0d40d417

Co-authored-by: Ian Ker-Seymer <ian.kerseymer@shopify.com>
  • Loading branch information
2 people authored and matzbot committed Aug 2, 2022
1 parent 9189c2d commit f4f6814
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 12 deletions.
8 changes: 7 additions & 1 deletion lib/bundler/gem_helpers.rb
Expand Up @@ -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?

Expand All @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions lib/bundler/lazy_specification.rb
Expand Up @@ -88,6 +88,8 @@ def materialize_for_installation
source.specs.search(self)
end

return self if candidates.empty?

__materialize__(candidates)
end

Expand All @@ -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
Expand Down
7 changes: 2 additions & 5 deletions lib/bundler/spec_set.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
47 changes: 47 additions & 0 deletions spec/bundler/install/gemfile/specific_platform_spec.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
22 changes: 18 additions & 4 deletions spec/bundler/install/gems/resolving_spec.rb
Expand Up @@ -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}"
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit f4f6814

Please sign in to comment.