Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: ModPerl: Dev

[Patch] Apache2::Reload

 

 

ModPerl dev RSS feed   Index | Next | Previous | View Threaded


ryan at livesite

Aug 28, 2009, 5:35 AM

Post #1 of 3 (739 views)
Permalink
[Patch] Apache2::Reload

This patch for Apache2::Reload does two things:

A) ignores require-hooks which exist in %INC. (fix)
B) reloads by file, not module name (fix?)

I consider B) to be a fix because I cannot imagine why anyone would
want the previous functionality. As such, the accompanying
configuration variable 'ReloadByModuleName' is used to enabled the old
behavior (as opposed to enabling the new behavior).

I was asked to include this patch inline in this email, however I know
the line-breaks will clobber the diff output, so I have attached it as
well.

Cheers,
-Ryan

--- Apache2/Reload.pm 2009-08-12 10:35:04.000000000 -0400
+++ Apache2/Reload.pm.v0.11 2009-08-28 08:28:25.000000000 -0400
@@ -20,7 +20,7 @@

use mod_perl2;

-our $VERSION = '0.10';
+our $VERSION = '0.11';

use Apache2::Const -compile => qw(OK);

@@ -86,6 +86,8 @@

my $DEBUG = ref($o) && (lc($o->dir_config("ReloadDebug") || '') eq
'on');
+ my $ReloadByModuleName = ref($o) &&
(lc($o->dir_config("ReloadByModuleName") || '') eq 'on'); +
my $TouchFile = ref($o) && $o->dir_config("ReloadTouchFile");

my $ConstantRedefineWarnings = ref($o) &&
@@ -138,6 +140,9 @@
my $file = $Apache2::Reload::INCS{$key};

next unless defined $file;
+ # TODO when $file is a hook, call the hook like require does
to
+ # determine the path (obtain a file handle). See %INC in
perlvar.
+ next if ref $file;
next if @watch_dirs && !grep { $file =~ /^$_/ } @watch_dirs;
warn "Apache2::Reload: Checking mtime of $key\n" if $DEBUG;

@@ -158,24 +163,27 @@
}

if ($mtime > $Stat{$file}) {
- push @changed, $key;
+ push @changed, [$key, $file];
}
$Stat{$file} = $mtime;
}

#First, let's unload all changed modules
- foreach my $module (@changed) {
- my $package = module_to_package($module);
+ foreach my $kvpair (@changed) {
+ my $package = module_to_package($kvpair->[0]);
ModPerl::Util::unload_package($package);
}
-
- #Then, let's reload them all, so that module dependencies can
satisfy
- #themselves in the correct order.
- foreach my $module (@changed) {
- my $package = module_to_package($module);
- require $module;
- warn("Apache2::Reload: process $$ reloading $package from
$module\n")
- if $DEBUG;
+
+ #Then, let's reload each changed file, so that module dependencies
can
+ #satisfy themselves in the correct order.
+ foreach my $kvpair (@changed) {
+ my $name = $ReloadByModuleName ? $kvpair->[0] : $kvpair->[1];
+ require $name;
+ if ($DEBUG) {
+ my $package = module_to_package($kvpair->[0]);
+ warn sprintf("Apache2::Reload: process %d reloading %s from
%s\n",
+ $$, $package, $kvpair->[1]);
+ }
}

return Apache2::Const::OK;
@@ -206,6 +214,7 @@
PerlSetVar ReloadAll Off
PerlSetVar ReloadModules "ModPerl::* Apache2::*"
#PerlSetVar ReloadDebug On
+ #PerlSetVar ReloadByModuleName On

# Reload a single module from within itself:
package My::Apache2::Module;
@@ -226,16 +235,28 @@
also do the check for modified modules, when a special touch-file has
been modified.

-Note that C<Apache2::Reload> operates on the current context of
-C<@INC>. Which means, when called as a C<Perl*Handler> it will not
-see C<@INC> paths added or removed by C<ModPerl::Registry> scripts, as
-the value of C<@INC> is saved on server startup and restored to that
-value after each request. In other words, if you want
-C<Apache2::Reload> to work with modules that live in custom C<@INC>
-paths, you should modify C<@INC> when the server is started. Besides,
-C<'use lib'> in the startup script, you can also set the C<PERL5LIB>
-variable in the httpd's environment to include any non-standard 'lib'
-directories that you choose. For example, to accomplish that you can
+Require-hooks, i.e., entries in %INC which are references, are
ignored. The +hook should modify %INC itself, adding the path to the
module file, for it to +be reloaded.
+
+C<Apache2::Reload> inspects and reloads the B<file> associated with a
given +module. Changes to @INC are not recognized, as it is the file
which is +being re-required, not the module name.
+
+In version 0.10 and earlier the B<module name>, not the file, is
re-required. +Meaning it operated on the the current context of @INC.
If you still want this +behavior set this environment variable in
I<httpd.conf>: +
+ PerlSetVar ReloadByModuleName On
+
+This means, when called as a C<Perl*Handler>, C<Apache2::Reload> will
not see +C<@INC> paths added or removed by C<ModPerl::Registry>
scripts, as the value of +C<@INC> is saved on server startup and
restored to that value after each +request. In other words, if you
want C<Apache2::Reload> to work with modules +that live in custom
C<@INC> paths, you should modify C<@INC> when the server is +started.
Besides, C<'use lib'> in the startup script, you can also set the
+C<PERL5LIB> variable in the httpd's environment to include any
non-standard +'lib' directories that you choose. For example, to
accomplish that you can include a line:
PERL5LIB=/home/httpd/perl/extra; export PERL5LIB
Attachments: Apache2-Reload.pm.patch (4.61 KB)


gozer at ectoplasm

Aug 28, 2009, 6:35 PM

Post #2 of 3 (685 views)
Permalink
Re: [Patch] Apache2::Reload [In reply to]

On 28/08/09 08:35 , Ryan Gies wrote:
> This patch for Apache2::Reload does two things:
>
> A) ignores require-hooks which exist in %INC. (fix)

That's a very good fix indeed, and should be fixed by itself, yes.

> B) reloads by file, not module name (fix?)

Good feature, good patch. I've got only one simple style nit with it.

Instead of:

+ foreach my $kvpair (@changed) {
+ my $name = $ReloadByModuleName ? $kvpair->[0] : $kvpair->[1];
+ require $name;


I just find $kvpair and $kvpair->[0] somewhat hard to read, could you
simply change it to something like:

+ foreach my $change (@changed) {
+ my $module = $change->[0];
+ my $file = $change->[1];
+ my $name = $ReloadByModuleName ? $module: $file;
+ require $name;

Apart from that, it's a great patch!

Could you resend 2 patches (splitting A) and B) in different ones) with
that small concern adressed ?

--
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Attachments: signature.asc (0.24 KB)


ryan at livesite

Aug 31, 2009, 5:38 AM

Post #3 of 3 (664 views)
Permalink
Re: [Patch] Apache2::Reload [In reply to]

On Fri, 28 Aug 2009 21:35:50 -0400
Philippe wrote:

> On 28/08/09 08:35 , Ryan Gies wrote:
> > This patch for Apache2::Reload does two things:
> >
> > A) ignores require-hooks which exist in %INC. (fix)
>
> That's a very good fix indeed, and should be fixed by itself, yes.
>
> > B) reloads by file, not module name (fix?)
>
> Good feature, good patch. I've got only one simple style nit with it.
>
> Instead of:
>
> + foreach my $kvpair (@changed) {
> + my $name = $ReloadByModuleName ? $kvpair->[0] : $kvpair->[1];
> + require $name;
>
>
> I just find $kvpair and $kvpair->[0] somewhat hard to read, could you
> simply change it to something like:
>
> + foreach my $change (@changed) {
> + my $module = $change->[0];
> + my $file = $change->[1];
> + my $name = $ReloadByModuleName ? $module: $file;
> + require $name;

Agreed. I used this style presuming it's equally readable:

my ($module, $file) = @$change;

>
> Apart from that, it's a great patch!
>
> Could you resend 2 patches (splitting A) and B) in different ones)
> with that small concern adressed ?
>

No problem. I created patch-B as a diff from patch-A, i.e., it's a
cumulative patch. I also bumped $VERSION once for each patch. I
have only ran tests against the final v0.12 code.

I also introduced a new change in patch-B. The debug output when an
entry is reloaded uses the format:

Apache2::Reload: process %d reloading %s from %s\n

the last %s *was* populated with the file's full path. It is *now*
populated with whatever value was passed to 'require'. For example, if
you are using the old-style (ReloadByModuleName) method, it will look
like this:

... from Apache2/Trace.pm

and by default (new-style) it will look like:

... from /usr/local/src/lsn/perl/lib/Apache2/Trace.pm

Please let me know if you have any other changes, questions, or
comments! Cheers,
-Ryan
Attachments: Apache2-Reload.pm.patch-A (1.14 KB)
  Apache2-Reload.pm.patch-B (3.91 KB)

ModPerl dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact lists@gossamer-threads.com
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.