JPADependencyProcessor "infecting" classpath with the wrong Javassist version

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

JPADependencyProcessor "infecting" classpath with the wrong Javassist version

Sanne Grinovero
Hi all,

Scott sent a nice PR to Wildfly a while back to fix the problem:
 - https://github.com/wildfly/wildfly/pull/9305

It wasn't merged, I guess it's not a priority for WildFly but let me
clarify that without such fixes it's impossible for people to use
newer versions of Hibernate ORM on WildFly, and I suspect lots of pain
as well for other libraries using Javassist.

There's quite some people in the Hibernate community who expressed
interest in using a not-so-stale version as the one which is typically
available in the latest stable release of WildFly.

To make this happen, all Hibernate projects are now publishing
"WildFly modules" which can be easily downloaded as additional drop-in
layer.
Granted these are not for everyone but we get good feedback from the
power users interested, and not least this allows us to develop all
our projects while regularly testing integration with WildFly, making
sure that the eventual integration goes smoother.

The current problem is that the WildFly JPADependencyProcessor adds
the wrong version of javassist to the deployments, and there's no way
for us to prevent this or override this, making it impossible to use a
recent version of Hibernate ORM as it requires a newer version.
 - https://github.com/wildfly/wildfly/blob/6b61a6003f704221f66dcd9f418bcb7af88fb9ab/jpa/subsystem/src/main/java/org/jboss/as/jpa/processor/JPADependencyProcessor.java#L95

We'd highly appreciate if that PR could be merged? Including on
product branches please, as enforcing a dependency which is neither
needed nor desired is going to break a long list of other frameworks
as well.

Thanks,
Sanne
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: JPADependencyProcessor "infecting" classpath with the wrong Javassist version

Scott Marlow
Is that the change you want merge or is it the
https://github.com/wildfly/wildfly/pull/8474 change which updates
JPADependencyProcessor to not export Javassist jars to the application
classloader?



On Fri, Jan 13, 2017 at 7:03 AM, Sanne Grinovero <[hidden email]> wrote:

> Hi all,
>
> Scott sent a nice PR to Wildfly a while back to fix the problem:
>  - https://github.com/wildfly/wildfly/pull/9305
>
> It wasn't merged, I guess it's not a priority for WildFly but let me
> clarify that without such fixes it's impossible for people to use
> newer versions of Hibernate ORM on WildFly, and I suspect lots of pain
> as well for other libraries using Javassist.
>
> There's quite some people in the Hibernate community who expressed
> interest in using a not-so-stale version as the one which is typically
> available in the latest stable release of WildFly.
>
> To make this happen, all Hibernate projects are now publishing
> "WildFly modules" which can be easily downloaded as additional drop-in
> layer.
> Granted these are not for everyone but we get good feedback from the
> power users interested, and not least this allows us to develop all
> our projects while regularly testing integration with WildFly, making
> sure that the eventual integration goes smoother.
>
> The current problem is that the WildFly JPADependencyProcessor adds
> the wrong version of javassist to the deployments, and there's no way
> for us to prevent this or override this, making it impossible to use a
> recent version of Hibernate ORM as it requires a newer version.
>  - https://github.com/wildfly/wildfly/blob/6b61a6003f704221f66dcd9f418bcb7af88fb9ab/jpa/subsystem/src/main/java/org/jboss/as/jpa/processor/JPADependencyProcessor.java#L95
>
> We'd highly appreciate if that PR could be merged? Including on
> product branches please, as enforcing a dependency which is neither
> needed nor desired is going to break a long list of other frameworks
> as well.
>
> Thanks,
> Sanne
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: JPADependencyProcessor "infecting" classpath with the wrong Javassist version

Sanne Grinovero
Hi Scott,
you're right, both would be very welcome but removing Javassist is the
most critical one.

Thanks,
Sanne

On 13 January 2017 at 14:46, Scott Marlow <[hidden email]> wrote:

> Is that the change you want merge or is it the
> https://github.com/wildfly/wildfly/pull/8474 change which updates
> JPADependencyProcessor to not export Javassist jars to the application
> classloader?
>
>
>
> On Fri, Jan 13, 2017 at 7:03 AM, Sanne Grinovero <[hidden email]> wrote:
>> Hi all,
>>
>> Scott sent a nice PR to Wildfly a while back to fix the problem:
>>  - https://github.com/wildfly/wildfly/pull/9305
>>
>> It wasn't merged, I guess it's not a priority for WildFly but let me
>> clarify that without such fixes it's impossible for people to use
>> newer versions of Hibernate ORM on WildFly, and I suspect lots of pain
>> as well for other libraries using Javassist.
>>
>> There's quite some people in the Hibernate community who expressed
>> interest in using a not-so-stale version as the one which is typically
>> available in the latest stable release of WildFly.
>>
>> To make this happen, all Hibernate projects are now publishing
>> "WildFly modules" which can be easily downloaded as additional drop-in
>> layer.
>> Granted these are not for everyone but we get good feedback from the
>> power users interested, and not least this allows us to develop all
>> our projects while regularly testing integration with WildFly, making
>> sure that the eventual integration goes smoother.
>>
>> The current problem is that the WildFly JPADependencyProcessor adds
>> the wrong version of javassist to the deployments, and there's no way
>> for us to prevent this or override this, making it impossible to use a
>> recent version of Hibernate ORM as it requires a newer version.
>>  - https://github.com/wildfly/wildfly/blob/6b61a6003f704221f66dcd9f418bcb7af88fb9ab/jpa/subsystem/src/main/java/org/jboss/as/jpa/processor/JPADependencyProcessor.java#L95
>>
>> We'd highly appreciate if that PR could be merged? Including on
>> product branches please, as enforcing a dependency which is neither
>> needed nor desired is going to break a long list of other frameworks
>> as well.
>>
>> Thanks,
>> Sanne
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: JPADependencyProcessor "infecting" classpath with the wrong Javassist version

Stuart Douglas
In reply to this post by Sanne Grinovero
Is there any reason why we can't update to the latest version of Javassist? I know this does not prevent similar issues in future, but we should probably be using the most recent version if there is no reason not to.

My issue with https://github.com/wildfly/wildfly/pull/8474 is that in general we should not be using export="true", as it makes it impossible to exclude the dependency using jboss-deployment-structure.xml. As a result I don't really see how this PR is an improvement, as it goes from the javassist deployment being added directly to the deployment to it being added indirectly via an export.

Stuart

On Fri, Jan 13, 2017 at 11:03 PM, Sanne Grinovero <[hidden email]> wrote:
Hi all,

Scott sent a nice PR to Wildfly a while back to fix the problem:
 - https://github.com/wildfly/wildfly/pull/9305

It wasn't merged, I guess it's not a priority for WildFly but let me
clarify that without such fixes it's impossible for people to use
newer versions of Hibernate ORM on WildFly, and I suspect lots of pain
as well for other libraries using Javassist.

There's quite some people in the Hibernate community who expressed
interest in using a not-so-stale version as the one which is typically
available in the latest stable release of WildFly.

To make this happen, all Hibernate projects are now publishing
"WildFly modules" which can be easily downloaded as additional drop-in
layer.
Granted these are not for everyone but we get good feedback from the
power users interested, and not least this allows us to develop all
our projects while regularly testing integration with WildFly, making
sure that the eventual integration goes smoother.

The current problem is that the WildFly JPADependencyProcessor adds
the wrong version of javassist to the deployments, and there's no way
for us to prevent this or override this, making it impossible to use a
recent version of Hibernate ORM as it requires a newer version.
 - https://github.com/wildfly/wildfly/blob/6b61a6003f704221f66dcd9f418bcb7af88fb9ab/jpa/subsystem/src/main/java/org/jboss/as/jpa/processor/JPADependencyProcessor.java#L95

We'd highly appreciate if that PR could be merged? Including on
product branches please, as enforcing a dependency which is neither
needed nor desired is going to break a long list of other frameworks
as well.

Thanks,
Sanne
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: JPADependencyProcessor "infecting" classpath with the wrong Javassist version

Sanne Grinovero
Hi Stuart,

On 15 January 2017 at 22:23, Stuart Douglas <[hidden email]> wrote:
> Is there any reason why we can't update to the latest version of Javassist?

Sure we can update Javassist as well, as long as we pick a version
which is compatible with the (slightly old) version of Hibernate ORM
included in WildFly.
This might require a backport of the Javassist upgrade to the older
stable branch of Hibernate ORM and a new release though, so I'd treat
it as an orthogonal issue especially as such a plan would require its
own set of approvers.

> I know this does not prevent similar issues in future, but we should
> probably be using the most recent version if there is no reason not to.

Right, but bear with me the "not prevent similar issues in future" is
crucial to my problem here. We need to regularly test next-gen
Hibernate ORM while we work on it: we want to have ci.hibernate.org
run the WildFly integration tests on each commit of Hibernate ORM, so
upgrading Javassist - or any other dependency - is an event which
might happen multiple times a week and we don't want to wait for a
WildFly release each time this is needed.
It's really about controlling our own dependencies, to keep
development quick and practical.

> My issue with https://github.com/wildfly/wildfly/pull/8474 is that in
> general we should not be using export="true", as it makes it impossible to
> exclude the dependency using jboss-deployment-structure.xml. As a result I
> don't really see how this PR is an improvement, as it goes from the
> javassist deployment being added directly to the deployment to it being
> added indirectly via an export.

Interesting point, I hadn't thought about users of
jboss-deployment-structure.xml. But still, anyone making use of
`org.hibernate:main` (as it is today) will strictly require to have
`org.javassist:main` on their classpath, so one will not want to
exclude it; while people using say `org.hibernate:5.2.6.Final` will
definitely not want `org.javassist:main` on their classpath as it will
cause subtle problems. In short, it's not a grey area but it's very
clear when it's required or not and we can automate this decision.
So I think the modules definitions of each `org.hibernate:xyz` module
should point to the right instance of the javassist module, rather
than force the users to craft an appropriate structure xml file to
amend for our bad choices?

I guess it is currently important for people to be able to exclude
`org.javassist` because it's being injected aggressively, for example
your apps get "infected" by javassist even if you want to use a
different JPA implementation which has no business at all with
Javassist.
On the other hand, I don't think we can support Hibernate users who
simply disable the right version of javassist using the deployment
structure, so if someone really wants to exclude org.javassist:main I
would suggest them to edit the module file of ORM, or better yet use
their own (and self-supported) ORM module.

By moving it to the static definition of the module, at least I have
the option to use my own set of Hibernate ORM modules and choose an
updated Javassist slot.

Hibernate ORM also has the option to use Byte Buddy now as an
alternative to Javassist; when using the new alternative enhancer one
might also want to get rid of the Javassist dependency altogether,
which power users can achieve by editing their own module, or using
the "next-gen" modules we regularly provide with each Hibernate
release. Fixing the JPADependencyProcessor would make it easy for
people to use newer modules by just layering the new edition.

Thanks,
Sanne


>
> Stuart
>
> On Fri, Jan 13, 2017 at 11:03 PM, Sanne Grinovero <[hidden email]>
> wrote:
>>
>> Hi all,
>>
>> Scott sent a nice PR to Wildfly a while back to fix the problem:
>>  - https://github.com/wildfly/wildfly/pull/9305
>>
>> It wasn't merged, I guess it's not a priority for WildFly but let me
>> clarify that without such fixes it's impossible for people to use
>> newer versions of Hibernate ORM on WildFly, and I suspect lots of pain
>> as well for other libraries using Javassist.
>>
>> There's quite some people in the Hibernate community who expressed
>> interest in using a not-so-stale version as the one which is typically
>> available in the latest stable release of WildFly.
>>
>> To make this happen, all Hibernate projects are now publishing
>> "WildFly modules" which can be easily downloaded as additional drop-in
>> layer.
>> Granted these are not for everyone but we get good feedback from the
>> power users interested, and not least this allows us to develop all
>> our projects while regularly testing integration with WildFly, making
>> sure that the eventual integration goes smoother.
>>
>> The current problem is that the WildFly JPADependencyProcessor adds
>> the wrong version of javassist to the deployments, and there's no way
>> for us to prevent this or override this, making it impossible to use a
>> recent version of Hibernate ORM as it requires a newer version.
>>  -
>> https://github.com/wildfly/wildfly/blob/6b61a6003f704221f66dcd9f418bcb7af88fb9ab/jpa/subsystem/src/main/java/org/jboss/as/jpa/processor/JPADependencyProcessor.java#L95
>>
>> We'd highly appreciate if that PR could be merged? Including on
>> product branches please, as enforcing a dependency which is neither
>> needed nor desired is going to break a long list of other frameworks
>> as well.
>>
>> Thanks,
>> Sanne
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: JPADependencyProcessor "infecting" classpath with the wrong Javassist version

Sanne Grinovero
This is how we documented the procedure in the Hibernate guides:
 - http://docs.jboss.org/hibernate/orm/5.2/topical/html_single/wildfly/Wildfly.html

I'd still prefer the Javassist module to not be enforced by the JPA
integration code.

Thanks,
Sanne



On 16 January 2017 at 11:13, Sanne Grinovero <[hidden email]> wrote:

> Hi Stuart,
>
> On 15 January 2017 at 22:23, Stuart Douglas <[hidden email]> wrote:
>> Is there any reason why we can't update to the latest version of Javassist?
>
> Sure we can update Javassist as well, as long as we pick a version
> which is compatible with the (slightly old) version of Hibernate ORM
> included in WildFly.
> This might require a backport of the Javassist upgrade to the older
> stable branch of Hibernate ORM and a new release though, so I'd treat
> it as an orthogonal issue especially as such a plan would require its
> own set of approvers.
>
>> I know this does not prevent similar issues in future, but we should
>> probably be using the most recent version if there is no reason not to.
>
> Right, but bear with me the "not prevent similar issues in future" is
> crucial to my problem here. We need to regularly test next-gen
> Hibernate ORM while we work on it: we want to have ci.hibernate.org
> run the WildFly integration tests on each commit of Hibernate ORM, so
> upgrading Javassist - or any other dependency - is an event which
> might happen multiple times a week and we don't want to wait for a
> WildFly release each time this is needed.
> It's really about controlling our own dependencies, to keep
> development quick and practical.
>
>> My issue with https://github.com/wildfly/wildfly/pull/8474 is that in
>> general we should not be using export="true", as it makes it impossible to
>> exclude the dependency using jboss-deployment-structure.xml. As a result I
>> don't really see how this PR is an improvement, as it goes from the
>> javassist deployment being added directly to the deployment to it being
>> added indirectly via an export.
>
> Interesting point, I hadn't thought about users of
> jboss-deployment-structure.xml. But still, anyone making use of
> `org.hibernate:main` (as it is today) will strictly require to have
> `org.javassist:main` on their classpath, so one will not want to
> exclude it; while people using say `org.hibernate:5.2.6.Final` will
> definitely not want `org.javassist:main` on their classpath as it will
> cause subtle problems. In short, it's not a grey area but it's very
> clear when it's required or not and we can automate this decision.
> So I think the modules definitions of each `org.hibernate:xyz` module
> should point to the right instance of the javassist module, rather
> than force the users to craft an appropriate structure xml file to
> amend for our bad choices?
>
> I guess it is currently important for people to be able to exclude
> `org.javassist` because it's being injected aggressively, for example
> your apps get "infected" by javassist even if you want to use a
> different JPA implementation which has no business at all with
> Javassist.
> On the other hand, I don't think we can support Hibernate users who
> simply disable the right version of javassist using the deployment
> structure, so if someone really wants to exclude org.javassist:main I
> would suggest them to edit the module file of ORM, or better yet use
> their own (and self-supported) ORM module.
>
> By moving it to the static definition of the module, at least I have
> the option to use my own set of Hibernate ORM modules and choose an
> updated Javassist slot.
>
> Hibernate ORM also has the option to use Byte Buddy now as an
> alternative to Javassist; when using the new alternative enhancer one
> might also want to get rid of the Javassist dependency altogether,
> which power users can achieve by editing their own module, or using
> the "next-gen" modules we regularly provide with each Hibernate
> release. Fixing the JPADependencyProcessor would make it easy for
> people to use newer modules by just layering the new edition.
>
> Thanks,
> Sanne
>
>
>>
>> Stuart
>>
>> On Fri, Jan 13, 2017 at 11:03 PM, Sanne Grinovero <[hidden email]>
>> wrote:
>>>
>>> Hi all,
>>>
>>> Scott sent a nice PR to Wildfly a while back to fix the problem:
>>>  - https://github.com/wildfly/wildfly/pull/9305
>>>
>>> It wasn't merged, I guess it's not a priority for WildFly but let me
>>> clarify that without such fixes it's impossible for people to use
>>> newer versions of Hibernate ORM on WildFly, and I suspect lots of pain
>>> as well for other libraries using Javassist.
>>>
>>> There's quite some people in the Hibernate community who expressed
>>> interest in using a not-so-stale version as the one which is typically
>>> available in the latest stable release of WildFly.
>>>
>>> To make this happen, all Hibernate projects are now publishing
>>> "WildFly modules" which can be easily downloaded as additional drop-in
>>> layer.
>>> Granted these are not for everyone but we get good feedback from the
>>> power users interested, and not least this allows us to develop all
>>> our projects while regularly testing integration with WildFly, making
>>> sure that the eventual integration goes smoother.
>>>
>>> The current problem is that the WildFly JPADependencyProcessor adds
>>> the wrong version of javassist to the deployments, and there's no way
>>> for us to prevent this or override this, making it impossible to use a
>>> recent version of Hibernate ORM as it requires a newer version.
>>>  -
>>> https://github.com/wildfly/wildfly/blob/6b61a6003f704221f66dcd9f418bcb7af88fb9ab/jpa/subsystem/src/main/java/org/jboss/as/jpa/processor/JPADependencyProcessor.java#L95
>>>
>>> We'd highly appreciate if that PR could be merged? Including on
>>> product branches please, as enforcing a dependency which is neither
>>> needed nor desired is going to break a long list of other frameworks
>>> as well.
>>>
>>> Thanks,
>>> Sanne
>>> _______________________________________________
>>> wildfly-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev