diff --git a/lib/bundler/match_platform.rb b/lib/bundler/match_platform.rb index 566067c905a0..fab1910be01b 100644 --- a/lib/bundler/match_platform.rb +++ b/lib/bundler/match_platform.rb @@ -31,11 +31,21 @@ def self.prefer_content_addressable(matching) return matching if addressable.empty? compatible = addressable.select(&:matches_current_metadata?) + compatible.reject! {|spec| longer_matching_content_address_available?(spec, compatible) } return compatible if compatible.any? non_addressable.any? ? non_addressable : matching end + def self.longer_matching_content_address_available?(spec, compatible_specs) + compatible_specs.any? do |compatible_spec| + compatible_spec.platform == spec.platform && + compatible_spec.content_address.length > spec.content_address.length && + compatible_spec.content_address.start_with?(spec.content_address) + end + end + private_class_method :longer_matching_content_address_available? + def self.select_best_local_platform_match(specs, force_ruby: false, locked_platforms: nil) local = Bundler.local_platform matching = select_all_platform_match(specs, local, force_ruby: force_ruby) diff --git a/lib/rubygems/content_address.rb b/lib/rubygems/content_address.rb index e815c45191c0..bed6a763d0c1 100644 --- a/lib/rubygems/content_address.rb +++ b/lib/rubygems/content_address.rb @@ -34,6 +34,13 @@ def self.match?(value) value.is_a?(String) && PATTERN.match?(value) end + ## + # Whether +value+ is a valid content address longer than the default. + + def self.widened?(value) + match?(value) && value.length > DEFAULT_LENGTH + end + ## # Whether +value+ is a well-formed Ruby ABI ("X.Y"). diff --git a/lib/rubygems/resolver.rb b/lib/rubygems/resolver.rb index ffe0e9f60785..c0b5ea1df0ca 100644 --- a/lib/rubygems/resolver.rb +++ b/lib/rubygems/resolver.rb @@ -477,10 +477,19 @@ def build_spec_for_cache(name) @all_specs[name].group_by(&:version).transform_values do |candidates| next candidates.first if candidates.length == 1 - # Prefer already-installed specs to avoid unnecessary downloads + # Prefer already-installed specs to avoid unnecessary downloads. installed = candidates.select {|s| s.is_a?(Gem::Resolver::InstalledSpecification) } - next installed.first if installed.length == 1 - candidates = installed if installed.any? + + superseded_installed = installed.select do |installed_spec| + candidates.any? {|candidate| widened_remote_spec_supersedes?(candidate, installed_spec) } + end + + if superseded_installed.any? + candidates -= superseded_installed + else + next installed.first if installed.length == 1 + candidates = installed if installed.any? + end # Among remaining candidates, prefer a content-addressed candidate # built for the running Ruby, then the most specific platform, then the @@ -496,6 +505,22 @@ def build_spec_for_cache(name) end end + def widened_remote_spec_supersedes?(candidate, installed_spec) + return false if candidate.is_a?(Gem::Resolver::InstalledSpecification) + + candidate_address = candidate.content_address + installed_address = installed_spec.content_address + + return false unless Gem::ContentAddress.match?(candidate_address) + return false unless installed_address&.length == Gem::ContentAddress::DEFAULT_LENGTH + return false unless candidate_address.length > installed_address.length + + candidate.platform == installed_spec.platform && + Gem::ContentAddress.ruby_abi_for(candidate.required_ruby_version) == + Gem::ContentAddress.ruby_abi_for(installed_spec.required_ruby_version) && + candidate_address.start_with?(installed_address) + end + def compute_dependencies(package, version) spec = spec_for(package.to_s, version) return {} unless spec diff --git a/spec/bundler/match_platform_spec.rb b/spec/bundler/match_platform_spec.rb new file mode 100644 index 000000000000..24f77ebf66b4 --- /dev/null +++ b/spec/bundler/match_platform_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +RSpec.describe Bundler::MatchPlatform do + describe ".prefer_content_addressable" do + it "prefers a widened address when its platform and prefix match" do + platform = Gem::Platform.new("arm64-darwin") + short = double( + :short, + platform: platform, + content_address: "ab123456", + matches_current_metadata?: true + ) + widened = double( + :widened, + platform: platform, + content_address: "ab1234567890", + matches_current_metadata?: true + ) + + matching = described_class.prefer_content_addressable([short, widened]) + + expect(matching).to contain_exactly(widened) + end + + it "does not replace a short address when the widened address has a different platform" do + short = double( + :short, + platform: Gem::Platform.new("arm64-darwin-27"), + content_address: "ab123456", + matches_current_metadata?: true + ) + widened = double( + :widened, + platform: Gem::Platform.new("arm64-darwin"), + content_address: "ab1234567890", + matches_current_metadata?: true + ) + + matching = described_class.prefer_content_addressable([short, widened]) + + expect(matching).to contain_exactly(short, widened) + end + end +end diff --git a/spec/install/gemfile/content_addressable_spec.rb b/spec/install/gemfile/content_addressable_spec.rb index 8493c89cc203..1363ddd32603 100644 --- a/spec/install/gemfile/content_addressable_spec.rb +++ b/spec/install/gemfile/content_addressable_spec.rb @@ -56,6 +56,50 @@ end end + it "prefers a widened server address over an installed short address" do + simulate_platform "x86_64-linux" do + build_repo2 do + build_gem "mygem", "1.0" do |s| + s.platform = Gem::Platform.new("x86_64-linux") + s.write "lib/mygem.rb", "MYGEM = '1.0 not_content_addressed'" + end + end + + build_gem "mygem", "1.0", ruby_abi: current_abi, path: gem_repo2("gems") do |s| + s.platform = Gem::Platform.new("x86_64-linux") + s.required_ruby_version = "~> #{current_abi}.0" + s.write "lib/mygem.rb", "MYGEM = '1.0 content_addressed'" + end + + install_gemfile <<~G, artifice: "compact_index_v2", env: { "BUNDLER_SPEC_GEM_REPO" => gem_repo2.to_s } + source "https://gem.repo2" + + gem "mygem" + G + + short_cache = Dir[default_bundle_path("cache", "mygem-1.0-*.gem").to_s].first + short_address = File.basename(short_cache, ".gem").rpartition("-").last + expect(short_address.length).to eq(Gem::ContentAddress::DEFAULT_LENGTH) + + digest = Digest::SHA256.file(short_cache).hexdigest + widened_address = digest[0, 12] + short_repo_gem = gem_repo2("gems", "mygem-1.0-#{short_address}.gem") + widened_repo_gem = gem_repo2("gems", "mygem-1.0-#{widened_address}.gem") + + update_repo2 do + FileUtils.mv short_repo_gem, widened_repo_gem + end + + bundle "update mygem", artifice: "compact_index_v2", env: { "BUNDLER_SPEC_GEM_REPO" => gem_repo2.to_s } + + expect(lockfile).to include("mygem (1.0-x86_64-linux) #{widened_address}") + expect(default_bundle_path("gems", "mygem-1.0-#{widened_address}")).to exist + expect(default_bundle_path("specifications", current_abi, "mygem-1.0-#{widened_address}.gemspec")).to exist + expect(default_bundle_path("cache", "mygem-1.0-#{widened_address}.gem")).to exist + expect(lockfile).not_to match(/mygem \(1\.0-x86_64-linux\) #{short_address}\s/) + end + end + it "resolves a content-addressed binary from the local cache after a lockfile round-trip" do simulate_platform "x86_64-linux" do build_repo2 do diff --git a/spec/support/shards.rb b/spec/support/shards.rb index 93e1ccb99462..983b56ccfa71 100644 --- a/spec/support/shards.rb +++ b/spec/support/shards.rb @@ -144,6 +144,7 @@ module Shards "spec/bundler/ci_detector_spec.rb", ], shard_d: [ + "spec/bundler/match_platform_spec.rb", "spec/bundler/cli/exec_spec.rb", "spec/bundler/rubygems_ext_spec.rb", "spec/bundler/resolver/cooldown_spec.rb", diff --git a/test/rubygems/test_gem_content_address.rb b/test/rubygems/test_gem_content_address.rb index 868d4ad6866d..775b3232e69c 100644 --- a/test/rubygems/test_gem_content_address.rb +++ b/test/rubygems/test_gem_content_address.rb @@ -26,6 +26,13 @@ def test_match_rejects_uppercase refute Gem::ContentAddress.match?("ABCDEF12") end + def test_widened + refute Gem::ContentAddress.widened?("a" * 8) + assert Gem::ContentAddress.widened?("a" * 9) + refute Gem::ContentAddress.widened?(nil) + refute Gem::ContentAddress.widened?("not-an-address") + end + def test_valid_ruby_abi assert Gem::ContentAddress.valid_ruby_abi?("3.4") assert Gem::ContentAddress.valid_ruby_abi?("10.0") diff --git a/test/rubygems/test_gem_resolver.rb b/test/rubygems/test_gem_resolver.rb index 9fff107e02d3..5ae71353e082 100644 --- a/test/rubygems/test_gem_resolver.rb +++ b/test/rubygems/test_gem_resolver.rb @@ -336,6 +336,108 @@ def test_prefers_content_addressed_gem_for_same_platform assert_resolves_to [ca_spec], resolver end + def test_prefers_remote_gem_with_widened_address_when_first_8_chars_match + current_abi = Gem.ruby_abi + util_pin_ruby_to_abi current_abi + installed_set = Gem::Resolver::CurrentSet.new + ca_spec = util_spec "a", "1" + ca_spec.platform = Gem::Platform.local + ca_spec.content_address = "ab123456" + ca_spec.required_ruby_version = "~> #{current_abi}.0" + ca_installed_spec = Gem::Resolver::InstalledSpecification.new installed_set, ca_spec + + # Add another installed spec for a different platform + # to confirm that this gem is preserved and not accidentally replaced by a widened remote spec. + other_platform_spec = util_spec "a", "1" + other_platform_spec.platform = "arm64-darwin" + other_platform_spec.content_address = "ab123456" + other_platform_spec.required_ruby_version = "~> #{current_abi}.0" + other_platform_installed_spec = Gem::Resolver::InstalledSpecification.new installed_set, other_platform_spec + + api_set = Gem::Resolver::APISet.new + data = { + name: "a", + number: "1", + suffix: "ab1234567890", + dependencies: [], + requirements: { platform: [Gem::Platform.local.to_s], ruby: ["~> #{current_abi}.0"] }, + } + widened_api_spec = Gem::Resolver::APISpecification.new api_set, data + + s = StaticSet.new([ca_installed_spec, other_platform_installed_spec, widened_api_spec]) + dependency = make_dep "a" + resolver = Gem::Resolver.new([dependency], s) + resolved_spec = resolver.resolve.first.spec + + assert_same widened_api_spec, resolved_spec + assert_equal "ab1234567890", resolved_spec.content_address + ensure + util_restore_RUBY_VERSION + end + + def test_does_not_replace_installed_gem_with_widened_address_for_different_ruby_abi + current_abi = Gem.ruby_abi + util_pin_ruby_to_abi current_abi + other_abi = current_abi == "2.7" ? "3.0" : "2.7" + installed_set = Gem::Resolver::CurrentSet.new + ca_spec = util_spec "a", "1" + ca_spec.platform = Gem::Platform.local + ca_spec.content_address = "ab123456" + ca_spec.required_ruby_version = "~> #{current_abi}.0" + ca_installed_spec = Gem::Resolver::InstalledSpecification.new installed_set, ca_spec + + api_set = Gem::Resolver::APISet.new + data = { + name: "a", + number: "1", + suffix: "ab1234567890", + dependencies: [], + requirements: { platform: [Gem::Platform.local.to_s], ruby: ["~> #{other_abi}.0"] }, + } + widened_api_spec = Gem::Resolver::APISpecification.new api_set, data + + s = StaticSet.new([ca_installed_spec, widened_api_spec]) + dependency = make_dep "a" + resolver = Gem::Resolver.new([dependency], s) + resolver.soft_missing = true + resolved_spec = resolver.resolve.first.spec + + assert_same ca_installed_spec, resolved_spec + ensure + util_restore_RUBY_VERSION + end + + def test_does_not_replace_installed_gem_with_widened_address_for_different_platform + util_set_arch "arm64-darwin-27" + current_abi = Gem.ruby_abi + util_pin_ruby_to_abi current_abi + installed_set = Gem::Resolver::CurrentSet.new + ca_spec = util_spec "a", "1" + ca_spec.platform = Gem::Platform.local + ca_spec.content_address = "ab123456" + ca_spec.required_ruby_version = "~> #{current_abi}.0" + ca_installed_spec = Gem::Resolver::InstalledSpecification.new installed_set, ca_spec + + api_set = Gem::Resolver::APISet.new + data = { + name: "a", + number: "1", + suffix: "ab1234567890", + dependencies: [], + requirements: { platform: ["= arm64-darwin"], ruby: ["~> #{current_abi}.0"] }, + } + widened_api_spec = Gem::Resolver::APISpecification.new api_set, data + + s = StaticSet.new([ca_installed_spec, widened_api_spec]) + dependency = make_dep "a" + resolver = Gem::Resolver.new([dependency], s) + resolved_spec = resolver.resolve.first.spec + + assert_same ca_installed_spec, resolved_spec + ensure + util_restore_RUBY_VERSION + end + def test_prefers_compatible_content_addressed_gem_over_more_specific_platform util_set_arch "arm64-darwin-27" current_abi = Gem.ruby_abi