Mailing-List: contact dev-help@perl.apache.org; run by ezmlm
Precedence: bulk
Subject: Re: [Patch mp2] PerlSections namespace
From: "Philippe M. Chiasson" <gozer@cpan.org>
To: Stas Bekman <stas@stason.org>
Cc: dev@perl.apache.org
In-Reply-To: <3FD67A68.3020407@stason.org>
References: <1071000267.29232.33.camel@localhost.localdomain>
	 <3FD67A68.3020407@stason.org>
Content-Type: multipart/signed; micalg=pgp-sha1;
 protocol="application/pgp-signature"; boundary="=-G/+neIWVyANIxOz1i3Lh"
Message-Id: <1071084252.32353.12.camel@localhost.localdomain>
Mime-Version: 1.0
Date: Wed, 10 Dec 2003 11:24:13 -0800

--=-G/+neIWVyANIxOz1i3Lh
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

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.
> >=20
> > The following patch follows a similar design than ModPerl::Registry,
> > putting each <Perl> block in it's own namespace, based on filename &
> > lineno.
>=20
> Coolio ;)
>=20
> > This now prevents infinite-recursion problems and makes $Includes from
> > within <Perl> sections work fine.
> >=20
> > 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.
>=20
> You mean it will break their code. In which case I'd suggest not to commi=
t=20
> 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.

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?

> > As usual, look at it and tell me if it breaks more stuff than it fixes
> > ;-)
>=20
> I suppose it was a proof of concept, since you need to fix quite a few st=
yle=20
> things (long lines, some missing indent, missing spaces around ops).

Yes, you supposed right ;-) I just wanted review on it.

> > 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?
>=20
> Good idea. I've just introduced the opposite function package2filename (t=
o fix=20
> the problems in modperl_mgv). I'm not sure it fits registrycooker, since =
it=20
> doesn't drop /, whereas your function does, collapsing everything into a=20
> single word. It may have problems, since these two distinct names will ma=
p=20
> into the same namespace:
>=20
> foo/barbaz
> foobar/baz
>=20
> 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 =3D> foo_barbaz
foobar/baz =3D> foobar_baz

//foobar/baz =3D> foobar_baz
/foobar/baz =3D> foobar_baz

>=20
> modperl_file2package has its own problems ;) a stash name can't start wit=
h=20
> numerical and ':'. See below:

Can't start with a numerical? Oh, I'll make sure I check for that as
well then.

> [...]
> > +#define MP_VALID_PKG_CHAR(c) (isalnum(c)||c=3D=3D':'||c=3D=3D'_')
> > +static const char *modperl_file2package(apr_pool_t *p, const char *fil=
e)
> > +{
> > +    char *package;
> > +    char *c;
> > +
> > +    c =3D package =3D apr_pcalloc(p, strlen(file));
> > +
> > +    /* First, skip invalid prefix characters */
> > +    while (!MP_VALID_PKG_CHAR(*file)) {
> > +        file++;
> > +    }
>=20
> You need here:
>=20
> #define MP_VALID_PKG_FIRST_CHAR(c) (isalpha(c) || c =3D=3D '_')
>=20
> /* First, skip invalid prefix characters */
> while (!MP_VALID_PKG_FIRST_CHAR(*file)) {
>        file++;
> }
>=20
> Neither you can have a single ':' in the package name (or triple, etc), s=
o you=20
> 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=20

C:\FooBar\Baz\Bof =3D> C_FooBar_Baz_Bof

> __________________________________________________________________
> 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
--=20
---------------------------------------------------------------------------=
-----
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634=
E37B)
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 ingenio=
us.
perl -e'$$=3D\${gozer};{$_=3Dunpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$+=
+&&redo}'

--=-G/+neIWVyANIxOz1i3Lh
Content-Type: application/pgp-signature; name=signature.asc
Content-Description: This is a digitally signed message part

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQA/13LcyzKhB4jDpaURAoqQAKCZo0fs3ckqbjD0IgP5YZSS7A4oLgCfZu/y
vUlrGrzavQddbwkvOZ41e4A=
=5yNt
-----END PGP SIGNATURE-----

--=-G/+neIWVyANIxOz1i3Lh--