From 4accf4449390c649bce0b1c9d84314d65fd41f8e Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 10 Jun 2010 08:47:10 +0000 Subject: respect "working_directory" wrt config.ru Since we added support for the "working_directory" parameter, it often became unclear where/when certain paths would be bound. There are some extremely nasty dependencies and ordering issues when doing this. It's all pretty fragile, but works for now and we even have a full integration test to keep it working. I plan on cleaning this up 2.x.x to be less offensive to look at (Rainbows! and Zbatery are a bit tied to this at the moment). Thanks to Pierre Baillet for reporting this. ref: http://mid.gmane.org/AANLkTimKb7JARr_69nfVrJLvMZH3Gvs1o_KwZFLKfuxy@mail.gmail.com --- bin/unicorn | 5 +--- bin/unicorn_rails | 17 ++++++++++---- lib/unicorn.rb | 7 +++--- lib/unicorn/configurator.rb | 56 ++++++++++++++++++++++++++++++++++++++++++++ lib/unicorn/launcher.rb | 5 ++-- t/t0003-working_directory.sh | 53 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 129 insertions(+), 14 deletions(-) create mode 100755 t/t0003-working_directory.sh diff --git a/bin/unicorn b/bin/unicorn index beece97..2cc10b1 100755 --- a/bin/unicorn +++ b/bin/unicorn @@ -107,10 +107,7 @@ opts = OptionParser.new("", 24, ' ') do |opts| opts.parse! ARGV end -ru = ARGV[0] || "config.ru" -abort "configuration file #{ru} not found" unless File.exist?(ru) - -app = Unicorn.builder(ru, opts) +app = Unicorn.builder(ARGV[0] || 'config.ru', opts) options[:listeners] << "#{host}:#{port}" if set_listener if $DEBUG diff --git a/bin/unicorn_rails b/bin/unicorn_rails index 2f88bc1..e9cac00 100755 --- a/bin/unicorn_rails +++ b/bin/unicorn_rails @@ -107,8 +107,6 @@ opts = OptionParser.new("", 24, ' ') do |opts| opts.parse! ARGV end -ru = ARGV[0] || (File.exist?('config.ru') ? 'config.ru' : nil) - def rails_dispatcher if ::Rails::VERSION::MAJOR >= 3 && ::File.exist?('config/application.rb') if ::File.read('config/application.rb') =~ /^module\s+([\w:]+)\s*$/ @@ -127,9 +125,20 @@ def rails_dispatcher result || abort("Unable to locate the application dispatcher class") end -def rails_builder(daemonize) +def rails_builder(ru, opts, daemonize) + return Unicorn.builder(ru, opts) if ru + + # allow Configurator to parse cli switches embedded in the ru file + Unicorn::Configurator::RACKUP.update(:file => :rails, :optparse => opts) + # this lambda won't run until after forking if preload_app is false + # this runs after config file reloading lambda do || + # Rails 3 includes a config.ru, use it if we find it after + # working_directory is bound. + ::File.exist?('config.ru') and + return Unicorn.builder('config.ru', opts).call + # Load Rails and (possibly) the private version of Rack it bundles. begin require ::File.expand_path('config/boot') @@ -179,7 +188,7 @@ def rails_builder(daemonize) end end -app = ru ? Unicorn.builder(ru, opts) : rails_builder(daemonize) +app = rails_builder(ARGV[0], opts, daemonize) options[:listeners] << "#{host}:#{port}" if set_listener if $DEBUG diff --git a/lib/unicorn.rb b/lib/unicorn.rb index c026236..a7b0646 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -33,11 +33,10 @@ module Unicorn # Unicorn config). The returned lambda will be called when it is # time to build the app. def builder(ru, opts) - if ru =~ /\.ru\z/ - # parse embedded command-line options in config.ru comments - /^#\\(.*)/ =~ File.read(ru) and opts.parse!($1.split(/\s+/)) - end + # allow Configurator to parse cli switches embedded in the ru file + Unicorn::Configurator::RACKUP.update(:file => ru, :optparse => opts) + # always called after config file parsing, may be called after forking lambda do || inner_app = case ru when /\.ru$/ diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb index e4305c2..0716e64 100644 --- a/lib/unicorn/configurator.rb +++ b/lib/unicorn/configurator.rb @@ -13,6 +13,12 @@ module Unicorn # nginx is also available at # http://unicorn.bogomips.org/examples/nginx.conf class Configurator < Struct.new(:set, :config_file, :after_reload) + # :stopdoc: + # used to stash stuff for deferred processing of cli options in + # config.ru after "working_directory" is bound. Do not rely on + # this being around later on... + RACKUP = {} + # :startdoc: # Default settings for Unicorn DEFAULTS = { @@ -51,6 +57,8 @@ module Unicorn def reload #:nodoc: instance_eval(File.read(config_file), config_file) if config_file + parse_rackup_file + # working_directory binds immediately (easier error checking that way), # now ensure any paths we changed are correctly set. [ :pid, :stderr_path, :stdout_path ].each do |var| @@ -66,6 +74,9 @@ module Unicorn def commit!(server, options = {}) #:nodoc: skip = options[:skip] || [] + if ready_pipe = RACKUP.delete(:ready_pipe) + server.ready_pipe = ready_pipe + end set.each do |key, value| value == :unset and next skip.include?(key) and next @@ -411,5 +422,50 @@ module Unicorn set[var] = my_proc end + # this is called _after_ working_directory is bound. This only + # parses the embedded switches in .ru files + # (for "rackup" compatibility) + def parse_rackup_file # :nodoc: + ru = RACKUP[:file] or return # we only return here in unit tests + + # :rails means use (old) Rails autodetect + if ru == :rails + File.readable?('config.ru') or return + ru = 'config.ru' + end + + File.readable?(ru) or + raise ArgumentError, "rackup file (#{ru}) not readable" + + # it could be a .rb file, too, we don't parse those manually + ru =~ /\.ru\z/ or return + + /^#\\(.*)/ =~ File.read(ru) or return + warn "ru cli opts: #{$1}" + RACKUP[:optparse].parse!($1.split(/\s+/)) + + # XXX ugly as hell, WILL FIX in 2.x (along with Rainbows!/Zbatery) + host, port, set_listener, options, daemonize = + eval("[ host, port, set_listener, options, daemonize ]", + TOPLEVEL_BINDING) + + warn [ :host, :port, :set_listener, :options, :daemonize ].inspect + warn [ ru, host, port, set_listener, options, daemonize ].inspect + # XXX duplicate code from bin/unicorn{,_rails} + set[:listeners] << "#{host}:#{port}" if set_listener + + if daemonize + # unicorn_rails wants a default pid path, (not plain 'unicorn') + if ru == :rails + spid = set[:pid] + pid('tmp/pids/unicorn.pid') if spid.nil? || spid == :unset + end + unless RACKUP[:daemonized] + Unicorn::Launcher.daemonize!(options) + RACKUP[:ready_pipe] = options.delete(:ready_pipe) + end + end + end + end end diff --git a/lib/unicorn/launcher.rb b/lib/unicorn/launcher.rb index 5ab04c7..7f3ffa6 100644 --- a/lib/unicorn/launcher.rb +++ b/lib/unicorn/launcher.rb @@ -56,8 +56,9 @@ class Unicorn::Launcher end end # $stderr/$stderr can/will be redirected separately in the Unicorn config - Unicorn::Configurator::DEFAULTS[:stderr_path] = "/dev/null" - Unicorn::Configurator::DEFAULTS[:stdout_path] = "/dev/null" + Unicorn::Configurator::DEFAULTS[:stderr_path] ||= "/dev/null" + Unicorn::Configurator::DEFAULTS[:stdout_path] ||= "/dev/null" + Unicorn::Configurator::RACKUP[:daemonized] = true end end diff --git a/t/t0003-working_directory.sh b/t/t0003-working_directory.sh new file mode 100755 index 0000000..3faa6c0 --- /dev/null +++ b/t/t0003-working_directory.sh @@ -0,0 +1,53 @@ +#!/bin/sh +. ./test-lib.sh +t_plan 4 "config.ru inside alt working_directory" + +t_begin "setup and start" && { + unicorn_setup + rtmpfiles unicorn_config_tmp + rm -rf $t_pfx.app + mkdir $t_pfx.app + + port=$(expr $listen : '[^:]*:\([0-9]\+\)') + host=$(expr $listen : '\([^:]*\):[0-9]\+') + + cat > $t_pfx.app/config.ru < $unicorn_config_tmp + + # the whole point of this exercise + echo "working_directory '$t_pfx.app'" >> $unicorn_config_tmp + + # allows ppid to be 1 in before_fork + echo "preload_app true" >> $unicorn_config_tmp + cat >> $unicorn_config_tmp <<\EOF +before_fork do |server,worker| + $master_ppid = Process.ppid # should be zero to detect daemonization +end +EOF + + mv $unicorn_config_tmp $unicorn_config + + # rely on --daemonize switch, no & or -D + unicorn -c $unicorn_config + unicorn_wait_start +} + +t_begin "hit with curl" && { + body=$(curl -sSf http://$listen/) +} + +t_begin "killing succeeds" && { + kill $unicorn_pid +} + +t_begin "response body ppid == 1 (daemonized)" && { + test "$body" -eq 1 +} + +t_done -- cgit v1.2.3-24-ge0c7