[hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly

[Scott Marlow]

We can call it a bad deployment, however, at the same time, we don't
want to break application compatibility in a way that requires the
application to be cracked open and modified (even if the fix is just a
simple MANIFEST.MF change).

On Thu, Oct 27, 2016 at 10:07 AM, Steve Ebersole <steve at hibernate.org> wrote:
> I would argue that that is a bad deployment.  IMO if you supply your own
> Hibernate jars, you'd be responsible for supplying its dependency jars as
> well.  Javassist is currently a non-optional dependency of ORM.
>
> On Thu, Oct 27, 2016 at 8:55 AM Scott Marlow <smarlow at redhat.com> wrote:
>>
>> I remember now why I didn't remove the JPADependencyProcessor
>> injection of Javassist in
>> https://github.com/wildfly/wildfly/pull/8474, it would cause a failure
>> in applications that embeds a copy of the ORM jars but the application
>> doesn't have the Javassisst jars on its classpath.  Currently,
>> JPADependencyProcessor ensures that Javassist is available to such an
>> application.
>>
>> I could easily work around the above in the
>>
>> wildfly/testsuite/compat/src/test/java/org/jboss/as/test/compat/jpa/hibernate/HibernateJarsInDeploymentTestCase.java
>> test, however, this would break application compatibility, which is
>> more important.  This goes back to around 2011 when we first started
>> auto injecting the Javassist classes to applications that are using
>> Hibernate as a JPA persistence provider.  The next time where we would
>> be allowed to break application compatibility, would be on an EAP
>> .zero release.
>>
>> If https://github.com/wildfly/wildfly/pull/8474 ever gets merged into
>> WildFly, perhaps we could enhance JPADependencyProcessor with a
>> persistence unit check for a "don't inject javassist" hint.  Or maybe
>> that could help the ORM testsuite now, in which case we could create
>> an independent pull request for the "don't inject javassist" hint
>> handling.
>>
>> On Thu, Oct 27, 2016 at 9:40 AM, Gunnar Morling <gunnar at hibernate.org>
>> wrote:
>> >
>> >
>> > 2016-10-27 14:59 GMT+02:00 Scott Marlow <smarlow at redhat.com>:
>> >>
>> >> On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling <gunnar at hibernate.org>
>> >> wrote:
>> >> > Hi,
>> >>
>> >> Hi,
>> >>
>> >> >
>> >> > 2016-10-27 4:27 GMT+02:00 Scott Marlow <smarlow at redhat.com>:
>> >> >>
>> >> >>
>> >> >> > Unless I am mistaken, Gunnar is suggesting that the Hibernate ORM
>> >> >> > module
>> >> >> > (the WF module) export Javassist.  Not the application.
>> >> >
>> >> >
>> >> > Right, Hibernate ORM's module should be the one exposing it, not the
>> >> > application nor JipiJapa.
>> >>
>> >> JipiJapa has zero to do with this, we will create a pr later today to
>> >> remove the unneeded dependencies, which has nothing to do with this
>> >> conversation.
>> >
>> >
>> > Yes, there is this superfluous dependency, thanks for removing it.
>> >
>> > But the other issue is hat in JPADependencyProcessor (that's what I
>> > meant
>> > when referring to "JipiJapa", sorry if that's not correct), a dependency
>> > to
>> > Javassist is injected. This shouldn't be the case for the reasons I've
>> > pointed out. Also with the PR
>> > https://github.com/wildfly/wildfly/pull/8474
>> > you mentioned this seems to be the case.
>> >>
>> >>
>> >> >
>> >> > I've done the following changes locally:
>> >> >
>> >> > 1) Altered JPADependencyProcessor to not add the Javassist dependency
>> >> > to
>> >> > the
>> >> > deployment
>> >> > 2) Altered module.xml of jipijapa-hibernate5 to not depend on
>> >> > Javassist
>> >>
>> >> I did the same locally, which is an unused dependency.  I removed some
>> >> other unused dependencies other as well.  Gail and I will talk later
>> >> about removing these unused dependencies.
>> >>
>> >> > 3) Added a new module for the latest Javassist
>> >> > 4) Altered module.xml of ORM itself to depend on that new module
>> >> > *and*
>> >> > re-export it
>> >> >
>> >> > With all this in place, the tests in Hibernate Search pass
>> >> > successfully.
>> >> >
>> >> > Scott, do you think we can try another PR with that? Again, it
>> >> > doesn't
>> >>
>> >> https://github.com/wildfly/wildfly/pull/8474 is still open, no need
>> >> for a new PR when we already have an open one, with one exception, as
>> >> I didn't actually remove the export of Javassist to the application
>> >> classpath.  I forget why.  I'll try that locally and run the WildFly
>> >> testsuite to see if there is a failure.
>> >
>> >