From fe1bace43c73d88a5721e9b97066e746a34f135c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Fri, 6 Sep 2024 09:34:00 +0200 Subject: [PATCH] [rubygems/rubygems] Fix `gem install does-not-exist` being super slow Every time a gem is not found in the Compact Index API, RubyGems will fallback to the full index, which is very slow. This is unnecessary because both indexes should be providing the same gems, so if a gem can't be found in the Compact Index API, it won't be found in the full index. We _do_ want a fallback to the full index, whenever the Compact Index API is not implemented. To detect that, we check that the API responds to the "/versions" endpoint, just like Bundler does. Before: ``` $ time gem install fooasdsfafs ERROR: Could not find a valid gem 'fooasdsfafs' (>= 0) in any repository gem 20,77s user 0,59s system 96% cpu 22,017 total ``` After: ``` $ time gem install fooasdsfafs ERROR: Could not find a valid gem 'fooasdsfafs' (>= 0) in any repository gem 5,02s user 0,09s system 91% cpu 5,568 total ``` https://github.com/rubygems/rubygems/commit/c0d6b9eea7 --- lib/rubygems/resolver/best_set.rb | 30 +------ lib/rubygems/source.rb | 2 +- test/rubygems/test_gem_resolver_best_set.rb | 86 ++----------------- test/rubygems/test_gem_source.rb | 2 +- .../test_gem_source_subpath_problem.rb | 2 +- 5 files changed, 10 insertions(+), 112 deletions(-) diff --git a/lib/rubygems/resolver/best_set.rb b/lib/rubygems/resolver/best_set.rb index a983f8c6b6..57d0d00375 100644 --- a/lib/rubygems/resolver/best_set.rb +++ b/lib/rubygems/resolver/best_set.rb @@ -29,10 +29,8 @@ class Gem::Resolver::BestSet < Gem::Resolver::ComposedSet pick_sets if @remote && @sets.empty? super - rescue Gem::RemoteFetcher::FetchError => e - replace_failed_api_set e - - retry + rescue Gem::RemoteFetcher::FetchError + [] end def prefetch(reqs) # :nodoc: @@ -50,28 +48,4 @@ class Gem::Resolver::BestSet < Gem::Resolver::ComposedSet q.pp @sets end end - - ## - # Replaces a failed APISet for the URI in +error+ with an IndexSet. - # - # If no matching APISet can be found the original +error+ is raised. - # - # The calling method must retry the exception to repeat the lookup. - - def replace_failed_api_set(error) # :nodoc: - uri = error.original_uri - uri = Gem::URI uri unless Gem::URI === uri - uri += "." - - raise error unless api_set = @sets.find do |set| - Gem::Resolver::APISet === set && set.dep_uri == uri - end - - index_set = Gem::Resolver::IndexSet.new api_set.source - - @sets.map! do |set| - next set unless set == api_set - index_set - end - end end diff --git a/lib/rubygems/source.rb b/lib/rubygems/source.rb index 4e5545982d..59dcc2e1e1 100644 --- a/lib/rubygems/source.rb +++ b/lib/rubygems/source.rb @@ -79,7 +79,7 @@ class Gem::Source uri end - bundler_api_uri = enforce_trailing_slash(fetch_uri) + bundler_api_uri = enforce_trailing_slash(fetch_uri) + "./versions" begin fetcher = Gem::RemoteFetcher.fetcher diff --git a/test/rubygems/test_gem_resolver_best_set.rb b/test/rubygems/test_gem_resolver_best_set.rb index 8a750cdf8f..02f542efc0 100644 --- a/test/rubygems/test_gem_resolver_best_set.rb +++ b/test/rubygems/test_gem_resolver_best_set.rb @@ -9,32 +9,16 @@ class TestGemResolverBestSet < Gem::TestCase assert_empty set.sets end - def test_find_all_index - spec_fetcher do |fetcher| - fetcher.spec "a", 1 - fetcher.spec "a", 2 - fetcher.spec "b", 1 - end - - set = Gem::Resolver::BestSet.new - - dependency = dep "a", "~> 1" - - req = Gem::Resolver::DependencyRequest.new dependency, nil - - found = set.find_all req - - assert_equal %w[a-1], found.map(&:full_name) - end - - def test_find_all_fallback + def test_find_all spec_fetcher do |fetcher| fetcher.spec "a", 1 end - set = Gem::Resolver::BestSet.new + api_uri = Gem::URI "#{@gem_repo}info/" - api_uri = Gem::URI(@gem_repo) + @fetcher.data["#{api_uri}a"] = "---\n1 " + + set = Gem::Resolver::BestSet.new set.sets << Gem::Resolver::APISet.new(api_uri) @@ -90,64 +74,4 @@ class TestGemResolverBestSet < Gem::TestCase assert_empty set.sets end - - def test_replace_failed_api_set - set = Gem::Resolver::BestSet.new - - api_uri = Gem::URI(@gem_repo) + "./info/" - api_set = Gem::Resolver::APISet.new api_uri - - set.sets << api_set - - error_uri = api_uri + "a" - - error = Gem::RemoteFetcher::FetchError.new "bogus", error_uri - - set.replace_failed_api_set error - - assert_equal 1, set.sets.size - - refute_includes set.sets, api_set - - assert_kind_of Gem::Resolver::IndexSet, set.sets.first - end - - def test_replace_failed_api_set_no_api_set - set = Gem::Resolver::BestSet.new - - index_set = Gem::Resolver::IndexSet.new Gem::Source.new @gem_repo - - set.sets << index_set - - error = Gem::RemoteFetcher::FetchError.new "bogus", @gem_repo - - e = assert_raise Gem::RemoteFetcher::FetchError do - set.replace_failed_api_set error - end - - assert_equal error, e - end - - def test_replace_failed_api_set_uri_with_credentials - set = Gem::Resolver::BestSet.new - - api_uri = Gem::URI(@gem_repo) + "./info/" - api_uri.user = "user" - api_uri.password = "pass" - api_set = Gem::Resolver::APISet.new api_uri - - set.sets << api_set - - error_uri = api_uri + "a" - - error = Gem::RemoteFetcher::FetchError.new "bogus", error_uri - - set.replace_failed_api_set error - - assert_equal 1, set.sets.size - - refute_includes set.sets, api_set - - assert_kind_of Gem::Resolver::IndexSet, set.sets.first - end end diff --git a/test/rubygems/test_gem_source.rb b/test/rubygems/test_gem_source.rb index 6baa203dcb..423abd6dd2 100644 --- a/test/rubygems/test_gem_source.rb +++ b/test/rubygems/test_gem_source.rb @@ -46,7 +46,7 @@ class TestGemSource < Gem::TestCase response = Gem::Net::HTTPResponse.new "1.1", 200, "OK" response.uri = Gem::URI("http://example") - @fetcher.data[@gem_repo] = response + @fetcher.data["#{@gem_repo}versions"] = response set = @source.dependency_resolver_set diff --git a/test/rubygems/test_gem_source_subpath_problem.rb b/test/rubygems/test_gem_source_subpath_problem.rb index a451a81a25..ad11529f67 100644 --- a/test/rubygems/test_gem_source_subpath_problem.rb +++ b/test/rubygems/test_gem_source_subpath_problem.rb @@ -24,7 +24,7 @@ class TestGemSourceSubpathProblem < Gem::TestCase response = Gem::Net::HTTPResponse.new "1.1", 200, "OK" response.uri = Gem::URI("http://example") - @fetcher.data["#{@gem_repo}/"] = response + @fetcher.data["#{@gem_repo}/versions"] = response set = @source.dependency_resolver_set