Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
Hi, 2016-10-27 4:27 GMT+02:00 Scott Marlow : > > > 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. This allows the ORM module to expose the right version of Javassist (or none at all eventually) without requiring JipiJapa to have the knowledge about this. As things stand, Javassist can be considered as part of ORM's API, hence it's module.xml should be the one making it available to the user. Of course eventually we don't want to expose Javassist at all, but as long as we do, the ORM module should be in charge of doing so. > > > > I agree, that is what our WF pull request did, which I think is an > incremental improvement but that wasn't approved for the reason I gave. > I don't quite get the reasoning here. We all agree that Javassist shouldn't be exposed at all (and there is good progress being made towards this by using ByteBuddy). But as long as we do, leaving this responsibility to the ORM module is the right thing to do IMO. E.g. what if a JPA provider doesn't need Javassist at all? Still JipiJapa would currently add it to the deployment. 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 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 change things in terms of exposure of Javassist to the application (it is exposed, just as it was before). But it puts the responsibility for doing so to ORM's module, allowing us (via the ORM module ZIP we provide) to expose a newer Javassist version as needed. Note that 2 - 4 can be done via the ORM module ZIP. But 1) is a change that needs to be done on the WF side, at least I don't see how this could be overridden? I.e. the module ZIP is rather pointless atm. and we don't have a good way to put the latest ORM into WF user's hands. Would be nice to change this. >> Is the ORM testsuite building the WildFly app server from source? > > > > > > Good god no :) > > > > > >> > >> Other option would be to use ByteBuddy to generate proxies instead of > >> Javassist to eliminate the application dependency on the Javassist > >> runtime classes. > > > > > > > > Rafael Winterhalter is actually pretty far along on defining support for > Byte Buddy in Hibernate[1]. So that might soon be an option. > > > > [1] https://hibernate.atlassian.net/browse/HHH-11152 > > > +100 :) > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
We could fix 1) as well by incorporating the sources for jipijapa-hibernate52 in ORM. I think it belongs within Hibernate, as it won't be the last time that a new ORM version requires some adaptation of bootstrap. On 27 Oct 2016 09:51, "Gunnar Morling" wrote: > Hi, > > 2016-10-27 4:27 GMT+02:00 Scott Marlow : > > > > > > 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. > > This allows the ORM module to expose the right version of Javassist (or > none at all eventually) without requiring JipiJapa to have the knowledge > about this. As things stand, Javassist can be considered as part of ORM's > API, hence it's module.xml should be the one making it available to the > user. > > Of course eventually we don't want to expose Javassist at all, but as long > as we do, the ORM module should be in charge of doing so. > > > > > > > > I agree, that is what our WF pull request did, which I think is an > > incremental improvement but that wasn't approved for the reason I gave. > > > > I don't quite get the reasoning here. We all agree that Javassist shouldn't > be exposed at all (and there is good progress being made towards this by > using ByteBuddy). > > But as long as we do, leaving this responsibility to the ORM module is the > right thing to do IMO. E.g. what if a JPA provider doesn't need Javassist > at all? Still JipiJapa would currently add it to the deployment. > > 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 > 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 > change things in terms of exposure of Javassist to the application (it is > exposed, just as it was before). But it puts the responsibility for doing > so to ORM's module, allowing us (via the ORM module ZIP we provide) to > expose a newer Javassist version as needed. > > Note that 2 - 4 can be done via the ORM module ZIP. But 1) is a change that > needs to be done on the WF side, at least I don't see how this could be > overridden? > > I.e. the module ZIP is rather pointless atm. and we don't have a good way > to put the latest ORM into WF user's hands. Would be nice to change this. > > >> Is the ORM testsuite building the WildFly app server from source? > > > > > > > > > Good god no :) > > > > > > > > >> > > >> Other option would be to use ByteBuddy to generate proxies instead of > > >> Javassist to eliminate the application dependency on the Javassist > > >> runtime classes. > > > > > > > > > > > > Rafael Winterhalter is actually pretty far along on defining support > for > > Byte Buddy in Hibernate[1]. So that might soon be an option. > > > > > > [1] https://hibernate.atlassian.net/browse/HHH-11152 > > > > > +100 :) > > > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
No, JPADependencyProcessor - which is adding the dependency - is part of "wildfly-jpa", not "jipijapa-hibernate". 2016-10-27 11:23 GMT+02:00 Sanne Grinovero : > We could fix 1) as well by incorporating the sources for > jipijapa-hibernate52 in ORM. > > I think it belongs within Hibernate, as it won't be the last time that a > new ORM version requires some adaptation of bootstrap. > > On 27 Oct 2016 09:51, "Gunnar Morling" wrote: > >> Hi, >> >> 2016-10-27 4:27 GMT+02:00 Scott Marlow : >> >> > >> > > 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. >> >> This allows the ORM module to expose the right version of Javassist (or >> none at all eventually) without requiring JipiJapa to have the knowledge >> about this. As things stand, Javassist can be considered as part of ORM's >> API, hence it's module.xml should be the one making it available to the >> user. >> >> Of course eventually we don't want to expose Javassist at all, but as long >> as we do, the ORM module should be in charge of doing so. >> >> > > >> > >> > I agree, that is what our WF pull request did, which I think is an >> > incremental improvement but that wasn't approved for the reason I gave. >> > >> >> I don't quite get the reasoning here. We all agree that Javassist >> shouldn't >> be exposed at all (and there is good progress being made towards this by >> using ByteBuddy). >> >> But as long as we do, leaving this responsibility to the ORM module is the >> right thing to do IMO. E.g. what if a JPA provider doesn't need Javassist >> at all? Still JipiJapa would currently add it to the deployment. >> >> 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 >> 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 >> change things in terms of exposure of Javassist to the application (it is >> exposed, just as it was before). But it puts the responsibility for doing >> so to ORM's module, allowing us (via the ORM module ZIP we provide) to >> expose a newer Javassist version as needed. >> >> Note that 2 - 4 can be done via the ORM module ZIP. But 1) is a change >> that >> needs to be done on the WF side, at least I don't see how this could be >> overridden? >> >> I.e. the module ZIP is rather pointless atm. and we don't have a good way >> to put the latest ORM into WF user's hands. Would be nice to change this. >> >> >> Is the ORM testsuite building the WildFly app server from source? >> > > >> > > >> > > Good god no :) >> > > >> > > >> > >> >> > >> Other option would be to use ByteBuddy to generate proxies instead of >> > >> Javassist to eliminate the application dependency on the Javassist >> > >> runtime classes. >> > > >> > > >> > > >> > > Rafael Winterhalter is actually pretty far along on defining support >> for >> > Byte Buddy in Hibernate[1]. So that might soon be an option. >> > > >> > > [1] https://hibernate.atlassian.net/browse/HHH-11152 >> > > >> > +100 :) >> > >> ___ >> hibernate-dev mailing list >> hibernate-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
Ok well that's also odd isn't it? On 27 Oct 2016 10:38, "Gunnar Morling" wrote: > No, JPADependencyProcessor - which is adding the dependency - is part of > "wildfly-jpa", not "jipijapa-hibernate". > > 2016-10-27 11:23 GMT+02:00 Sanne Grinovero : > >> We could fix 1) as well by incorporating the sources for >> jipijapa-hibernate52 in ORM. >> >> I think it belongs within Hibernate, as it won't be the last time that a >> new ORM version requires some adaptation of bootstrap. >> >> On 27 Oct 2016 09:51, "Gunnar Morling" wrote: >> >>> Hi, >>> >>> 2016-10-27 4:27 GMT+02:00 Scott Marlow : >>> >>> > >>> > > 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. >>> >>> This allows the ORM module to expose the right version of Javassist (or >>> none at all eventually) without requiring JipiJapa to have the knowledge >>> about this. As things stand, Javassist can be considered as part of ORM's >>> API, hence it's module.xml should be the one making it available to the >>> user. >>> >>> Of course eventually we don't want to expose Javassist at all, but as >>> long >>> as we do, the ORM module should be in charge of doing so. >>> >>> > > >>> > >>> > I agree, that is what our WF pull request did, which I think is an >>> > incremental improvement but that wasn't approved for the reason I gave. >>> > >>> >>> I don't quite get the reasoning here. We all agree that Javassist >>> shouldn't >>> be exposed at all (and there is good progress being made towards this by >>> using ByteBuddy). >>> >>> But as long as we do, leaving this responsibility to the ORM module is >>> the >>> right thing to do IMO. E.g. what if a JPA provider doesn't need Javassist >>> at all? Still JipiJapa would currently add it to the deployment. >>> >>> 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 >>> 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 >>> change things in terms of exposure of Javassist to the application (it is >>> exposed, just as it was before). But it puts the responsibility for doing >>> so to ORM's module, allowing us (via the ORM module ZIP we provide) to >>> expose a newer Javassist version as needed. >>> >>> Note that 2 - 4 can be done via the ORM module ZIP. But 1) is a change >>> that >>> needs to be done on the WF side, at least I don't see how this could be >>> overridden? >>> >>> I.e. the module ZIP is rather pointless atm. and we don't have a good way >>> to put the latest ORM into WF user's hands. Would be nice to change this. >>> >>> >> Is the ORM testsuite building the WildFly app server from source? >>> > > >>> > > >>> > > Good god no :) >>> > > >>> > > >>> > >> >>> > >> Other option would be to use ByteBuddy to generate proxies instead >>> of >>> > >> Javassist to eliminate the application dependency on the Javassist >>> > >> runtime classes. >>> > > >>> > > >>> > > >>> > > Rafael Winterhalter is actually pretty far along on defining support >>> for >>> > Byte Buddy in Hibernate[1]. So that might soon be an option. >>> > > >>> > > [1] https://hibernate.atlassian.net/browse/HHH-11152 >>> > > >>> > +100 :) >>> > >>> ___ >>> hibernate-dev mailing list >>> hibernate-dev@lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>> >> > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
JPADependencyProcessor deals with adding JPA dependencies generally (i.e. it works for whatever JPA provider is configured), so it seems reasonable by itself. But as said shouldn't be adding the dependency to Javassist as that's better left to the module(s) of the specific provider(s) needing it. 2016-10-27 12:12 GMT+02:00 Sanne Grinovero : > Ok well that's also odd isn't it? > > On 27 Oct 2016 10:38, "Gunnar Morling" wrote: > >> No, JPADependencyProcessor - which is adding the dependency - is part of >> "wildfly-jpa", not "jipijapa-hibernate". >> >> 2016-10-27 11:23 GMT+02:00 Sanne Grinovero : >> >>> We could fix 1) as well by incorporating the sources for >>> jipijapa-hibernate52 in ORM. >>> >>> I think it belongs within Hibernate, as it won't be the last time that a >>> new ORM version requires some adaptation of bootstrap. >>> >>> On 27 Oct 2016 09:51, "Gunnar Morling" wrote: >>> Hi, 2016-10-27 4:27 GMT+02:00 Scott Marlow : > > > 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. This allows the ORM module to expose the right version of Javassist (or none at all eventually) without requiring JipiJapa to have the knowledge about this. As things stand, Javassist can be considered as part of ORM's API, hence it's module.xml should be the one making it available to the user. Of course eventually we don't want to expose Javassist at all, but as long as we do, the ORM module should be in charge of doing so. > > > > I agree, that is what our WF pull request did, which I think is an > incremental improvement but that wasn't approved for the reason I gave. > I don't quite get the reasoning here. We all agree that Javassist shouldn't be exposed at all (and there is good progress being made towards this by using ByteBuddy). But as long as we do, leaving this responsibility to the ORM module is the right thing to do IMO. E.g. what if a JPA provider doesn't need Javassist at all? Still JipiJapa would currently add it to the deployment. 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 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 change things in terms of exposure of Javassist to the application (it is exposed, just as it was before). But it puts the responsibility for doing so to ORM's module, allowing us (via the ORM module ZIP we provide) to expose a newer Javassist version as needed. Note that 2 - 4 can be done via the ORM module ZIP. But 1) is a change that needs to be done on the WF side, at least I don't see how this could be overridden? I.e. the module ZIP is rather pointless atm. and we don't have a good way to put the latest ORM into WF user's hands. Would be nice to change this. >> Is the ORM testsuite building the WildFly app server from source? > > > > > > Good god no :) > > > > > >> > >> Other option would be to use ByteBuddy to generate proxies instead of > >> Javassist to eliminate the application dependency on the Javassist > >> runtime classes. > > > > > > > > Rafael Winterhalter is actually pretty far along on defining support for > Byte Buddy in Hibernate[1]. So that might soon be an option. > > > > [1] https://hibernate.atlassian.net/browse/HHH-11152 > > > +100 :) > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev >>> >> ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
Only a WildFly deployment unit processor (DUP) can add dependencies to an application deployment. Jipijapa is not a DUP. On Thu, Oct 27, 2016 at 6:12 AM, Sanne Grinovero wrote: > Ok well that's also odd isn't it? > > > On 27 Oct 2016 10:38, "Gunnar Morling" wrote: >> >> No, JPADependencyProcessor - which is adding the dependency - is part of >> "wildfly-jpa", not "jipijapa-hibernate". >> >> >> 2016-10-27 11:23 GMT+02:00 Sanne Grinovero : >>> >>> We could fix 1) as well by incorporating the sources for >>> jipijapa-hibernate52 in ORM. >>> >>> I think it belongs within Hibernate, as it won't be the last time that a >>> new ORM version requires some adaptation of bootstrap. >>> >>> >>> On 27 Oct 2016 09:51, "Gunnar Morling" wrote: Hi, 2016-10-27 4:27 GMT+02:00 Scott Marlow : > > > 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. This allows the ORM module to expose the right version of Javassist (or none at all eventually) without requiring JipiJapa to have the knowledge about this. As things stand, Javassist can be considered as part of ORM's API, hence it's module.xml should be the one making it available to the user. Of course eventually we don't want to expose Javassist at all, but as long as we do, the ORM module should be in charge of doing so. > > > > I agree, that is what our WF pull request did, which I think is an > incremental improvement but that wasn't approved for the reason I > gave. > I don't quite get the reasoning here. We all agree that Javassist shouldn't be exposed at all (and there is good progress being made towards this by using ByteBuddy). But as long as we do, leaving this responsibility to the ORM module is the right thing to do IMO. E.g. what if a JPA provider doesn't need Javassist at all? Still JipiJapa would currently add it to the deployment. 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 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 change things in terms of exposure of Javassist to the application (it is exposed, just as it was before). But it puts the responsibility for doing so to ORM's module, allowing us (via the ORM module ZIP we provide) to expose a newer Javassist version as needed. Note that 2 - 4 can be done via the ORM module ZIP. But 1) is a change that needs to be done on the WF side, at least I don't see how this could be overridden? I.e. the module ZIP is rather pointless atm. and we don't have a good way to put the latest ORM into WF user's hands. Would be nice to change this. >> Is the ORM testsuite building the WildFly app server from source? > > > > > > Good god no :) > > > > > >> > >> Other option would be to use ByteBuddy to generate proxies instead > >> of > >> Javassist to eliminate the application dependency on the Javassist > >> runtime classes. > > > > > > > > Rafael Winterhalter is actually pretty far along on defining support > > for > Byte Buddy in Hibernate[1]. So that might soon be an option. > > > > [1] https://hibernate.atlassian.net/browse/HHH-11152 > > > +100 :) > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >> > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling wrote: > Hi, Hi, > > 2016-10-27 4:27 GMT+02:00 Scott Marlow : >> >> >> > 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. > > 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
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
On 27 October 2016 at 13:38, Scott Marlow wrote: > Only a WildFly deployment unit processor (DUP) can add dependencies to > an application deployment. Jipijapa is not a DUP. Ah, thanks Scott that helps to understand this all. Still, I agree with Gunnar that any additional dependencies belong in the static module definition of the JPA provider implementation. Could you remove that javassist injection please? While at it, are other dependencies being injected? Would be nice to remove as much as possible, and leave it up to the specific implementations to choose its dependencies. Thanks, Sanne > > On Thu, Oct 27, 2016 at 6:12 AM, Sanne Grinovero wrote: >> Ok well that's also odd isn't it? >> >> >> On 27 Oct 2016 10:38, "Gunnar Morling" wrote: >>> >>> No, JPADependencyProcessor - which is adding the dependency - is part of >>> "wildfly-jpa", not "jipijapa-hibernate". >>> >>> >>> 2016-10-27 11:23 GMT+02:00 Sanne Grinovero : We could fix 1) as well by incorporating the sources for jipijapa-hibernate52 in ORM. I think it belongs within Hibernate, as it won't be the last time that a new ORM version requires some adaptation of bootstrap. On 27 Oct 2016 09:51, "Gunnar Morling" wrote: > > Hi, > > 2016-10-27 4:27 GMT+02:00 Scott Marlow : > > > > > > 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. > > This allows the ORM module to expose the right version of Javassist (or > none at all eventually) without requiring JipiJapa to have the knowledge > about this. As things stand, Javassist can be considered as part of > ORM's > API, hence it's module.xml should be the one making it available to the > user. > > Of course eventually we don't want to expose Javassist at all, but as > long > as we do, the ORM module should be in charge of doing so. > > > > > > > > I agree, that is what our WF pull request did, which I think is an > > incremental improvement but that wasn't approved for the reason I > > gave. > > > > I don't quite get the reasoning here. We all agree that Javassist > shouldn't > be exposed at all (and there is good progress being made towards this by > using ByteBuddy). > > But as long as we do, leaving this responsibility to the ORM module is > the > right thing to do IMO. E.g. what if a JPA provider doesn't need > Javassist > at all? Still JipiJapa would currently add it to the deployment. > > 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 > 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 > change things in terms of exposure of Javassist to the application (it > is > exposed, just as it was before). But it puts the responsibility for > doing > so to ORM's module, allowing us (via the ORM module ZIP we provide) to > expose a newer Javassist version as needed. > > Note that 2 - 4 can be done via the ORM module ZIP. But 1) is a change > that > needs to be done on the WF side, at least I don't see how this could be > overridden? > > I.e. the module ZIP is rather pointless atm. and we don't have a good > way > to put the latest ORM into WF user's hands. Would be nice to change > this. > > >> Is the ORM testsuite building the WildFly app server from source? > > > > > > > > > Good god no :) > > > > > > > > >> > > >> Other option would be to use ByteBuddy to generate proxies instead > > >> of > > >> Javassist to eliminate the application dependency on the Javassist > > >> runtime classes. > > > > > > > > > > > > Rafael Winterhalter is actually pretty far along on defining support > > > for > > Byte Buddy in Hibernate[1]. So that might soon be an option. > > > > > > [1] https://hibernate.atlassian.net/browse/HHH-11152 > > > > > +100 :) > > > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>> >>> >> ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
2016-10-27 14:59 GMT+02:00 Scott Marlow : > On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling > wrote: > > Hi, > > Hi, > > > > > 2016-10-27 4:27 GMT+02:00 Scott Marlow : > >> > >> > >> > 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
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
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 wrote: > > > 2016-10-27 14:59 GMT+02:00 Scott Marlow : >> >> On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling >> wrote: >> > Hi, >> >> Hi, >> >> > >> > 2016-10-27 4:27 GMT+02:00 Scott Marlow : >> >> >> >> >> >> > 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
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
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 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 > wrote: > > > > > > 2016-10-27 14:59 GMT+02:00 Scott Marlow : > >> > >> On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling > >> wrote: > >> > Hi, > >> > >> Hi, > >> > >> > > >> > 2016-10-27 4:27 GMT+02:00 Scott Marlow : > >> >> > >> >> > >> >> > 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
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
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 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 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 >> wrote: >> > >> > >> > 2016-10-27 14:59 GMT+02:00 Scott Marlow : >> >> >> >> On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling >> >> wrote: >> >> > Hi, >> >> >> >> Hi, >> >> >> >> > >> >> > 2016-10-27 4:27 GMT+02:00 Scott Marlow : >> >> >> >> >> >> >> >> >> > 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
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
Dunno. To me this is a bug that Jipijapa/WF/DUP adds this dependency at all. How that gets resolved or not is completely up to Jipijapa/WF/DUP as it is the one(s) doing this. There is nothing Hibernate can do here. On Thu, Oct 27, 2016 at 9:11 AM Scott Marlow wrote: > 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 > 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 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 > >> wrote: > >> > > >> > > >> > 2016-10-27 14:59 GMT+02:00 Scott Marlow : > >> >> > >> >> 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 : > >> >> >> > >> >> >> > >> >> >> > 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/mail
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
2016-10-27 15:55 GMT+02:00 Scott Marlow : > 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. > Why would one support such use case? If a user is adding their own ORM in the app, I'd expect them to do it completely, with all the dependencies of *that version*. I mean, how would you even know whether the Javassist version you are adding is the right one? And what happens if the user *is* providing Javassist with the app? Tbh. allowing the user to add their own ORM in the app seems even more a justification to not inject Javassist. 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. > Would that hint be another "knob" to be set by the user? If so, I wouldn't like it too much, it's just such a technical detail we shouldn't bother the user with. Another idea: can you examine the ORM module you are about to add and check wether it exports Javassist and if so, not auto-inject Javassist? Not sure whether it's doable in terms of capabilities of the module API. > > On Thu, Oct 27, 2016 at 9:40 AM, Gunnar Morling > wrote: > > > > > > 2016-10-27 14:59 GMT+02:00 Scott Marlow : > >> > >> On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling > >> wrote: > >> > Hi, > >> > >> Hi, > >> > >> > > >> > 2016-10-27 4:27 GMT+02:00 Scott Marlow : > >> >> > >> >> > >> >> > 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
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
On Thu, Oct 27, 2016 at 10:09 AM, Gunnar Morling wrote: > 2016-10-27 15:55 GMT+02:00 Scott Marlow : >> >> 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. > > > Why would one support such use case? If a user is adding their own ORM in > the app, I'd expect them to do it completely, with all the dependencies of > *that version*. I mean, how would you even know whether the Javassist > version you are adding is the right one? I think that exporting the Javassist classes from ORM was considered a bad idea back then in 2011. Now, we don't want to lose application compatibility. > > And what happens if the user *is* providing Javassist with the app? Tbh. > allowing the user to add their own ORM in the app seems even more a > justification to not inject Javassist. Sure, lets get to the point where we can allow the user to have their own Javassist. > >> 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. > > > Would that hint be another "knob" to be set by the user? If so, I wouldn't > like it too much, it's just such a technical detail we shouldn't bother the > user with. Yes, it would be another knob, however, it is likely to only be used by the Hibernate ORM testsuite, as I haven't heard of any user request for this. > > Another idea: can you examine the ORM module you are about to add and check > wether it exports Javassist and if so, not auto-inject Javassist? Not sure > whether it's doable in terms of capabilities of the module API. I'm not 100%, we would likely need to explore what is available from the application deployment org.jboss.as.server.deployment.module.ModuleSpecification at the early deployment phase that JPADependencyProcessor is called in. ModuleSpecification seems to have getSystemDependencies() + getUserDependencies() + getLocalDependencies() calls, but I'm not sure how (deployment) expensive it would be to sequentially walk through each module dependency and look for javassist classes. > >> >> >> On Thu, Oct 27, 2016 at 9:40 AM, Gunnar Morling >> wrote: >> > >> > >> > 2016-10-27 14:59 GMT+02:00 Scott Marlow : >> >> >> >> On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling >> >> wrote: >> >> > Hi, >> >> >> >> Hi, >> >> >> >> > >> >> > 2016-10-27 4:27 GMT+02:00 Scott Marlow : >> >> >> >> >> >> >> >> >> > 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 t
[hibernate-dev] Hibernate Validator 5.3.1.Final is out
Hi, I'm pleased to announce the first maintenance release of Hibernate Validator 5.3. For more information, please see the full announcement here: http://in.relation.to/2016/10/27/hibernate-validator-531-final-out/ . Have a nice day! -- Guillaume ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
I updated https://github.com/wildfly/wildfly/pull/8474 to make a push for applying the pull request and also mentioned the desire to update JPADependencyProcessor to not inject Javassist into applications. Perhaps others from the WildFly team will disagree with me and make an exception to break applications with Hibernate ORM embedded in a small way, that has a tiny (one line change) workaround needed. :-) On Thu, Oct 27, 2016 at 10:39 AM, Scott Marlow wrote: > On Thu, Oct 27, 2016 at 10:09 AM, Gunnar Morling wrote: >> 2016-10-27 15:55 GMT+02:00 Scott Marlow : >>> >>> 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. >> >> >> Why would one support such use case? If a user is adding their own ORM in >> the app, I'd expect them to do it completely, with all the dependencies of >> *that version*. I mean, how would you even know whether the Javassist >> version you are adding is the right one? > > I think that exporting the Javassist classes from ORM was considered a > bad idea back then in 2011. Now, we don't want to lose application > compatibility. > >> >> And what happens if the user *is* providing Javassist with the app? Tbh. >> allowing the user to add their own ORM in the app seems even more a >> justification to not inject Javassist. > > Sure, lets get to the point where we can allow the user to have their > own Javassist. > >> >>> 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. >> >> >> Would that hint be another "knob" to be set by the user? If so, I wouldn't >> like it too much, it's just such a technical detail we shouldn't bother the >> user with. > > Yes, it would be another knob, however, it is likely to only be used > by the Hibernate ORM testsuite, as I haven't heard of any user request > for this. > >> >> Another idea: can you examine the ORM module you are about to add and check >> wether it exports Javassist and if so, not auto-inject Javassist? Not sure >> whether it's doable in terms of capabilities of the module API. > > I'm not 100%, we would likely need to explore what is available from > the application deployment > org.jboss.as.server.deployment.module.ModuleSpecification at the early > deployment phase that JPADependencyProcessor is called in. > ModuleSpecification seems to have getSystemDependencies() + > getUserDependencies() + getLocalDependencies() calls, but I'm not sure > how (deployment) expensive it would be to sequentially walk through > each module dependency and look for javassist classes. > >> >>> >>> >>> On Thu, Oct 27, 2016 at 9:40 AM, Gunnar Morling >>> wrote: >>> > >>> > >>> > 2016-10-27 14:59 GMT+02:00 Scott Marlow : >>> >> >>> >> On Thu, Oct 27, 2016 at 4:49 AM, Gunnar Morling >>> >> wrote: >>> >> > Hi, >>> >> >>> >> Hi, >>> >> >>> >> > >>> >> > 2016-10-27 4:27 GMT+02:00 Scott Marlow : >>> >> >> >>> >> >> >>> >> >> > 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. >>> >> >>> >> >
Re: [hibernate-dev] Javassist dependency conflict in the ORM modules for WildFly
On Thu, Oct 27, 2016 at 10:14 AM, Steve Ebersole wrote: > Dunno. To me this is a bug that Jipijapa/WF/DUP adds this dependency at > all. How that gets resolved or not is completely up to Jipijapa/WF/DUP as > it is the one(s) doing this. There is nothing Hibernate can do here. > > I updated the https://github.com/scottmarlow/wildfly/tree/WFLY-459_javassistnative pull request as requested, JPADependencyProcessor will no longer add Javassist to the application deployment (if the change is merged). What changed? If applications embed Hibernate jars, the app developer shouldn't be surprised by this change that requires them to specify their own Javassist dependency. I'm not sure if it would help but Sanne could comment on the pull request that he approves the PR (if Sanne agrees the updated change is now correct). That might negate earlier feedback against the pull request, not being correct. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
