Releasing a new version of Geronimo JCDI 2.0?

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

Releasing a new version of Geronimo JCDI 2.0?

John D. Ament
Hi

I pushed up a small fix to the CDI spec.  Would it make sense to cut a release of that?

John
Reply | Threaded
Open this post in threaded view
|

Re: Releasing a new version of Geronimo JCDI 2.0?

Romain Manni-Bucau
Doesn't break anything so +1

Now a few side note on it:

1. private static volatile CDI shouldn't be static IMO since it is broken in any container (including SE if not under a flat classloading of the cdi impl)
2. i'm not sure how the patch changes much in practise (= maybe it does worth mentioning it in the class?)
3. just realized we are not aligned on https://docs.jboss.org/cdi/api/2.0/ "protected" behavior. This looks like an abuse of the javadoc "discovered API" but wonder if we want to be aligned on the spec or make the spec cleaned up

If unclear: none of these point block a release but since we revisit this "to be enhanced" area I thought it was important to mention them


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | JavaEE Factory

2017-08-27 14:22 GMT+02:00 John D. Ament <[hidden email]>:
Hi

I pushed up a small fix to the CDI spec.  Would it make sense to cut a release of that?

John

Reply | Threaded
Open this post in threaded view
|

Re: Releasing a new version of Geronimo JCDI 2.0?

John D. Ament
Hmmm so you're thinking we need to keep a map of classloader/CDIProvider pairs?

On Sun, Aug 27, 2017 at 12:16 PM Romain Manni-Bucau <[hidden email]> wrote:
Doesn't break anything so +1

Now a few side note on it:

1. private static volatile CDI shouldn't be static IMO since it is broken in any container (including SE if not under a flat classloading of the cdi impl)
2. i'm not sure how the patch changes much in practise (= maybe it does worth mentioning it in the class?)
3. just realized we are not aligned on https://docs.jboss.org/cdi/api/2.0/ "protected" behavior. This looks like an abuse of the javadoc "discovered API" but wonder if we want to be aligned on the spec or make the spec cleaned up

If unclear: none of these point block a release but since we revisit this "to be enhanced" area I thought it was important to mention them


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | JavaEE Factory

2017-08-27 14:22 GMT+02:00 John D. Ament <[hidden email]>:
Hi

I pushed up a small fix to the CDI spec.  Would it make sense to cut a release of that?

John

Reply | Threaded
Open this post in threaded view
|

Re: Releasing a new version of Geronimo JCDI 2.0?

Romain Manni-Bucau
I don't like much this impl since it enforces a single CDI impl and flat classloader for the container but if really in the API we must, if just a leak from the javadoc we don't have to.


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | JavaEE Factory

2017-08-27 18:31 GMT+02:00 John D. Ament <[hidden email]>:
Hmmm so you're thinking we need to keep a map of classloader/CDIProvider pairs?


On Sun, Aug 27, 2017 at 12:16 PM Romain Manni-Bucau <[hidden email]> wrote:
Doesn't break anything so +1

Now a few side note on it:

1. private static volatile CDI shouldn't be static IMO since it is broken in any container (including SE if not under a flat classloading of the cdi impl)
2. i'm not sure how the patch changes much in practise (= maybe it does worth mentioning it in the class?)
3. just realized we are not aligned on https://docs.jboss.org/cdi/api/2.0/ "protected" behavior. This looks like an abuse of the javadoc "discovered API" but wonder if we want to be aligned on the spec or make the spec cleaned up

If unclear: none of these point block a release but since we revisit this "to be enhanced" area I thought it was important to mention them


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | JavaEE Factory

2017-08-27 14:22 GMT+02:00 John D. Ament <[hidden email]>:
Hi

I pushed up a small fix to the CDI spec.  Would it make sense to cut a release of that?

John


Reply | Threaded
Open this post in threaded view
|

Re: Releasing a new version of Geronimo JCDI 2.0?

Jean-Baptiste Onofré
Agree with Romain.

Regards
JB
On Aug 27, 2017, at 17:48, Romain Manni-Bucau <[hidden email]> wrote:
I don't like much this impl since it enforces a single CDI impl and flat classloader for the container but if really in the API we must, if just a leak from the javadoc we don't have to.


Romain Manni-Bucau
@rmannibucau |   Blog | Old BlogGithub | LinkedIn | JavaEE Factory

2017-08-27 18:31 GMT+02:00 John D. Ament <[hidden email]>:
Hmmm so you're thinking we need to keep a map of classloader/CDIProvider pairs?


On Sun, Aug 27, 2017 at 12:16 PM Romain Manni-Bucau < [hidden email]> wrote:
Doesn't break anything so +1

Now a few side note on it:

1. private static volatile CDI shouldn't be static IMO since it is broken in any container (including SE if not under a flat classloading of the cdi impl)
2. i'm not sure how the patch changes much in practise (= maybe it does worth mentioning it in the class?)
3. just realized we are not aligned on  https://docs.jboss.org/cdi/api/2.0/ "protected" behavior. This looks like an abuse of the javadoc "discovered API" but wonder if we want to be aligned on the spec or make the spec cleaned up

If unclear: none of these point block a release but since we revisit this "to be enhanced" area I thought it was important to mention them


Romain Manni-Bucau
@rmannibucau |   Blog | Old BlogGithub | LinkedIn | JavaEE Factory

2017-08-27 14:22 GMT+02:00 John D. Ament <[hidden email]>:
Hi

I pushed up a small fix to the CDI spec.  Would it make sense to cut a release of that?

John


Reply | Threaded
Open this post in threaded view
|

Re: Releasing a new version of Geronimo JCDI 2.0?

Mark Struberg
Well, theoretically a Map<CL, CDIProvider> might be more correct.
But there is already a static (!) setter for it. So I fear there is not much we can do.
Plus the CL trick is broken in most containers anyway for such low-level things.
Think about EARs. You would only be able to set it for the main CL...

It's also not practical to have different CDI containers which share the same API via a parent ClassLoader.
If you have multiple CDI containers then they are fully separated - means including the API.
Otherwise you are bound to a very specific version anyway. And thus it makes not much sense.

LieGrue,
strub


> Am 27.08.2017 um 19:41 schrieb Jean-Baptiste Onofré <[hidden email]>:
>
> Agree with Romain.
>
> Regards
> JB
> On Aug 27, 2017, at 17:48, Romain Manni-Bucau <[hidden email]> wrote:
> I don't like much this impl since it enforces a single CDI impl and flat classloader for the container but if really in the API we must, if just a leak from the javadoc we don't have to.
>
>
> Romain Manni-Bucau
> @rmannibucau |   Blog | Old Blog |  Github | LinkedIn | JavaEE Factory
>
> 2017-08-27 18:31 GMT+02:00 John D. Ament <[hidden email]>:
> Hmmm so you're thinking we need to keep a map of classloader/CDIProvider pairs?
>
>
> On Sun, Aug 27, 2017 at 12:16 PM Romain Manni-Bucau < [hidden email]> wrote:
> Doesn't break anything so +1
>
> Now a few side note on it:
>
> 1. private static volatile CDI shouldn't be static IMO since it is broken in any container (including SE if not under a flat classloading of the cdi impl)
> 2. i'm not sure how the patch changes much in practise (= maybe it does worth mentioning it in the class?)
> 3. just realized we are not aligned on  https://docs.jboss.org/cdi/api/2.0/ "protected" behavior. This looks like an abuse of the javadoc "discovered API" but wonder if we want to be aligned on the spec or make the spec cleaned up
>
> If unclear: none of these point block a release but since we revisit this "to be enhanced" area I thought it was important to mention them
>
>
> Romain Manni-Bucau
> @rmannibucau |   Blog | Old Blog |  Github | LinkedIn | JavaEE Factory
>
> 2017-08-27 14:22 GMT+02:00 John D. Ament <[hidden email]>:
> Hi
>
> I pushed up a small fix to the CDI spec.  Would it make sense to cut a release of that?
>
> John
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Releasing a new version of Geronimo JCDI 2.0?

Romain Manni-Bucau


Le 28 août 2017 03:29, "Mark Struberg" <[hidden email]> a écrit :
Well, theoretically a Map<CL, CDIProvider> might be more correct.
But there is already a static (!) setter for it. So I fear there is not much we can do.
Plus the CL trick is broken in most containers anyway for such low-level things.
Think about EARs. You would only be able to set it for the main CL...


Not really cause ear classloader is defined enough to have a lookup strategy. Also keep in mind setting it at runtime - from the app - leads to undefined behavior by design.  The static setter is fine and doesnt mean it is backed by a singleton.


It's also not practical to have different CDI containers which share the same API via a parent ClassLoader.
If you have multiple CDI containers then they are fully separated - means including the API.
Otherwise you are bound to a very specific version anyway. And thus it makes not much sense.

Well it can - have exactly it but not with cdi - while you dont break the api. I know cdi was not that good on it - with its beansattributes for instance - but it should so it is a valid case. Also have to admit i was more thinking about a deployer where you choose your provider more than the spec version.


LieGrue,
strub


> Am 27.08.2017 um 19:41 schrieb Jean-Baptiste Onofré <[hidden email]>:
>
> Agree with Romain.
>
> Regards
> JB
> On Aug 27, 2017, at 17:48, Romain Manni-Bucau <[hidden email]> wrote:
> I don't like much this impl since it enforces a single CDI impl and flat classloader for the container but if really in the API we must, if just a leak from the javadoc we don't have to.
>
>
> Romain Manni-Bucau
> @rmannibucau |   Blog | Old Blog |  Github | LinkedIn | JavaEE Factory
>
> 2017-08-27 18:31 GMT+02:00 John D. Ament <[hidden email]>:
> Hmmm so you're thinking we need to keep a map of classloader/CDIProvider pairs?
>
>
> On Sun, Aug 27, 2017 at 12:16 PM Romain Manni-Bucau < [hidden email]> wrote:
> Doesn't break anything so +1
>
> Now a few side note on it:
>
> 1. private static volatile CDI shouldn't be static IMO since it is broken in any container (including SE if not under a flat classloading of the cdi impl)
> 2. i'm not sure how the patch changes much in practise (= maybe it does worth mentioning it in the class?)
> 3. just realized we are not aligned on  https://docs.jboss.org/cdi/api/2.0/ "protected" behavior. This looks like an abuse of the javadoc "discovered API" but wonder if we want to be aligned on the spec or make the spec cleaned up
>
> If unclear: none of these point block a release but since we revisit this "to be enhanced" area I thought it was important to mention them
>
>
> Romain Manni-Bucau
> @rmannibucau |   Blog | Old Blog |  Github | LinkedIn | JavaEE Factory
>
> 2017-08-27 14:22 GMT+02:00 John D. Ament <[hidden email]>:
> Hi
>
> I pushed up a small fix to the CDI spec.  Would it make sense to cut a release of that?
>
> John
>
>


Reply | Threaded
Open this post in threaded view
|

Re: Releasing a new version of Geronimo JCDI 2.0?

John D. Ament
Alright, I'm going to plan to roll the release "1.0.1" tonight.

John

On Mon, Aug 28, 2017 at 1:19 AM Romain Manni-Bucau <[hidden email]> wrote:


Le 28 août 2017 03:29, "Mark Struberg" <[hidden email]> a écrit :
Well, theoretically a Map<CL, CDIProvider> might be more correct.
But there is already a static (!) setter for it. So I fear there is not much we can do.
Plus the CL trick is broken in most containers anyway for such low-level things.
Think about EARs. You would only be able to set it for the main CL...


Not really cause ear classloader is defined enough to have a lookup strategy. Also keep in mind setting it at runtime - from the app - leads to undefined behavior by design.  The static setter is fine and doesnt mean it is backed by a singleton.


It's also not practical to have different CDI containers which share the same API via a parent ClassLoader.
If you have multiple CDI containers then they are fully separated - means including the API.
Otherwise you are bound to a very specific version anyway. And thus it makes not much sense.

Well it can - have exactly it but not with cdi - while you dont break the api. I know cdi was not that good on it - with its beansattributes for instance - but it should so it is a valid case. Also have to admit i was more thinking about a deployer where you choose your provider more than the spec version.


LieGrue,
strub


> Am 27.08.2017 um 19:41 schrieb Jean-Baptiste Onofré <[hidden email]>:
>
> Agree with Romain.
>
> Regards
> JB
> On Aug 27, 2017, at 17:48, Romain Manni-Bucau <[hidden email]> wrote:
> I don't like much this impl since it enforces a single CDI impl and flat classloader for the container but if really in the API we must, if just a leak from the javadoc we don't have to.
>
>
> Romain Manni-Bucau
> @rmannibucau |   Blog | Old Blog |  Github | LinkedIn | JavaEE Factory
>
> 2017-08-27 18:31 GMT+02:00 John D. Ament <[hidden email]>:
> Hmmm so you're thinking we need to keep a map of classloader/CDIProvider pairs?
>
>
> On Sun, Aug 27, 2017 at 12:16 PM Romain Manni-Bucau < [hidden email]> wrote:
> Doesn't break anything so +1
>
> Now a few side note on it:
>
> 1. private static volatile CDI shouldn't be static IMO since it is broken in any container (including SE if not under a flat classloading of the cdi impl)
> 2. i'm not sure how the patch changes much in practise (= maybe it does worth mentioning it in the class?)
> 3. just realized we are not aligned on  https://docs.jboss.org/cdi/api/2.0/ "protected" behavior. This looks like an abuse of the javadoc "discovered API" but wonder if we want to be aligned on the spec or make the spec cleaned up
>
> If unclear: none of these point block a release but since we revisit this "to be enhanced" area I thought it was important to mention them
>
>
> Romain Manni-Bucau
> @rmannibucau |   Blog | Old Blog |  Github | LinkedIn | JavaEE Factory
>
> 2017-08-27 14:22 GMT+02:00 John D. Ament <[hidden email]>:
> Hi
>
> I pushed up a small fix to the CDI spec.  Would it make sense to cut a release of that?
>
> John
>
>