unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
* unicorn_rails cleanup (possible fix for Rails3) pushed
@ 2010-06-04  1:58 Eric Wong
  2010-06-04  2:13 ` Michael Guterl
  2010-06-08  2:49 ` Eric Wong
  0 siblings, 2 replies; 9+ messages in thread
From: Eric Wong @ 2010-06-04  1:58 UTC (permalink / raw)
  To: mongrel-unicorn

Hi all,

I've pushed the following patch out go git://git.bogomips.org/unicorn
along with a few other Rails-related test updates.  This is more of a
shotgun fix (but less code is better :) since I haven't been able to
reproduce the brokeness people have been seeing with "unicorn_rails"
and Rails 3 betas.

Even though "unicorn" works perfectly well for Rails3, some folks will
inevitably run "unicorn_rails" because of the "rails" in its name.

If I were to do it all over again, I would've just made everybody write
config.ru files for Rails.  But yes, programming is like sex, make one
mistake and support it for life :)

You can grab a git gem here, too:

  http://unicorn.bogomips.org/files/unicorn-0.99.0.16.g59a625.gem

>From 4b44e21957e4cb8ec6ace5604fbe096dfd8959d2 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Thu, 3 Jun 2010 22:52:11 +0000
Subject: [PATCH] unicorn_rails: avoid duplicating config.ru logic

This should allow "unicorn_rails" to be used seamlessly
with Rails 3 projects which package config.ru for you.
---
 bin/unicorn_rails |   50 ++++++++++++++------------------------------------
 1 files changed, 14 insertions(+), 36 deletions(-)

diff --git a/bin/unicorn_rails b/bin/unicorn_rails
index 72ab288..45a9b11 100755
--- a/bin/unicorn_rails
+++ b/bin/unicorn_rails
@@ -109,53 +109,30 @@ end
 
 ru = ARGV[0] || (File.exist?('config.ru') ? 'config.ru' : nil)
 
-if ru && ru =~ /\.ru\z/
-  # parse embedded command-line options in config.ru comments
-  /^#\\(.*)/ =~ File.read(ru) and opts.parse!($1.split(/\s+/))
-end
-
-def rails_builder(ru, daemonize)
+def rails_builder(daemonize)
   # this lambda won't run until after forking if preload_app is false
   lambda do ||
     # Load Rails and (possibly) the private version of Rack it bundles.
     begin
       require 'config/boot'
+      require 'config/environment'
     rescue LoadError => err
       abort "#$0 must be run inside RAILS_ROOT: #{err.inspect}"
     end
 
-    inner_app = case ru
-    when nil
-      require 'config/environment'
-
-      defined?(::Rails::VERSION::STRING) or
-        abort "Rails::VERSION::STRING not defined by config/{boot,environment}"
-      # it seems Rails >=2.2 support Rack, but only >=2.3 requires it
-      old_rails = case ::Rails::VERSION::MAJOR
-      when 0, 1 then true
-      when 2 then Rails::VERSION::MINOR < 3 ? true : false
-      else
-        false
-      end
-
-      if old_rails
-        require 'unicorn/app/old_rails'
-        Unicorn::App::OldRails.new
-      else
-        ActionController::Dispatcher.new
-      end
-    when /\.ru$/
-      raw = File.read(ru)
-      raw.sub!(/^__END__\n.*/, '')
-      eval("Rack::Builder.new {(#{raw}\n)}.to_app", TOPLEVEL_BINDING, ru)
+    defined?(::Rails::VERSION::STRING) or
+      abort "Rails::VERSION::STRING not defined by config/{boot,environment}"
+    # it seems Rails >=2.2 support Rack, but only >=2.3 requires it
+    old_rails = case ::Rails::VERSION::MAJOR
+    when 0, 1 then true
+    when 2 then Rails::VERSION::MINOR < 3 ? true : false
     else
-      require ru
-      Object.const_get(File.basename(ru, '.rb').capitalize)
+      false
     end
 
     Rack::Builder.new do
       map_path = ENV['RAILS_RELATIVE_URL_ROOT'] || '/'
-      if inner_app.class.to_s == "Unicorn::App::OldRails"
+      if old_rails
         if map_path != '/'
           # patches + tests welcome, but I really cbf to deal with this
           # since all apps I've ever dealt with just use "/" ...
@@ -163,23 +140,24 @@ def rails_builder(ru, daemonize)
         end
         $stderr.puts "LogTailer not available for Rails < 2.3" unless daemonize
         $stderr.puts "Debugger not available" if $DEBUG
+        require 'unicorn/app/old_rails'
         map(map_path) do
           use Unicorn::App::OldRails::Static
-          run inner_app
+          run Unicorn::App::OldRails.new
         end
       else
         use Rails::Rack::LogTailer unless daemonize
         use Rails::Rack::Debugger if $DEBUG
         map(map_path) do
           use Rails::Rack::Static
-          run inner_app
+          run ActionController::Dispatcher.new
         end
       end
     end.to_app
   end
 end
 
-app = rails_builder(ru, daemonize)
+app = ru ? Unicorn.builder(ru, opts) : rails_builder(daemonize)
 options[:listeners] << "#{host}:#{port}" if set_listener
 
 if $DEBUG
-- 
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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: unicorn_rails cleanup (possible fix for Rails3) pushed
  2010-06-04  1:58 unicorn_rails cleanup (possible fix for Rails3) pushed Eric Wong
@ 2010-06-04  2:13 ` Michael Guterl
  2010-06-04  2:48   ` Eric Wong
  2010-06-08  2:49 ` Eric Wong
  1 sibling, 1 reply; 9+ messages in thread
From: Michael Guterl @ 2010-06-04  2:13 UTC (permalink / raw)
  To: unicorn list

On Thu, Jun 3, 2010 at 9:58 PM, Eric Wong <normalperson@yhbt.net> wrote:
<snip>
> If I were to do it all over again, I would've just made everybody write
> config.ru files for Rails.  But yes, programming is like sex, make one
> mistake and support it for life :)

That is probably one of the funniest things I've read in awhile.
Thanks for the unexpected source of humor! :)

Best,
Michael Guterl
_______________________________________________
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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unicorn_rails cleanup (possible fix for Rails3) pushed
  2010-06-04  2:13 ` Michael Guterl
@ 2010-06-04  2:48   ` Eric Wong
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2010-06-04  2:48 UTC (permalink / raw)
  To: unicorn list

Michael Guterl <mguterl@gmail.com> wrote:
> On Thu, Jun 3, 2010 at 9:58 PM, Eric Wong <normalperson@yhbt.net> wrote:
> <snip>
> > If I were to do it all over again, I would've just made everybody write
> > config.ru files for Rails.  But yes, programming is like sex, make one
> > mistake and support it for life :)
> 
> That is probably one of the funniest things I've read in awhile.
> Thanks for the unexpected source of humor! :)

You're welcome :>  It's not my line, but I heard it many years ago.

-- 
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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unicorn_rails cleanup (possible fix for Rails3) pushed
  2010-06-04  1:58 unicorn_rails cleanup (possible fix for Rails3) pushed Eric Wong
  2010-06-04  2:13 ` Michael Guterl
@ 2010-06-08  2:49 ` Eric Wong
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Wong @ 2010-06-08  2:49 UTC (permalink / raw)
  To: mongrel-unicorn

Eric Wong <normalperson@yhbt.net> wrote:
> Hi all,
> 
> I've pushed the following patch out go git://git.bogomips.org/unicorn
> along with a few other Rails-related test updates.  This is more of a
> shotgun fix (but less code is better :) since I haven't been able to
> reproduce the brokeness people have been seeing with "unicorn_rails"
> and Rails 3 betas.

<snip>

> You can grab a git gem here, too:
> 
>   http://unicorn.bogomips.org/files/unicorn-0.99.0.16.g59a625.gem

Anybody get a chance to try this?

-- 
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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unicorn_rails cleanup (possible fix for Rails3) pushed
@ 2010-06-08 12:24 Hongli Lai
  2010-06-08 19:20 ` Eric Wong
  0 siblings, 1 reply; 9+ messages in thread
From: Hongli Lai @ 2010-06-08 12:24 UTC (permalink / raw)
  To: mongrel-unicorn

Hi Eric.

It would appear that recent Rails 3 changes have broken unicorn_rails,
just like they broke Phusion Passenger. Here's a patch which fixes the
problem.
http://gist.github.com/429944

With kind regards,
Hongli Lai

-- 
Phusion | The Computer Science Company

Web: http://www.phusion.nl/
E-mail: info@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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unicorn_rails cleanup (possible fix for Rails3) pushed
  2010-06-08 12:24 Hongli Lai
@ 2010-06-08 19:20 ` Eric Wong
  2010-06-08 19:25   ` Hongli Lai
  2010-06-11 20:32   ` Eric Wong
  0 siblings, 2 replies; 9+ messages in thread
From: Eric Wong @ 2010-06-08 19:20 UTC (permalink / raw)
  To: unicorn list; +Cc: Hongli Lai

Hongli Lai <hongli@phusion.nl> wrote:
> Hi Eric.
> 
> It would appear that recent Rails 3 changes have broken unicorn_rails,
> just like they broke Phusion Passenger. Here's a patch which fixes the
> problem.
> http://gist.github.com/429944

Thanks Hongli, so this only affects people who remove the
config.ru that Rails 3 creates for them?  Yikes...

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.

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.

I'll also push out a few Rails 3 tests that exercise the missing
config.ru cases.

>From 222ae0a353eda446a480e5c4b473a218304f9594 Mon Sep 17 00:00:00 2001
From: Hongli Lai (Phusion) <hongli@phusion.nl>
Date: Tue, 8 Jun 2010 13:22:25 +0200
Subject: [PATCH] Fix unicorn_rails compatibility with the latest Rails 3 code

This allows us to properly detect Rails 3 installations
in cases where config.ru is not present.

[ew: expanded commit message
 fixed static file serving,
 more flexible regexp for matching module ]

ref: mid.gmane.org/AANLkTiksBxIo_PFWoiPTWi1entXZRb7D2uE-Rl7H3lbw@mail.gmail.com
Acked-by: Eric Wong <normalperson@yhbt.net>
---
 bin/unicorn_rails |   26 ++++++++++++++++++++++++--
 1 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/bin/unicorn_rails b/bin/unicorn_rails
index 45a9b11..ed235ba 100755
--- a/bin/unicorn_rails
+++ b/bin/unicorn_rails
@@ -109,6 +109,24 @@ 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*$/
+      app_module = Object.const_get($1)
+      begin
+        result = app_module::Application
+      rescue NameError
+      end
+    end
+  end
+
+  if result.nil? && defined?(ActionController::Dispatcher)
+    result = ActionController::Dispatcher.new
+  end
+
+  result || abort("Unable to locate the application dispatcher class")
+end
+
 def rails_builder(daemonize)
   # this lambda won't run until after forking if preload_app is false
   lambda do ||
@@ -149,8 +167,12 @@ def rails_builder(daemonize)
         use Rails::Rack::LogTailer unless daemonize
         use Rails::Rack::Debugger if $DEBUG
         map(map_path) do
-          use Rails::Rack::Static
-          run ActionController::Dispatcher.new
+          if defined?(ActionDispatch::Static)
+            use ActionDispatch::Static, "#{Rails.root}/public"
+          else
+            use Rails::Rack::Static
+          end
+          run rails_dispatcher
         end
       end
     end.to_app
-- 
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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: unicorn_rails cleanup (possible fix for Rails3) pushed
  2010-06-08 19:20 ` Eric Wong
@ 2010-06-08 19:25   ` Hongli Lai
  2010-06-08 20:55     ` Eric Wong
  2010-06-11 20:32   ` Eric Wong
  1 sibling, 1 reply; 9+ messages in thread
From: Hongli Lai @ 2010-06-08 19:25 UTC (permalink / raw)
  To: Eric Wong; +Cc: unicorn list

On Tue, Jun 8, 2010 at 9:20 PM, Eric Wong <normalperson@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: info@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unicorn_rails cleanup (possible fix for Rails3) pushed
  2010-06-08 19:25   ` Hongli Lai
@ 2010-06-08 20:55     ` Eric Wong
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2010-06-08 20:55 UTC (permalink / raw)
  To: unicorn list; +Cc: Hongli Lai

Hongli Lai <hongli@phusion.nl> wrote:
> On Tue, Jun 8, 2010 at 9:20 PM, Eric Wong <normalperson@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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: unicorn_rails cleanup (possible fix for Rails3) pushed
  2010-06-08 19:20 ` Eric Wong
  2010-06-08 19:25   ` Hongli Lai
@ 2010-06-11 20:32   ` Eric Wong
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Wong @ 2010-06-11 20:32 UTC (permalink / raw)
  To: unicorn list; +Cc: Hongli Lai

Eric Wong <normalperson@yhbt.net> wrote:
> Hongli Lai <hongli@phusion.nl> wrote:
> > Hi Eric.
> > 
> > It would appear that recent Rails 3 changes have broken unicorn_rails,
> > just like they broke Phusion Passenger. Here's a patch which fixes the
> > problem.
> > http://gist.github.com/429944
> 
> Thanks Hongli, so this only affects people who remove the
> config.ru that Rails 3 creates for them?  Yikes...

And for the completely bizzare: Rails 3 is ultra aggressive
when searching for config.ru, it walks up the directory tree,
even going all the way up to "/" if it can't find config.ru.

I had a $HOME/config.ru leftover from a test on one of my boxes, and
usually have my working directory in $HOME/unicorn.   This was was
causing the unicorn_rails test for this (t0301) to fail because that
test relies on Rails /not/ being able to find config.ru.

So yes, don't leave random artifacts lying around and love strace :)

-- 
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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2010-06-11 21:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-04  1:58 unicorn_rails cleanup (possible fix for Rails3) pushed Eric Wong
2010-06-04  2:13 ` Michael Guterl
2010-06-04  2:48   ` Eric Wong
2010-06-08  2:49 ` Eric Wong
  -- strict thread matches above, loose matches on Subject: below --
2010-06-08 12:24 Hongli Lai
2010-06-08 19:20 ` Eric Wong
2010-06-08 19:25   ` Hongli Lai
2010-06-08 20:55     ` Eric Wong
2010-06-11 20:32   ` Eric Wong

Code repositories for project(s) associated with this public inbox

	https://yhbt.net/unicorn.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).