You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Jeff Genender <jg...@savoirtech.com> on 2005/09/29 21:04:20 UTC
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on
Logging ClassCastException
> David Jencks commented on GERONIMO-518:
> ---------------------------------------
>
> I'm ok with including commons-logging in the hardcoded list of exclusions
> since we are really and inextricably using it as part of the app server.
> I agree that making the exclusion list configurable in the plan is a very
> good idea but I do not want to try to get it implemented for M5. Can we
> open a separate issue for that for 1.0?
Yes...then feel free to apply the patch (after Kevan fixes it) and open a
new issue (or you can leave this one open ;-)
But I want to emphasize my discomfort with harcoding a commons package
into these loaders as it ultimately takes waya control from the user. As
long as we can back this out and do a proper exclusion list in a
configurable plan, then I am cool with it.
Thanks,
Jeff
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on Logging ClassCastException
Posted by "Geir Magnusson Jr." <ge...@apache.org>.
On Oct 1, 2005, at 11:50 AM, David Jencks wrote:
>
> On Oct 1, 2005, at 7:34 AM, Aaron Mulder wrote:
>
>
>> On 9/29/05, Jeff Genender <jg...@savoirtech.com> wrote:
>>
>>> But I want to emphasize my discomfort with harcoding a commons
>>> package
>>> into these loaders as it ultimately takes waya control from the
>>> user. As
>>> long as we can back this out and do a proper exclusion list in a
>>> configurable plan, then I am cool with it.
>>>
>>
>> Could you explain what you mean here? I think we've already seen
>> that
>> in the case of commons logging, if you give that control to the user
>> (by exposing a list including commons logging) and they exercise it
>> (by removing commons logging from the list and including their own
>> commons logging JAR), they get a ClassCastException -- which is to
>> say, it doesn't work include your own commons logging and try to use
>> it instead of Geronimo's version. I think the only way to work
>> around
>> that is a much more detailed restructuring of our ClassLoader
>> hierarchy. Do you have another proposal for "making commons logging
>> overridable"?
>>
>>
> With our current classloaders I think we need to include commons
> logging in the exclusion list for the reasons Aaron explains.
> There may be a possibility of actually solving the problem with
> more sophisticated classloaders that actually hide all the server
> classes from the applications. From some conversations with Dain I
> think the OSGI classloaders are capable of doing this, and I think
> he is looking into the possibility of using them or something like
> them. I suspect this would be a post 1.0 change however. For 1.0
> I think the best we can hope for is a configurable exclusion list,
> and my understanding is that you should not be able to remove
> commons logging from the "fixed" part of the exclusion list.
>
If there's ever a reason why a user would want to remove it from the
"fixed" part of the exclusion list, maybe allow a minus sign or
something in front of the class name in the exclusion configuration,
like "-org.apache.Clogging" which says "Don't exclusde - I know what
I'm doing so let me do this"
Is that a resonable compromise?
geir
--
Geir Magnusson Jr +1-203-665-6437
geirm@apache.org
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on Logging ClassCastException
Posted by Aaron Mulder <am...@alumni.princeton.edu>.
On 10/1/05, Jeff Genender <jg...@savoirtech.com> wrote:
> Yes. Lets make a more real scenario. Lets say I write...not my own
> logging, but my own factory to do whatever, that needs package access to
> the o.a.c.l because something is protect in there. With our hard code,
> I cannot use my class. By creating an exclusionary list, I can exclude
> packages and classes, but allow my new class through.
OK, I believe that if you create some new classes in the same
org.apache.commons.logging package and don't modify any existing
commons-logging packages and then use *only* parent-first class
loading then you could manage to load your custom classes. Just like
if you added a new class to javax.servlet, in the absence of package
loading restrictions, you could then load the class javax.servlet.Foo
from your web app.
However, I don't think it's necessary for us to support this scenario.
I'm comfortable saying that if people want to provide their own
commons logging implementations or whatever then they should do it in
a different package.
If someone shows up with a compelling reason -- something really
useful that their app depends on and requires access to package-level
stuff in commons logging, OK, we should reconsider.
But in the mean time, let's just forcibly exclude the commons logging
package. I think eliminating all possible bug reports caused by
reading commons logging code in WEB-INF/lib is worth the down side of
not allowing the scenario you present, at least until that compelling
reason shows up.
As you said on IRC, hopefully we'll move on to a better class loader
arrangement or drop commons-logging before that need to reconsider
rolls around.
Thanks,
Aaron
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on
Logging ClassCastException
Posted by Jeff Genender <jg...@savoirtech.com>.
Aaron Mulder wrote:
>
> OK, so lets follow this scenario. You create a custom
> commons-logging. In order to get your app to use it, you have 2
> options:
>
> 1) Replace the commons-logging that Geronimo uses (just copy over the
> file in the repository or whatever).
>
> Result: Whatever we do with exclusion lists is irrelevant in this
> case, since there's only 1 version of commons-logging -- your patched
> one. Your patched one is used.
IMHO, this seems almost as bad as forcing the user to remove their own
commons-logging from web-inf/lib.
>
> 2) You add your patched version to WEB-INF/lib so your web app will
> use it, but Geronimo won't. If we have an exclusion list, you remove
> commons-logging from it. You set the web app class loading flag to
> load from the WEB-INF/lib before the parent.
>
> Result: You get a ClassCastException because your version in
> WEB-INF/lib has to touch our global logging instances which implement
> our commons logging interfaces, not yours.
>
> Do you disagree?
Yes. Lets make a more real scenario. Lets say I write...not my own
logging, but my own factory to do whatever, that needs package access to
the o.a.c.l because something is protect in there. With our hard code,
I cannot use my class. By creating an exclusionary list, I can exclude
packages and classes, but allow my new class through.
Jeff
>
> Aaron
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on Logging ClassCastException
Posted by Aaron Mulder <am...@alumni.princeton.edu>.
Jeff, I think I'm OK with your conclusion, but I am not at all in sync
with your logic.
On 10/1/05, Jeff Genender <jg...@savoirtech.com> wrote:
> I don't agree with you about not removing the hard coded part. What if
> I write my own patches to commons-logging to fix some of these issues
> and do my own specialized logging.
OK, so lets follow this scenario. You create a custom
commons-logging. In order to get your app to use it, you have 2
options:
1) Replace the commons-logging that Geronimo uses (just copy over the
file in the repository or whatever).
Result: Whatever we do with exclusion lists is irrelevant in this
case, since there's only 1 version of commons-logging -- your patched
one. Your patched one is used.
2) You add your patched version to WEB-INF/lib so your web app will
use it, but Geronimo won't. If we have an exclusion list, you remove
commons-logging from it. You set the web app class loading flag to
load from the WEB-INF/lib before the parent.
Result: You get a ClassCastException because your version in
WEB-INF/lib has to touch our global logging instances which implement
our commons logging interfaces, not yours.
Do you disagree?
Aaron
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on
Logging ClassCastException
Posted by Jeff Genender <jg...@savoirtech.com>.
David Jencks wrote:
>>
> With our current classloaders I think we need to include commons logging
> in the exclusion list for the reasons Aaron explains.
Agreed - there never was any argument here. Already done..GERONIMO-518
patches have been applied and committed to HEAD and M5.
> There may be a
> possibility of actually solving the problem with more sophisticated
> classloaders that actually hide all the server classes from the
> applications. From some conversations with Dain I think the OSGI
> classloaders are capable of doing this, and I think he is looking into
> the possibility of using them or something like them.
+1. ;-) I look forward to working on this stuff.
> I suspect this
> would be a post 1.0 change however. For 1.0 I think the best we can
> hope for is a configurable exclusion list, and my understanding is that
> you should not be able to remove commons logging from the "fixed" part
> of the exclusion list.
I don't agree with you about not removing the hard coded part. What if
I write my own patches to commons-logging to fix some of these issues
and do my own specialized logging. I then need the o.a.c.l package
structure, as a user. By hardcoding this, you have taken that ability
for me to do this...my jars will never get used. I am against
this...this takes away complete control from the user to patch and make
their own o.a.c.l versions. What happens then? Do we tell the user
"too bad"...or should we give them a choice of excluding it via a
configuration, so they can roll their own o.a.c.l library?
IMHO...
1) Hard code now (M5)
2) Remove hard code and create an exclusions list (1.0)
3) Remove exclusions list and fix the classloaders (Post 1.0)
For number 2, we could easily create a warning if the user has
commons-logging in the web-inf/lib and its not excluded. But lets give
the user a choice. This takes us out of the loop completely from a
support perspective.
>
> thanks
> david jencks
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on Logging ClassCastException
Posted by David Jencks <da...@yahoo.com>.
On Oct 1, 2005, at 7:34 AM, Aaron Mulder wrote:
> On 9/29/05, Jeff Genender <jg...@savoirtech.com> wrote:
>> But I want to emphasize my discomfort with harcoding a commons package
>> into these loaders as it ultimately takes waya control from the user.
>> As
>> long as we can back this out and do a proper exclusion list in a
>> configurable plan, then I am cool with it.
>
> Could you explain what you mean here? I think we've already seen that
> in the case of commons logging, if you give that control to the user
> (by exposing a list including commons logging) and they exercise it
> (by removing commons logging from the list and including their own
> commons logging JAR), they get a ClassCastException -- which is to
> say, it doesn't work include your own commons logging and try to use
> it instead of Geronimo's version. I think the only way to work around
> that is a much more detailed restructuring of our ClassLoader
> hierarchy. Do you have another proposal for "making commons logging
> overridable"?
>
With our current classloaders I think we need to include commons
logging in the exclusion list for the reasons Aaron explains. There
may be a possibility of actually solving the problem with more
sophisticated classloaders that actually hide all the server classes
from the applications. From some conversations with Dain I think the
OSGI classloaders are capable of doing this, and I think he is looking
into the possibility of using them or something like them. I suspect
this would be a post 1.0 change however. For 1.0 I think the best we
can hope for is a configurable exclusion list, and my understanding is
that you should not be able to remove commons logging from the "fixed"
part of the exclusion list.
thanks
david jencks
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on
Logging ClassCastException
Posted by Jeff Genender <jg...@savoirtech.com>.
Aaron Mulder wrote:
> On 9/29/05, Jeff Genender <jg...@savoirtech.com> wrote:
>
>>But I want to emphasize my discomfort with harcoding a commons package
>>into these loaders as it ultimately takes waya control from the user. As
>>long as we can back this out and do a proper exclusion list in a
>>configurable plan, then I am cool with it.
>
>
> Could you explain what you mean here? I think we've already seen that
> in the case of commons logging, if you give that control to the user
> (by exposing a list including commons logging) and they exercise it
> (by removing commons logging from the list and including their own
> commons logging JAR), they get a ClassCastException -- which is to
> say, it doesn't work include your own commons logging and try to use
> it instead of Geronimo's version.
To answer your question, it goes back to my original statement. I think
we should not be patching with hardcoding for other APIs. Why? Because
we will undoubtably run into this with other packages in time. The next
commons-[insert your package here] that implements a similar technique,
then are we to do another hardcode? Once we start hardcoding others'
APIs, then we start to become responsible for the fixes if they don't
work right. The mere fact that the author of this package advocated
removal from the web-inf/lib directory, in and of itself, is a clear
warning that we are patching of others' issues. Minimally we should
allow the user to make the final decision of whether he/she wants
delegation to the server classes or not. IMHO...I would rather give
more control to the user and let them decide to exclude it. I don't
think that decision should be forced by us. But this is a semi-short
term solution...read the answer to your next question below.
> I think the only way to work around
> that is a much more detailed restructuring of our ClassLoader
> hierarchy. Do you have another proposal for "making commons logging
> overridable"?
I agree and I think David Jencks' email on this (as well as yours and my
email discussions) may be the ultimate solution. The problem is
essentially due to our classloader being too permissive about access. I
think you pointed this out earlier, and you and I had consensus on this.
The really short term solution is to hard code the commons-logging
package. The semi-short term solution is a plan based exclusion list.
The long term is to fix the class loaders. Do we agree on this?
Jeff
>
> Thanks,
> Aaron
Re: [jira] Commented: (GERONIMO-518) Deploying Struts app fails on Logging ClassCastException
Posted by Aaron Mulder <am...@alumni.princeton.edu>.
On 9/29/05, Jeff Genender <jg...@savoirtech.com> wrote:
> But I want to emphasize my discomfort with harcoding a commons package
> into these loaders as it ultimately takes waya control from the user. As
> long as we can back this out and do a proper exclusion list in a
> configurable plan, then I am cool with it.
Could you explain what you mean here? I think we've already seen that
in the case of commons logging, if you give that control to the user
(by exposing a list including commons logging) and they exercise it
(by removing commons logging from the list and including their own
commons logging JAR), they get a ClassCastException -- which is to
say, it doesn't work include your own commons logging and try to use
it instead of Geronimo's version. I think the only way to work around
that is a much more detailed restructuring of our ClassLoader
hierarchy. Do you have another proposal for "making commons logging
overridable"?
Thanks,
Aaron