[wildfly-dev] private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...
Steve Ebersole
steve at hibernate.org
Fri Feb 19 15:59:24 EST 2016
Personally I do not see this distinction as the important factor. I get
what you are saying, and if we intentionally were exposing Javassist as an
API detail I would agree that copying and shading would be bad. But I say
it is not important because even though I do *not* view Javassist as
something we intentionally expose as an API detail I still think it is
important to copy/shade only as a last resort.
The mistake in our current OOTB bytecode manipulation implementation seems
to be that we expose some of its impl details (usage of Javassist) into the
manipulated bytecode. What I like specifically about the some of the
suggestions here so far (and Gunnar's work IIUC) is that it splits this
problem into distinct concerns:
1. Actually performing the manipulation and/or proxy generation
2. Runtime usage of that manipulated class or proxy.
So I can use Javassist or ByteBuddy or ASM or ... to deal with (1) as long
as that choice does not leak into (2). That is the important part to me.
That's the best option IMO if possible. And I think it is.
On Fri, Feb 19, 2016 at 1:56 PM Paul Benedict <pbenedict at apache.org> wrote:
> I think the answer should depend on how the Hibernate developers view
> Javassist. Is Javassist seen as a pluggable part of the API? Or is it seen
> as fundamentally an internal component? If it's the former, then do not
> shade it; otherwise do shade it. I think the latter is very sensible as
> long as that perspective is maintained in the code base.
>
> Cheers,
> Paul
>
> On Fri, Feb 19, 2016 at 11:47 AM, Scott Marlow <smarlow at redhat.com> wrote:
>
>>
>>
>> On 02/18/2016 01:17 PM, Carlo de Wolf wrote:
>> > Looks to me like Hibernate is exposing bad proxies to the user.
>>
>> Its not bad or new, just how we always did it. Other Javassist users
>> have also done the same and ended up shading Javassist classes.
>>
>> >
>> > Would it not be possible to use a custom class loader at the point where
>> > the proxy is defined?
>> > Than you could use one that takes the version of javassist that
>> > Hibernate requires and delegates everything else to the deployment class
>> > loader.
>>
>> This sounds similar to my https://github.com/wildfly/wildfly/pull/8474
>> pull request that changes the Hibernate ORM static module to export the
>> Javassist classes.
>>
>> >
>> > I don't like to see any sort of shading as this means the full
>> > maintenance burden of those classes are carried over into Hibernate.
>> >
>> > Carlo
>> >
>> > On 02/18/2016 12:11 PM, Sanne Grinovero wrote:
>> >> It seems we're discussing this issue in multiple places,
>> >> so to let you all know I'll repeat it hare:
>> >> I think shading is a really really bad idea :)
>> >>
>> >> Could we try to have the enhanced entities to not need Javassist in in
>> >> their *direct* classloader; we can still have a normal Javassist as a
>> >> module dependency of Hibernate?
>> >> That would require to just make sure the generated bytecode doesn't
>> >> directly refer to Javassist types but uses an indirection controlled
>> >> by Hibernate code.. which in turn can use Javassist or even
>> >> alternatives in future, if we'd like to experiment.
>> >>
>> >> I'm not familiar enough with Javassist to know if that's an option
>> >> as-is but we can either improve Javassist to allow such a thing or use
>> >> some alternatives, like Gunnar and Hardy also suggested on the
>> >> hibernate-dev mailing list.
>> >>
>> >> To summarize, I agree with Stuart and would hope that Scott's branch
>> >> can be improved by minimizing the amount of Javassist code which
>> >> actually needs to be copied by using some simple delegation to
>> >> Hibernte types, which in turn can use a private, non-shaded Javassist
>> >> taking advantage of the isolation provided by JBoss Modules.
>> >>
>> >> --Sanne
>> >>
>> >>
>> >>
>> >> On 12 February 2016 at 03:19, Scott Marlow <smarlow at redhat.com> wrote:
>> >>> What if Javassist packaged these same (proxy/runtime) classes in a
>> >>> separate javassist-runtime jar and we shaded only the proxy/runtime
>> >>> classes? That way we only repackage the same classes that we included
>> >>> for this hack test (e.g.
>> >>> org.hibernate.bytecode.internal.javassist.proxy.*).
>> >>>
>> >>> Early testing results of the hack test look good
>> >>> (https://gist.github.com/scottmarlow/ad878968c5a7c6fbbfb7).
>> >>>
>> >>> Scott
>> >>>
>> >>> On 02/11/2016 09:04 PM, Stuart Douglas wrote:
>> >>>> It depends if you are going to shade all the javassist classes or
>> just
>> >>>> the "javassist.util.proxy" package (not sure if this is actually
>> >>>> possible with the shade plugin).
>> >>>>
>> >>>> The main advantage is that you can upgrade javassist to get fixes to
>> >>>> issues that affect bytecode generation. So if JDK9 comes out with new
>> >>>> bytecodes that the current version of Javassist does not understand
>> >>>> then
>> >>>> upgrading javassist will allow the older version of hibernate to work
>> >>>> with classes compiled against the newer JDK version. If all of
>> >>>> javassist
>> >>>> is shaded into hibernate then that version of hibernate will never
>> work
>> >>>> with the newer bytecodes.
>> >>>>
>> >>>> I think this is less of an issue if you are still publishing the
>> >>>> non-Javassist shaded hibernate as well as a shaded version, but if
>> the
>> >>>> only published artifact has javassist shaded in then it may limit
>> >>>> forward compatibility.
>> >>>>
>> >>>> Stuart
>> >>>>
>> >>>>
>> >>>> On Fri, 12 Feb 2016 at 12:53 Steve Ebersole <steve at hibernate.org
>> >>>> <mailto:steve at hibernate.org>> wrote:
>> >>>>
>> >>>> Ugh. That is an awful lot of classes copied over. What
>> >>>> exactly was
>> >>>> the benefit of this over shading again? I mean both case lose
>> the
>> >>>> ability to simply drop in fixes from upstream Javassist. So
>> what
>> >>>> does this "clone" approach gain versus shadowing?
>> >>>>
>> >>>>
>> >>>>
>> >>>> On Thu, Feb 11, 2016 at 7:13 PM Scott Marlow <
>> smarlow at redhat.com
>> >>>> <mailto:smarlow at redhat.com>> wrote:
>> >>>>
>> >>>> >>
>> >>>> >> On 02/11/2016 03:02 PM, Stuart Douglas wrote:
>> >>>> >> > Have you considered a 3rd alternative, which is
>> to
>> >>>> use a custom
>> >>>> >> > ProxyFactory instead of javassists built in one?
>> >>>> >> >
>> >>>> >> > AFAIK the main issue is that javassist proxies
>> >>>> require access to the
>> >>>> >> >
>> >>>> 'javassist.util.proxy.MethodHandler|RuntimeSupport'
>> >>>> classes. You
>> >>>> >> could
>> >>>> >> > create a similar org.hibernate interface, and a
>> >>>> proxy factory
>> >>>> >> that uses
>> >>>> >> > this method handler instead.
>> >>>> >> >
>> >>>> >> > Basically you just copy the code from
>> >>>> javassist.util.proxy into
>> >>>> >> > hibernate. This is a relatively small amount of
>> >>>> code, so it
>> >>>> >> should not
>> >>>> >> > really add any maintenance burden.
>> >>>> >>
>> >>>> >> We talked about this as well via [1]. I
>> >>>> understand the
>> >>>> concept but have
>> >>>> >> not tried doing this. I like this approach as
>> >>>> well, if
>> >>>> it works. One
>> >>>> >> of the cons with cloning that Steve Ebersole
>> pointed
>> >>>> out (see response
>> >>>> >> on Feb-03-2016 9:01am), is that that users lose the
>> >>>> ability to drop a
>> >>>> >> different version of Javassist in (since we
>> maintain
>> >>>> our own cloned copy
>> >>>> >> of the Javassist proxy/runtime code).
>> >>>> >>
>> >>>> >>
>> >>>> >> The proxy code is a relatively small part of javassist,
>> so
>> >>>> unless a bug
>> >>>> >> is in the proxy code itself this should not be that big
>> >>>> a deal.
>> >>>> >
>> >>>> > Thanks for the encouragement to go down this path. :)
>> >>>> >
>> >>>>
>> >>>> Started a hack attempt at the clone via
>> >>>>
>> >>>> https://github.com/scottmarlow/hibernate-orm/tree/javassistproxy.
>> >>>> Seems
>> >>>> to pass the Hibernate ORM unit tests.
>> >>>>
>> >>>> Scott
>> >>>>
>> >>>> _______________________________________________
>> >>>> wildfly-dev mailing list
>> >>>> wildfly-dev at lists.jboss.org
>> >>>> <mailto:wildfly-dev at lists.jboss.org>
>> >>>>
>> >>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>> >>>>
>> >>> _______________________________________________
>> >>> wildfly-dev mailing list
>> >>> wildfly-dev at lists.jboss.org
>> >>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>> >> _______________________________________________
>> >> wildfly-dev mailing list
>> >> wildfly-dev at lists.jboss.org
>> >> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>> >>
>> >>
>> >
>> _______________________________________________
>> wildfly-dev mailing list
>> wildfly-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>
> _______________________________________________
> wildfly-dev mailing list
> wildfly-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/wildfly-dev/attachments/20160219/f368f8b7/attachment.html
More information about the wildfly-dev
mailing list