You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2003/11/24 10:50:00 UTC
[mp2] finfo collisions with Apache::compat
finfo() from Apache::compat collides with the new $r->finfo implementation,
the two can't coexist...
'make test' fails to detect that, but t/SMOKE did, if compat gets loaded after
APR::Finfo, we get:
[Sun Nov 23 14:01:44 2003] [error] [client 127.0.0.1] Can't locate object
method "pool" via package "IO::Handle" at
/home/stas/apache.org/mp2-cvs/t/response/TestAPR/finfo.pm line 36.
IO::Handler comes from finfo in Apache::compat.
I couldn't get the shortest sequence, and the one given by t/SMOKE is too
long, but it's clear what the problem is.
Solutions?
p.s. We need to fix the test suite to respect command line -maxclients no
matter what the minimum value is, if that was the case we could reproduce the
problem by forcing the same_interpreter with:
% t/TEST -maxclients 1 -v apr/finfo.t compat/apache_uri.t apr/finfo.t
__________________________________________________________________
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
Re: [mp2] finfo collisions with Apache::compat
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>>I still think this is a solution worthy of investigation. I just
>>>don't care
>>>about 1) and I think we might be able to work around 2) somewhat - I've
>>>created subclasses that return my own objects for $r->connection, so I
>>>would
>>>assume we can do the same for $r->server, $r->parsed_uri, etc. which
>>>leaves
>>>only stuff like Apache->server_root_relative. we can probably come up
>>>with
>>>something clever there too :)
>>
>>
>>This still requires heavy modifications in the user code. If they do so,
>>they should just move to the new API. The idea behind compat.pm is to
>>have to run the code unmodified (so neither my renaming idea fits in).
>
>
> hmm. I don't see "heavy modification." what I'm suggesting is that
> Apache::compat be a subclass of Apache. thus, like any true subclass,
> everything remains the same for the user save the constructor.
>
> the modification would now be two lines
>
> use Apache::compat;
>
> my $r = Apache::compat->new($r);
>
> rather than just
>
> use Apacahe::compat;
>
> I don't see heavy modification here at all for anything that comes from $r,
> $c, or $s. where it gets tricky are the class methods like
> server_root_relative(), and yes, those would be problematic for this approach.
>
> and, of course, it's lots of work for us to rewrite it all, too :) and I
> really don't have much of an interest in the compat layer anyway, so I'm not
> all that motivated to work on it.
Yes, it's too much work and it won't work for non-methods.
>>The smoke is only showing that users may have the same issues in their
>>code, besides not letting to smoke real problems out.
>
>
> in the case of compat there definitely will be conflicts whether it shows up
> in smoke or not. that's been well established, right? this is why I don't
> think that smoke is that big a deal with compat - we know the issues are
> there, so fixing finfo specifically is just a short-term solution. what
> really needs to happen is that we need to fix the compat layer so that it
> won't cause the problem for users. that, or just live with smoke driving
> known issues to the surface.
I guess you don't realize what the problem is with smoke. These known issues
prevent from running smoke to discover unknown ones. It's only finfo that gets
on the way.
>>If there is no co-existence solution it needs to be removed. But not
>>having a compat layer at all is not a good solution. I suggest to move
>>all those colliding methods to a different package and let people use it
>>on their own risk. Or an alternative approach is to keep Apache::compat
>>but have an import method which will enable the colliding functions on
>>demand. So if you'd want finfo, you'd say:
>>
>>use Apache::compat qw(Apache::RequestRec::finfo);
>>
>>and it'll build this compat method. There will be a big warnings
>>suggesting possible collisions with the 2.0 API.
>>
>>At the same time the harmless compat API will work as before.
>
>
> yeah, I think we had discussed that too, and it seems like a decent approach.
OK, let's do it then.
__________________________________________________________________
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
Re: [mp2] finfo collisions with Apache::compat
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> I still think this is a solution worthy of investigation. I just
>> don't care
>> about 1) and I think we might be able to work around 2) somewhat - I've
>> created subclasses that return my own objects for $r->connection, so I
>> would
>> assume we can do the same for $r->server, $r->parsed_uri, etc. which
>> leaves
>> only stuff like Apache->server_root_relative. we can probably come up
>> with
>> something clever there too :)
>
>
> This still requires heavy modifications in the user code. If they do so,
> they should just move to the new API. The idea behind compat.pm is to
> have to run the code unmodified (so neither my renaming idea fits in).
hmm. I don't see "heavy modification." what I'm suggesting is that
Apache::compat be a subclass of Apache. thus, like any true subclass,
everything remains the same for the user save the constructor.
the modification would now be two lines
use Apache::compat;
my $r = Apache::compat->new($r);
rather than just
use Apacahe::compat;
I don't see heavy modification here at all for anything that comes from $r,
$c, or $s. where it gets tricky are the class methods like
server_root_relative(), and yes, those would be problematic for this approach.
and, of course, it's lots of work for us to rewrite it all, too :) and I
really don't have much of an interest in the compat layer anyway, so I'm not
all that motivated to work on it.
> The smoke is only showing that users may have the same issues in their
> code, besides not letting to smoke real problems out.
in the case of compat there definitely will be conflicts whether it shows up
in smoke or not. that's been well established, right? this is why I don't
think that smoke is that big a deal with compat - we know the issues are
there, so fixing finfo specifically is just a short-term solution. what
really needs to happen is that we need to fix the compat layer so that it
won't cause the problem for users. that, or just live with smoke driving
known issues to the surface.
>
> If there is no co-existence solution it needs to be removed. But not
> having a compat layer at all is not a good solution. I suggest to move
> all those colliding methods to a different package and let people use it
> on their own risk. Or an alternative approach is to keep Apache::compat
> but have an import method which will enable the colliding functions on
> demand. So if you'd want finfo, you'd say:
>
> use Apache::compat qw(Apache::RequestRec::finfo);
>
> and it'll build this compat method. There will be a big warnings
> suggesting possible collisions with the 2.0 API.
>
> At the same time the harmless compat API will work as before.
yeah, I think we had discussed that too, and it seems like a decent approach.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] finfo collisions with Apache::compat
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Stas Bekman wrote:
>
>>Geoff, any chance we can get this one resolved? As we may make a new
>>release soonish, we can't have this collision in Apache::compat and
>>APR::Finfo get out from the dev release.
>
>
> I really don't know the answer to this. the new finfo API is a proper
> interface, and I don't see removing it just to appease the compat layer,
> especially since it's just exposing a problem that many compat methods have.
I've never suggested that. I'm just pushing to get the conflict resolved
before we release a new version.
>>>what about a new namespace? instead of finfo_old make everyone using
>>>compat do this
>>>
>>>my $r = Apache::compat($r);
>>>
>>>then $r->finfo would be certain to call Apache::compat::finfo.
>
>
> actually, I meant Apache::compat->new($r) but you get the idea :)
>
>
>>It's a cool idea, but it 1) requires users to change their code, whereas
>>for most case they don't need to with the current Apache::compat 2) it
>>won't work for functions which aren't invoked on $r
>
>
> I still think this is a solution worthy of investigation. I just don't care
> about 1) and I think we might be able to work around 2) somewhat - I've
> created subclasses that return my own objects for $r->connection, so I would
> assume we can do the same for $r->server, $r->parsed_uri, etc. which leaves
> only stuff like Apache->server_root_relative. we can probably come up with
> something clever there too :)
This still requires heavy modifications in the user code. If they do so, they
should just move to the new API. The idea behind compat.pm is to have to run
the code unmodified (so neither my renaming idea fits in).
>>So if we already require users to change their code, let's just rename
>>the method. How about adding _mp1 postfix for those methods that collide?
>>
>
>
> I don't think that's a good idea at all - you might as well not have a
> compat layer at all then.
>
> in the short term, though, I'd be in favor of removing
> Apache::compat::finfo() if the smoke failures are bothersome - finfo() just
> isn't all that popular (unfortunately).
The smoke is only showing that users may have the same issues in their code,
besides not letting to smoke real problems out.
If there is no co-existence solution it needs to be removed. But not having a
compat layer at all is not a good solution. I suggest to move all those
colliding methods to a different package and let people use it on their own
risk. Or an alternative approach is to keep Apache::compat but have an import
method which will enable the colliding functions on demand. So if you'd want
finfo, you'd say:
use Apache::compat qw(Apache::RequestRec::finfo);
and it'll build this compat method. There will be a big warnings suggesting
possible collisions with the 2.0 API.
At the same time the harmless compat API will work as before.
__________________________________________________________________
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
Re: [mp2] finfo collisions with Apache::compat
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Geoff, any chance we can get this one resolved? As we may make a new
> release soonish, we can't have this collision in Apache::compat and
> APR::Finfo get out from the dev release.
I really don't know the answer to this. the new finfo API is a proper
interface, and I don't see removing it just to appease the compat layer,
especially since it's just exposing a problem that many compat methods have.
>> what about a new namespace? instead of finfo_old make everyone using
>> compat do this
>>
>> my $r = Apache::compat($r);
>>
>> then $r->finfo would be certain to call Apache::compat::finfo.
actually, I meant Apache::compat->new($r) but you get the idea :)
> It's a cool idea, but it 1) requires users to change their code, whereas
> for most case they don't need to with the current Apache::compat 2) it
> won't work for functions which aren't invoked on $r
I still think this is a solution worthy of investigation. I just don't care
about 1) and I think we might be able to work around 2) somewhat - I've
created subclasses that return my own objects for $r->connection, so I would
assume we can do the same for $r->server, $r->parsed_uri, etc. which leaves
only stuff like Apache->server_root_relative. we can probably come up with
something clever there too :)
>
> So if we already require users to change their code, let's just rename
> the method. How about adding _mp1 postfix for those methods that collide?
>
I don't think that's a good idea at all - you might as well not have a
compat layer at all then.
in the short term, though, I'd be in favor of removing
Apache::compat::finfo() if the smoke failures are bothersome - finfo() just
isn't all that popular (unfortunately).
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] finfo collisions with Apache::compat
Posted by Stas Bekman <st...@stason.org>.
Geoff, any chance we can get this one resolved? As we may make a new release
soonish, we can't have this collision in Apache::compat and APR::Finfo get out
from the dev release.
Thanks.
__________________________________________________________________
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
Re: [mp2] finfo collisions with Apache::compat
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
>>>> finfo() from Apache::compat collides with the new $r->finfo
>>>> implementation, the two can't coexist...
>>>
>>>
>>>
>>>
>>> this is actually a problem with more than just $r->finfo - in places
>>> where the method is redefined and the new one takes different
>>> arguments than the old, right?
>>
>>
>>
>> Different arguments but also the same number of them or none, yes. If
>> the number of arguments is different we can code around it in compat.
>
>
> actually, finfo might be worse than the others - it returns an object
> where it used to return _, so we have a whole new class of
> $finfo->method calls we didn't have before, which will only resolve at
> runtime. ack.
there are others of its kind.
>> But the problem with $r->finfo is that it breaks the smoking, whereas
>> others aren't (I think other aren't exercised in both forms)
>
>
> well, I guess we need to add more compat tests so that finfo won't be so
> lonely ;)
;)
> we could disable the compat finfo tests but keep both implementations
> until we come up with something better.
there aren't compat finfo tests. it's about the force of reload of
Apache::compat in one of the tests.
> really, I'm not too worried
> about the compat tests. we ought to test as much as we can there, of
> course, but the compat layer has lots of issues now for people who want
> to use both compat and the new api (as we all know), so I don't see the
> need to worry too much about tests that illustrate this brokenness.
We need to resolve this for this and other methods which have this problem.
It's not so much about the tests but the end users.
[...]
>> May be renaming finfo to finfo_old in compat.pm, so that at least
>> it'll be easier to do the initial transition by just adjusting only
>> one bit and not many. In fact that what I think we should do for all
>> functions in compat.pm that collide. What do you think? May be we
>> should ask the users?
>
>
> hmm...
>
> what about a new namespace? instead of finfo_old make everyone using
> compat do this
>
> my $r = Apache::compat($r);
>
> then $r->finfo would be certain to call Apache::compat::finfo. in fact,
> that approach might help us lots in the long run - CPAN modules could
> now use Apache::compat (though we still would not endorse it) and not
> muck around with all the other mp2 namespaces out there. well, maybe it
> wouldn't help with Apache::Constants and the other non-$r based APIs,
> but it would be an improvement over how things are currently, at least.
It's a cool idea, but it 1) requires users to change their code, whereas for
most case they don't need to with the current Apache::compat 2) it won't work
for functions which aren't invoked on $r 3) definititely not a good idea of
using compat in CPAN modules because at least of (2).
So if we already require users to change their code, let's just rename the
method. How about adding _mp1 postfix for those methods that collide?
__________________________________________________________________
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
Re: [mp2] finfo collisions with Apache::compat
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>>> finfo() from Apache::compat collides with the new $r->finfo
>>> implementation, the two can't coexist...
>>
>>
>>
>> this is actually a problem with more than just $r->finfo - in places
>> where the method is redefined and the new one takes different
>> arguments than the old, right?
>
>
> Different arguments but also the same number of them or none, yes. If
> the number of arguments is different we can code around it in compat.
actually, finfo might be worse than the others - it returns an object where
it used to return _, so we have a whole new class of $finfo->method calls we
didn't have before, which will only resolve at runtime. ack.
> But the problem with $r->finfo is that it breaks the smoking, whereas
> others aren't (I think other aren't exercised in both forms)
well, I guess we need to add more compat tests so that finfo won't be so
lonely ;)
we could disable the compat finfo tests but keep both implementations until
we come up with something better. really, I'm not too worried about the
compat tests. we ought to test as much as we can there, of course, but the
compat layer has lots of issues now for people who want to use both compat
and the new api (as we all know), so I don't see the need to worry too much
about tests that illustrate this brokenness.
>
>>> Solutions?
>>
>>
>>
>> I have no idea. I'm not inclined to change anything in 2.0 core just
>> because the compat layer has issues - presumably, things are "right"
>> in core (finfo and otherwise).
>>
>> the easiest fix would be to disable finfo from compat, which I'd be ok
>> with since finfo wasn't really all that popular anyway (unfortunately).
>
>
> Disabling is imperfect, since the new code is way too different.
imperfect, yes.
> May be
> renaming finfo to finfo_old in compat.pm, so that at least it'll be
> easier to do the initial transition by just adjusting only one bit and
> not many. In fact that what I think we should do for all functions in
> compat.pm that collide. What do you think? May be we should ask the users?
hmm...
what about a new namespace? instead of finfo_old make everyone using compat
do this
my $r = Apache::compat($r);
then $r->finfo would be certain to call Apache::compat::finfo. in fact,
that approach might help us lots in the long run - CPAN modules could now
use Apache::compat (though we still would not endorse it) and not muck
around with all the other mp2 namespaces out there. well, maybe it wouldn't
help with Apache::Constants and the other non-$r based APIs, but it would be
an improvement over how things are currently, at least.
?
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] finfo collisions with Apache::compat
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
>
> Stas Bekman wrote:
>
>> finfo() from Apache::compat collides with the new $r->finfo
>> implementation, the two can't coexist...
>
>
> this is actually a problem with more than just $r->finfo - in places
> where the method is redefined and the new one takes different arguments
> than the old, right?
Different arguments but also the same number of them or none, yes. If the
number of arguments is different we can code around it in compat. But the
problem with $r->finfo is that it breaks the smoking, whereas others aren't (I
think other aren't exercised in both forms)
>> Solutions?
>
>
> I have no idea. I'm not inclined to change anything in 2.0 core just
> because the compat layer has issues - presumably, things are "right" in
> core (finfo and otherwise).
>
> the easiest fix would be to disable finfo from compat, which I'd be ok
> with since finfo wasn't really all that popular anyway (unfortunately).
Disabling is imperfect, since the new code is way too different. May be
renaming finfo to finfo_old in compat.pm, so that at least it'll be easier to
do the initial transition by just adjusting only one bit and not many. In fact
that what I think we should do for all functions in compat.pm that collide.
What do you think? May be we should ask the users?
__________________________________________________________________
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
Re: [mp2] finfo collisions with Apache::compat
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> finfo() from Apache::compat collides with the new $r->finfo
> implementation, the two can't coexist...
this is actually a problem with more than just $r->finfo - in places where
the method is redefined and the new one takes different arguments than the
old, right?
> Solutions?
I have no idea. I'm not inclined to change anything in 2.0 core just
because the compat layer has issues - presumably, things are "right" in core
(finfo and otherwise).
the easiest fix would be to disable finfo from compat, which I'd be ok with
since finfo wasn't really all that popular anyway (unfortunately).
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org