Re: unicorn_rails cleanup (possible fix for Rails3) pushed

2010-06-08 Thread Hongli Lai
On Tue, Jun 8, 2010 at 9:20 PM, Eric Wong normalper...@yhbt.net wrote:
 Thanks Hongli, so this only affects people who remove the
 config.ru that Rails 3 creates for them?  Yikes...

No. The problem even occurs if you already have config.ru. But the
thing is, Rails 3 has deprecated ActionController::Dispatcher a few
weeks ago and replaced it with a stub. Rails::Rack::Static changed its
interface and must be constructed differently. The only way to obtain
a valid Rack endpoint for the app seems to be parsing
config/application.rb


 A few comments:

 +  if ::Rails::VERSION::MAJOR = 3  ::File.exist?('config/application.rb')
 +    ::File.read('config/application.rb') =~ /^module (.+)$/

 Maybe a more flexible regexp like this: /^module\s+([\w:]+)\s*$/ (or
 maybe even starting with: ^\s*) would be more reliable for folks who
 leave extra spaces around.

I think it's easier to just call 'strip'. :)

 Unfortunately, Ruby is not Python :)

 @@ -148,9 +167,9 @@ def rails_builder(daemonize)
       else
         use Rails::Rack::LogTailer unless daemonize
         use Rails::Rack::Debugger if $DEBUG
 +        use Rails::Rack::Static, map_path
         map(map_path) do
 -          use Rails::Rack::Static
 -          run ActionController::Dispatcher.new
 +          run rails_dispatcher

 Changing the call to use Rails::Rack::Static there is wrong.  map_path
 is the URI prefix (RAILS_RELATIVE_URL_ROOT) and not the static file
 path we serve from.  It appears the deprecation in Rails 3 broke some
 things and ActionDispatch::Static is configured slightly differently.

 Let me know if the edited patch below looks alright to you.

Yes it looks fine. A bit overcomplicated regexp compared to using
'strip' but whatever works. :)

With kind regards,
Hongli Lai
-- 
Phusion | The Computer Science Company

Web: http://www.phusion.nl/
E-mail: i...@phusion.nl
Chamber of commerce no: 08173483 (The Netherlands)
___
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying


Re: unicorn_rails cleanup (possible fix for Rails3) pushed

2010-06-08 Thread Eric Wong
Hongli Lai hon...@phusion.nl wrote:
 On Tue, Jun 8, 2010 at 9:20 PM, Eric Wong normalper...@yhbt.net wrote:
  Thanks Hongli, so this only affects people who remove the
  config.ru that Rails 3 creates for them?  Yikes...
 
 No. The problem even occurs if you already have config.ru. But the
 thing is, Rails 3 has deprecated ActionController::Dispatcher a few
 weeks ago and replaced it with a stub. Rails::Rack::Static changed its
 interface and must be constructed differently. The only way to obtain
 a valid Rack endpoint for the app seems to be parsing
 config/application.rb

Actually, I made unicorn_rails completely bypass the rails_builder
method if there's a config.ru, so it should never hit the
ActionController::Dispatcher stuff.

  Let me know if the edited patch below looks alright to you.
 
 Yes it looks fine. A bit overcomplicated regexp compared to using
 'strip' but whatever works. :)

I just kept the regexp as-is, works for me.

I just managed to push this to git://git.bogomips.org/unicorn.git before
my Internet connection died on me earlier today.  I've beefed up the
tests a bit but will probably do more later.

Eric Wong (4):
  t0300: Rails 3 test actually uses unicorn_rails
  tests: libify common rails3 setup code
  unicorn_rails: fix requires for Ruby 1.9.2
  tests: add Rails 3 test for the missing config.ru case

Hongli Lai (Phusion) (1):
  Fix unicorn_rails compatibility with the latest Rails 3 code

Thanks again!

-- 
Eric Wong
___
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying