perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philippe M. Chiasson" <go...@cpan.org>
Subject Re: [Patch mp2] PerlSections namespace
Date Wed, 10 Dec 2003 20:41:40 GMT
On Wed, 2003-12-10 at 11:49, Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> > On Tue, 2003-12-09 at 17:44, Stas Bekman wrote:
> > 
> >>Philippe M. Chiasson wrote:
> >>
> >>>It's been a long time that perl sections have been having trouble with
> >>>recursive inclusion. This has been discussed before and has to do with
> >>>the fact that all PerlSections are evaluated in the same namespace
> >>>Apache::ReadSections.
> >>>
> >>>The following patch follows a similar design than ModPerl::Registry,
> >>>putting each <Perl> block in it's own namespace, based on filename
&
> >>>lineno.
> >>
> >>Coolio ;)
> >>
> >>
> >>>This now prevents infinite-recursion problems and makes $Includes from
> >>>within <Perl> sections work fine.
> >>>
> >>>There is still one little problem left with this, people will not be
> >>>able to put stuff directly in the Apache::ReadSections namespace
> >>>themselves. I do have a plan for fixing that as well in a subsequent
> >>>patch.
> >>
> >>You mean it will break their code. In which case I'd suggest not to commit 
> >>this patch till you get it all ready.
> > 
> > 
> > Yeah, well, things are already broken quite a bit w/r to <Perl >
> > sections, so I wanted to fix a problem at a time, even though if it
> > temporarely introduces another little different problem.
> 
> Yes, but your change is going to introduce a new problem, and if it's not 
> fixed by the time of the new release, it'll break people's code which worked 
> fine with the previous release. Not too friendly, hey? And I think we should 
> do a new release rsn. What do you think?

You are quite right about this. Especially with a release on the
horizon. In that case, I'd lean towards solution #1, easily achieved by
having PerlSections iterate over symbols in the generated namespace AND
%Apache::ReadConfig on each pass. This ends up being just like before,
and would fix recursive <Perl > section problems as well.

> > Now that we are sticking <Perl> sections in their own distinct
> > namespaces, it means that for people who put stuff directly in
> > Apache::ReadConfig, I can think of 2 ways of handling it.
> 
> > 1. Apache::PerlSections defaults to reading from the unique package and
> > also process the contents of Apache::ReadConfig.
> > 
> > Problem with this (same as it is right now) , is that since people just
> > add to the %Apache::ReadConfig:: namespace, each time a <Perl> block
> > ends, the sum of its content is fed back into apache, so you get
> > warnings and errors, since directives are processed multiple times.
> > 
> > 2. Leave %Apache::ReadConfig:: alone until right after the config phase
> > and feed it to Apache::PerlSections in one go
> > 
> > Problem with this that processing of Apache::ReadConfig (for modules and
> > code that push into it themselves) will be delayed at the very end and
> > might break existing things that expects it to be processed at the end
> > of the <Perl> block that ran...
> > 
> > I dunno about anybody else, but I prefer solution 2, but what do you
> > think?
> 
> I think you also have a problem of ordering thing. As you may end up with 
> overriding order problem, no? I think 1 is safer and making it deprecated will 
> eventually make the problem go away completely. Am I correct to say that 
> people don't need to work directly with Apache::ReadConfig? I think the only 
> place this is needed when people mix config with non-config perl code in 
> <Perl>, so that they need to switch 'package Apache::ReadConfig;' to start 
> feeding things into the config.

As far as I can tell, now that we have Apache::Server->add_config, I'd
be more than happy to deprecate direct usage of the
%Apache::ReadConfig:: namespace.

Ordering as of today is still a problem anyways (in Location,
VirtualHost and other containers). In mp1 we were using Tie::IxHash to
work around this, and I am planning on doing the same thing in the near
future to address that problem.

> [...]
> 
> >>>I had to introduce a function in mod_perl_util.c, modperl_file2package,
> >>>that makes a package-safe name from a filepath, so maybe it could be
> >>>exposed and used by ModPerl::Registry as well?
> >>
> >>Good idea. I've just introduced the opposite function package2filename (to fix

> >>the problems in modperl_mgv). I'm not sure it fits registrycooker, since it 
> >>doesn't drop /, whereas your function does, collapsing everything into a 
> >>single word. It may have problems, since these two distinct names will map 
> >>into the same namespace:
> >>
> >>foo/barbaz
> >>foobar/baz
> >>
> >>so the concept of dropping / is wrong. You need to replace each '/' with '::'.
> > 
> > 
> > I am not dropping it, just if it's at the beginning, for example
> > 
> > foo/barbaz => foo_barbaz
> > foobar/baz => foobar_baz
> > 
> > //foobar/baz => foobar_baz
> > /foobar/baz => foobar_baz
> 
> OK, but I can still see how it it breaks. Consider:
> 
> foo-/baz => foo_baz
> foo/-baz => foo_baz
> 
> whereas this works:
> 
> foo-/baz => foo_::baz
> foo/-baz => foo::_baz
> 
> I think you shouldn't replace groups of chars with a single _, you must 
> preserve the lent

Correct, I didn't realize that. One question I have though is for win32.

Would it be 100% safe to :

1. replace occurences of '/' and '\' (path separators) with '::'
2. replace non alphanumeric with '_'
3. insure resulting package doesn't start with '::' or numeric

I know File::Spec would be the nicest way of going around doing it:

my $name = join('::', File::Spec->splitpath($filename));
$name =~ s/[^a-zA-Z0-9]/_/g;

> >>modperl_file2package has its own problems ;) a stash name can't start with 
> >>numerical and ':'. See below:
> > 
> > 
> > Can't start with a numerical? Oh, I'll make sure I check for that as
> > well then.
> 
> It can start only with _ or alpha
> 
> >>[...]
> >>
> >>>+#define MP_VALID_PKG_CHAR(c) (isalnum(c)||c==':'||c=='_')
> >>>+static const char *modperl_file2package(apr_pool_t *p, const char *file)
> >>>+{
> >>>+    char *package;
> >>>+    char *c;
> >>>+
> >>>+    c = package = apr_pcalloc(p, strlen(file));
> >>>+
> >>>+    /* First, skip invalid prefix characters */
> >>>+    while (!MP_VALID_PKG_CHAR(*file)) {
> >>>+        file++;
> >>>+    }
> >>
> >>You need here:
> >>
> >>#define MP_VALID_PKG_FIRST_CHAR(c) (isalpha(c) || c == '_')
> >>
> >>/* First, skip invalid prefix characters */
> >>while (!MP_VALID_PKG_FIRST_CHAR(*file)) {
> >>       file++;
> >>}
> >>
> >>Neither you can have a single ':' in the package name (or triple, etc), so you

> >>should check that they come in doubles...
> > 
> > 
> > Well, even simpler, if I just remove ':' from the allowed characters,
> > I'll avoid that problem (and keep the string the same size or smaller)
> > 
> > So 
> > 
> > C:\FooBar\Baz\Bof => C_FooBar_Baz_Bof
> 
> I think you shouldn't replace groups of chars with a single _, you must 
> preserve their length, but even then you are prone to collisions:
> 
> foo--+bar => foo___bar
> foo+--bar => foo___bar
> 
> I don't think there is a good solution here, as you apply a lossful 
> transformation. We have the same problem with registry, but nobody has ever 
> complained, since most files include alphanumerics only.

So, can we agree on what will agree to be doing ? Then I'll be happy to
write it, stick it in modperl_util.c, expose it thru ModPerl::Util (for
registry) and use it for PerlSection's namespaces

> __________________________________________________________________
> 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
-- 
--------------------------------------------------------------------------------
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}'

Mime
View raw message