From 3cb093176ae49dbf190b7e8fb3d7d05ef81fd018 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 25 May 2020 02:14:31 +1200 Subject: [PATCH] Remove options from `Builder#parse_file` & `Builder#load_file`. --- CHANGELOG.md | 1 + lib/rack/builder.rb | 40 +++++++++++++++------------------------- lib/rack/server.rb | 4 +--- test/builder/options.ru | 4 ---- test/spec_builder.rb | 13 ++----------- test/spec_server.rb | 12 ------------ 6 files changed, 19 insertions(+), 55 deletions(-) delete mode 100644 test/builder/options.ru diff --git a/CHANGELOG.md b/CHANGELOG.md index 28d0701ee..d439a12bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file. For info on - Relax validations around `Rack::Request#host` and `Rack::Request#hostname`. ([#1606](https://github.com/rack/rack/issues/1606), [@pvande](https://github.com/pvande)) - Removed antiquated handlers: FCGI, LSWS, SCGI, Thin. ([#1658](https://github.com/rack/rack/pull/1658), [@ioquatix](https://github.com/ioquatix)) +- Removed options from `Rack::Builder.parse_file` and `Rack::Builder.load_file`. ([#1663](https://github.com/rack/rack/pull/1663), [@ioquatix](https://github.com/ioquatix)) ### Fixed diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb index 816ecf620..7f298e8dd 100644 --- a/lib/rack/builder.rb +++ b/lib/rack/builder.rb @@ -39,13 +39,11 @@ class Builder # # If the config file ends in +.ru+, it is treated as a # rackup file and the contents will be treated as if - # specified inside a Rack::Builder block, using the given - # options. + # specified inside a Rack::Builder block. # # If the config file does not end in +.ru+, it is # required and Rack will use the basename of the file # to guess which constant will be the Rack application to run. - # The options given will be ignored in this case. # # Examples: # @@ -61,23 +59,18 @@ class Builder # # requires ./my_app.rb, which should be in the # # process's current directory. After requiring, # # assumes MyApp constant contains Rack application - def self.parse_file(config, opts = Server::Options.new) - if config.end_with?('.ru') - return self.load_file(config, opts) + def self.parse_file(path) + if path.end_with?('.ru') + return self.load_file(path) else - require config - app = Object.const_get(::File.basename(config, '.rb').split('_').map(&:capitalize).join('')) - return app, {} + require path + return Object.const_get(::File.basename(path, '.rb').split('_').map(&:capitalize).join('')) end end # Load the given file as a rackup file, treating the # contents as if specified inside a Rack::Builder block. # - # Treats the first comment at the beginning of a line - # that starts with a backslash as options similar to - # options passed on a rackup command line. - # # Ignores content in the file after +__END__+, so that # use of +__END__+ will not result in a syntax error. # @@ -90,21 +83,17 @@ def self.parse_file(config, opts = Server::Options.new) # use Rack::ContentLength # require './app.rb' # run App - def self.load_file(path, opts = Server::Options.new) - options = {} - - cfgfile = ::File.read(path) - cfgfile.slice!(/\A#{UTF_8_BOM}/) if cfgfile.encoding == Encoding::UTF_8 + def self.load_file(path) + config = ::File.read(path) + config.slice!(/\A#{UTF_8_BOM}/) if config.encoding == Encoding::UTF_8 - if cfgfile[/^#\\(.*)/] && opts - warn "Parsing options from the first comment line is deprecated!" - options = opts.parse! $1.split(/\s+/) + if config[/^#\\(.*)/] + fail "Parsing options from the first comment line is deprecated: #{path}" end - cfgfile.sub!(/^__END__\n.*\Z/m, '') - app = new_from_string cfgfile, path + config.sub!(/^__END__\n.*\Z/m, '') - return app, options + return new_from_string(config, path) end # Evaluate the given +builder_script+ string in the context of @@ -114,7 +103,8 @@ def self.new_from_string(builder_script, file = "(rackup)") # We cannot use instance_eval(String) as that would resolve constants differently. binding, builder = TOPLEVEL_BINDING.eval('Rack::Builder.new.instance_eval { [binding, self] }') eval builder_script, binding, file - builder.to_app + + return builder.to_app end # Initialize a new Rack::Builder instance. +default_app+ specifies the diff --git a/lib/rack/server.rb b/lib/rack/server.rb index c1f2f5caa..2e584617d 100644 --- a/lib/rack/server.rb +++ b/lib/rack/server.rb @@ -346,9 +346,7 @@ def build_app_and_options_from_config abort "configuration #{options[:config]} not found" end - app, options = Rack::Builder.parse_file(self.options[:config], opt_parser) - @options.merge!(options) { |key, old, new| old } - app + return Rack::Builder.parse_file(self.options[:config]) end def handle_profiling(heapfile, profile_mode, filename) diff --git a/test/builder/options.ru b/test/builder/options.ru deleted file mode 100644 index dca48fd91..000000000 --- a/test/builder/options.ru +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -#\ -d -p 2929 --env test -run lambda { |env| [200, { 'Content-Type' => 'text/plain' }, ['OK']] } diff --git a/test/spec_builder.rb b/test/spec_builder.rb index c0f59c182..eb13a9766 100644 --- a/test/spec_builder.rb +++ b/test/spec_builder.rb @@ -229,23 +229,14 @@ def config_file(name) File.join(File.dirname(__FILE__), 'builder', name) end - it "parses commented options" do - app, options = Rack::Builder.parse_file config_file('options.ru') - options[:debug].must_equal true - options[:environment].must_equal 'test' - options[:Port].must_equal '2929' - Rack::MockRequest.new(app).get("/").body.to_s.must_equal 'OK' - end - it "removes __END__ before evaluating app" do app, _ = Rack::Builder.parse_file config_file('end.ru') Rack::MockRequest.new(app).get("/").body.to_s.must_equal 'OK' end it "supports multi-line comments" do - proc, env = Rack::Builder.parse_file(config_file('comment.ru')) - proc.must_be_kind_of Proc - env.must_equal({}) + app = Rack::Builder.parse_file(config_file('comment.ru')) + app.must_be_kind_of(Proc) end it 'requires an_underscore_app not ending in .ru' do diff --git a/test/spec_server.rb b/test/spec_server.rb index a6968cfb9..22fb90f28 100644 --- a/test/spec_server.rb +++ b/test/spec_server.rb @@ -102,18 +102,6 @@ def with_stderr server.options[:daemonize].must_equal true end - it "only override non-passed options from parsed .ru file" do - builder_file = File.join(File.dirname(__FILE__), 'builder', 'options.ru') - SPEC_ARGV[0..-1] = ['--debug', '-sthin', '--env', 'production', builder_file] - server = Rack::Server.new - server.app # force .ru file to be parsed - - server.options[:debug].must_equal true - server.options[:server].must_equal 'thin' - server.options[:environment].must_equal 'production' - server.options[:Port].must_equal '2929' - end - def test_options_server(*args) SPEC_ARGV[0..-1] = args output = String.new