Skip to content

Commit 3996b60

Browse files
segiddinsclaude
andcommitted
Look up gems for gem fetch through the compact index
`gem fetch` looks gems up through Gem::SpecFetcher, which asks a source for its entire index -- the compact index `versions` file when the source serves one, 23MB of it for rubygems.org on a cold cache, and the Marshal index otherwise -- and then fetches a Marshal gemspec for every version that matches. `gem install` resolves through Gem::Resolver::BestSet, which reads only the `info` file of the gem it is looking for. Look gems up through a BestSet instead, so `gem fetch a` reads `/info/a` rather than the whole index, and fetches only the gemspec of the version it downloads. Platform preference and the reporting of specs rejected for another platform are unchanged. Gem::Source uses Gem::RemoteFetcher without requiring it, relying on its callers to have loaded it, and it cannot require it eagerly: Bundler loads rubygems/source on boot, where pulling in net/http breaks bundles that do not include its dependencies. Require it from the command instead, the way the sources command already does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 86cbb81 commit 3996b60

2 files changed

Lines changed: 98 additions & 8 deletions

File tree

lib/rubygems/commands/fetch_command.rb

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
require_relative "../command"
44
require_relative "../local_remote_options"
5+
require_relative "../remote_fetcher"
6+
require_relative "../resolver"
57
require_relative "../version_option"
68

79
class Gem::Commands::FetchCommand < Gem::Command
@@ -79,31 +81,59 @@ def fetch_gems
7981
platform = Gem.platforms.last
8082
gem_names = get_all_gem_names_and_versions
8183

84+
# A BestSet reads a source's compact index when it serves one, the same way
85+
# gems are looked up when installing them.
86+
remote_set = Gem::Resolver::BestSet.new
87+
8288
gem_names.each do |gem_name, gem_version|
8389
gem_version ||= version
8490
dep = Gem::Dependency.new gem_name, gem_version
8591
dep.prerelease = options[:prerelease]
8692
suppress_suggestions = !options[:suggest_alternate]
8793

88-
specs_and_sources, errors =
89-
Gem::SpecFetcher.fetcher.spec_for_dependency dep
94+
remote_specs, errors = find_remote_specs dep, remote_set
9095

9196
if platform
92-
filtered = specs_and_sources.select {|s,| s.platform == platform }
93-
specs_and_sources = filtered unless filtered.empty?
97+
filtered = remote_specs.select {|s| s.platform == platform }
98+
remote_specs = filtered unless filtered.empty?
9499
end
95100

96-
spec, source = specs_and_sources.max_by {|s,| s }
101+
remote_spec = remote_specs.max_by {|s| [s.version, Gem::Platform.sort_priority(s.platform)] }
97102

98-
if spec.nil?
103+
if remote_spec.nil?
99104
show_lookup_failure gem_name, gem_version, errors, suppress_suggestions, options[:domain]
100105
exit_code |= 2
101106
next
102107
end
103-
source.download spec
108+
109+
spec = remote_spec.spec
110+
remote_spec.source.download spec
104111
say "Downloaded #{spec.full_name}"
105112
end
106113

107114
exit_code
108115
end
116+
117+
# Find specs in +set+ that match +dep+ and can be used on this platform,
118+
# along with the reasons any other spec was rejected.
119+
120+
def find_remote_specs(dep, set)
121+
set.prerelease = dep.prerelease?
122+
123+
request = Gem::Resolver::DependencyRequest.new dep, nil
124+
125+
matching, mismatched = set.find_all(request).partition do |spec|
126+
Gem::Platform.match_spec? spec
127+
end
128+
129+
[matching, set.errors + platform_mismatches(mismatched)]
130+
end
131+
132+
def platform_mismatches(specs)
133+
specs.group_by {|spec| [spec.name, spec.version] }.map do |(name, version), group|
134+
mismatch = Gem::PlatformMismatch.new name, version
135+
group.each {|spec| mismatch.add_platform spec.platform.to_s }
136+
mismatch
137+
end
138+
end
109139
end

test/rubygems/test_gem_commands_fetch_command.rb

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def test_execute_platform
8181

8282
@cmd.options[:args] = %w[a]
8383

84-
@fetcher.data["#{@gem_repo}latest_specs.#{Gem.marshal_version}.gz"] = util_gzip(Marshal.dump([
84+
@fetcher.data["#{@gem_repo}specs.#{Gem.marshal_version}.gz"] = util_gzip(Marshal.dump([
8585
Gem::NameTuple.new(a2_spec.name, a2_spec.version, a2_spec.platform),
8686
Gem::NameTuple.new(a2_universal_darwin_spec.name, a2_universal_darwin_spec.version, a2_universal_darwin_spec.platform),
8787
]))
@@ -100,6 +100,66 @@ def test_execute_platform
100100
"#{a2_universal_darwin_spec.full_name} not fetched")
101101
end
102102

103+
def test_execute_compact_index
104+
specs = spec_fetcher do |fetcher|
105+
fetcher.gem "a", 1
106+
fetcher.gem "a", 2
107+
end
108+
109+
util_setup_compact_index(*specs.values)
110+
111+
@cmd.options[:args] = %w[a]
112+
113+
execute_with_exit_code
114+
115+
a2 = specs["a-2"]
116+
117+
assert_path_exist(File.join(@tempdir, a2.file_name),
118+
"#{a2.full_name} not fetched")
119+
120+
assert_includes @fetcher.paths, "#{@gem_repo}info/a"
121+
refute_includes @fetcher.paths, "#{@gem_repo}specs.#{Gem.marshal_version}.gz"
122+
end
123+
124+
def test_execute_compact_index_platform
125+
specs = spec_fetcher do |fetcher|
126+
fetcher.gem "a", 2
127+
fetcher.gem("a", 2) {|s| s.platform = "universal-darwin" }
128+
end
129+
130+
util_setup_compact_index(*specs.values)
131+
132+
@cmd.options[:args] = %w[a]
133+
134+
util_set_arch "arm64-darwin20" do
135+
execute_with_exit_code
136+
end
137+
138+
a2_universal_darwin = specs["a-2-universal-darwin"]
139+
140+
assert_path_exist(File.join(@tempdir, a2_universal_darwin.file_name),
141+
"#{a2_universal_darwin.full_name} not fetched")
142+
end
143+
144+
def test_execute_compact_index_platform_mismatch
145+
specs = spec_fetcher do |fetcher|
146+
fetcher.spec("a", 2) {|s| s.platform = "java" }
147+
end
148+
149+
util_setup_compact_index(*specs.values)
150+
151+
@cmd.options[:args] = %w[a]
152+
153+
execute_with_term_error
154+
155+
expected = <<-EXPECTED
156+
ERROR: Could not find a valid gem 'a' (>= 0), here is why:
157+
Found a (2), but was for platform java
158+
EXPECTED
159+
160+
assert_equal expected, @ui.error
161+
end
162+
103163
def test_execute_specific_prerelease
104164
specs = spec_fetcher do |fetcher|
105165
fetcher.gem "a", 2

0 commit comments

Comments
 (0)