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 <st...@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 <smar...@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 <gun...@hibernate.org> >> wrote: >> > >> > >> > 2016-10-27 14:59 GMT+02:00 Scott Marlow <smar...@redhat.com>: >> >> >> >> On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling <gun...@hibernate.org> >> >> wrote: >> >> > Hi, >> >> >> >> Hi, >> >> >> >> > >> >> > 2016-10-27 4:27 GMT+02:00 Scott Marlow <smar...@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. >> > >> > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
