On Thu, 2003-06-19 at 16:16, Steve Hay wrote:
> Hi Stas,
>
> Stas Bekman wrote:
>
> >>> Look at ModPerl::RegistryCooker::flush_namespace_normal, which does
> >>> the necessary undef'ing. You really need just a chunk of it.
> >>
> >>
> >>
> >> I've had a look at this, and can't get it to work properly. I've
> >> combined the subroutine undef'ing from that function with the
> >> constant subroutine mandatory warning handling from the mp2 version
> >> of Apache::Reload, but when I try it out I find that (a) all the
> >> constant subroutines produce warnings about prototype mismatches, and
> >> (b) all the other subroutines still produce warnings about being
> >> redefined!
> >>
> >> The following short test script reproduces both of these problems:
> >>
> >> ### START
> >>
> >> use strict;
> >> use warnings;
> >>
> >> use constant FOO => 1;
> >> sub bar { 1; }
> >>
> >> BEGIN {
> >> $SIG{__WARN__} = sub {
> >> return if $_[0] =~ /^Constant subroutine [\w:]+ redefined at/;
> >> CORE::warn(@_);
> >> };
> >> foreach my $fullname ('main::FOO', 'main::bar') {
> >> if (defined &$fullname) {
> >> if (defined(my $p = prototype $fullname)) {
> >> no strict 'refs';
> >> no warnings 'redefine';
> >> *{$fullname} = eval "sub ($p) {}";
> >> }
> >> else {
> >> no strict 'refs';
> >> no warnings 'redefine';
> >> *{$fullname} = sub {};
> >> }
> >> undef &$fullname;
> >> }
> >> }
> >> }
> >>
> >> use constant FOO => 2;
> >> sub bar { 2; }
> >>
> >> ### END
> >>
> >> This program produces the following warnings:
> >>
> >> Prototype mismatch: sub main::FOO vs () at C:/perl5/lib/constant.pm
> >> line 108.
> >> Subroutine bar redefined at C:\Temp\test.pl line 30.
> >>
> >> By contrast, the solution that I was originally working towards,
> >> using Apache::Symbol::undef(), looks much simpler:
> >>
> >> ### START
> >>
> >> use strict;
> >> use warnings;
> >>
> >> use constant FOO => 1;
> >> sub bar { 1; }
> >>
> >> BEGIN {
> >> require Apache::Symbol;
> >> foreach my $fullname ('main::FOO', 'main::bar') {
> >> no strict 'refs';
> >> Apache::Symbol::undef(*{$fullname}{CODE});
> >> }
> >> }
> >>
> >> use constant FOO => 2;
> >> sub bar { 2; }
> >>
> >> ### END
> >>
> >> and produces no warnings.
> >>
> >> Unless I'm missing something, this suggests to me that we do still
> >> need (or least, could greatly benefit from having) Apache::Symbol in
> >> mp2. I would like to see it put back so that I can produce an
> >> Apache::Reload patch that uses it.
> >>
> >> There are three options here:
> >>
> >> 1. Put back Apache::Symbol and use it's undef() function.
> >> 2. Copy the Apache::Symbol::undef() function into Apache::Reload.
> >> 3. Explain why the first test script above doesn't work and fix it!
> >>
> >> I favour option 1. What's your vote?
> >
> >
> > We should certainly fix the problems you have mentioned. Thanks for
> > raising that issue. Is this still the same list after Philippe has
> > posted his patch?
>
> I guess the three options still stand, but now Philippe has proposed a
> fourth option -- copy the Apache::Symbol::undef() function into
> ModPerl::Util for mp2, and just use Apache::Symbol for mp1.Yes, and as soon as we can agree on a good name for it, I'll get comitted ;) > As for fixing the problems shown up by the first test script above, > maybe the best thing to do is "option five": put the remove_package() > function that my last patch proposed for Apache::Reload (based on > Philippe's code) into some other module (maybe ModPerl::Util) so that it > can be used instead of ModPerl::RegistryCooker::flush_namespace_normal()? Yes, it's quite generic enough and might be usefull to anybody doing some kind of smart code caching/reloading. So I'd definitely be for adding something like remove_package() into mp2's core somewhere. > In fact, the Symbol module already contains a function called > delete_package() which does a similar job again, though even less well > still! (It doesn't appear to have any magic to circumvent those > mandatory warnings about constant subroutines, or to avoid undef'ing > stuff imported from other packages.) True, but after looking at it, I believe it does 1-2 things a bit smarter then my suggested code (i.e. deleting the namespace) and has the added benefit of being self-contained, with no dependencies on other modules, core or non-core. > The "best" solution, I suppose, > would be "option six": get Symbol::delete_package() updated using > Philippe's code and the undef() function from Apache::Symbol, and then > have both Apache::Reload and ModPerl::RegistryCooker use > Symbol::delete_package() instead of having their own versions of what is > basically a "core" (as opposed to mod_perl-specific) function. Not really, see below. > However, nothing's perfect in this world. Option six would make > Apache::Reload dependent on whatever version of Perl the fixed Symbol > module appeared in (5.8.1 at the earliest) because Symbol isn't > distributed separately on CPAN. So maybe option five is the most practical. I believe the simplest thing would be to rip out the code from Symbol, add a bit of the undefication magic I proposed and move it all into mp2 somewhere under ModPerl::Util::*. While at it, I might give a shot at writing it in C for speed and availability to the core, like <Perl > sections who need a good way to wipe namespaces (to name only one possible usage). That way we are not depending on Symbol or anything else for that matter. Of course, I might also consider posting a patch to Symbol.pm to include some of the magic to avoid warnings as well. Gozer out. > Steve > > > --------------------------------------------------------------------- > 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
