Skip to content

Commit

Permalink
Remove options from Builder#parse_file & Builder#load_file.
Browse files Browse the repository at this point in the history
  • Loading branch information
ioquatix committed May 24, 2020
1 parent 8e4e8d6 commit 3cb0931
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 55 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
40 changes: 15 additions & 25 deletions lib/rack/builder.rb
Expand Up @@ -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:
#
Expand All @@ -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.
#
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions lib/rack/server.rb
Expand Up @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions test/builder/options.ru

This file was deleted.

13 changes: 2 additions & 11 deletions test/spec_builder.rb
Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions test/spec_server.rb
Expand Up @@ -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
Expand Down

0 comments on commit 3cb0931

Please sign in to comment.