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([EMAIL PROTECTED], [ $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:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com > http://modperlbook.org http://apache.org http://ticketmaster.com > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] -- -------------------------------------------------------------------------------- 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}'
signature.asc
Description: This is a digitally signed message part
