From: Tomas Doran Date: Wed, 19 Jan 2011 23:44:06 +0000 (+0000) Subject: Additional notes and cleanup X-Git-Tag: 5.89000~3 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=acbecf084395e9b46e607a3fe244faa3c1bd3abb Additional notes and cleanup --- diff --git a/TODO b/TODO index 1425968..748bbde 100644 --- a/TODO +++ b/TODO @@ -30,11 +30,18 @@ http://github.com/willert/catalyst-plugin-log4perl-simple/tree ### Blockers - * properly pass along server arguments * Add some tests for Catalyst::Test::local_request + * Docs + * Test all the options work on all of the scripts + * Test (and fix if needed) Engine::Stomp and ::Wx + * Document how to use your own .psgi + * Document migration for setting engine in setup + * Document migration for setting engine in $ENV + * Document what to do if you're a Prefork person ### Nice to have + * Do we need to do something else about middleware than let the user provide a .psgi? * throw out Catalyst::Test's remote_request in favour of Plack::Test::ExternalServer * make sure we're running under a server that support psgi.streaming - maybe diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm index a3cd9ad..173483a 100644 --- a/lib/Catalyst.pm +++ b/lib/Catalyst.pm @@ -1116,7 +1116,10 @@ sub setup { $class->setup_log( delete $flags->{log} ); $class->setup_plugins( delete $flags->{plugins} ); $class->setup_dispatcher( delete $flags->{dispatcher} ); - $class->setup_engine( delete $flags->{engine} ); + if (my $engine = delete $flags->{engine}) { + $class->log->warn("Specifying the engine in ->setup is no longer supported, XXX FIXME"); + } + $class->setup_engine(); $class->setup_stats( delete $flags->{stats} ); for my $flag ( sort keys %{$flags} ) { @@ -2587,51 +2590,11 @@ Sets up engine. =cut sub setup_engine { - my ($class, $engine) = @_; - - unless ($engine) { - $engine = $class->engine_class; - } - else { - $engine = String::RewritePrefix->rewrite( { '' => 'Catalyst::Engine::', '+' => '' }, $engine ); - } - - $engine = 'Catalyst::Engine' if $engine eq 'Catalyst::Engine::HTTP'; + my ($class) = @_; + my $engine = $class->engine_class; Class::MOP::load_class($engine); - # check for old engines that are no longer compatible - my $old_engine; - if ( $engine->isa('Catalyst::Engine::Apache') - && !Catalyst::Engine::Apache->VERSION ) - { - $old_engine = 1; - } - - elsif ( $engine->isa('Catalyst::Engine::Server::Base') - && Catalyst::Engine::Server->VERSION le '0.02' ) - { - $old_engine = 1; - } - - elsif ($engine->isa('Catalyst::Engine::HTTP::POE') - && $engine->VERSION eq '0.01' ) - { - $old_engine = 1; - } - - elsif ($engine->isa('Catalyst::Engine::Zeus') - && $engine->VERSION eq '0.01' ) - { - $old_engine = 1; - } - - if ($old_engine) { - Catalyst::Exception->throw( message => - qq/Engine "$engine" is not supported by this version of Catalyst/ - ); - } - if ($ENV{MOD_PERL}) { require 'Catalyst/Engine/Loader.pm'; my $apache = Catalyst::Engine::Loader->auto; diff --git a/lib/Catalyst/Engine.pm b/lib/Catalyst/Engine.pm index ae4bf94..52c38cb 100644 --- a/lib/Catalyst/Engine.pm +++ b/lib/Catalyst/Engine.pm @@ -14,6 +14,7 @@ use Moose::Util::TypeConstraints; use Plack::Loader; use Plack::Middleware::Conditional; use Plack::Middleware::ReverseProxy; +use Catalyst::Engine::Loader; use Encode (); use utf8; @@ -775,18 +776,32 @@ The amount of input data that has already been read. =head2 $self->run($app, $server) Start the engine. Builds a PSGI application and calls the -run method on the server passed in.. +run method on the server passed in, which then causes the +engine to loop, handling requests.. =cut sub run { my ($self, $app, $psgi, @args) = @_; - # FIXME - Do something sensible with the options we're passed + # @args left here rather than just a $options, $server for back compat with the + # old style scripts which send a few args, then a hashref + + # They should never actually be used in the normal case as the Plack engine is + # passed in got all the 'standard' args via the loader in the script already. + + # FIXME - we should stash the options in an attribute so that custom args + # like Gitalist's --git_dir are possible to get from the app without stupid tricks. my $server = pop @args if blessed $args[-1]; - $server ||= Plack::Loader->auto(); # We're not being called from a script, - # so auto detect what backend to run on. - # This does *NOT* cover mod_perl. - $server->run($psgi); + my $options = pop @args if ref($args[-1]) eq 'HASH'; + if (! $server ) { + $server = Catalyst::Engine::Loader->auto(); # We're not being called from a script, + # so auto detect what backend to run on. + # This should never happen, as mod_perl + # never calls ->run, instead the $app->handle + # method is called per request. + $app->log->warn("Not supplied a Plack engine, falling back to engine auto-loader (are your scripts ancient?)") + } + $server->run($psgi, $options); } =head2 build_psgi_app ($app, @args) diff --git a/lib/Catalyst/Engine/Loader.pm b/lib/Catalyst/Engine/Loader.pm index 026a07c..0b81933 100644 --- a/lib/Catalyst/Engine/Loader.pm +++ b/lib/Catalyst/Engine/Loader.pm @@ -1,10 +1,17 @@ package Catalyst::Engine::Loader; use Moose; use Catalyst::Exception; +use Catalyst::Utils; use namespace::autoclean; extends 'Plack::Loader'; +has application_name => ( + isa => 'Str', + is => 'ro', + required => 1, +); + around guess => sub { my ($orig, $self) = (shift, shift); my $engine = $self->$orig(@_); @@ -37,10 +44,31 @@ around guess => sub { } } + my $old_engine = Catalyst::Utils::env_value($self->application_name, 'ENGINE'); + if (!defined $old_engine) { # Not overridden + } + elsif ($old_engine =~ /^(CGI|FCGI|HTTP|Apache.*)$/) { + # Trust autodetect + } + elsif ($old_engine eq "HTTP::Prefork") { # Too bad if you're customising, we don't handle options + # write yourself a script to collect and pass in the options + $engine = "Starman"; + } + elsif ($old_engine eq "HTTP::POE") { + Catalyst::Exception->throw("HTTP::POE engine no longer works, recommend you use Twiggy instead"); + } + elsif ($old_engine eq "Zeus") { + Catalyst::Exception->throw("Zeus engine no longer works"); + } + else { + warn("You asked for an unrecognised engine '$old_engine' which is no longer supported, this has been ignored.\n"); + } + return $engine; }; -__PACKAGE__->meta->make_immutable( inline_constructor => 0 ); +# Force constructor inlining +__PACKAGE__->meta->make_immutable( replace_constructor => 1 ); 1; diff --git a/lib/Catalyst/ScriptRole.pm b/lib/Catalyst/ScriptRole.pm index af8c636..f726626 100644 --- a/lib/Catalyst/ScriptRole.pm +++ b/lib/Catalyst/ScriptRole.pm @@ -33,7 +33,8 @@ has loader_class => ( has _loader => ( isa => 'Plack::Loader', default => sub { - shift->loader_class->new + my $self = shift; + $self->loader_class->new(application_name => $self->application_name); }, handles => { load_engine => 'load', @@ -65,7 +66,8 @@ sub _application_args { } sub _plack_loader_args { - my @app_args = shift->_application_args; + my $self = shift; + my @app_args = $self->_application_args; return (port => $app_args[0]); } diff --git a/t/aggregate/live_view_warnings.t b/t/aggregate/live_view_warnings.t index 1387c1b..bcfaeb7 100644 --- a/t/aggregate/live_view_warnings.t +++ b/t/aggregate/live_view_warnings.t @@ -2,6 +2,7 @@ use strict; use warnings; +no warnings 'once'; use FindBin; use lib "$FindBin::Bin/../lib";