Re: [wildfly-dev] private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

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(a)redhat.com&gt; 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(a)redhat.com&gt; 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(a)hibernate.org > >>>> <mailto:steve@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(a)redhat.com > >>>> <mailto:smarlow@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(a)lists.jboss.org > >>>> <mailto:wildfly-dev@lists.jboss.org> > >>>> > >>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev > >>>> > >>> _______________________________________________ > >>> wildfly-dev mailing list > >>> wildfly-dev(a)lists.jboss.org > >>> https://lists.jboss.org/mailman/listinfo/wildfly-dev > >> _______________________________________________ > >> wildfly-dev mailing list > >> wildfly-dev(a)lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/wildfly-dev > >> > >> > > > _______________________________________________ > wildfly-dev mailing list > wildfly-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/wildfly-dev > _______________________________________________ wildfly-dev mailing list wildfly-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/wildfly-dev