You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2003/12/15 03:33:47 UTC
[mp2 patch] introducing Apache::compat "scoping"
The following is yet another attempt to avoid collisions between
Apache::compat and the real mp2 APIs.
Similar to 'use' and 'no' pragma, I'm suggesting to introduce the new
functions override_mp2_api and restore_mp2_api for those APIs in
Apache::compat that collide with 2.0 API. The only difference is that one
needs an explicit call to restore_mp2_api, which I suppose could be done by
returning an object, which if made a lexically scoped will call
restore_mp2_api on its own (i.e. DESTROY), but since it's going to be used
very infrequently and eventually won't be needed at all, I don't see a reason
to bother. See the compat/request test change for an example in the patch below.
I decided not to use the import() method, but have an explicit function call,
since import() may make some people think that it imports the overriden
methods, which it doesn't.
I think there are only two functions at the moment that collide with mp2 API:
Apache::RequestRec::notes and Apache::RequestRec::finfo. There is also
APR::URI::unparse, which introduces a special case but I think it's harmless.
in June I posted an implementation for 2 more colliding functions:
Apache::Connection::local_addr
Apache::Connection::remote_addr
http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=105452446932154&w=2
which I will now be able to commit using this new functionality.
The cool thing is that we can introduce a sub-class of Registry which will do
the wrapping into a handler like so:
'sub handler {' .
'Apache::compat::override_mp2_api('Apache::RequestRec::notes');' .
$code .
'Apache::compat::restore_mp2_api('Apache::RequestRec::notes');' .
'}';
or something like that (overriding other subs as well).
Here is the patch:
Index: lib/Apache/compat.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/Apache/compat.pm,v
retrieving revision 1.90
diff -u -r1.90 compat.pm
--- lib/Apache/compat.pm 19 Nov 2003 19:30:11 -0000 1.90
+++ lib/Apache/compat.pm 15 Dec 2003 02:02:10 -0000
@@ -50,6 +50,86 @@
$INC{'Apache/Table.pm'} = __FILE__;
}
+# api => "overriding code"
+# the overriding code, needs to "return" the original CODE reference
+# when eval'ed , so that it can be restored later
+my %overridable_mp2_api = (
+ 'Apache::RequestRec::notes' => <<'EOI',
+{
+ require Apache::RequestRec;
+ my $notes_sub = *Apache::RequestRec::notes{CODE};
+ *Apache::RequestRec::notes = sub {
+ my $r = shift;
+ return wantarray()
+ ? ($r->table_get_set(scalar($r->$notes_sub), @_))
+ : scalar($r->table_get_set(scalar($r->$notes_sub), @_));
+ };
+ $notes_sub;
+}
+EOI
+
+ 'Apache::RequestRec::finfo' => <<'EOI',
+{
+ require APR::Finfo;
+ my $finfo_sub = *APR::Finfo::finfo{CODE};
+ sub Apache::RequestRec::finfo {
+ my $r = shift;
+ stat $r->filename;
+ \*_;
+ }
+ $finfo_sub;
+}
+EOI
+);
+
+my %overridden_mp2_api = ();
+
+# this function enables back-compatible APIs which can't coexist with
+# mod_perl 2.0 APIs with the same name and therefore it should be
+# avoided if possible.
+#
+# it expects a list of fully qualified functions, like
+# "Apache::RequestRec::finfo"
+sub override_mp2_api {
+ my (@subs) = @_;
+
+ for my $sub (@subs) {
+ unless (exists $overridable_mp2_api{$sub}) {
+ die __PACKAGE__ . ": $sub is not overridable";
+ }
+ if(exists $overridden_mp2_api{$sub}) {
+ warn __PACKAGE__ . ": $sub has been already overridden";
+ next;
+ }
+ $overridden_mp2_api{$sub} = eval $overridable_mp2_api{$sub};
+ unless (exists $overridden_mp2_api{$sub} &&
+ ref($overridden_mp2_api{$sub}) eq 'CODE') {
+ die "overriding $sub didn't return a CODE ref";
+ }
+ }
+}
+
+# restore_mp2_api does the opposite of override_mp2_api(), it removes
+# the overriden API and restores the original mod_perl 2.0 API
+sub restore_mp2_api {
+ my (@subs) = @_;
+
+ for my $sub (@subs) {
+ unless (exists $overridable_mp2_api{$sub}) {
+ die __PACKAGE__ . ": $sub is not overridable";
+ }
+ unless (exists $overridden_mp2_api{$sub}) {
+ warn __PACKAGE__ . ": can't restore $sub, " .
+ "as it has not been overridden";
+ next;
+ }
+ my $original_sub = delete $overridden_mp2_api{$sub};
+ no warnings 'redefine';
+ no strict 'refs';
+ *$sub = $original_sub;
+ }
+}
+
sub request {
my $what = shift;
@@ -249,15 +329,6 @@
: scalar($r->table_get_set(scalar($r->err_headers_out), @_));
}
-{
- my $notes_sub = *Apache::RequestRec::notes{CODE};
- *Apache::RequestRec::notes = sub {
- my $r = shift;
- return wantarray()
- ? ($r->table_get_set(scalar($r->$notes_sub), @_))
- : scalar($r->table_get_set(scalar($r->$notes_sub), @_));
- }
-}
sub register_cleanup {
shift->pool->cleanup_register(@_);
@@ -345,12 +416,6 @@
sub chdir_file {
#XXX resolve '.' in @INC to basename $r->filename
-}
-
-sub finfo {
- my $r = shift;
- stat $r->filename;
- \*_;
}
*log_reason = \&log_error;
Index: t/response/TestCompat/request.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/response/TestCompat/request.pm,v
retrieving revision 1.3
diff -u -r1.3 request.pm
--- t/response/TestCompat/request.pm 11 Apr 2003 07:34:03 -0000 1.3
+++ t/response/TestCompat/request.pm 15 Dec 2003 02:02:10 -0000
@@ -75,6 +75,8 @@
# $r->notes
{
+ Apache::compat::override_mp2_api('Apache::RequestRec::notes');
+
my $key = 'notes-test';
# get/set scalar context
{
@@ -98,6 +100,10 @@
$r->notes->add($key => $_) for @exp;
ok t_cmp(\@exp, [ $r->notes($key) ], "\$r->notes in list context");
}
+
+ # restore the real 2.0 notes() method, now that we are done
+ # with the compat one
+ Apache::compat::restore_mp2_api('Apache::RequestRec::notes');
}
# get_remote_host()
__________________________________________________________________
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
Re: [mp2 patch] introducing Apache::compat "scoping"
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> On Mon, 2003-12-15 at 16:09, Stas Bekman wrote:
>
>>Philippe M. Chiasson wrote:
>>
>>>On Sun, 2003-12-14 at 18:33, Stas Bekman wrote:
>>>
>>>
>>>>The following is yet another attempt to avoid collisions between
>>>>Apache::compat and the real mp2 APIs.
>>
>>[...]
>>
>>>>+}
>>>>+EOI
>>>>+
>>>>+ 'Apache::RequestRec::finfo' => <<'EOI',
>>>>+{
>>>>+ require APR::Finfo;
>>>>+ my $finfo_sub = *APR::Finfo::finfo{CODE};
>>>>+ sub Apache::RequestRec::finfo {
>>>>+ my $r = shift;
>>>>+ stat $r->filename;
>>>>+ \*_;
>>>>+ }
>>>>+ $finfo_sub;
>>>>+}
>>>>+EOI
>>>>+);
>>>
>>>
>>>Why make this code a SCALAR, instead of creating an anonymous sub right
>>>there?
>>
>>What do you mean?
>
>
> I meant:
>
> 'Apache::RequestRec::finfo' => sub {
> require APR::Finfo;
> my $finfo_sub = *$APR::Finfo::finfo{CODE};
> *Apache::RequestRec::finfo{CODE} = sub {
> my $r = shift;
> stat $r->filename;
> \*_;
> }
> return $finfo_sub;
> }
> }
>
> That way, in mp2_override_api() you don't have to do that extra eval,
> and can just call this anonsub directly, no ?
That's not the same thing. If you do that than you need to run this anon sub
to get the CODE ref. So why not just eval that code. I can't see how it better
than just having a string of code to be evaled only if it's ever going to be used.
__________________________________________________________________
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
Re: [mp2 patch] introducing Apache::compat "scoping"
Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Mon, 2003-12-15 at 16:09, Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> > On Sun, 2003-12-14 at 18:33, Stas Bekman wrote:
> >
> >>The following is yet another attempt to avoid collisions between
> >>Apache::compat and the real mp2 APIs.
> [...]
> >>+}
> >>+EOI
> >>+
> >>+ 'Apache::RequestRec::finfo' => <<'EOI',
> >>+{
> >>+ require APR::Finfo;
> >>+ my $finfo_sub = *APR::Finfo::finfo{CODE};
> >>+ sub Apache::RequestRec::finfo {
> >>+ my $r = shift;
> >>+ stat $r->filename;
> >>+ \*_;
> >>+ }
> >>+ $finfo_sub;
> >>+}
> >>+EOI
> >>+);
> >
> >
> > Why make this code a SCALAR, instead of creating an anonymous sub right
> > there?
>
> What do you mean?
I meant:
'Apache::RequestRec::finfo' => sub {
require APR::Finfo;
my $finfo_sub = *$APR::Finfo::finfo{CODE};
*Apache::RequestRec::finfo{CODE} = sub {
my $r = shift;
stat $r->filename;
\*_;
}
return $finfo_sub;
}
}
That way, in mp2_override_api() you don't have to do that extra eval,
and can just call this anonsub directly, no ?
>
> __________________________________________________________________
> 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
--
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'
Re: [mp2 patch] introducing Apache::compat "scoping"
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> On Sun, 2003-12-14 at 18:33, Stas Bekman wrote:
>
>>The following is yet another attempt to avoid collisions between
>>Apache::compat and the real mp2 APIs.
>
>
> That is indeed an annoying problem.
>
>
>>Similar to 'use' and 'no' pragma, I'm suggesting to introduce the new
>>functions override_mp2_api and restore_mp2_api for those APIs in
>>Apache::compat that collide with 2.0 API. The only difference is that one
>>needs an explicit call to restore_mp2_api, which I suppose could be done by
>>returning an object, which if made a lexically scoped will call
>>restore_mp2_api on its own (i.e. DESTROY), but since it's going to be used
>>very infrequently and eventually won't be needed at all, I don't see a reason
>>to bother. See the compat/request test change for an example in the patch below.
>>
>>I decided not to use the import() method, but have an explicit function call,
>>since import() may make some people think that it imports the overriden
>>methods, which it doesn't.
>
>
> I would probably prefer to use import() just to make things as easy as
> possible for the potential users of Apache::compat(), but as long as
> it's documented, I don't think I'd mind much.
You shouldn't need to use this function at all ;)
I prefer not to call it import, because it explicitly says override_mp2_api
>> 'sub handler {' .
>> 'Apache::compat::override_mp2_api('Apache::RequestRec::notes');' .
>> $code .
>> 'Apache::compat::restore_mp2_api('Apache::RequestRec::notes');' .
>> '}';
>>
>>or something like that (overriding other subs as well).
>>
>>Here is the patch:
>>
>>Index: lib/Apache/compat.pm
>>===================================================================
>>RCS file: /home/cvs/modperl-2.0/lib/Apache/compat.pm,v
>>retrieving revision 1.90
>>diff -u -r1.90 compat.pm
>>--- lib/Apache/compat.pm 19 Nov 2003 19:30:11 -0000 1.90
>>+++ lib/Apache/compat.pm 15 Dec 2003 02:02:10 -0000
>>@@ -50,6 +50,86 @@
>> $INC{'Apache/Table.pm'} = __FILE__;
>> }
>>
>>+# api => "overriding code"
>>+# the overriding code, needs to "return" the original CODE reference
>>+# when eval'ed , so that it can be restored later
>>+my %overridable_mp2_api = (
>>+ 'Apache::RequestRec::notes' => <<'EOI',
>>+{
>>+ require Apache::RequestRec;
>>+ my $notes_sub = *Apache::RequestRec::notes{CODE};
>>+ *Apache::RequestRec::notes = sub {
>>+ my $r = shift;
>>+ return wantarray()
>>+ ? ($r->table_get_set(scalar($r->$notes_sub), @_))
>>+ : scalar($r->table_get_set(scalar($r->$notes_sub), @_));
>>+ };
>>+ $notes_sub;
>>+}
>>+EOI
>>+
>>+ 'Apache::RequestRec::finfo' => <<'EOI',
>>+{
>>+ require APR::Finfo;
>>+ my $finfo_sub = *APR::Finfo::finfo{CODE};
>>+ sub Apache::RequestRec::finfo {
>>+ my $r = shift;
>>+ stat $r->filename;
>>+ \*_;
>>+ }
>>+ $finfo_sub;
>>+}
>>+EOI
>>+);
>
>
> Why make this code a SCALAR, instead of creating an anonymous sub right
> there?
What do you mean?
__________________________________________________________________
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
Re: [mp2 patch] introducing Apache::compat "scoping"
Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Sun, 2003-12-14 at 18:33, Stas Bekman wrote:
> The following is yet another attempt to avoid collisions between
> Apache::compat and the real mp2 APIs.
That is indeed an annoying problem.
> Similar to 'use' and 'no' pragma, I'm suggesting to introduce the new
> functions override_mp2_api and restore_mp2_api for those APIs in
> Apache::compat that collide with 2.0 API. The only difference is that one
> needs an explicit call to restore_mp2_api, which I suppose could be done by
> returning an object, which if made a lexically scoped will call
> restore_mp2_api on its own (i.e. DESTROY), but since it's going to be used
> very infrequently and eventually won't be needed at all, I don't see a reason
> to bother. See the compat/request test change for an example in the patch below.
>
> I decided not to use the import() method, but have an explicit function call,
> since import() may make some people think that it imports the overriden
> methods, which it doesn't.
I would probably prefer to use import() just to make things as easy as
possible for the potential users of Apache::compat(), but as long as
it's documented, I don't think I'd mind much.
> I think there are only two functions at the moment that collide with mp2 API:
> Apache::RequestRec::notes and Apache::RequestRec::finfo. There is also
> APR::URI::unparse, which introduces a special case but I think it's harmless.
>
> in June I posted an implementation for 2 more colliding functions:
> Apache::Connection::local_addr
> Apache::Connection::remote_addr
> http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=105452446932154&w=2
> which I will now be able to commit using this new functionality.
>
> The cool thing is that we can introduce a sub-class of Registry which will do
> the wrapping into a handler like so:
Coolness indeed !
> 'sub handler {' .
> 'Apache::compat::override_mp2_api('Apache::RequestRec::notes');' .
> $code .
> 'Apache::compat::restore_mp2_api('Apache::RequestRec::notes');' .
> '}';
>
> or something like that (overriding other subs as well).
>
> Here is the patch:
>
> Index: lib/Apache/compat.pm
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/lib/Apache/compat.pm,v
> retrieving revision 1.90
> diff -u -r1.90 compat.pm
> --- lib/Apache/compat.pm 19 Nov 2003 19:30:11 -0000 1.90
> +++ lib/Apache/compat.pm 15 Dec 2003 02:02:10 -0000
> @@ -50,6 +50,86 @@
> $INC{'Apache/Table.pm'} = __FILE__;
> }
>
> +# api => "overriding code"
> +# the overriding code, needs to "return" the original CODE reference
> +# when eval'ed , so that it can be restored later
> +my %overridable_mp2_api = (
> + 'Apache::RequestRec::notes' => <<'EOI',
> +{
> + require Apache::RequestRec;
> + my $notes_sub = *Apache::RequestRec::notes{CODE};
> + *Apache::RequestRec::notes = sub {
> + my $r = shift;
> + return wantarray()
> + ? ($r->table_get_set(scalar($r->$notes_sub), @_))
> + : scalar($r->table_get_set(scalar($r->$notes_sub), @_));
> + };
> + $notes_sub;
> +}
> +EOI
> +
> + 'Apache::RequestRec::finfo' => <<'EOI',
> +{
> + require APR::Finfo;
> + my $finfo_sub = *APR::Finfo::finfo{CODE};
> + sub Apache::RequestRec::finfo {
> + my $r = shift;
> + stat $r->filename;
> + \*_;
> + }
> + $finfo_sub;
> +}
> +EOI
> +);
Why make this code a SCALAR, instead of creating an anonymous sub right
there?
> +my %overridden_mp2_api = ();
> +
> +# this function enables back-compatible APIs which can't coexist with
> +# mod_perl 2.0 APIs with the same name and therefore it should be
> +# avoided if possible.
> +#
> +# it expects a list of fully qualified functions, like
> +# "Apache::RequestRec::finfo"
> +sub override_mp2_api {
> + my (@subs) = @_;
> +
> + for my $sub (@subs) {
> + unless (exists $overridable_mp2_api{$sub}) {
> + die __PACKAGE__ . ": $sub is not overridable";
> + }
> + if(exists $overridden_mp2_api{$sub}) {
> + warn __PACKAGE__ . ": $sub has been already overridden";
> + next;
> + }
> + $overridden_mp2_api{$sub} = eval $overridable_mp2_api{$sub};
> + unless (exists $overridden_mp2_api{$sub} &&
> + ref($overridden_mp2_api{$sub}) eq 'CODE') {
> + die "overriding $sub didn't return a CODE ref";
> + }
> + }
> +}
> +
> +# restore_mp2_api does the opposite of override_mp2_api(), it removes
> +# the overriden API and restores the original mod_perl 2.0 API
> +sub restore_mp2_api {
> + my (@subs) = @_;
> +
> + for my $sub (@subs) {
> + unless (exists $overridable_mp2_api{$sub}) {
> + die __PACKAGE__ . ": $sub is not overridable";
> + }
> + unless (exists $overridden_mp2_api{$sub}) {
> + warn __PACKAGE__ . ": can't restore $sub, " .
> + "as it has not been overridden";
> + next;
> + }
> + my $original_sub = delete $overridden_mp2_api{$sub};
> + no warnings 'redefine';
> + no strict 'refs';
> + *$sub = $original_sub;
> + }
> +}
> sub request {
> my $what = shift;
>
> @@ -249,15 +329,6 @@
> : scalar($r->table_get_set(scalar($r->err_headers_out), @_));
> }
>
> -{
> - my $notes_sub = *Apache::RequestRec::notes{CODE};
> - *Apache::RequestRec::notes = sub {
> - my $r = shift;
> - return wantarray()
> - ? ($r->table_get_set(scalar($r->$notes_sub), @_))
> - : scalar($r->table_get_set(scalar($r->$notes_sub), @_));
> - }
> -}
>
> sub register_cleanup {
> shift->pool->cleanup_register(@_);
> @@ -345,12 +416,6 @@
>
> sub chdir_file {
> #XXX resolve '.' in @INC to basename $r->filename
> -}
> -
> -sub finfo {
> - my $r = shift;
> - stat $r->filename;
> - \*_;
> }
>
> *log_reason = \&log_error;
> Index: t/response/TestCompat/request.pm
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/t/response/TestCompat/request.pm,v
> retrieving revision 1.3
> diff -u -r1.3 request.pm
> --- t/response/TestCompat/request.pm 11 Apr 2003 07:34:03 -0000 1.3
> +++ t/response/TestCompat/request.pm 15 Dec 2003 02:02:10 -0000
> @@ -75,6 +75,8 @@
>
> # $r->notes
> {
> + Apache::compat::override_mp2_api('Apache::RequestRec::notes');
> +
> my $key = 'notes-test';
> # get/set scalar context
> {
> @@ -98,6 +100,10 @@
> $r->notes->add($key => $_) for @exp;
> ok t_cmp(\@exp, [ $r->notes($key) ], "\$r->notes in list context");
> }
> +
> + # restore the real 2.0 notes() method, now that we are done
> + # with the compat one
> + Apache::compat::restore_mp2_api('Apache::RequestRec::notes');
> }
>
> # get_remote_host()
>
>
> __________________________________________________________________
> 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
--
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'