Skip to content

Commit

Permalink
Fix Bundler::GemNotFound errors for nio4r gem (#2427)
Browse files Browse the repository at this point in the history
* Update extra_runtime_dependencies test to test master proccess's gems

The `extra_runtime_dependencies` option allows one to activate gems in
the puma master process after "pruning" the master process with
`prune_bundler`. This is useful for activating gems that need to be
loaded in the master process, such as `puma_worker_killer`.

The integration test for `extra_runtime_dependencies` tested the
`$LOAD_PATH` of the worker process instead. Since workers are forked
from the master, it's normally fine to do this, but we might as well
test the master process's `$LOAD_PATH` directly if we can.

* Add test to refute that nio4r is loaded into puma master process

* Remove nio4r from puma master $LOAD_PATH

* Remove list of gems to activate from puma-wild

* Update History.md

Co-authored-by: Nate Berkopec <nate.berkopec@gmail.com>
  • Loading branch information
cjlarose and nateberkopec committed Oct 20, 2020
1 parent fdb936d commit 6502c5b
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 33 deletions.
1 change: 1 addition & 0 deletions History.md
Expand Up @@ -6,6 +6,7 @@

* Bugfixes
* Cleanup daemonization in rc.d script (#2409)
* Fix `Bundler::GemNotFound` errors for `nio4r` gem during phased restarts (#2427)
* Fire `on_booted` after server starts

* Refactor
Expand Down
12 changes: 3 additions & 9 deletions bin/puma-wild
Expand Up @@ -5,24 +5,18 @@

require 'rubygems'

gems = ARGV.shift
cli_arg = ARGV.shift

inc = ""

if gems == "-I"
if cli_arg == "-I"
inc = ARGV.shift
$LOAD_PATH.concat inc.split(":")
gems = ARGV.shift
end

gems.split(",").each do |s|
name, ver = s.split(":",2)
gem name, ver
end

module Puma; end

Puma.const_set("WILD_ARGS", ["-I", inc, gems])
Puma.const_set("WILD_ARGS", ["-I", inc])

require 'puma/cli'

Expand Down
14 changes: 5 additions & 9 deletions lib/puma/launcher.rb
Expand Up @@ -264,15 +264,11 @@ def restart!
end
end

# @!attribute [r] dependencies_and_files_to_require_after_prune
def dependencies_and_files_to_require_after_prune
# @!attribute [r] files_to_require_after_prune
def files_to_require_after_prune
puma = spec_for_gem("puma")

deps = puma.runtime_dependencies.map do |d|
"#{d.name}:#{spec_for_gem(d.name).version}"
end

[deps, require_paths_for_gem(puma) + extra_runtime_deps_directories]
require_paths_for_gem(puma) + extra_runtime_deps_directories
end

# @!attribute [r] extra_runtime_deps_directories
Expand Down Expand Up @@ -304,7 +300,7 @@ def prune_bundler
return
end

deps, dirs = dependencies_and_files_to_require_after_prune
dirs = files_to_require_after_prune

log '* Pruning Bundler environment'
home = ENV['GEM_HOME']
Expand All @@ -313,7 +309,7 @@ def prune_bundler
ENV['GEM_HOME'] = home
ENV['BUNDLE_GEMFILE'] = bundle_gemfile
ENV['PUMA_BUNDLER_PRUNED'] = '1'
args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':'), deps.join(',')] + @original_argv
args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':')] + @original_argv
# Ruby 2.0+ defaults to true which breaks socket activation
args += [{:close_others => false}]
Kernel.exec(*args)
Expand Down
2 changes: 1 addition & 1 deletion lib/puma/reactor.rb
@@ -1,6 +1,5 @@
# frozen_string_literal: true

require 'nio'
require 'puma/queue_close' if RUBY_VERSION < '2.3'

module Puma
Expand All @@ -20,6 +19,7 @@ class Reactor
# The provided block will be invoked when an IO has data available to read,
# its timeout elapses, or when the Reactor shuts down.
def initialize(&block)
require 'nio'
@selector = NIO::Selector.new
@input = Queue.new
@timeouts = []
Expand Down
5 changes: 5 additions & 0 deletions test/config/prune_bundler_with_deps.rb
@@ -1,2 +1,7 @@
prune_bundler true
extra_runtime_dependencies ["rdoc"]
before_fork do
$LOAD_PATH.each do |path|
puts "LOAD_PATH: #{path}"
end
end
3 changes: 0 additions & 3 deletions test/rackup/hello-last-load-path.ru

This file was deleted.

22 changes: 19 additions & 3 deletions test/test_integration_cluster.rb
Expand Up @@ -239,10 +239,26 @@ def test_prune_bundler_with_multiple_workers
end

def test_load_path_includes_extra_deps
cli_server "-w #{workers} -C test/config/prune_bundler_with_deps.rb test/rackup/hello-last-load-path.ru"
last_load_path = read_body(connect)
cli_server "-w #{workers} -C test/config/prune_bundler_with_deps.rb test/rackup/hello.ru"

assert_match(%r{gems/rdoc-[\d.]+/lib$}, last_load_path)
load_path = []
while (line = @server.gets) =~ /^LOAD_PATH/
load_path << line.gsub(/^LOAD_PATH: /, '')
end
assert_match(%r{gems/rdoc-[\d.]+/lib$}, load_path.last)
end

def test_load_path_does_not_include_nio4r
cli_server "-w #{workers} -C test/config/prune_bundler_with_deps.rb test/rackup/hello.ru"

load_path = []
while (line = @server.gets) =~ /^LOAD_PATH/
load_path << line.gsub(/^LOAD_PATH: /, '')
end

load_path.each do |path|
refute_match(%r{gems/nio4r-[\d.]+/lib}, path)
end
end

private
Expand Down
12 changes: 4 additions & 8 deletions test/test_launcher.rb
Expand Up @@ -7,29 +7,25 @@
class TestLauncher < Minitest::Test
include TmpPath

def test_dependencies_and_files_to_require_after_prune_is_correctly_built_for_no_extra_deps
def test_files_to_require_after_prune_is_correctly_built_for_no_extra_deps
skip_on :no_bundler

deps, dirs = launcher.send(:dependencies_and_files_to_require_after_prune)
dirs = launcher.send(:files_to_require_after_prune)

assert_equal(1, deps.length)
assert_match(%r{^nio4r:[\d.]+$}, deps.first)
assert_equal(2, dirs.length)
assert_match(%r{puma/lib$}, dirs[0]) # lib dir
assert_match(%r{puma-#{Puma::Const::PUMA_VERSION}$}, dirs[1]) # native extension dir
refute_match(%r{gems/rdoc-[\d.]+/lib$}, dirs[2])
end

def test_dependencies_and_files_to_require_after_prune_is_correctly_built_with_extra_deps
def test_files_to_require_after_prune_is_correctly_built_with_extra_deps
skip_on :no_bundler
conf = Puma::Configuration.new do |c|
c.extra_runtime_dependencies ['rdoc']
end

deps, dirs = launcher(conf).send(:dependencies_and_files_to_require_after_prune)
dirs = launcher(conf).send(:files_to_require_after_prune)

assert_equal(1, deps.length)
assert_match(%r{^nio4r:[\d.]+$}, deps.first)
assert_equal(3, dirs.length)
assert_match(%r{puma/lib$}, dirs[0]) # lib dir
assert_match(%r{puma-#{Puma::Const::PUMA_VERSION}$}, dirs[1]) # native extension dir
Expand Down

0 comments on commit 6502c5b

Please sign in to comment.