Skip to content

Commit

Permalink
Fix bundle update warnings about nil gemspecs
Browse files Browse the repository at this point in the history
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'
```
  • Loading branch information
deivid-rodriguez committed Feb 27, 2021
1 parent 8864be6 commit 3e8a939
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 0 deletions.
2 changes: 2 additions & 0 deletions bundler/lib/bundler/installer.rb
Expand Up @@ -89,6 +89,8 @@ def run(options)
end
install(options)

Gem::Specification.reset # invalidate gem specification cache so that installed gems are immediately available

lock unless Bundler.frozen_bundle?
Standalone.new(options[:standalone], @definition).generate if options[:standalone]
end
Expand Down
47 changes: 47 additions & 0 deletions bundler/lib/bundler/rubygems_gem_installer.rb
Expand Up @@ -8,6 +8,53 @@ def check_executable_overwrite(filename)
# Bundler needs to install gems regardless of binstub overwriting
end

def install
pre_install_checks

run_pre_install_hooks

spec.loaded_from = spec_file

# Completely remove any previous gem files
FileUtils.rm_rf gem_dir
FileUtils.rm_rf spec.extension_dir

FileUtils.mkdir_p gem_dir, :mode => 0o755

extract_files

build_extensions
write_build_info_file
run_post_build_hooks

generate_bin
generate_plugins

write_spec
write_cache_file

say spec.post_install_message unless spec.post_install_message.nil?

run_post_install_hooks

spec
end

def generate_plugins
return unless Gem::Installer.instance_methods(false).include?(:generate_plugins)

latest = Gem::Specification.stubs_for(spec.name).first
return if latest && latest.version > spec.version

ensure_writable_dir @plugins_dir

if spec.plugins.empty?
remove_plugins_for(spec, @plugins_dir)
else
regenerate_plugins_for(spec, @plugins_dir)
end
end

def pre_install_checks
super && validate_bundler_checksum(options[:bundler_expected_checksum])
end
Expand Down
117 changes: 117 additions & 0 deletions bundler/spec/install/gemfile/sources_spec.rb
Expand Up @@ -378,6 +378,123 @@
end
end
end

context "when the lockfile has aggregated rubygems sources and newer versions of dependencies are available" do
before do
build_repo gem_repo2 do
build_gem "activesupport", "6.0.3.4" do |s|
s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2"
s.add_dependency "i18n", ">= 0.7", "< 2"
s.add_dependency "minitest", "~> 5.1"
s.add_dependency "tzinfo", "~> 1.1"
s.add_dependency "zeitwerk", "~> 2.2", ">= 2.2.2"
end

build_gem "activesupport", "6.1.2.1" do |s|
s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2"
s.add_dependency "i18n", ">= 1.6", "< 2"
s.add_dependency "minitest", ">= 5.1"
s.add_dependency "tzinfo", "~> 2.0"
s.add_dependency "zeitwerk", "~> 2.3"
end

build_gem "concurrent-ruby", "1.1.8"
build_gem "concurrent-ruby", "1.1.9"
build_gem "connection_pool", "2.2.3"

build_gem "i18n", "1.8.9" do |s|
s.add_dependency "concurrent-ruby", "~> 1.0"
end

build_gem "minitest", "5.14.3"
build_gem "rack", "2.2.3"
build_gem "redis", "4.2.5"

build_gem "sidekiq", "6.1.3" do |s|
s.add_dependency "connection_pool", ">= 2.2.2"
s.add_dependency "rack", "~> 2.0"
s.add_dependency "redis", ">= 4.2.0"
end

build_gem "thread_safe", "0.3.6"

build_gem "tzinfo", "1.2.9" do |s|
s.add_dependency "thread_safe", "~> 0.1"
end

build_gem "tzinfo", "2.0.4" do |s|
s.add_dependency "concurrent-ruby", "~> 1.0"
end

build_gem "zeitwerk", "2.4.2"
end

build_repo gem_repo3 do
build_gem "sidekiq-pro", "5.2.1" do |s|
s.add_dependency "connection_pool", ">= 2.2.3"
s.add_dependency "sidekiq", ">= 6.1.0"
end
end

gemfile <<-G
# frozen_string_literal: true
source "#{file_uri_for(gem_repo2)}"
gem "activesupport"
source "#{file_uri_for(gem_repo3)}" do
gem "sidekiq-pro"
end
G

lockfile <<~L
GEM
remote: #{file_uri_for(gem_repo2)}/
remote: #{file_uri_for(gem_repo3)}/
specs:
activesupport (6.0.3.4)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 0.7, < 2)
minitest (~> 5.1)
tzinfo (~> 1.1)
zeitwerk (~> 2.2, >= 2.2.2)
concurrent-ruby (1.1.8)
connection_pool (2.2.3)
i18n (1.8.9)
concurrent-ruby (~> 1.0)
minitest (5.14.3)
rack (2.2.3)
redis (4.2.5)
sidekiq (6.1.3)
connection_pool (>= 2.2.2)
rack (~> 2.0)
redis (>= 4.2.0)
sidekiq-pro (5.2.1)
connection_pool (>= 2.2.3)
sidekiq (>= 6.1.0)
thread_safe (0.3.6)
tzinfo (1.2.9)
thread_safe (~> 0.1)
zeitwerk (2.4.2)
PLATFORMS
#{specific_local_platform}
DEPENDENCIES
activesupport
sidekiq-pro!
BUNDLED WITH
#{Bundler::VERSION}
L
end

it "upgrades gems when running bundle update, without printing any warnings or errors" do
bundle "update --all"
expect(err).to be_empty
end
end
end

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

0 comments on commit 3e8a939

Please sign in to comment.