Mailing-List: contact dev-help@perl.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <3FD778D0.4010003@stason.org>
Date: Wed, 10 Dec 2003 11:49:36 -0800
From: Stas Bekman <stas@stason.org>
Organization: Hope, Humanized
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030630
MIME-Version: 1.0
To: "Philippe M. Chiasson" <gozer@cpan.org>
Cc: dev@perl.apache.org
Subject: Re: [Patch mp2] PerlSections namespace
References: <1071000267.29232.33.camel@localhost.localdomain>
	 <3FD67A68.3020407@stason.org>
 <1071084252.32353.12.camel@localhost.localdomain>
In-Reply-To: <1071084252.32353.12.camel@localhost.localdomain>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

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?

> 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:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org