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?
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.
[...]
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
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.
__________________________________________________________________ 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]
