Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove options from Builder#parse_file & Builder#load_file. #1663

Merged
merged 1 commit into from May 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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