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:[EMAIL PROTECTED] 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}'
signature.asc
Description: This is a digitally signed message part
