perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: [Patch mp2] PerlConfigRequire & PerlPostConfigRequire ( was: a new directive: PerlStartupFile? )
Date Sat, 11 Dec 2004 16:37:40 GMT
Philippe M. Chiasson wrote:

> After some talk about exactly what to call these directive, here is a 
> patch. It's late, so it doesn't include
> docs or tests, but it should give a good idea of what's required to get 
> this working.
> 
> PerlConfigRequire is basically just PerlRequire with an immediate perl 
> startup
> PerlPostConfigRequire delays requiring the files up to the last possible 
> moment, before interpreters begin
> to get cloned, at the begining of mp's post_config phase.
> 
> Feedback most welcome!

gozer++

comments below

> Index: src/modules/perl/mod_perl.c
> ===================================================================
> @@ -646,6 +665,11 @@
>      MP_dSCFG(s);
>      dTHXa(scfg->mip->parent->perl);
>  #endif
> +    
> +    if (!modperl_post_config_require(s, pconf)) {
> +        exit(1);
> +    }

shouldn't it return an error (code) here, rather than exit?

also I find modperl_post_config_require not very intuitive, but I can't 
think of anything better. I guess it's OK, since it's only local. may be 
modperl_post_config_require_apply. dunno.

> Index: src/modules/perl/modperl_config.c
> ===================================================================
> --- src/modules/perl/modperl_config.c	(revision 111578)
> +++ src/modules/perl/modperl_config.c	(working copy)
> @@ -155,6 +155,7 @@
>  
>      scfg->PerlModule  = apr_array_make(p, 2, sizeof(char *));
>      scfg->PerlRequire = apr_array_make(p, 2, sizeof(char *));
> +    scfg->PerlPostConfigRequire = apr_array_make(p, 2, sizeof(char *));

probably 1 table entry should be enough here.

> Index: src/modules/perl/modperl_types.h
> ===================================================================
> --- src/modules/perl/modperl_types.h	(revision 111578)
> +++ src/modules/perl/modperl_types.h	(working copy)
> @@ -65,6 +65,11 @@
>  };
>  
>  typedef struct {
> +    const char *file;
> +    PerlInterpreter *perl;  
> +} modperl_require_file_t;

shouldn't *perl be ifdef'ed by

#ifdef USE_ITHREADS
     PerlInterpreter *perl;
#endif

You don't have aTHX under non-threaded perl.

Same of course goes to the population of this variable and its usage (see 
APR__Pool.h for a similar case).

> Index: t/conf/post_config_startup.pl
> ===================================================================
> --- t/conf/post_config_startup.pl	(revision 111578)
> +++ t/conf/post_config_startup.pl	(working copy)
> @@ -148,3 +148,5 @@
>  }
>  
>  1;
> +
> +die "blargh";

what's this die for?

> Index: t/conf/extra.conf.in
> ===================================================================
> --- t/conf/extra.conf.in	(revision 111578)
> +++ t/conf/extra.conf.in	(working copy)
> @@ -1,6 +1,8 @@
>  # needed to test $r->psignature
>  ServerSignature On
>  
> +PerlPostConfigRequire @ServerRoot@/conf/post_config_startup.pl
> +
>  # The following tests require more than one interpreter during the
>  # same request:
>  #
> Index: todo/release

Also we discussed that we need to add a similar PerlPostConfigRequire for 
t/vhost and add a test similar to t/hooks/TestHooks/startup.pm and 
modperl_extra.pl:test_hooks_startup();

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Mime
View raw message