Skip to content

Commit 3e8a939

Browse files
Fix bundle update warnings about nil gemspecs
Sometimes, as a result of parallel installation of gems reading & writing gemspecs concurrently during `bundle update`, some thread can end up printing warnings like the following: ``` [/home/runner/work/rubygems/rubygems/bundler/tmp/2/gems/system/specifications/zeitwerk-2.4.2.gemspec] isn't a Gem::Specification (NilClass instead). ``` This commit fixes the issue by completely overriding rubygems installer and removing all the unnecessary stuff. In particular, the `Gem::Specification.reset` call that was constantly invalidating gem specification caches and thus mutating shared state has been moved to the main thread, after after installation of all gems is finished. As a results, `bundle install` time is noticiably faster. A quick test on my system (with ~500 gems installed) on an app with about ~100 locked gems (to avoid resolution overhead) without extensions (to avoid extension compilation overhead), with a fully loaded gem cache (to avoid network overhead), it results in ~15% speed up in `bundle install` time: ``` $ hyperfine -p 'rm -rf vendor/bundle' 'bundle install' 'bundle _2.2.11_ install' Benchmark #1: bundle install Time (mean ± σ): 5.713 s ± 0.387 s [User: 5.215 s, System: 2.163 s] Range (min … max): 5.443 s … 6.610 s 10 runs Benchmark #2: bundle _2.2.11_ install Time (mean ± σ): 6.549 s ± 0.106 s [User: 6.087 s, System: 2.353 s] Range (min … max): 6.412 s … 6.773 s 10 runs Summary 'bundle install' ran 1.15 ± 0.08 times faster than 'bundle _2.2.11_ install' ```
1 parent 8864be6 commit 3e8a939

File tree

3 files changed

+166
-0
lines changed

3 files changed

+166
-0
lines changed

bundler/lib/bundler/installer.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ def run(options)
8989
end
9090
install(options)
9191

92+
Gem::Specification.reset # invalidate gem specification cache so that installed gems are immediately available
93+
9294
lock unless Bundler.frozen_bundle?
9395
Standalone.new(options[:standalone], @definition).generate if options[:standalone]
9496
end

bundler/lib/bundler/rubygems_gem_installer.rb

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,53 @@ def check_executable_overwrite(filename)
88
# Bundler needs to install gems regardless of binstub overwriting
99
end
1010

11+
def install
12+
pre_install_checks
13+
14+
run_pre_install_hooks
15+
16+
spec.loaded_from = spec_file
17+
18+
# Completely remove any previous gem files
19+
FileUtils.rm_rf gem_dir
20+
FileUtils.rm_rf spec.extension_dir
21+
22+
FileUtils.mkdir_p gem_dir, :mode => 0o755
23+
24+
extract_files
25+
26+
build_extensions
27+
write_build_info_file
28+
run_post_build_hooks
29+
30+
generate_bin
31+
generate_plugins
32+
33+
write_spec
34+
write_cache_file
35+
36+
say spec.post_install_message unless spec.post_install_message.nil?
37+
38+
run_post_install_hooks
39+
40+
spec
41+
end
42+
43+
def generate_plugins
44+
return unless Gem::Installer.instance_methods(false).include?(:generate_plugins)
45+
46+
latest = Gem::Specification.stubs_for(spec.name).first
47+
return if latest && latest.version > spec.version
48+
49+
ensure_writable_dir @plugins_dir
50+
51+
if spec.plugins.empty?
52+
remove_plugins_for(spec, @plugins_dir)
53+
else
54+
regenerate_plugins_for(spec, @plugins_dir)
55+
end
56+
end
57+
1158
def pre_install_checks
1259
super && validate_bundler_checksum(options[:bundler_expected_checksum])
1360
end

bundler/spec/install/gemfile/sources_spec.rb

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,123 @@
378378
end
379379
end
380380
end
381+
382+
context "when the lockfile has aggregated rubygems sources and newer versions of dependencies are available" do
383+
before do
384+
build_repo gem_repo2 do
385+
build_gem "activesupport", "6.0.3.4" do |s|
386+
s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2"
387+
s.add_dependency "i18n", ">= 0.7", "< 2"
388+
s.add_dependency "minitest", "~> 5.1"
389+
s.add_dependency "tzinfo", "~> 1.1"
390+
s.add_dependency "zeitwerk", "~> 2.2", ">= 2.2.2"
391+
end
392+
393+
build_gem "activesupport", "6.1.2.1" do |s|
394+
s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2"
395+
s.add_dependency "i18n", ">= 1.6", "< 2"
396+
s.add_dependency "minitest", ">= 5.1"
397+
s.add_dependency "tzinfo", "~> 2.0"
398+
s.add_dependency "zeitwerk", "~> 2.3"
399+
end
400+
401+
build_gem "concurrent-ruby", "1.1.8"
402+
build_gem "concurrent-ruby", "1.1.9"
403+
build_gem "connection_pool", "2.2.3"
404+
405+
build_gem "i18n", "1.8.9" do |s|
406+
s.add_dependency "concurrent-ruby", "~> 1.0"
407+
end
408+
409+
build_gem "minitest", "5.14.3"
410+
build_gem "rack", "2.2.3"
411+
build_gem "redis", "4.2.5"
412+
413+
build_gem "sidekiq", "6.1.3" do |s|
414+
s.add_dependency "connection_pool", ">= 2.2.2"
415+
s.add_dependency "rack", "~> 2.0"
416+
s.add_dependency "redis", ">= 4.2.0"
417+
end
418+
419+
build_gem "thread_safe", "0.3.6"
420+
421+
build_gem "tzinfo", "1.2.9" do |s|
422+
s.add_dependency "thread_safe", "~> 0.1"
423+
end
424+
425+
build_gem "tzinfo", "2.0.4" do |s|
426+
s.add_dependency "concurrent-ruby", "~> 1.0"
427+
end
428+
429+
build_gem "zeitwerk", "2.4.2"
430+
end
431+
432+
build_repo gem_repo3 do
433+
build_gem "sidekiq-pro", "5.2.1" do |s|
434+
s.add_dependency "connection_pool", ">= 2.2.3"
435+
s.add_dependency "sidekiq", ">= 6.1.0"
436+
end
437+
end
438+
439+
gemfile <<-G
440+
# frozen_string_literal: true
441+
442+
source "#{file_uri_for(gem_repo2)}"
443+
444+
gem "activesupport"
445+
446+
source "#{file_uri_for(gem_repo3)}" do
447+
gem "sidekiq-pro"
448+
end
449+
G
450+
451+
lockfile <<~L
452+
GEM
453+
remote: #{file_uri_for(gem_repo2)}/
454+
remote: #{file_uri_for(gem_repo3)}/
455+
specs:
456+
activesupport (6.0.3.4)
457+
concurrent-ruby (~> 1.0, >= 1.0.2)
458+
i18n (>= 0.7, < 2)
459+
minitest (~> 5.1)
460+
tzinfo (~> 1.1)
461+
zeitwerk (~> 2.2, >= 2.2.2)
462+
concurrent-ruby (1.1.8)
463+
connection_pool (2.2.3)
464+
i18n (1.8.9)
465+
concurrent-ruby (~> 1.0)
466+
minitest (5.14.3)
467+
rack (2.2.3)
468+
redis (4.2.5)
469+
sidekiq (6.1.3)
470+
connection_pool (>= 2.2.2)
471+
rack (~> 2.0)
472+
redis (>= 4.2.0)
473+
sidekiq-pro (5.2.1)
474+
connection_pool (>= 2.2.3)
475+
sidekiq (>= 6.1.0)
476+
thread_safe (0.3.6)
477+
tzinfo (1.2.9)
478+
thread_safe (~> 0.1)
479+
zeitwerk (2.4.2)
480+
481+
PLATFORMS
482+
#{specific_local_platform}
483+
484+
DEPENDENCIES
485+
activesupport
486+
sidekiq-pro!
487+
488+
BUNDLED WITH
489+
#{Bundler::VERSION}
490+
L
491+
end
492+
493+
it "upgrades gems when running bundle update, without printing any warnings or errors" do
494+
bundle "update --all"
495+
expect(err).to be_empty
496+
end
497+
end
381498
end
382499

383500
context "with a gem that is only found in the wrong source" do

0 commit comments

Comments
 (0)