Skip to content

Commit

Permalink
[Fix #3044] Set conditional config defaults after CLI options are par…
Browse files Browse the repository at this point in the history
…sed and config files are loaded
  • Loading branch information
joshuay03 committed Dec 24, 2023
1 parent 83c5b32 commit 0b9d9ed
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 13 deletions.
6 changes: 2 additions & 4 deletions lib/puma/cli.rb
Expand Up @@ -39,10 +39,8 @@ def initialize(argv, log_writer = LogWriter.stdio, events = Events.new)
@control_url = nil
@control_options = {}

setup_options

begin
@parser.parse! @argv
setup_options

if file = @argv.shift
@conf.configure do |user_config, file_config|
Expand Down Expand Up @@ -240,7 +238,7 @@ def setup_options
$stdout.puts o
exit 0
end
end
end.parse! @argv
end
end
end
Expand Down
14 changes: 10 additions & 4 deletions lib/puma/configuration.rb
Expand Up @@ -182,13 +182,11 @@ def initialize(user_options={}, default_options = {}, &block)
@file_dsl = DSL.new(@options.file_options, self)
@default_dsl = DSL.new(@options.default_options, self)

if !@options[:prune_bundler]
default_options[:preload_app] = (@options[:workers] > 1) && Puma.forkable?
end

if block
configure(&block)
end

set_conditional_default_options
end

attr_reader :options, :plugins
Expand Down Expand Up @@ -238,6 +236,8 @@ def puma_options_from_env
def load
config_files.each { |config_file| @file_dsl._load_from(config_file) }

set_conditional_default_options

@options
end

Expand Down Expand Up @@ -380,6 +380,12 @@ def load_rackup
rack_app
end

def set_conditional_default_options
if !@options[:prune_bundler]
@options.default_options[:preload_app] = (@options[:workers] > 1) && Puma.forkable?
end
end

def self.random_token
require 'securerandom' unless defined?(SecureRandom)

Expand Down
1 change: 1 addition & 0 deletions test/config/workers_0.rb
@@ -0,0 +1 @@
workers 0
1 change: 1 addition & 0 deletions test/config/workers_2.rb
@@ -0,0 +1 @@
workers 2
23 changes: 18 additions & 5 deletions test/test_cli.rb
Expand Up @@ -352,7 +352,6 @@ def test_environment_app_env
ENV['APP_ENV'] = 'test'

cli = Puma::CLI.new []
cli.send(:setup_options)

assert_equal 'test', cli.instance_variable_get(:@conf).environment
ensure
Expand All @@ -364,7 +363,6 @@ def test_environment_rack_env
ENV['RACK_ENV'] = @environment

cli = Puma::CLI.new []
cli.send(:setup_options)

assert_equal @environment, cli.instance_variable_get(:@conf).environment
end
Expand All @@ -374,7 +372,6 @@ def test_environment_rails_env
ENV['RAILS_ENV'] = @environment

cli = Puma::CLI.new []
cli.send(:setup_options)

assert_equal @environment, cli.instance_variable_get(:@conf).environment
ensure
Expand All @@ -383,7 +380,6 @@ def test_environment_rails_env

def test_silent
cli = Puma::CLI.new ['--silent']
cli.send(:setup_options)

log_writer = cli.instance_variable_get(:@log_writer)

Expand All @@ -396,9 +392,26 @@ def test_plugins
assert_empty Puma::Plugins.instance_variable_get(:@plugins)

cli = Puma::CLI.new ['--plugin', 'tmp_restart', '--plugin', 'systemd']
cli.send(:setup_options)

assert Puma::Plugins.find("tmp_restart")
assert Puma::Plugins.find("systemd")
end

def test_config_does_not_preload_app_with_workers
skip_unless :fork

cli = Puma::CLI.new ['-w 0']
config = Puma.cli_config

assert_equal false, config.options[:preload_app]
end

def test_config_preloads_app_with_workers
skip_unless :fork

cli = Puma::CLI.new ['-w 2']
config = Puma.cli_config

assert_equal true, config.options[:preload_app]
end
end
28 changes: 28 additions & 0 deletions test/test_config.rb
Expand Up @@ -556,6 +556,34 @@ def test_http_content_length_limit
assert_equal 10000, conf.final_options[:http_content_length_limit]
end

def test_config_does_not_preload_app_if_not_using_workers
conf = Puma::Configuration.new({ workers: 0 })

assert_equal false, conf.options.default_options[:preload_app]
end

def test_config_preloads_app_if_using_workers
conf = Puma::Configuration.new({ workers: 2 })
preload = Puma.forkable?

assert_equal preload, conf.options.default_options[:preload_app]
end

def test_config_file_does_not_preload_app_if_not_using_workers
conf = Puma::Configuration.new { |c| c.load 'test/config/workers_0.rb' }
conf.load

assert_equal false, conf.options.default_options[:preload_app]
end

def test_config_file_preloads_app_if_using_workers
conf = Puma::Configuration.new { |c| c.load 'test/config/workers_2.rb' }
conf.load
preload = Puma.forkable?

assert_equal preload, conf.options.default_options[:preload_app]
end

private

def assert_run_hooks(hook_name, options = {})
Expand Down

0 comments on commit 0b9d9ed

Please sign in to comment.