From 0b9d9ed29ea96978c6507b572e7201f7b3ac7d2f Mon Sep 17 00:00:00 2001 From: Joshua Young Date: Sun, 24 Dec 2023 19:23:20 +1000 Subject: [PATCH] [Fix #3044] Set conditional config defaults after CLI options are parsed and config files are loaded --- lib/puma/cli.rb | 6 ++---- lib/puma/configuration.rb | 14 ++++++++++---- test/config/workers_0.rb | 1 + test/config/workers_2.rb | 1 + test/test_cli.rb | 23 ++++++++++++++++++----- test/test_config.rb | 28 ++++++++++++++++++++++++++++ 6 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 test/config/workers_0.rb create mode 100644 test/config/workers_2.rb diff --git a/lib/puma/cli.rb b/lib/puma/cli.rb index 384acd33e8..651275c313 100644 --- a/lib/puma/cli.rb +++ b/lib/puma/cli.rb @@ -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| @@ -240,7 +238,7 @@ def setup_options $stdout.puts o exit 0 end - end + end.parse! @argv end end end diff --git a/lib/puma/configuration.rb b/lib/puma/configuration.rb index 1441d32027..c1c44c30d8 100644 --- a/lib/puma/configuration.rb +++ b/lib/puma/configuration.rb @@ -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 @@ -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 @@ -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) diff --git a/test/config/workers_0.rb b/test/config/workers_0.rb new file mode 100644 index 0000000000..b2cdf8009a --- /dev/null +++ b/test/config/workers_0.rb @@ -0,0 +1 @@ +workers 0 diff --git a/test/config/workers_2.rb b/test/config/workers_2.rb new file mode 100644 index 0000000000..988555fade --- /dev/null +++ b/test/config/workers_2.rb @@ -0,0 +1 @@ +workers 2 diff --git a/test/test_cli.rb b/test/test_cli.rb index bb97b06016..d2dcc3a4a4 100644 --- a/test/test_cli.rb +++ b/test/test_cli.rb @@ -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 @@ -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 @@ -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 @@ -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) @@ -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 diff --git a/test/test_config.rb b/test/test_config.rb index ec8e21c606..938665b3aa 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -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 = {})