You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Romain Manni-Bucau <rm...@gmail.com> on 2018/01/19 12:18:03 UTC

cdi-api leaking?

Hi guys,

cdi-api is still in maven lib and breaks any plugin using it since it is an
old version, can it be dropped or at least isolated from plugin
classloaders?

Thanks,
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau>

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
Perfect, thanks a lot for the help. Let me know if I can support him - even
if I guess you are already doing it ;).

Le mer. 9 déc. 2020 à 21:02, Michael Osipov <mi...@apache.org> a écrit :

> Am 2020-12-09 um 11:39 schrieb Romain Manni-Bucau:
> > Any issue moving forward on https://github.com/apache/maven/pull/408 ?
>
> No issue, I have delegated the IT to a former colleague who is looking
> for simple issues to contribute. Turning your test case into an IT seems
> like a perfect fit.
>
> M
>
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <
> https://github.com/rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >
> >
> > Le jeu. 3 déc. 2020 à 20:08, Romain Manni-Bucau <rm...@gmail.com>
> a
> > écrit :
> >
> >> created https://github.com/apache/maven/pull/408 about it
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> <https://rmannibucau.metawerx.net/> | Old Blog
> >> <http://rmannibucau.wordpress.com> | Github
> >> <https://github.com/rmannibucau> | LinkedIn
> >> <https://www.linkedin.com/in/rmannibucau> | Book
> >> <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >>
> >>
> >> Le mar. 1 déc. 2020 à 17:50, Romain Manni-Bucau <rm...@gmail.com>
> a
> >> écrit :
> >>
> >>> Up,
> >>>
> >>> Encountered a few bugs related to this regression, wonder how we want
> to
> >>> tackle it.
> >>> My 2cts would be to drop cdi-api and replace the single used
> >>> annotation from there by a maven one.
> >>> If we don't want to break plugins (not sure any use that) we can
> rewrite
> >>> it with asm or equivalent at load time since we own the classloading.
> >>>
> >>> Anyone having an opinion on that?
> >>>
> >>> Romain Manni-Bucau
> >>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>> <http://rmannibucau.wordpress.com> | Github
> >>> <https://github.com/rmannibucau> | LinkedIn
> >>> <https://www.linkedin.com/in/rmannibucau> | Book
> >>> <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >>>
> >>>
> >>> Le mar. 6 févr. 2018 à 18:02, Romain Manni-Bucau <
> rmannibucau@gmail.com>
> >>> a écrit :
> >>>
> >>>> commented inline
> >>>>
> >>>>
> >>>> Romain Manni-Bucau
> >>>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>> <http://rmannibucau.wordpress.com> | Github
> >>>> <https://github.com/rmannibucau> | LinkedIn
> >>>> <https://www.linkedin.com/in/rmannibucau> | Book
> >>>> <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >>>>
> >>>> 2018-02-06 17:51 GMT+01:00 Stuart McCulloch <mc...@gmail.com>:
> >>>>
> >>>>> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
> >>>>>> 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mcculls@gmail.com
> >>>>> (mailto:mcculls@gmail.com)>:
> >>>>>>
> >>>>>>> The "javax.enterprise.inject" CDI package was explicitly exported
> >>>>> as part
> >>>>>>> of the ongoing effort to migrate legacy Plexus components onto more
> >>>>> modern
> >>>>>>> standard annotations.
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Hmm, this looks wrong from my window cause Maven doesn't support CDI
> >>>>> API -
> >>>>>> guice doesn't. So it is an interpretation of a well defined API
> which
> >>>>> is by
> >>>>>> defintion a bad public API, no?
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> Raw Guice supports JSR330, but requires programmatic configuration
> (ie.
> >>>>> bindings defined in modules)
> >>>>>
> >>>>> Sisu builds on Guice to add support for things like annotation
> scanning
> >>>>> and wiring, injectable collections that dynamically update as
> plugins come
> >>>>> and go, property placeholders, etc.
> >>>>>
> >>>>> If the CDI annotations are on the classpath then it also honours the
> >>>>> @Typed annotation. This was a feature request to help migrate certain
> >>>>> components from other Plexus-based applications over to JSR330 +
> @Typed. At
> >>>>> the time there was a consideration that the rest of the CDI
> annotations
> >>>>> would eventually be supported, as another compatibility layer.
> >>>>>
> >>>>> Sisu also provides a Plexus compatibility layer that supports Plexus
> >>>>> annotations and XML
> >>>>>
> >>>>> Maven 3.x switched to Sisu so old Plexus-based components can still
> be
> >>>>> used, while modern components can be written with JSR330. At the
> time of
> >>>>> the switch it was decided to enable support for @Typed in Maven
> >>>>> plugins/extensions (because there was a developer need for this
> feature,
> >>>>> but that may well have changed and no longer be relevant).
> >>>>>
> >>>>> So Maven currently honours using @Typed on components - if it’s
> decided
> >>>>> that Maven doesn’t want to support @Typed in plugins then just
> remove the
> >>>>> export and exclude the cdi-api jar. As mentioned previously support
> for
> >>>>> @Typed is used by other downstream non-Maven applications so it will
> always
> >>>>> be something the container supports, but it's totally optional so if
> you
> >>>>> don’t want it then you don’t need to ship the cdi-api jar.
> >>>>>
> >>>>
> >>>> Yes but having the full API for one class is luxury (see later comment
> >>>> for the detail)
> >>>>
> >>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Specifically, the @javax.enterprise.inject.Typed annotation lets
> >>>>>>> components state they are only visible for injection under a
> >>>>> specific type,
> >>>>>>> rather than any type in their hierarchy.
> >>>>>>>
> >>>>>>> There’s no annotation to control binding visibility in JSR330,
> >>>>> because it
> >>>>>>> deliberately avoids configuration concerns, which is why we went
> >>>>> with the
> >>>>>>> closest standard annotation (@Typed from JSR299 aka CDI). While we
> >>>>> could
> >>>>>>> have decided to use our own annotation - and the container does in
> >>>>> fact
> >>>>>>> support using @org.eclipse.sisu.Typed - this is not standardised or
> >>>>>>> portable. Also note the container will continue to support this
> >>>>> (optional)
> >>>>>>> feature for other downstream users, regardless of what’s decided
> >>>>> here - the
> >>>>>>> question is whether Maven still wants to use this feature and
> >>>>> whether it
> >>>>>>> wants to use the standard annotation or not.
> >>>>>>>
> >>>>>>> Another point is that whichever annotation is chosen must be
> >>>>>>> visible/defined from the same classloader to both core and plugins.
> >>>>> If the
> >>>>>>> annotation is not exported then core and each plugin will end up
> >>>>> with a
> >>>>>>> different @Typed class, defined by different classloaders. Any use
> >>>>> of
> >>>>>>> @Typed in plugins would then effectively be invisible to the
> >>>>> container,
> >>>>>>> because the JVM’s AnnotatedElement API (getAnnotation,
> >>>>> isAnnotationPresent,
> >>>>>>> etc.) work off classes and not name equivalence.
> >>>>>>>
> >>>>>>> Similarly shading won’t work because neither the plugin’s
> >>>>> components nor
> >>>>>>> the container would know about the shaded package.
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Hmm, not sure. I mean it works in most projects and it is easy to
> >>>>> expose
> >>>>>> the shaded API so not a big deal *technically*. Agree it would be a
> >>>>> bad
> >>>>>> solution to use a misused API publicly.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> By “not work” I really meant “not practical”. It’s not enough to just
> >>>>> shade the CDI jar, you’d also need to shade the container - being
> careful
> >>>>> that its reflective calls were properly updated (since it uses
> reflection
> >>>>> to decide whether to load the feature or not). TBH all that work is
> >>>>> overkill, since the container already supports an alternative
> annotation:
> >>>>> @org.eclipse.sisu.Typed
> >>>>>
> >>>>
> >>>> works for me
> >>>>
> >>>>
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> As you can see from the thread in http://maven.40175.n5.nabble.
> >>>>>>> com/Linkage-error-td5784411.html a number of alternative solutions
> >>>>> have
> >>>>>>> been discussed before, including narrowing the export to:
> >>>>>>>
> >>>>>>> "javax.enterprise.inject.Typed"
> >>>>>>>
> >>>>>>> as that’s the only annotation we’re currently interested in. Since
> >>>>> @Typed
> >>>>>>> hasn’t changed between 1.x and 2.x that should be a workable
> >>>>> solution,
> >>>>>>> assuming you wanted to keep using the standard annotation.
> >>>>>>>
> >>>>>>> Removing the export (and thereby removing the feature to limit
> >>>>> injection
> >>>>>>> visibility to a specific type) was also discussed, and at the time
> >>>>> Igor
> >>>>>>> asked for it to be kept:
> >>>>>>>
> >>>>>>> “Please keep @Typed annotation available outside of core.
> >>>>>>>
> >>>>>>> I use @Typed annotation in one of my (private) core extensions
> >>>>> where I
> >>>>>>> need a class to implement an interface but not make that interface
> >>>>>>> visible for injection in other components.”
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Issue is I can say the opposite "I use this in my plugin cause I use
> >>>>> CDI to
> >>>>>> impl my plugin, please ignore it for all Maven usage". Both are
> valid
> >>>>> and
> >>>>>> therefore the Maven API shouldn't have any overlapping.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> Whenever you embed a container inside any kind of plugin you’re at
> the
> >>>>> mercy of what’s exposed to that plugin - whether that plugin is
> running in
> >>>>> Maven, Jenkins, or an IDE. If you want full control/sanity then use a
> >>>>> custom classloader to isolate the embedded container from the
> plugin’s
> >>>>> environment, and just let through those packages you expect to be
> provided.
> >>>>>
> >>>>> For example, say we did fully support CDI 1.x components inside
> plugins
> >>>>> (as in the entire API was supported). You’d still have an issue
> embedding a
> >>>>> CDI 2.x container, because of the API clash, unless you used a custom
> >>>>> classloader between the plugin and the embedded container.
> >>>>>
> >>>>
> >>>> Yes but would have complained way earlier ;)
> >>>>
> >>>>
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Assuming Igor still needs this feature then the only other option
> >>>>> would be
> >>>>>>> to ask him if he can move to the non-standard
> >>>>> @org.eclipse.sisu.Typed. The
> >>>>>>> existing CDI export could then be replaced by exporting
> >>>>> “org.eclipse.sisu”.
> >>>>>>> Once that was done then the cdi-api dependency could be excluded
> >>>>> from the
> >>>>>>> distribution, as the container will still work without it on the
> >>>>> classpath
> >>>>>>> (it’s only required if you want to use the standard CDI
> annotation).
> >>>>>>>
> >>>>>>> So to summarise, the options are:
> >>>>>>>
> >>>>>>> a) Continue to support the standard API, but narrow the entry in
> >>>>>>> META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
> >>>>>>>
> >>>>>>> b) Switch to support @org.eclipse.sisu.Typed
> >>>>>>>
> >>>>>>> c) Remove this feature completely from Maven
> >>>>>>
> >>>>>>  From what I'm concerned b and c would solve it but I guess sisu
> users
> >>>>> can
> >>>>>> have the same issue - not sure how likely it is.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> Sisu users typically have control over the container classpath and
> can
> >>>>> choose whether to include CDI or not (and at which level)
> >>>>>> There is a d) option: add in @Mojo a list of imported API.
> ClassRealm
> >>>>> can
> >>>>>> support filtering from the parent classloader and therefore I could
> >>>>> use:
> >>>>>>
> >>>>>> @Mojo(name ="...", pluginPackages={"javax", ...})
> >>>>>>
> >>>>>> This would allow to keep current setup and let mojo to override it.
> >>>>>> Compared to a) it is defined in plugin.xml and not extension.xml.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> At the moment there’s a single Maven API realm, which imports all the
> >>>>> packages listed in the core extension.xml from the core classloader.
> >>>>> Plugins then import that realm wholesale, so they automatically get
> all
> >>>>> exported packages. However, it should be possible to be more
> selective,
> >>>>> whether that’s using a whitelisting or blacklisting approach.
> >>>>>
> >>>>> That said, it would be much simpler to either remove the export or
> >>>>> switch to @org.eclipse.inject.Typed (since use of the annotation in
> Maven
> >>>>> is currently very limited)
> >>>>>
> >>>>
> >>>> A last alternative is to still support @Typed without providing it.
> >>>> Concretely it means maven drops cdi (sadly not inject jar) and use
> asm to
> >>>> check if @Typed if here. Sounds the less breaking compromise even if
> not
> >>>> the most sexy in terms of impl.
> >>>>
> >>>>
> >>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> Cheers, Stuart
> >>>>>>>
> >>>>>>>
> >>>>>>> On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
> >>>>>>>
> >>>>>>>> 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org
> >>>>> (mailto:tibordigana@apache.org) (mailto:
> >>>>>>> tibordigana@apache.org (mailto:tibordigana@apache.org))>:
> >>>>>>>>
> >>>>>>>>> Personally I would like to see a new Git branch with CDI 2.0
> >>>>> and the
> >>>>>>>>> integration test results on Jenkins.
> >>>>>>>>> This would give us more confidence.
> >>>>>>>>> Question: Does the CDI 2.0 have any NEW mandatory descriptive
> >>>>> methods
> >>>>>>>>> without default value already introduced in OLD annotations CDI
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> 1.0/1.1?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> It is more a change in the hierarchy. It doesn't break the user
> >>>>> API since
> >>>>>>>> cdi is designed to be provided but it is broken if new code uses
> >>>>> old API.
> >>>>>>>>
> >>>>>>>> Side note: if the idea behind this answer is to ensure the default
> >>>>>>> provided
> >>>>>>>> API is the last one then it doesn't work cause an API has a few
> >>>>> logic
> >>>>>>>
> >>>>>>> which
> >>>>>>>> can require to be overriden (like the SPI and defaults handling).
> >>>>>>>> Maven uses its own API and exposing CDI is a leaking abuse IMHO.
> >>>>>>>>
> >>>>>>>> Note that this is an old bug which should be fixed now IMO before
> >>>>> maven
> >>>>>>>> considers CDI being exposed as part of the contract.
> >>>>>>>>
> >>>>>>>> For reference, older threads:
> >>>>>>>>
> >>>>>>>> http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
> >>>>>>> td5828015.html
> >>>>>>>>
> >>>>>
> http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> There is no risk removing it, worse case plugins would add the
> >>>>> API as
> >>>>>>>> compile instead of provided which should likely already be the
> >>>>> case.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
> >>>>>>> rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> >>>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> For the reproducer here it is https://github.com/
> >>>>>>>>>> rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
> >>>>>>>>>>
> >>>>>>>>>> 2018-02-06 8:05 GMT+01:00 Tibor Digana <
> >>>>> tibordigana@apache.org (mailto:tibordigana@apache.org)
> >>>>>>> (mailto:tibordigana@apache.org)>:
> >>>>>>>>>>
> >>>>>>>>>>> Changing the package would not be possible in 3.x.
> >>>>>>>>>>
> >>>>>>>>>> Why? In particular since it is an old regression already
> >>>>> reported on
> >>>>>>> the
> >>>>>>>>>> list due to guice introduction it shouldn't be delayed for
> >>>>> this kind
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> of
> >>>>>>>>>> reason IMHO.
> >>>>>>>>>> Was less visible until CDI 2 was released cause the API
> >>>>> difference
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> was
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> not
> >>>>>>>>>> triggered but now there are new entries it breaks immediately.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> Guessing the version 4.0.0.
> >>>>>>>>>>> WDYT?
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Would stay a blocker until 4 is out which is that soon so not
> >>>>> sure
> >>>>>>> it is
> >>>>>>>>>> an option.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
> >>>>>>> tibordigana@apache.org (mailto:tibordigana@apache.org)>
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>> The question is maybe about what is realistic for Maven
> >>>>> devs.
> >>>>>>>>>>>> Shading the CPI package (to something like
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> org.apache.maven.cdi.*)
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> would
> >>>>>>>>>>>> be maybe the case instead of removing the original CDI and
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> reinventing
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> the
> >>>>>>>>>>>> wheel.
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
> >>>>>>> herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
> >>>>>>>>>>>> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>> and does the MNG issue contain a reproducible test case
> >>>>> for us
> >>>>>>> to
> >>>>>>>>>>>>> investigate
> >>>>>>>>>>>>> more precisely?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Regards,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Hervé
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a
> >>>>> écrit :
> >>>>>>>>>>>>>> Is there a MNG[1] issue?
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Robert
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/MNG
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com
> >>>>> )>
> >>>>>>> wrote:
> >>>>>>>>>>>>>>> Up?
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
> >>>>>>>>> rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> >>>>>>>>>>>>> a
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> écrit :
> >>>>>>>>>>>>>>>> Hi guys,
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> cdi-api is still in maven lib and breaks any
> >>>>> plugin
> >>>>>>> using it
> >>>>>>>>> since
> >>>>>>>>>>>>> it is
> >>>>>>>>>>>>>>>> an old version, can it be dropped or at least
> >>>>> isolated
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> from
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> plugin
> >>>>>>>>>>>>>>>> classloaders?
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>>>>> Romain Manni-Bucau
> >>>>>>>>>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> |
> >>>>> Blog
> >>>>>>>>>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>>>>>>>>>>> <http://rmannibucau.wordpress.com> | Github
> >>>>>>>>>>>>>>>> <https://github.com/rmannibucau> | LinkedIn
> >>>>>>>>>>>>>>>> <https://www.linkedin.com/in/rmannibucau>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>> ------------------------------------------------------------
> >>>>>>>>> ---------
> >>>>>>>>>>>>>> To unsubscribe, e-mail:
> >>>>> dev-unsubscribe@maven.apache.org (mailto:
> >>>>> dev-unsubscribe@maven.apache.org)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> (mailto:dev-unsubscribe@maven.apache.org)
> >>>>>>>>>>>>>> For additional commands, e-mail:
> >>>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> (mailto:dev-help@maven.apache.org)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>> ------------------------------------------------------------
> >>>>>>> ---------
> >>>>>>>>>>>>> To unsubscribe, e-mail:
> >>>>> dev-unsubscribe@maven.apache.org (mailto:
> >>>>> dev-unsubscribe@maven.apache.org)
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> (mailto:dev-unsubscribe@maven.apache.org)
> >>>>>>>>>>>>> For additional commands, e-mail:
> >>>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> (mailto:dev-help@maven.apache.org)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: cdi-api leaking?

Posted by Michael Osipov <mi...@apache.org>.
Am 2020-12-09 um 11:39 schrieb Romain Manni-Bucau:
> Any issue moving forward on https://github.com/apache/maven/pull/408 ?

No issue, I have delegated the IT to a former colleague who is looking 
for simple issues to contribute. Turning your test case into an IT seems 
like a perfect fit.

M

> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
> 
> 
> Le jeu. 3 déc. 2020 à 20:08, Romain Manni-Bucau <rm...@gmail.com> a
> écrit :
> 
>> created https://github.com/apache/maven/pull/408 about it
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>>
>> Le mar. 1 déc. 2020 à 17:50, Romain Manni-Bucau <rm...@gmail.com> a
>> écrit :
>>
>>> Up,
>>>
>>> Encountered a few bugs related to this regression, wonder how we want to
>>> tackle it.
>>> My 2cts would be to drop cdi-api and replace the single used
>>> annotation from there by a maven one.
>>> If we don't want to break plugins (not sure any use that) we can rewrite
>>> it with asm or equivalent at load time since we own the classloading.
>>>
>>> Anyone having an opinion on that?
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github
>>> <https://github.com/rmannibucau> | LinkedIn
>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>
>>>
>>> Le mar. 6 févr. 2018 à 18:02, Romain Manni-Bucau <rm...@gmail.com>
>>> a écrit :
>>>
>>>> commented inline
>>>>
>>>>
>>>> Romain Manni-Bucau
>>>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>> <http://rmannibucau.wordpress.com> | Github
>>>> <https://github.com/rmannibucau> | LinkedIn
>>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>>
>>>> 2018-02-06 17:51 GMT+01:00 Stuart McCulloch <mc...@gmail.com>:
>>>>
>>>>> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
>>>>>> 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mcculls@gmail.com
>>>>> (mailto:mcculls@gmail.com)>:
>>>>>>
>>>>>>> The "javax.enterprise.inject" CDI package was explicitly exported
>>>>> as part
>>>>>>> of the ongoing effort to migrate legacy Plexus components onto more
>>>>> modern
>>>>>>> standard annotations.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> Hmm, this looks wrong from my window cause Maven doesn't support CDI
>>>>> API -
>>>>>> guice doesn't. So it is an interpretation of a well defined API which
>>>>> is by
>>>>>> defintion a bad public API, no?
>>>>>>
>>>>>>
>>>>>
>>>>> Raw Guice supports JSR330, but requires programmatic configuration (ie.
>>>>> bindings defined in modules)
>>>>>
>>>>> Sisu builds on Guice to add support for things like annotation scanning
>>>>> and wiring, injectable collections that dynamically update as plugins come
>>>>> and go, property placeholders, etc.
>>>>>
>>>>> If the CDI annotations are on the classpath then it also honours the
>>>>> @Typed annotation. This was a feature request to help migrate certain
>>>>> components from other Plexus-based applications over to JSR330 + @Typed. At
>>>>> the time there was a consideration that the rest of the CDI annotations
>>>>> would eventually be supported, as another compatibility layer.
>>>>>
>>>>> Sisu also provides a Plexus compatibility layer that supports Plexus
>>>>> annotations and XML
>>>>>
>>>>> Maven 3.x switched to Sisu so old Plexus-based components can still be
>>>>> used, while modern components can be written with JSR330. At the time of
>>>>> the switch it was decided to enable support for @Typed in Maven
>>>>> plugins/extensions (because there was a developer need for this feature,
>>>>> but that may well have changed and no longer be relevant).
>>>>>
>>>>> So Maven currently honours using @Typed on components - if it’s decided
>>>>> that Maven doesn’t want to support @Typed in plugins then just remove the
>>>>> export and exclude the cdi-api jar. As mentioned previously support for
>>>>> @Typed is used by other downstream non-Maven applications so it will always
>>>>> be something the container supports, but it's totally optional so if you
>>>>> don’t want it then you don’t need to ship the cdi-api jar.
>>>>>
>>>>
>>>> Yes but having the full API for one class is luxury (see later comment
>>>> for the detail)
>>>>
>>>>
>>>>>>
>>>>>>>
>>>>>>> Specifically, the @javax.enterprise.inject.Typed annotation lets
>>>>>>> components state they are only visible for injection under a
>>>>> specific type,
>>>>>>> rather than any type in their hierarchy.
>>>>>>>
>>>>>>> There’s no annotation to control binding visibility in JSR330,
>>>>> because it
>>>>>>> deliberately avoids configuration concerns, which is why we went
>>>>> with the
>>>>>>> closest standard annotation (@Typed from JSR299 aka CDI). While we
>>>>> could
>>>>>>> have decided to use our own annotation - and the container does in
>>>>> fact
>>>>>>> support using @org.eclipse.sisu.Typed - this is not standardised or
>>>>>>> portable. Also note the container will continue to support this
>>>>> (optional)
>>>>>>> feature for other downstream users, regardless of what’s decided
>>>>> here - the
>>>>>>> question is whether Maven still wants to use this feature and
>>>>> whether it
>>>>>>> wants to use the standard annotation or not.
>>>>>>>
>>>>>>> Another point is that whichever annotation is chosen must be
>>>>>>> visible/defined from the same classloader to both core and plugins.
>>>>> If the
>>>>>>> annotation is not exported then core and each plugin will end up
>>>>> with a
>>>>>>> different @Typed class, defined by different classloaders. Any use
>>>>> of
>>>>>>> @Typed in plugins would then effectively be invisible to the
>>>>> container,
>>>>>>> because the JVM’s AnnotatedElement API (getAnnotation,
>>>>> isAnnotationPresent,
>>>>>>> etc.) work off classes and not name equivalence.
>>>>>>>
>>>>>>> Similarly shading won’t work because neither the plugin’s
>>>>> components nor
>>>>>>> the container would know about the shaded package.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> Hmm, not sure. I mean it works in most projects and it is easy to
>>>>> expose
>>>>>> the shaded API so not a big deal *technically*. Agree it would be a
>>>>> bad
>>>>>> solution to use a misused API publicly.
>>>>>>
>>>>>>
>>>>>
>>>>> By “not work” I really meant “not practical”. It’s not enough to just
>>>>> shade the CDI jar, you’d also need to shade the container - being careful
>>>>> that its reflective calls were properly updated (since it uses reflection
>>>>> to decide whether to load the feature or not). TBH all that work is
>>>>> overkill, since the container already supports an alternative annotation:
>>>>> @org.eclipse.sisu.Typed
>>>>>
>>>>
>>>> works for me
>>>>
>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> As you can see from the thread in http://maven.40175.n5.nabble.
>>>>>>> com/Linkage-error-td5784411.html a number of alternative solutions
>>>>> have
>>>>>>> been discussed before, including narrowing the export to:
>>>>>>>
>>>>>>> "javax.enterprise.inject.Typed"
>>>>>>>
>>>>>>> as that’s the only annotation we’re currently interested in. Since
>>>>> @Typed
>>>>>>> hasn’t changed between 1.x and 2.x that should be a workable
>>>>> solution,
>>>>>>> assuming you wanted to keep using the standard annotation.
>>>>>>>
>>>>>>> Removing the export (and thereby removing the feature to limit
>>>>> injection
>>>>>>> visibility to a specific type) was also discussed, and at the time
>>>>> Igor
>>>>>>> asked for it to be kept:
>>>>>>>
>>>>>>> “Please keep @Typed annotation available outside of core.
>>>>>>>
>>>>>>> I use @Typed annotation in one of my (private) core extensions
>>>>> where I
>>>>>>> need a class to implement an interface but not make that interface
>>>>>>> visible for injection in other components.”
>>>>>>>
>>>>>>
>>>>>>
>>>>>> Issue is I can say the opposite "I use this in my plugin cause I use
>>>>> CDI to
>>>>>> impl my plugin, please ignore it for all Maven usage". Both are valid
>>>>> and
>>>>>> therefore the Maven API shouldn't have any overlapping.
>>>>>>
>>>>>>
>>>>>
>>>>> Whenever you embed a container inside any kind of plugin you’re at the
>>>>> mercy of what’s exposed to that plugin - whether that plugin is running in
>>>>> Maven, Jenkins, or an IDE. If you want full control/sanity then use a
>>>>> custom classloader to isolate the embedded container from the plugin’s
>>>>> environment, and just let through those packages you expect to be provided.
>>>>>
>>>>> For example, say we did fully support CDI 1.x components inside plugins
>>>>> (as in the entire API was supported). You’d still have an issue embedding a
>>>>> CDI 2.x container, because of the API clash, unless you used a custom
>>>>> classloader between the plugin and the embedded container.
>>>>>
>>>>
>>>> Yes but would have complained way earlier ;)
>>>>
>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Assuming Igor still needs this feature then the only other option
>>>>> would be
>>>>>>> to ask him if he can move to the non-standard
>>>>> @org.eclipse.sisu.Typed. The
>>>>>>> existing CDI export could then be replaced by exporting
>>>>> “org.eclipse.sisu”.
>>>>>>> Once that was done then the cdi-api dependency could be excluded
>>>>> from the
>>>>>>> distribution, as the container will still work without it on the
>>>>> classpath
>>>>>>> (it’s only required if you want to use the standard CDI annotation).
>>>>>>>
>>>>>>> So to summarise, the options are:
>>>>>>>
>>>>>>> a) Continue to support the standard API, but narrow the entry in
>>>>>>> META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
>>>>>>>
>>>>>>> b) Switch to support @org.eclipse.sisu.Typed
>>>>>>>
>>>>>>> c) Remove this feature completely from Maven
>>>>>>
>>>>>>  From what I'm concerned b and c would solve it but I guess sisu users
>>>>> can
>>>>>> have the same issue - not sure how likely it is.
>>>>>>
>>>>>>
>>>>>
>>>>> Sisu users typically have control over the container classpath and can
>>>>> choose whether to include CDI or not (and at which level)
>>>>>> There is a d) option: add in @Mojo a list of imported API. ClassRealm
>>>>> can
>>>>>> support filtering from the parent classloader and therefore I could
>>>>> use:
>>>>>>
>>>>>> @Mojo(name ="...", pluginPackages={"javax", ...})
>>>>>>
>>>>>> This would allow to keep current setup and let mojo to override it.
>>>>>> Compared to a) it is defined in plugin.xml and not extension.xml.
>>>>>>
>>>>>>
>>>>>
>>>>> At the moment there’s a single Maven API realm, which imports all the
>>>>> packages listed in the core extension.xml from the core classloader.
>>>>> Plugins then import that realm wholesale, so they automatically get all
>>>>> exported packages. However, it should be possible to be more selective,
>>>>> whether that’s using a whitelisting or blacklisting approach.
>>>>>
>>>>> That said, it would be much simpler to either remove the export or
>>>>> switch to @org.eclipse.inject.Typed (since use of the annotation in Maven
>>>>> is currently very limited)
>>>>>
>>>>
>>>> A last alternative is to still support @Typed without providing it.
>>>> Concretely it means maven drops cdi (sadly not inject jar) and use asm to
>>>> check if @Typed if here. Sounds the less breaking compromise even if not
>>>> the most sexy in terms of impl.
>>>>
>>>>
>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Cheers, Stuart
>>>>>>>
>>>>>>>
>>>>>>> On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
>>>>>>>
>>>>>>>> 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org
>>>>> (mailto:tibordigana@apache.org) (mailto:
>>>>>>> tibordigana@apache.org (mailto:tibordigana@apache.org))>:
>>>>>>>>
>>>>>>>>> Personally I would like to see a new Git branch with CDI 2.0
>>>>> and the
>>>>>>>>> integration test results on Jenkins.
>>>>>>>>> This would give us more confidence.
>>>>>>>>> Question: Does the CDI 2.0 have any NEW mandatory descriptive
>>>>> methods
>>>>>>>>> without default value already introduced in OLD annotations CDI
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> 1.0/1.1?
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> It is more a change in the hierarchy. It doesn't break the user
>>>>> API since
>>>>>>>> cdi is designed to be provided but it is broken if new code uses
>>>>> old API.
>>>>>>>>
>>>>>>>> Side note: if the idea behind this answer is to ensure the default
>>>>>>> provided
>>>>>>>> API is the last one then it doesn't work cause an API has a few
>>>>> logic
>>>>>>>
>>>>>>> which
>>>>>>>> can require to be overriden (like the SPI and defaults handling).
>>>>>>>> Maven uses its own API and exposing CDI is a leaking abuse IMHO.
>>>>>>>>
>>>>>>>> Note that this is an old bug which should be fixed now IMO before
>>>>> maven
>>>>>>>> considers CDI being exposed as part of the contract.
>>>>>>>>
>>>>>>>> For reference, older threads:
>>>>>>>>
>>>>>>>> http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
>>>>>>> td5828015.html
>>>>>>>>
>>>>> http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
>>>>>>>>
>>>>>>>>
>>>>>>>> There is no risk removing it, worse case plugins would add the
>>>>> API as
>>>>>>>> compile instead of provided which should likely already be the
>>>>> case.
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
>>>>>>> rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> For the reproducer here it is https://github.com/
>>>>>>>>>> rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
>>>>>>>>>>
>>>>>>>>>> 2018-02-06 8:05 GMT+01:00 Tibor Digana <
>>>>> tibordigana@apache.org (mailto:tibordigana@apache.org)
>>>>>>> (mailto:tibordigana@apache.org)>:
>>>>>>>>>>
>>>>>>>>>>> Changing the package would not be possible in 3.x.
>>>>>>>>>>
>>>>>>>>>> Why? In particular since it is an old regression already
>>>>> reported on
>>>>>>> the
>>>>>>>>>> list due to guice introduction it shouldn't be delayed for
>>>>> this kind
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> of
>>>>>>>>>> reason IMHO.
>>>>>>>>>> Was less visible until CDI 2 was released cause the API
>>>>> difference
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> was
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> not
>>>>>>>>>> triggered but now there are new entries it breaks immediately.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Guessing the version 4.0.0.
>>>>>>>>>>> WDYT?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Would stay a blocker until 4 is out which is that soon so not
>>>>> sure
>>>>>>> it is
>>>>>>>>>> an option.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
>>>>>>> tibordigana@apache.org (mailto:tibordigana@apache.org)>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> The question is maybe about what is realistic for Maven
>>>>> devs.
>>>>>>>>>>>> Shading the CPI package (to something like
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> org.apache.maven.cdi.*)
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> would
>>>>>>>>>>>> be maybe the case instead of removing the original CDI and
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> reinventing
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> the
>>>>>>>>>>>> wheel.
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
>>>>>>> herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> and does the MNG issue contain a reproducible test case
>>>>> for us
>>>>>>> to
>>>>>>>>>>>>> investigate
>>>>>>>>>>>>> more precisely?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hervé
>>>>>>>>>>>>>
>>>>>>>>>>>>> Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a
>>>>> écrit :
>>>>>>>>>>>>>> Is there a MNG[1] issue?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Robert
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/MNG
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com
>>>>> )>
>>>>>>> wrote:
>>>>>>>>>>>>>>> Up?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
>>>>>>>>> rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>>>>>>>>>>> a
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> écrit :
>>>>>>>>>>>>>>>> Hi guys,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> cdi-api is still in maven lib and breaks any
>>>>> plugin
>>>>>>> using it
>>>>>>>>> since
>>>>>>>>>>>>> it is
>>>>>>>>>>>>>>>> an old version, can it be dropped or at least
>>>>> isolated
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> from
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> plugin
>>>>>>>>>>>>>>>> classloaders?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Romain Manni-Bucau
>>>>>>>>>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> |
>>>>> Blog
>>>>>>>>>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>>>>>>>>>>> <http://rmannibucau.wordpress.com> | Github
>>>>>>>>>>>>>>>> <https://github.com/rmannibucau> | LinkedIn
>>>>>>>>>>>>>>>> <https://www.linkedin.com/in/rmannibucau>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>> ------------------------------------------------------------
>>>>>>>>> ---------
>>>>>>>>>>>>>> To unsubscribe, e-mail:
>>>>> dev-unsubscribe@maven.apache.org (mailto:
>>>>> dev-unsubscribe@maven.apache.org)
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> (mailto:dev-unsubscribe@maven.apache.org)
>>>>>>>>>>>>>> For additional commands, e-mail:
>>>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> (mailto:dev-help@maven.apache.org)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>> ------------------------------------------------------------
>>>>>>> ---------
>>>>>>>>>>>>> To unsubscribe, e-mail:
>>>>> dev-unsubscribe@maven.apache.org (mailto:
>>>>> dev-unsubscribe@maven.apache.org)
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> (mailto:dev-unsubscribe@maven.apache.org)
>>>>>>>>>>>>> For additional commands, e-mail:
>>>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> (mailto:dev-help@maven.apache.org)
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org

Re: cdi-api leaking?

Posted by Robert Scholte <rf...@apache.org>.
I'm missing an integration-test that confirms the need for this, otherwise I expect this discussion might pop up again every several years.
Just ensure it breaks if the exported packages are activated again for clear reasons.


Robert
On 9-12-2020 11:39:35, Romain Manni-Bucau <rm...@gmail.com> wrote:
Any issue moving forward on https://github.com/apache/maven/pull/408 ?

Romain Manni-Bucau
@rmannibucau | Blog
| Old Blog
| Github |
LinkedIn | Book



Le jeu. 3 déc. 2020 à 20:08, Romain Manni-Bucau a
écrit :

> created https://github.com/apache/maven/pull/408 about it
>
> Romain Manni-Bucau
> @rmannibucau | Blog
> | Old Blog
> | Github
> | LinkedIn
> | Book
>
>
>
> Le mar. 1 déc. 2020 à 17:50, Romain Manni-Bucau a
> écrit :
>
>> Up,
>>
>> Encountered a few bugs related to this regression, wonder how we want to
>> tackle it.
>> My 2cts would be to drop cdi-api and replace the single used
>> annotation from there by a maven one.
>> If we don't want to break plugins (not sure any use that) we can rewrite
>> it with asm or equivalent at load time since we own the classloading.
>>
>> Anyone having an opinion on that?
>>
>> Romain Manni-Bucau
>> @rmannibucau | Blog
>> | Old Blog
>> | Github
>> | LinkedIn
>> | Book
>>
>>
>>
>> Le mar. 6 févr. 2018 à 18:02, Romain Manni-Bucau
>> a écrit :
>>
>>> commented inline
>>>
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau | Blog
>>> | Old Blog
>>> | Github
>>> | LinkedIn
>>> | Book
>>>
>>>
>>> 2018-02-06 17:51 GMT+01:00 Stuart McCulloch :
>>>
>>>> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
>>>> > 2018-02-06 12:25 GMT+01:00 Stuart McCulloch
>>>> (mailto:mcculls@gmail.com)>:
>>>> >
>>>> > > The "javax.enterprise.inject" CDI package was explicitly exported
>>>> as part
>>>> > > of the ongoing effort to migrate legacy Plexus components onto more
>>>> modern
>>>> > > standard annotations.
>>>> > >
>>>> >
>>>> >
>>>> > Hmm, this looks wrong from my window cause Maven doesn't support CDI
>>>> API -
>>>> > guice doesn't. So it is an interpretation of a well defined API which
>>>> is by
>>>> > defintion a bad public API, no?
>>>> >
>>>> >
>>>>
>>>> Raw Guice supports JSR330, but requires programmatic configuration (ie.
>>>> bindings defined in modules)
>>>>
>>>> Sisu builds on Guice to add support for things like annotation scanning
>>>> and wiring, injectable collections that dynamically update as plugins come
>>>> and go, property placeholders, etc.
>>>>
>>>> If the CDI annotations are on the classpath then it also honours the
>>>> @Typed annotation. This was a feature request to help migrate certain
>>>> components from other Plexus-based applications over to JSR330 + @Typed. At
>>>> the time there was a consideration that the rest of the CDI annotations
>>>> would eventually be supported, as another compatibility layer.
>>>>
>>>> Sisu also provides a Plexus compatibility layer that supports Plexus
>>>> annotations and XML
>>>>
>>>> Maven 3.x switched to Sisu so old Plexus-based components can still be
>>>> used, while modern components can be written with JSR330. At the time of
>>>> the switch it was decided to enable support for @Typed in Maven
>>>> plugins/extensions (because there was a developer need for this feature,
>>>> but that may well have changed and no longer be relevant).
>>>>
>>>> So Maven currently honours using @Typed on components - if it’s decided
>>>> that Maven doesn’t want to support @Typed in plugins then just remove the
>>>> export and exclude the cdi-api jar. As mentioned previously support for
>>>> @Typed is used by other downstream non-Maven applications so it will always
>>>> be something the container supports, but it's totally optional so if you
>>>> don’t want it then you don’t need to ship the cdi-api jar.
>>>>
>>>
>>> Yes but having the full API for one class is luxury (see later comment
>>> for the detail)
>>>
>>>
>>>> >
>>>> > >
>>>> > > Specifically, the @javax.enterprise.inject.Typed annotation lets
>>>> > > components state they are only visible for injection under a
>>>> specific type,
>>>> > > rather than any type in their hierarchy.
>>>> > >
>>>> > > There’s no annotation to control binding visibility in JSR330,
>>>> because it
>>>> > > deliberately avoids configuration concerns, which is why we went
>>>> with the
>>>> > > closest standard annotation (@Typed from JSR299 aka CDI). While we
>>>> could
>>>> > > have decided to use our own annotation - and the container does in
>>>> fact
>>>> > > support using @org.eclipse.sisu.Typed - this is not standardised or
>>>> > > portable. Also note the container will continue to support this
>>>> (optional)
>>>> > > feature for other downstream users, regardless of what’s decided
>>>> here - the
>>>> > > question is whether Maven still wants to use this feature and
>>>> whether it
>>>> > > wants to use the standard annotation or not.
>>>> > >
>>>> > > Another point is that whichever annotation is chosen must be
>>>> > > visible/defined from the same classloader to both core and plugins.
>>>> If the
>>>> > > annotation is not exported then core and each plugin will end up
>>>> with a
>>>> > > different @Typed class, defined by different classloaders. Any use
>>>> of
>>>> > > @Typed in plugins would then effectively be invisible to the
>>>> container,
>>>> > > because the JVM’s AnnotatedElement API (getAnnotation,
>>>> isAnnotationPresent,
>>>> > > etc.) work off classes and not name equivalence.
>>>> > >
>>>> > > Similarly shading won’t work because neither the plugin’s
>>>> components nor
>>>> > > the container would know about the shaded package.
>>>> > >
>>>> >
>>>> >
>>>> > Hmm, not sure. I mean it works in most projects and it is easy to
>>>> expose
>>>> > the shaded API so not a big deal *technically*. Agree it would be a
>>>> bad
>>>> > solution to use a misused API publicly.
>>>> >
>>>> >
>>>>
>>>> By “not work” I really meant “not practical”. It’s not enough to just
>>>> shade the CDI jar, you’d also need to shade the container - being careful
>>>> that its reflective calls were properly updated (since it uses reflection
>>>> to decide whether to load the feature or not). TBH all that work is
>>>> overkill, since the container already supports an alternative annotation:
>>>> @org.eclipse.sisu.Typed
>>>>
>>>
>>> works for me
>>>
>>>
>>>> >
>>>> >
>>>> > >
>>>> > > As you can see from the thread in http://maven.40175.n5.nabble.
>>>> > > com/Linkage-error-td5784411.html a number of alternative solutions
>>>> have
>>>> > > been discussed before, including narrowing the export to:
>>>> > >
>>>> > > "javax.enterprise.inject.Typed"
>>>> > >
>>>> > > as that’s the only annotation we’re currently interested in. Since
>>>> @Typed
>>>> > > hasn’t changed between 1.x and 2.x that should be a workable
>>>> solution,
>>>> > > assuming you wanted to keep using the standard annotation.
>>>> > >
>>>> > > Removing the export (and thereby removing the feature to limit
>>>> injection
>>>> > > visibility to a specific type) was also discussed, and at the time
>>>> Igor
>>>> > > asked for it to be kept:
>>>> > >
>>>> > > “Please keep @Typed annotation available outside of core.
>>>> > >
>>>> > > I use @Typed annotation in one of my (private) core extensions
>>>> where I
>>>> > > need a class to implement an interface but not make that interface
>>>> > > visible for injection in other components.”
>>>> > >
>>>> >
>>>> >
>>>> > Issue is I can say the opposite "I use this in my plugin cause I use
>>>> CDI to
>>>> > impl my plugin, please ignore it for all Maven usage". Both are valid
>>>> and
>>>> > therefore the Maven API shouldn't have any overlapping.
>>>> >
>>>> >
>>>>
>>>> Whenever you embed a container inside any kind of plugin you’re at the
>>>> mercy of what’s exposed to that plugin - whether that plugin is running in
>>>> Maven, Jenkins, or an IDE. If you want full control/sanity then use a
>>>> custom classloader to isolate the embedded container from the plugin’s
>>>> environment, and just let through those packages you expect to be provided.
>>>>
>>>> For example, say we did fully support CDI 1.x components inside plugins
>>>> (as in the entire API was supported). You’d still have an issue embedding a
>>>> CDI 2.x container, because of the API clash, unless you used a custom
>>>> classloader between the plugin and the embedded container.
>>>>
>>>
>>> Yes but would have complained way earlier ;)
>>>
>>>
>>>> >
>>>> >
>>>> > >
>>>> > > Assuming Igor still needs this feature then the only other option
>>>> would be
>>>> > > to ask him if he can move to the non-standard
>>>> @org.eclipse.sisu.Typed. The
>>>> > > existing CDI export could then be replaced by exporting
>>>> “org.eclipse.sisu”.
>>>> > > Once that was done then the cdi-api dependency could be excluded
>>>> from the
>>>> > > distribution, as the container will still work without it on the
>>>> classpath
>>>> > > (it’s only required if you want to use the standard CDI annotation).
>>>> > >
>>>> > > So to summarise, the options are:
>>>> > >
>>>> > > a) Continue to support the standard API, but narrow the entry in
>>>> > > META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
>>>> > >
>>>> > > b) Switch to support @org.eclipse.sisu.Typed
>>>> > >
>>>> > > c) Remove this feature completely from Maven
>>>> >
>>>> > From what I'm concerned b and c would solve it but I guess sisu users
>>>> can
>>>> > have the same issue - not sure how likely it is.
>>>> >
>>>> >
>>>>
>>>> Sisu users typically have control over the container classpath and can
>>>> choose whether to include CDI or not (and at which level)
>>>> > There is a d) option: add in @Mojo a list of imported API. ClassRealm
>>>> can
>>>> > support filtering from the parent classloader and therefore I could
>>>> use:
>>>> >
>>>> > @Mojo(name ="...", pluginPackages={"javax", ...})
>>>> >
>>>> > This would allow to keep current setup and let mojo to override it.
>>>> > Compared to a) it is defined in plugin.xml and not extension.xml.
>>>> >
>>>> >
>>>>
>>>> At the moment there’s a single Maven API realm, which imports all the
>>>> packages listed in the core extension.xml from the core classloader.
>>>> Plugins then import that realm wholesale, so they automatically get all
>>>> exported packages. However, it should be possible to be more selective,
>>>> whether that’s using a whitelisting or blacklisting approach.
>>>>
>>>> That said, it would be much simpler to either remove the export or
>>>> switch to @org.eclipse.inject.Typed (since use of the annotation in Maven
>>>> is currently very limited)
>>>>
>>>
>>> A last alternative is to still support @Typed without providing it.
>>> Concretely it means maven drops cdi (sadly not inject jar) and use asm to
>>> check if @Typed if here. Sounds the less breaking compromise even if not
>>> the most sexy in terms of impl.
>>>
>>>
>>>> >
>>>> > >
>>>> > > --
>>>> > > Cheers, Stuart
>>>> > >
>>>> > >
>>>> > > On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
>>>> > >
>>>> > > > 2018-02-06 9:41 GMT+01:00 Tibor Digana
>>>> (mailto:tibordigana@apache.org) (mailto:
>>>> > > tibordigana@apache.org (mailto:tibordigana@apache.org))>:
>>>> > > >
>>>> > > > > Personally I would like to see a new Git branch with CDI 2.0
>>>> and the
>>>> > > > > integration test results on Jenkins.
>>>> > > > > This would give us more confidence.
>>>> > > > > Question: Does the CDI 2.0 have any NEW mandatory descriptive
>>>> methods
>>>> > > > > without default value already introduced in OLD annotations CDI
>>>> > > > >
>>>> > > >
>>>> > > >
>>>> > >
>>>> > > 1.0/1.1?
>>>> > > > >
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > It is more a change in the hierarchy. It doesn't break the user
>>>> API since
>>>> > > > cdi is designed to be provided but it is broken if new code uses
>>>> old API.
>>>> > > >
>>>> > > > Side note: if the idea behind this answer is to ensure the default
>>>> > > provided
>>>> > > > API is the last one then it doesn't work cause an API has a few
>>>> logic
>>>> > >
>>>> > > which
>>>> > > > can require to be overriden (like the SPI and defaults handling).
>>>> > > > Maven uses its own API and exposing CDI is a leaking abuse IMHO.
>>>> > > >
>>>> > > > Note that this is an old bug which should be fixed now IMO before
>>>> maven
>>>> > > > considers CDI being exposed as part of the contract.
>>>> > > >
>>>> > > > For reference, older threads:
>>>> > > >
>>>> > > > http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
>>>> > > td5828015.html
>>>> > > >
>>>> http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
>>>> > > >
>>>> > > >
>>>> > > > There is no risk removing it, worse case plugins would add the
>>>> API as
>>>> > > > compile instead of provided which should likely already be the
>>>> case.
>>>> > > >
>>>> > > >
>>>> > > > > On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau
>>>> > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>> > > > > wrote:
>>>> > > > >
>>>> > > > > >
>>>> > > > > > For the reproducer here it is https://github.com/
>>>> > > > > > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
>>>> > > > > >
>>>> > > > > > 2018-02-06 8:05 GMT+01:00 Tibor Digana
>>>> tibordigana@apache.org (mailto:tibordigana@apache.org)
>>>> > > (mailto:tibordigana@apache.org)>:
>>>> > > > > >
>>>> > > > > > > Changing the package would not be possible in 3.x.
>>>> > > > > >
>>>> > > > > > Why? In particular since it is an old regression already
>>>> reported on
>>>> > > the
>>>> > > > > > list due to guice introduction it shouldn't be delayed for
>>>> this kind
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > of
>>>> > > > > > reason IMHO.
>>>> > > > > > Was less visible until CDI 2 was released cause the API
>>>> difference
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > was
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > not
>>>> > > > > > triggered but now there are new entries it breaks immediately.
>>>> > > > > >
>>>> > > > > >
>>>> > > > > > > Guessing the version 4.0.0.
>>>> > > > > > > WDYT?
>>>> > > > > > >
>>>> > > > > >
>>>> > > > > >
>>>> > > > > >
>>>> > > > > > Would stay a blocker until 4 is out which is that soon so not
>>>> sure
>>>> > > it is
>>>> > > > > > an option.
>>>> > > > > >
>>>> > > > > >
>>>> > > > > > >
>>>> > > > > > > On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana
>>>> > > tibordigana@apache.org (mailto:tibordigana@apache.org)>
>>>> > > > > > > wrote:
>>>> > > > > > >
>>>> > > > > > > > The question is maybe about what is realistic for Maven
>>>> devs.
>>>> > > > > > > > Shading the CPI package (to something like
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > org.apache.maven.cdi.*)
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > would
>>>> > > > > > > > be maybe the case instead of removing the original CDI and
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > reinventing
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > the
>>>> > > > > > > > wheel.
>>>> > > > > > > >
>>>> > > > > > > > On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY
>>>> > > herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
>>>> > > > > > > > wrote:
>>>> > > > > > > >
>>>> > > > > > > > > and does the MNG issue contain a reproducible test case
>>>> for us
>>>> > > to
>>>> > > > > > > > > investigate
>>>> > > > > > > > > more precisely?
>>>> > > > > > > > >
>>>> > > > > > > > > Regards,
>>>> > > > > > > > >
>>>> > > > > > > > > Hervé
>>>> > > > > > > > >
>>>> > > > > > > > > Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a
>>>> écrit :
>>>> > > > > > > > > > Is there a MNG[1] issue?
>>>> > > > > > > > > >
>>>> > > > > > > > > > Robert
>>>> > > > > > > > > >
>>>> > > > > > > > > > [1] https://issues.apache.org/jira/browse/MNG
>>>> > > > > > > > > >
>>>> > > > > > > > > > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>>>> > > > > > > > > >
>>>> > > > > > > > > >
>>>> )>
>>>> > > wrote:
>>>> > > > > > > > > > > Up?
>>>> > > > > > > > > > >
>>>> > > > > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau"
>>>> > > > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>> > > > > > > > > a
>>>> > > > > > > > > > >
>>>> > > > > > > > > > > écrit :
>>>> > > > > > > > > > > > Hi guys,
>>>> > > > > > > > > > > >
>>>> > > > > > > > > > > > cdi-api is still in maven lib and breaks any
>>>> plugin
>>>> > > using it
>>>> > > > > since
>>>> > > > > > > > > it is
>>>> > > > > > > > > > > > an old version, can it be dropped or at least
>>>> isolated
>>>> > > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > from
>>>> > > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > plugin
>>>> > > > > > > > > > > > classloaders?
>>>> > > > > > > > > > > >
>>>> > > > > > > > > > > > Thanks,
>>>> > > > > > > > > > > > Romain Manni-Bucau
>>>> > > > > > > > > > > > @rmannibucau |
>>>> Blog
>>>> > > > > > > > > > > > | Old Blog
>>>> > > > > > > > > > > > | Github
>>>> > > > > > > > > > > > | LinkedIn
>>>> > > > > > > > > > > >
>>>> > > > > > > > > > > >
>>>> > > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > > >
>>>> ------------------------------------------------------------
>>>> > > > > ---------
>>>> > > > > > > > > > To unsubscribe, e-mail:
>>>> dev-unsubscribe@maven.apache.org (mailto:
>>>> dev-unsubscribe@maven.apache.org)
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > (mailto:dev-unsubscribe@maven.apache.org)
>>>> > > > > > > > > > For additional commands, e-mail:
>>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > (mailto:dev-help@maven.apache.org)
>>>> > > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> ------------------------------------------------------------
>>>> > > ---------
>>>> > > > > > > > > To unsubscribe, e-mail:
>>>> dev-unsubscribe@maven.apache.org (mailto:
>>>> dev-unsubscribe@maven.apache.org)
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > (mailto:dev-unsubscribe@maven.apache.org)
>>>> > > > > > > > > For additional commands, e-mail:
>>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > (mailto:dev-help@maven.apache.org)
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > >
>>>> >
>>>> >
>>>> >
>>>>
>>>>
>>>>
>>>

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
Any issue moving forward on https://github.com/apache/maven/pull/408 ?

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>


Le jeu. 3 déc. 2020 à 20:08, Romain Manni-Bucau <rm...@gmail.com> a
écrit :

> created https://github.com/apache/maven/pull/408 about it
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
>
> Le mar. 1 déc. 2020 à 17:50, Romain Manni-Bucau <rm...@gmail.com> a
> écrit :
>
>> Up,
>>
>> Encountered a few bugs related to this regression, wonder how we want to
>> tackle it.
>> My 2cts would be to drop cdi-api and replace the single used
>> annotation from there by a maven one.
>> If we don't want to break plugins (not sure any use that) we can rewrite
>> it with asm or equivalent at load time since we own the classloading.
>>
>> Anyone having an opinion on that?
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>>
>> Le mar. 6 févr. 2018 à 18:02, Romain Manni-Bucau <rm...@gmail.com>
>> a écrit :
>>
>>> commented inline
>>>
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github
>>> <https://github.com/rmannibucau> | LinkedIn
>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>
>>> 2018-02-06 17:51 GMT+01:00 Stuart McCulloch <mc...@gmail.com>:
>>>
>>>> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
>>>> > 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mcculls@gmail.com
>>>> (mailto:mcculls@gmail.com)>:
>>>> >
>>>> > > The "javax.enterprise.inject" CDI package was explicitly exported
>>>> as part
>>>> > > of the ongoing effort to migrate legacy Plexus components onto more
>>>> modern
>>>> > > standard annotations.
>>>> > >
>>>> >
>>>> >
>>>> > Hmm, this looks wrong from my window cause Maven doesn't support CDI
>>>> API -
>>>> > guice doesn't. So it is an interpretation of a well defined API which
>>>> is by
>>>> > defintion a bad public API, no?
>>>> >
>>>> >
>>>>
>>>> Raw Guice supports JSR330, but requires programmatic configuration (ie.
>>>> bindings defined in modules)
>>>>
>>>> Sisu builds on Guice to add support for things like annotation scanning
>>>> and wiring, injectable collections that dynamically update as plugins come
>>>> and go, property placeholders, etc.
>>>>
>>>> If the CDI annotations are on the classpath then it also honours the
>>>> @Typed annotation. This was a feature request to help migrate certain
>>>> components from other Plexus-based applications over to JSR330 + @Typed. At
>>>> the time there was a consideration that the rest of the CDI annotations
>>>> would eventually be supported, as another compatibility layer.
>>>>
>>>> Sisu also provides a Plexus compatibility layer that supports Plexus
>>>> annotations and XML
>>>>
>>>> Maven 3.x switched to Sisu so old Plexus-based components can still be
>>>> used, while modern components can be written with JSR330. At the time of
>>>> the switch it was decided to enable support for @Typed in Maven
>>>> plugins/extensions (because there was a developer need for this feature,
>>>> but that may well have changed and no longer be relevant).
>>>>
>>>> So Maven currently honours using @Typed on components - if it’s decided
>>>> that Maven doesn’t want to support @Typed in plugins then just remove the
>>>> export and exclude the cdi-api jar. As mentioned previously support for
>>>> @Typed is used by other downstream non-Maven applications so it will always
>>>> be something the container supports, but it's totally optional so if you
>>>> don’t want it then you don’t need to ship the cdi-api jar.
>>>>
>>>
>>> Yes but having the full API for one class is luxury (see later comment
>>> for the detail)
>>>
>>>
>>>> >
>>>> > >
>>>> > > Specifically, the @javax.enterprise.inject.Typed annotation lets
>>>> > > components state they are only visible for injection under a
>>>> specific type,
>>>> > > rather than any type in their hierarchy.
>>>> > >
>>>> > > There’s no annotation to control binding visibility in JSR330,
>>>> because it
>>>> > > deliberately avoids configuration concerns, which is why we went
>>>> with the
>>>> > > closest standard annotation (@Typed from JSR299 aka CDI). While we
>>>> could
>>>> > > have decided to use our own annotation - and the container does in
>>>> fact
>>>> > > support using @org.eclipse.sisu.Typed - this is not standardised or
>>>> > > portable. Also note the container will continue to support this
>>>> (optional)
>>>> > > feature for other downstream users, regardless of what’s decided
>>>> here - the
>>>> > > question is whether Maven still wants to use this feature and
>>>> whether it
>>>> > > wants to use the standard annotation or not.
>>>> > >
>>>> > > Another point is that whichever annotation is chosen must be
>>>> > > visible/defined from the same classloader to both core and plugins.
>>>> If the
>>>> > > annotation is not exported then core and each plugin will end up
>>>> with a
>>>> > > different @Typed class, defined by different classloaders. Any use
>>>> of
>>>> > > @Typed in plugins would then effectively be invisible to the
>>>> container,
>>>> > > because the JVM’s AnnotatedElement API (getAnnotation,
>>>> isAnnotationPresent,
>>>> > > etc.) work off classes and not name equivalence.
>>>> > >
>>>> > > Similarly shading won’t work because neither the plugin’s
>>>> components nor
>>>> > > the container would know about the shaded package.
>>>> > >
>>>> >
>>>> >
>>>> > Hmm, not sure. I mean it works in most projects and it is easy to
>>>> expose
>>>> > the shaded API so not a big deal *technically*. Agree it would be a
>>>> bad
>>>> > solution to use a misused API publicly.
>>>> >
>>>> >
>>>>
>>>> By “not work” I really meant “not practical”. It’s not enough to just
>>>> shade the CDI jar, you’d also need to shade the container - being careful
>>>> that its reflective calls were properly updated (since it uses reflection
>>>> to decide whether to load the feature or not). TBH all that work is
>>>> overkill, since the container already supports an alternative annotation:
>>>> @org.eclipse.sisu.Typed
>>>>
>>>
>>> works for me
>>>
>>>
>>>> >
>>>> >
>>>> > >
>>>> > > As you can see from the thread in http://maven.40175.n5.nabble.
>>>> > > com/Linkage-error-td5784411.html a number of alternative solutions
>>>> have
>>>> > > been discussed before, including narrowing the export to:
>>>> > >
>>>> > > "javax.enterprise.inject.Typed"
>>>> > >
>>>> > > as that’s the only annotation we’re currently interested in. Since
>>>> @Typed
>>>> > > hasn’t changed between 1.x and 2.x that should be a workable
>>>> solution,
>>>> > > assuming you wanted to keep using the standard annotation.
>>>> > >
>>>> > > Removing the export (and thereby removing the feature to limit
>>>> injection
>>>> > > visibility to a specific type) was also discussed, and at the time
>>>> Igor
>>>> > > asked for it to be kept:
>>>> > >
>>>> > > “Please keep @Typed annotation available outside of core.
>>>> > >
>>>> > > I use @Typed annotation in one of my (private) core extensions
>>>> where I
>>>> > > need a class to implement an interface but not make that interface
>>>> > > visible for injection in other components.”
>>>> > >
>>>> >
>>>> >
>>>> > Issue is I can say the opposite "I use this in my plugin cause I use
>>>> CDI to
>>>> > impl my plugin, please ignore it for all Maven usage". Both are valid
>>>> and
>>>> > therefore the Maven API shouldn't have any overlapping.
>>>> >
>>>> >
>>>>
>>>> Whenever you embed a container inside any kind of plugin you’re at the
>>>> mercy of what’s exposed to that plugin - whether that plugin is running in
>>>> Maven, Jenkins, or an IDE. If you want full control/sanity then use a
>>>> custom classloader to isolate the embedded container from the plugin’s
>>>> environment, and just let through those packages you expect to be provided.
>>>>
>>>> For example, say we did fully support CDI 1.x components inside plugins
>>>> (as in the entire API was supported). You’d still have an issue embedding a
>>>> CDI 2.x container, because of the API clash, unless you used a custom
>>>> classloader between the plugin and the embedded container.
>>>>
>>>
>>> Yes but would have complained way earlier ;)
>>>
>>>
>>>> >
>>>> >
>>>> > >
>>>> > > Assuming Igor still needs this feature then the only other option
>>>> would be
>>>> > > to ask him if he can move to the non-standard
>>>> @org.eclipse.sisu.Typed. The
>>>> > > existing CDI export could then be replaced by exporting
>>>> “org.eclipse.sisu”.
>>>> > > Once that was done then the cdi-api dependency could be excluded
>>>> from the
>>>> > > distribution, as the container will still work without it on the
>>>> classpath
>>>> > > (it’s only required if you want to use the standard CDI annotation).
>>>> > >
>>>> > > So to summarise, the options are:
>>>> > >
>>>> > > a) Continue to support the standard API, but narrow the entry in
>>>> > > META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
>>>> > >
>>>> > > b) Switch to support @org.eclipse.sisu.Typed
>>>> > >
>>>> > > c) Remove this feature completely from Maven
>>>> >
>>>> > From what I'm concerned b and c would solve it but I guess sisu users
>>>> can
>>>> > have the same issue - not sure how likely it is.
>>>> >
>>>> >
>>>>
>>>> Sisu users typically have control over the container classpath and can
>>>> choose whether to include CDI or not (and at which level)
>>>> > There is a d) option: add in @Mojo a list of imported API. ClassRealm
>>>> can
>>>> > support filtering from the parent classloader and therefore I could
>>>> use:
>>>> >
>>>> > @Mojo(name ="...", pluginPackages={"javax", ...})
>>>> >
>>>> > This would allow to keep current setup and let mojo to override it.
>>>> > Compared to a) it is defined in plugin.xml and not extension.xml.
>>>> >
>>>> >
>>>>
>>>> At the moment there’s a single Maven API realm, which imports all the
>>>> packages listed in the core extension.xml from the core classloader.
>>>> Plugins then import that realm wholesale, so they automatically get all
>>>> exported packages. However, it should be possible to be more selective,
>>>> whether that’s using a whitelisting or blacklisting approach.
>>>>
>>>> That said, it would be much simpler to either remove the export or
>>>> switch to @org.eclipse.inject.Typed (since use of the annotation in Maven
>>>> is currently very limited)
>>>>
>>>
>>> A last alternative is to still support @Typed without providing it.
>>> Concretely it means maven drops cdi (sadly not inject jar) and use asm to
>>> check if @Typed if here. Sounds the less breaking compromise even if not
>>> the most sexy in terms of impl.
>>>
>>>
>>>> >
>>>> > >
>>>> > > --
>>>> > > Cheers, Stuart
>>>> > >
>>>> > >
>>>> > > On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
>>>> > >
>>>> > > > 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org
>>>> (mailto:tibordigana@apache.org) (mailto:
>>>> > > tibordigana@apache.org (mailto:tibordigana@apache.org))>:
>>>> > > >
>>>> > > > > Personally I would like to see a new Git branch with CDI 2.0
>>>> and the
>>>> > > > > integration test results on Jenkins.
>>>> > > > > This would give us more confidence.
>>>> > > > > Question: Does the CDI 2.0 have any NEW mandatory descriptive
>>>> methods
>>>> > > > > without default value already introduced in OLD annotations CDI
>>>> > > > >
>>>> > > >
>>>> > > >
>>>> > >
>>>> > > 1.0/1.1?
>>>> > > > >
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > It is more a change in the hierarchy. It doesn't break the user
>>>> API since
>>>> > > > cdi is designed to be provided but it is broken if new code uses
>>>> old API.
>>>> > > >
>>>> > > > Side note: if the idea behind this answer is to ensure the default
>>>> > > provided
>>>> > > > API is the last one then it doesn't work cause an API has a few
>>>> logic
>>>> > >
>>>> > > which
>>>> > > > can require to be overriden (like the SPI and defaults handling).
>>>> > > > Maven uses its own API and exposing CDI is a leaking abuse IMHO.
>>>> > > >
>>>> > > > Note that this is an old bug which should be fixed now IMO before
>>>> maven
>>>> > > > considers CDI being exposed as part of the contract.
>>>> > > >
>>>> > > > For reference, older threads:
>>>> > > >
>>>> > > > http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
>>>> > > td5828015.html
>>>> > > >
>>>> http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
>>>> > > >
>>>> > > >
>>>> > > > There is no risk removing it, worse case plugins would add the
>>>> API as
>>>> > > > compile instead of provided which should likely already be the
>>>> case.
>>>> > > >
>>>> > > >
>>>> > > > > On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
>>>> > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>> > > > > wrote:
>>>> > > > >
>>>> > > > > >
>>>> > > > > > For the reproducer here it is https://github.com/
>>>> > > > > > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
>>>> > > > > >
>>>> > > > > > 2018-02-06 8:05 GMT+01:00 Tibor Digana <
>>>> tibordigana@apache.org (mailto:tibordigana@apache.org)
>>>> > > (mailto:tibordigana@apache.org)>:
>>>> > > > > >
>>>> > > > > > > Changing the package would not be possible in 3.x.
>>>> > > > > >
>>>> > > > > > Why? In particular since it is an old regression already
>>>> reported on
>>>> > > the
>>>> > > > > > list due to guice introduction it shouldn't be delayed for
>>>> this kind
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > of
>>>> > > > > > reason IMHO.
>>>> > > > > > Was less visible until CDI 2 was released cause the API
>>>> difference
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > was
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > not
>>>> > > > > > triggered but now there are new entries it breaks immediately.
>>>> > > > > >
>>>> > > > > >
>>>> > > > > > > Guessing the version 4.0.0.
>>>> > > > > > > WDYT?
>>>> > > > > > >
>>>> > > > > >
>>>> > > > > >
>>>> > > > > >
>>>> > > > > > Would stay a blocker until 4 is out which is that soon so not
>>>> sure
>>>> > > it is
>>>> > > > > > an option.
>>>> > > > > >
>>>> > > > > >
>>>> > > > > > >
>>>> > > > > > > On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
>>>> > > tibordigana@apache.org (mailto:tibordigana@apache.org)>
>>>> > > > > > > wrote:
>>>> > > > > > >
>>>> > > > > > > > The question is maybe about what is realistic for Maven
>>>> devs.
>>>> > > > > > > > Shading the CPI package (to something like
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > org.apache.maven.cdi.*)
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > would
>>>> > > > > > > > be maybe the case instead of removing the original CDI and
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > reinventing
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > the
>>>> > > > > > > > wheel.
>>>> > > > > > > >
>>>> > > > > > > > On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
>>>> > > herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
>>>> > > > > > > > wrote:
>>>> > > > > > > >
>>>> > > > > > > > > and does the MNG issue contain a reproducible test case
>>>> for us
>>>> > > to
>>>> > > > > > > > > investigate
>>>> > > > > > > > > more precisely?
>>>> > > > > > > > >
>>>> > > > > > > > > Regards,
>>>> > > > > > > > >
>>>> > > > > > > > > Hervé
>>>> > > > > > > > >
>>>> > > > > > > > > Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a
>>>> écrit :
>>>> > > > > > > > > > Is there a MNG[1] issue?
>>>> > > > > > > > > >
>>>> > > > > > > > > > Robert
>>>> > > > > > > > > >
>>>> > > > > > > > > > [1] https://issues.apache.org/jira/browse/MNG
>>>> > > > > > > > > >
>>>> > > > > > > > > > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>>>> > > > > > > > > >
>>>> > > > > > > > > > <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com
>>>> )>
>>>> > > wrote:
>>>> > > > > > > > > > > Up?
>>>> > > > > > > > > > >
>>>> > > > > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
>>>> > > > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>> > > > > > > > > a
>>>> > > > > > > > > > >
>>>> > > > > > > > > > > écrit :
>>>> > > > > > > > > > > > Hi guys,
>>>> > > > > > > > > > > >
>>>> > > > > > > > > > > > cdi-api is still in maven lib and breaks any
>>>> plugin
>>>> > > using it
>>>> > > > > since
>>>> > > > > > > > > it is
>>>> > > > > > > > > > > > an old version, can it be dropped or at least
>>>> isolated
>>>> > > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > from
>>>> > > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > > > plugin
>>>> > > > > > > > > > > > classloaders?
>>>> > > > > > > > > > > >
>>>> > > > > > > > > > > > Thanks,
>>>> > > > > > > > > > > > Romain Manni-Bucau
>>>> > > > > > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> |
>>>> Blog
>>>> > > > > > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
>>>> > > > > > > > > > > > <http://rmannibucau.wordpress.com> | Github
>>>> > > > > > > > > > > > <https://github.com/rmannibucau> | LinkedIn
>>>> > > > > > > > > > > > <https://www.linkedin.com/in/rmannibucau>
>>>> > > > > > > > > > > >
>>>> > > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > > >
>>>> > > > > > > > > >
>>>> ------------------------------------------------------------
>>>> > > > > ---------
>>>> > > > > > > > > > To unsubscribe, e-mail:
>>>> dev-unsubscribe@maven.apache.org (mailto:
>>>> dev-unsubscribe@maven.apache.org)
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > (mailto:dev-unsubscribe@maven.apache.org)
>>>> > > > > > > > > > For additional commands, e-mail:
>>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > (mailto:dev-help@maven.apache.org)
>>>> > > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> > > > > > > > >
>>>> ------------------------------------------------------------
>>>> > > ---------
>>>> > > > > > > > > To unsubscribe, e-mail:
>>>> dev-unsubscribe@maven.apache.org (mailto:
>>>> dev-unsubscribe@maven.apache.org)
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > (mailto:dev-unsubscribe@maven.apache.org)
>>>> > > > > > > > > For additional commands, e-mail:
>>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > > (mailto:dev-help@maven.apache.org)
>>>> > > > > > > > >
>>>> > > > > > > >
>>>> > > > > > >
>>>> > > > > >
>>>> > > > >
>>>> > > >
>>>> > >
>>>> > >
>>>> >
>>>> >
>>>> >
>>>>
>>>>
>>>>
>>>

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
created https://github.com/apache/maven/pull/408 about it

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>


Le mar. 1 déc. 2020 à 17:50, Romain Manni-Bucau <rm...@gmail.com> a
écrit :

> Up,
>
> Encountered a few bugs related to this regression, wonder how we want to
> tackle it.
> My 2cts would be to drop cdi-api and replace the single used
> annotation from there by a maven one.
> If we don't want to break plugins (not sure any use that) we can rewrite
> it with asm or equivalent at load time since we own the classloading.
>
> Anyone having an opinion on that?
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
>
> Le mar. 6 févr. 2018 à 18:02, Romain Manni-Bucau <rm...@gmail.com>
> a écrit :
>
>> commented inline
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>> 2018-02-06 17:51 GMT+01:00 Stuart McCulloch <mc...@gmail.com>:
>>
>>> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
>>> > 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mcculls@gmail.com
>>> (mailto:mcculls@gmail.com)>:
>>> >
>>> > > The "javax.enterprise.inject" CDI package was explicitly exported as
>>> part
>>> > > of the ongoing effort to migrate legacy Plexus components onto more
>>> modern
>>> > > standard annotations.
>>> > >
>>> >
>>> >
>>> > Hmm, this looks wrong from my window cause Maven doesn't support CDI
>>> API -
>>> > guice doesn't. So it is an interpretation of a well defined API which
>>> is by
>>> > defintion a bad public API, no?
>>> >
>>> >
>>>
>>> Raw Guice supports JSR330, but requires programmatic configuration (ie.
>>> bindings defined in modules)
>>>
>>> Sisu builds on Guice to add support for things like annotation scanning
>>> and wiring, injectable collections that dynamically update as plugins come
>>> and go, property placeholders, etc.
>>>
>>> If the CDI annotations are on the classpath then it also honours the
>>> @Typed annotation. This was a feature request to help migrate certain
>>> components from other Plexus-based applications over to JSR330 + @Typed. At
>>> the time there was a consideration that the rest of the CDI annotations
>>> would eventually be supported, as another compatibility layer.
>>>
>>> Sisu also provides a Plexus compatibility layer that supports Plexus
>>> annotations and XML
>>>
>>> Maven 3.x switched to Sisu so old Plexus-based components can still be
>>> used, while modern components can be written with JSR330. At the time of
>>> the switch it was decided to enable support for @Typed in Maven
>>> plugins/extensions (because there was a developer need for this feature,
>>> but that may well have changed and no longer be relevant).
>>>
>>> So Maven currently honours using @Typed on components - if it’s decided
>>> that Maven doesn’t want to support @Typed in plugins then just remove the
>>> export and exclude the cdi-api jar. As mentioned previously support for
>>> @Typed is used by other downstream non-Maven applications so it will always
>>> be something the container supports, but it's totally optional so if you
>>> don’t want it then you don’t need to ship the cdi-api jar.
>>>
>>
>> Yes but having the full API for one class is luxury (see later comment
>> for the detail)
>>
>>
>>> >
>>> > >
>>> > > Specifically, the @javax.enterprise.inject.Typed annotation lets
>>> > > components state they are only visible for injection under a
>>> specific type,
>>> > > rather than any type in their hierarchy.
>>> > >
>>> > > There’s no annotation to control binding visibility in JSR330,
>>> because it
>>> > > deliberately avoids configuration concerns, which is why we went
>>> with the
>>> > > closest standard annotation (@Typed from JSR299 aka CDI). While we
>>> could
>>> > > have decided to use our own annotation - and the container does in
>>> fact
>>> > > support using @org.eclipse.sisu.Typed - this is not standardised or
>>> > > portable. Also note the container will continue to support this
>>> (optional)
>>> > > feature for other downstream users, regardless of what’s decided
>>> here - the
>>> > > question is whether Maven still wants to use this feature and
>>> whether it
>>> > > wants to use the standard annotation or not.
>>> > >
>>> > > Another point is that whichever annotation is chosen must be
>>> > > visible/defined from the same classloader to both core and plugins.
>>> If the
>>> > > annotation is not exported then core and each plugin will end up
>>> with a
>>> > > different @Typed class, defined by different classloaders. Any use of
>>> > > @Typed in plugins would then effectively be invisible to the
>>> container,
>>> > > because the JVM’s AnnotatedElement API (getAnnotation,
>>> isAnnotationPresent,
>>> > > etc.) work off classes and not name equivalence.
>>> > >
>>> > > Similarly shading won’t work because neither the plugin’s components
>>> nor
>>> > > the container would know about the shaded package.
>>> > >
>>> >
>>> >
>>> > Hmm, not sure. I mean it works in most projects and it is easy to
>>> expose
>>> > the shaded API so not a big deal *technically*. Agree it would be a bad
>>> > solution to use a misused API publicly.
>>> >
>>> >
>>>
>>> By “not work” I really meant “not practical”. It’s not enough to just
>>> shade the CDI jar, you’d also need to shade the container - being careful
>>> that its reflective calls were properly updated (since it uses reflection
>>> to decide whether to load the feature or not). TBH all that work is
>>> overkill, since the container already supports an alternative annotation:
>>> @org.eclipse.sisu.Typed
>>>
>>
>> works for me
>>
>>
>>> >
>>> >
>>> > >
>>> > > As you can see from the thread in http://maven.40175.n5.nabble.
>>> > > com/Linkage-error-td5784411.html a number of alternative solutions
>>> have
>>> > > been discussed before, including narrowing the export to:
>>> > >
>>> > > "javax.enterprise.inject.Typed"
>>> > >
>>> > > as that’s the only annotation we’re currently interested in. Since
>>> @Typed
>>> > > hasn’t changed between 1.x and 2.x that should be a workable
>>> solution,
>>> > > assuming you wanted to keep using the standard annotation.
>>> > >
>>> > > Removing the export (and thereby removing the feature to limit
>>> injection
>>> > > visibility to a specific type) was also discussed, and at the time
>>> Igor
>>> > > asked for it to be kept:
>>> > >
>>> > > “Please keep @Typed annotation available outside of core.
>>> > >
>>> > > I use @Typed annotation in one of my (private) core extensions where
>>> I
>>> > > need a class to implement an interface but not make that interface
>>> > > visible for injection in other components.”
>>> > >
>>> >
>>> >
>>> > Issue is I can say the opposite "I use this in my plugin cause I use
>>> CDI to
>>> > impl my plugin, please ignore it for all Maven usage". Both are valid
>>> and
>>> > therefore the Maven API shouldn't have any overlapping.
>>> >
>>> >
>>>
>>> Whenever you embed a container inside any kind of plugin you’re at the
>>> mercy of what’s exposed to that plugin - whether that plugin is running in
>>> Maven, Jenkins, or an IDE. If you want full control/sanity then use a
>>> custom classloader to isolate the embedded container from the plugin’s
>>> environment, and just let through those packages you expect to be provided.
>>>
>>> For example, say we did fully support CDI 1.x components inside plugins
>>> (as in the entire API was supported). You’d still have an issue embedding a
>>> CDI 2.x container, because of the API clash, unless you used a custom
>>> classloader between the plugin and the embedded container.
>>>
>>
>> Yes but would have complained way earlier ;)
>>
>>
>>> >
>>> >
>>> > >
>>> > > Assuming Igor still needs this feature then the only other option
>>> would be
>>> > > to ask him if he can move to the non-standard
>>> @org.eclipse.sisu.Typed. The
>>> > > existing CDI export could then be replaced by exporting
>>> “org.eclipse.sisu”.
>>> > > Once that was done then the cdi-api dependency could be excluded
>>> from the
>>> > > distribution, as the container will still work without it on the
>>> classpath
>>> > > (it’s only required if you want to use the standard CDI annotation).
>>> > >
>>> > > So to summarise, the options are:
>>> > >
>>> > > a) Continue to support the standard API, but narrow the entry in
>>> > > META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
>>> > >
>>> > > b) Switch to support @org.eclipse.sisu.Typed
>>> > >
>>> > > c) Remove this feature completely from Maven
>>> >
>>> > From what I'm concerned b and c would solve it but I guess sisu users
>>> can
>>> > have the same issue - not sure how likely it is.
>>> >
>>> >
>>>
>>> Sisu users typically have control over the container classpath and can
>>> choose whether to include CDI or not (and at which level)
>>> > There is a d) option: add in @Mojo a list of imported API. ClassRealm
>>> can
>>> > support filtering from the parent classloader and therefore I could
>>> use:
>>> >
>>> > @Mojo(name ="...", pluginPackages={"javax", ...})
>>> >
>>> > This would allow to keep current setup and let mojo to override it.
>>> > Compared to a) it is defined in plugin.xml and not extension.xml.
>>> >
>>> >
>>>
>>> At the moment there’s a single Maven API realm, which imports all the
>>> packages listed in the core extension.xml from the core classloader.
>>> Plugins then import that realm wholesale, so they automatically get all
>>> exported packages. However, it should be possible to be more selective,
>>> whether that’s using a whitelisting or blacklisting approach.
>>>
>>> That said, it would be much simpler to either remove the export or
>>> switch to @org.eclipse.inject.Typed (since use of the annotation in Maven
>>> is currently very limited)
>>>
>>
>> A last alternative is to still support @Typed without providing it.
>> Concretely it means maven drops cdi (sadly not inject jar) and use asm to
>> check if @Typed if here. Sounds the less breaking compromise even if not
>> the most sexy in terms of impl.
>>
>>
>>> >
>>> > >
>>> > > --
>>> > > Cheers, Stuart
>>> > >
>>> > >
>>> > > On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
>>> > >
>>> > > > 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org
>>> (mailto:tibordigana@apache.org) (mailto:
>>> > > tibordigana@apache.org (mailto:tibordigana@apache.org))>:
>>> > > >
>>> > > > > Personally I would like to see a new Git branch with CDI 2.0 and
>>> the
>>> > > > > integration test results on Jenkins.
>>> > > > > This would give us more confidence.
>>> > > > > Question: Does the CDI 2.0 have any NEW mandatory descriptive
>>> methods
>>> > > > > without default value already introduced in OLD annotations CDI
>>> > > > >
>>> > > >
>>> > > >
>>> > >
>>> > > 1.0/1.1?
>>> > > > >
>>> > > >
>>> > > >
>>> > > >
>>> > > > It is more a change in the hierarchy. It doesn't break the user
>>> API since
>>> > > > cdi is designed to be provided but it is broken if new code uses
>>> old API.
>>> > > >
>>> > > > Side note: if the idea behind this answer is to ensure the default
>>> > > provided
>>> > > > API is the last one then it doesn't work cause an API has a few
>>> logic
>>> > >
>>> > > which
>>> > > > can require to be overriden (like the SPI and defaults handling).
>>> > > > Maven uses its own API and exposing CDI is a leaking abuse IMHO.
>>> > > >
>>> > > > Note that this is an old bug which should be fixed now IMO before
>>> maven
>>> > > > considers CDI being exposed as part of the contract.
>>> > > >
>>> > > > For reference, older threads:
>>> > > >
>>> > > > http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
>>> > > td5828015.html
>>> > > >
>>> http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
>>> > > >
>>> > > >
>>> > > > There is no risk removing it, worse case plugins would add the API
>>> as
>>> > > > compile instead of provided which should likely already be the
>>> case.
>>> > > >
>>> > > >
>>> > > > > On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
>>> > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>> > > > > wrote:
>>> > > > >
>>> > > > > >
>>> > > > > > For the reproducer here it is https://github.com/
>>> > > > > > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
>>> > > > > >
>>> > > > > > 2018-02-06 8:05 GMT+01:00 Tibor Digana <tibordigana@apache.org
>>> (mailto:tibordigana@apache.org)
>>> > > (mailto:tibordigana@apache.org)>:
>>> > > > > >
>>> > > > > > > Changing the package would not be possible in 3.x.
>>> > > > > >
>>> > > > > > Why? In particular since it is an old regression already
>>> reported on
>>> > > the
>>> > > > > > list due to guice introduction it shouldn't be delayed for
>>> this kind
>>> > > > >
>>> > > >
>>> > >
>>> > > of
>>> > > > > > reason IMHO.
>>> > > > > > Was less visible until CDI 2 was released cause the API
>>> difference
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > > was
>>> > > > > >
>>> > > > >
>>> > > > >
>>> > > > > not
>>> > > > > > triggered but now there are new entries it breaks immediately.
>>> > > > > >
>>> > > > > >
>>> > > > > > > Guessing the version 4.0.0.
>>> > > > > > > WDYT?
>>> > > > > > >
>>> > > > > >
>>> > > > > >
>>> > > > > >
>>> > > > > > Would stay a blocker until 4 is out which is that soon so not
>>> sure
>>> > > it is
>>> > > > > > an option.
>>> > > > > >
>>> > > > > >
>>> > > > > > >
>>> > > > > > > On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
>>> > > tibordigana@apache.org (mailto:tibordigana@apache.org)>
>>> > > > > > > wrote:
>>> > > > > > >
>>> > > > > > > > The question is maybe about what is realistic for Maven
>>> devs.
>>> > > > > > > > Shading the CPI package (to something like
>>> > > > > > > >
>>> > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > > org.apache.maven.cdi.*)
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > > >
>>> > > > > would
>>> > > > > > > > be maybe the case instead of removing the original CDI and
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > > reinventing
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > > >
>>> > > > > the
>>> > > > > > > > wheel.
>>> > > > > > > >
>>> > > > > > > > On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
>>> > > herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
>>> > > > > > > > wrote:
>>> > > > > > > >
>>> > > > > > > > > and does the MNG issue contain a reproducible test case
>>> for us
>>> > > to
>>> > > > > > > > > investigate
>>> > > > > > > > > more precisely?
>>> > > > > > > > >
>>> > > > > > > > > Regards,
>>> > > > > > > > >
>>> > > > > > > > > Hervé
>>> > > > > > > > >
>>> > > > > > > > > Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a
>>> écrit :
>>> > > > > > > > > > Is there a MNG[1] issue?
>>> > > > > > > > > >
>>> > > > > > > > > > Robert
>>> > > > > > > > > >
>>> > > > > > > > > > [1] https://issues.apache.org/jira/browse/MNG
>>> > > > > > > > > >
>>> > > > > > > > > > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>>> > > > > > > > > >
>>> > > > > > > > > > <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>> > > wrote:
>>> > > > > > > > > > > Up?
>>> > > > > > > > > > >
>>> > > > > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
>>> > > > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>> > > > > > > > > a
>>> > > > > > > > > > >
>>> > > > > > > > > > > écrit :
>>> > > > > > > > > > > > Hi guys,
>>> > > > > > > > > > > >
>>> > > > > > > > > > > > cdi-api is still in maven lib and breaks any plugin
>>> > > using it
>>> > > > > since
>>> > > > > > > > > it is
>>> > > > > > > > > > > > an old version, can it be dropped or at least
>>> isolated
>>> > > > > > > > > > >
>>> > > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > > from
>>> > > > > > > > > > >
>>> > > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > > >
>>> > > > > plugin
>>> > > > > > > > > > > > classloaders?
>>> > > > > > > > > > > >
>>> > > > > > > > > > > > Thanks,
>>> > > > > > > > > > > > Romain Manni-Bucau
>>> > > > > > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> |
>>> Blog
>>> > > > > > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
>>> > > > > > > > > > > > <http://rmannibucau.wordpress.com> | Github
>>> > > > > > > > > > > > <https://github.com/rmannibucau> | LinkedIn
>>> > > > > > > > > > > > <https://www.linkedin.com/in/rmannibucau>
>>> > > > > > > > > > > >
>>> > > > > > > > > > >
>>> > > > > > > > > >
>>> > > > > > > > > >
>>> > > > > > > > > >
>>> > > > > > > > > >
>>> ------------------------------------------------------------
>>> > > > > ---------
>>> > > > > > > > > > To unsubscribe, e-mail:
>>> dev-unsubscribe@maven.apache.org (mailto:
>>> dev-unsubscribe@maven.apache.org)
>>> > > > > > > > >
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > > (mailto:dev-unsubscribe@maven.apache.org)
>>> > > > > > > > > > For additional commands, e-mail:
>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>> > > > > > > > >
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > > (mailto:dev-help@maven.apache.org)
>>> > > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > > >
>>> ------------------------------------------------------------
>>> > > ---------
>>> > > > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> (mailto:dev-unsubscribe@maven.apache.org)
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > > (mailto:dev-unsubscribe@maven.apache.org)
>>> > > > > > > > > For additional commands, e-mail:
>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > > (mailto:dev-help@maven.apache.org)
>>> > > > > > > > >
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> > >
>>> >
>>> >
>>> >
>>>
>>>
>>>
>>

Re: cdi-api leaking?

Posted by Mark Struberg <st...@yahoo.de.INVALID>.
+1 for dropping. Also already had problems with it.

LieGrue,
strub

> Am 01.12.2020 um 17:50 schrieb Romain Manni-Bucau <rm...@gmail.com>:
> 
> Up,
> 
> Encountered a few bugs related to this regression, wonder how we want to
> tackle it.
> My 2cts would be to drop cdi-api and replace the single used
> annotation from there by a maven one.
> If we don't want to break plugins (not sure any use that) we can rewrite it
> with asm or equivalent at load time since we own the classloading.
> 
> Anyone having an opinion on that?
> 
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
> 
> 
> Le mar. 6 févr. 2018 à 18:02, Romain Manni-Bucau <rm...@gmail.com> a
> écrit :
> 
>> commented inline
>> 
>> 
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>> 
>> 2018-02-06 17:51 GMT+01:00 Stuart McCulloch <mc...@gmail.com>:
>> 
>>> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
>>>> 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mcculls@gmail.com (mailto:
>>> mcculls@gmail.com)>:
>>>> 
>>>>> The "javax.enterprise.inject" CDI package was explicitly exported as
>>> part
>>>>> of the ongoing effort to migrate legacy Plexus components onto more
>>> modern
>>>>> standard annotations.
>>>>> 
>>>> 
>>>> 
>>>> Hmm, this looks wrong from my window cause Maven doesn't support CDI
>>> API -
>>>> guice doesn't. So it is an interpretation of a well defined API which
>>> is by
>>>> defintion a bad public API, no?
>>>> 
>>>> 
>>> 
>>> Raw Guice supports JSR330, but requires programmatic configuration (ie.
>>> bindings defined in modules)
>>> 
>>> Sisu builds on Guice to add support for things like annotation scanning
>>> and wiring, injectable collections that dynamically update as plugins come
>>> and go, property placeholders, etc.
>>> 
>>> If the CDI annotations are on the classpath then it also honours the
>>> @Typed annotation. This was a feature request to help migrate certain
>>> components from other Plexus-based applications over to JSR330 + @Typed. At
>>> the time there was a consideration that the rest of the CDI annotations
>>> would eventually be supported, as another compatibility layer.
>>> 
>>> Sisu also provides a Plexus compatibility layer that supports Plexus
>>> annotations and XML
>>> 
>>> Maven 3.x switched to Sisu so old Plexus-based components can still be
>>> used, while modern components can be written with JSR330. At the time of
>>> the switch it was decided to enable support for @Typed in Maven
>>> plugins/extensions (because there was a developer need for this feature,
>>> but that may well have changed and no longer be relevant).
>>> 
>>> So Maven currently honours using @Typed on components - if it’s decided
>>> that Maven doesn’t want to support @Typed in plugins then just remove the
>>> export and exclude the cdi-api jar. As mentioned previously support for
>>> @Typed is used by other downstream non-Maven applications so it will always
>>> be something the container supports, but it's totally optional so if you
>>> don’t want it then you don’t need to ship the cdi-api jar.
>>> 
>> 
>> Yes but having the full API for one class is luxury (see later comment for
>> the detail)
>> 
>> 
>>>> 
>>>>> 
>>>>> Specifically, the @javax.enterprise.inject.Typed annotation lets
>>>>> components state they are only visible for injection under a specific
>>> type,
>>>>> rather than any type in their hierarchy.
>>>>> 
>>>>> There’s no annotation to control binding visibility in JSR330,
>>> because it
>>>>> deliberately avoids configuration concerns, which is why we went with
>>> the
>>>>> closest standard annotation (@Typed from JSR299 aka CDI). While we
>>> could
>>>>> have decided to use our own annotation - and the container does in
>>> fact
>>>>> support using @org.eclipse.sisu.Typed - this is not standardised or
>>>>> portable. Also note the container will continue to support this
>>> (optional)
>>>>> feature for other downstream users, regardless of what’s decided here
>>> - the
>>>>> question is whether Maven still wants to use this feature and whether
>>> it
>>>>> wants to use the standard annotation or not.
>>>>> 
>>>>> Another point is that whichever annotation is chosen must be
>>>>> visible/defined from the same classloader to both core and plugins.
>>> If the
>>>>> annotation is not exported then core and each plugin will end up with
>>> a
>>>>> different @Typed class, defined by different classloaders. Any use of
>>>>> @Typed in plugins would then effectively be invisible to the
>>> container,
>>>>> because the JVM’s AnnotatedElement API (getAnnotation,
>>> isAnnotationPresent,
>>>>> etc.) work off classes and not name equivalence.
>>>>> 
>>>>> Similarly shading won’t work because neither the plugin’s components
>>> nor
>>>>> the container would know about the shaded package.
>>>>> 
>>>> 
>>>> 
>>>> Hmm, not sure. I mean it works in most projects and it is easy to expose
>>>> the shaded API so not a big deal *technically*. Agree it would be a bad
>>>> solution to use a misused API publicly.
>>>> 
>>>> 
>>> 
>>> By “not work” I really meant “not practical”. It’s not enough to just
>>> shade the CDI jar, you’d also need to shade the container - being careful
>>> that its reflective calls were properly updated (since it uses reflection
>>> to decide whether to load the feature or not). TBH all that work is
>>> overkill, since the container already supports an alternative annotation:
>>> @org.eclipse.sisu.Typed
>>> 
>> 
>> works for me
>> 
>> 
>>>> 
>>>> 
>>>>> 
>>>>> As you can see from the thread in http://maven.40175.n5.nabble.
>>>>> com/Linkage-error-td5784411.html a number of alternative solutions
>>> have
>>>>> been discussed before, including narrowing the export to:
>>>>> 
>>>>> "javax.enterprise.inject.Typed"
>>>>> 
>>>>> as that’s the only annotation we’re currently interested in. Since
>>> @Typed
>>>>> hasn’t changed between 1.x and 2.x that should be a workable solution,
>>>>> assuming you wanted to keep using the standard annotation.
>>>>> 
>>>>> Removing the export (and thereby removing the feature to limit
>>> injection
>>>>> visibility to a specific type) was also discussed, and at the time
>>> Igor
>>>>> asked for it to be kept:
>>>>> 
>>>>> “Please keep @Typed annotation available outside of core.
>>>>> 
>>>>> I use @Typed annotation in one of my (private) core extensions where I
>>>>> need a class to implement an interface but not make that interface
>>>>> visible for injection in other components.”
>>>>> 
>>>> 
>>>> 
>>>> Issue is I can say the opposite "I use this in my plugin cause I use
>>> CDI to
>>>> impl my plugin, please ignore it for all Maven usage". Both are valid
>>> and
>>>> therefore the Maven API shouldn't have any overlapping.
>>>> 
>>>> 
>>> 
>>> Whenever you embed a container inside any kind of plugin you’re at the
>>> mercy of what’s exposed to that plugin - whether that plugin is running in
>>> Maven, Jenkins, or an IDE. If you want full control/sanity then use a
>>> custom classloader to isolate the embedded container from the plugin’s
>>> environment, and just let through those packages you expect to be provided.
>>> 
>>> For example, say we did fully support CDI 1.x components inside plugins
>>> (as in the entire API was supported). You’d still have an issue embedding a
>>> CDI 2.x container, because of the API clash, unless you used a custom
>>> classloader between the plugin and the embedded container.
>>> 
>> 
>> Yes but would have complained way earlier ;)
>> 
>> 
>>>> 
>>>> 
>>>>> 
>>>>> Assuming Igor still needs this feature then the only other option
>>> would be
>>>>> to ask him if he can move to the non-standard
>>> @org.eclipse.sisu.Typed. The
>>>>> existing CDI export could then be replaced by exporting
>>> “org.eclipse.sisu”.
>>>>> Once that was done then the cdi-api dependency could be excluded from
>>> the
>>>>> distribution, as the container will still work without it on the
>>> classpath
>>>>> (it’s only required if you want to use the standard CDI annotation).
>>>>> 
>>>>> So to summarise, the options are:
>>>>> 
>>>>> a) Continue to support the standard API, but narrow the entry in
>>>>> META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
>>>>> 
>>>>> b) Switch to support @org.eclipse.sisu.Typed
>>>>> 
>>>>> c) Remove this feature completely from Maven
>>>> 
>>>> From what I'm concerned b and c would solve it but I guess sisu users
>>> can
>>>> have the same issue - not sure how likely it is.
>>>> 
>>>> 
>>> 
>>> Sisu users typically have control over the container classpath and can
>>> choose whether to include CDI or not (and at which level)
>>>> There is a d) option: add in @Mojo a list of imported API. ClassRealm
>>> can
>>>> support filtering from the parent classloader and therefore I could use:
>>>> 
>>>> @Mojo(name ="...", pluginPackages={"javax", ...})
>>>> 
>>>> This would allow to keep current setup and let mojo to override it.
>>>> Compared to a) it is defined in plugin.xml and not extension.xml.
>>>> 
>>>> 
>>> 
>>> At the moment there’s a single Maven API realm, which imports all the
>>> packages listed in the core extension.xml from the core classloader.
>>> Plugins then import that realm wholesale, so they automatically get all
>>> exported packages. However, it should be possible to be more selective,
>>> whether that’s using a whitelisting or blacklisting approach.
>>> 
>>> That said, it would be much simpler to either remove the export or switch
>>> to @org.eclipse.inject.Typed (since use of the annotation in Maven is
>>> currently very limited)
>>> 
>> 
>> A last alternative is to still support @Typed without providing it.
>> Concretely it means maven drops cdi (sadly not inject jar) and use asm to
>> check if @Typed if here. Sounds the less breaking compromise even if not
>> the most sexy in terms of impl.
>> 
>> 
>>>> 
>>>>> 
>>>>> --
>>>>> Cheers, Stuart
>>>>> 
>>>>> 
>>>>> On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
>>>>> 
>>>>>> 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org
>>> (mailto:tibordigana@apache.org) (mailto:
>>>>> tibordigana@apache.org (mailto:tibordigana@apache.org))>:
>>>>>> 
>>>>>>> Personally I would like to see a new Git branch with CDI 2.0 and
>>> the
>>>>>>> integration test results on Jenkins.
>>>>>>> This would give us more confidence.
>>>>>>> Question: Does the CDI 2.0 have any NEW mandatory descriptive
>>> methods
>>>>>>> without default value already introduced in OLD annotations CDI
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>>>> 1.0/1.1?
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> It is more a change in the hierarchy. It doesn't break the user API
>>> since
>>>>>> cdi is designed to be provided but it is broken if new code uses
>>> old API.
>>>>>> 
>>>>>> Side note: if the idea behind this answer is to ensure the default
>>>>> provided
>>>>>> API is the last one then it doesn't work cause an API has a few
>>> logic
>>>>> 
>>>>> which
>>>>>> can require to be overriden (like the SPI and defaults handling).
>>>>>> Maven uses its own API and exposing CDI is a leaking abuse IMHO.
>>>>>> 
>>>>>> Note that this is an old bug which should be fixed now IMO before
>>> maven
>>>>>> considers CDI being exposed as part of the contract.
>>>>>> 
>>>>>> For reference, older threads:
>>>>>> 
>>>>>> http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
>>>>> td5828015.html
>>>>>> 
>>> http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
>>>>>> 
>>>>>> 
>>>>>> There is no risk removing it, worse case plugins would add the API
>>> as
>>>>>> compile instead of provided which should likely already be the case.
>>>>>> 
>>>>>> 
>>>>>>> On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> 
>>>>>>>> For the reproducer here it is https://github.com/
>>>>>>>> rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
>>>>>>>> 
>>>>>>>> 2018-02-06 8:05 GMT+01:00 Tibor Digana <tibordigana@apache.org
>>> (mailto:tibordigana@apache.org)
>>>>> (mailto:tibordigana@apache.org)>:
>>>>>>>> 
>>>>>>>>> Changing the package would not be possible in 3.x.
>>>>>>>> 
>>>>>>>> Why? In particular since it is an old regression already
>>> reported on
>>>>> the
>>>>>>>> list due to guice introduction it shouldn't be delayed for this
>>> kind
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> of
>>>>>>>> reason IMHO.
>>>>>>>> Was less visible until CDI 2 was released cause the API
>>> difference
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> was
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> not
>>>>>>>> triggered but now there are new entries it breaks immediately.
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> Guessing the version 4.0.0.
>>>>>>>>> WDYT?
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Would stay a blocker until 4 is out which is that soon so not
>>> sure
>>>>> it is
>>>>>>>> an option.
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
>>>>> tibordigana@apache.org (mailto:tibordigana@apache.org)>
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> The question is maybe about what is realistic for Maven
>>> devs.
>>>>>>>>>> Shading the CPI package (to something like
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> org.apache.maven.cdi.*)
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> would
>>>>>>>>>> be maybe the case instead of removing the original CDI and
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> reinventing
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> the
>>>>>>>>>> wheel.
>>>>>>>>>> 
>>>>>>>>>> On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
>>>>> herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
>>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>>> and does the MNG issue contain a reproducible test case
>>> for us
>>>>> to
>>>>>>>>>>> investigate
>>>>>>>>>>> more precisely?
>>>>>>>>>>> 
>>>>>>>>>>> Regards,
>>>>>>>>>>> 
>>>>>>>>>>> Hervé
>>>>>>>>>>> 
>>>>>>>>>>> Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a
>>> écrit :
>>>>>>>>>>>> Is there a MNG[1] issue?
>>>>>>>>>>>> 
>>>>>>>>>>>> Robert
>>>>>>>>>>>> 
>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/MNG
>>>>>>>>>>>> 
>>>>>>>>>>>> On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>>>>>>>>>>>> 
>>>>>>>>>>>> <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>>> wrote:
>>>>>>>>>>>>> Up?
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
>>>>>>> rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>>>>>>>>>>> a
>>>>>>>>>>>>> 
>>>>>>>>>>>>> écrit :
>>>>>>>>>>>>>> Hi guys,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> cdi-api is still in maven lib and breaks any plugin
>>>>> using it
>>>>>>> since
>>>>>>>>>>> it is
>>>>>>>>>>>>>> an old version, can it be dropped or at least
>>> isolated
>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> from
>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> plugin
>>>>>>>>>>>>>> classloaders?
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Romain Manni-Bucau
>>>>>>>>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> |
>>> Blog
>>>>>>>>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>>>>>>>>> <http://rmannibucau.wordpress.com> | Github
>>>>>>>>>>>>>> <https://github.com/rmannibucau> | LinkedIn
>>>>>>>>>>>>>> <https://www.linkedin.com/in/rmannibucau>
>>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>> ------------------------------------------------------------
>>>>>>> ---------
>>>>>>>>>>>> To unsubscribe, e-mail:
>>> dev-unsubscribe@maven.apache.org (mailto:dev-unsubscribe@maven.apache.org
>>> )
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> (mailto:dev-unsubscribe@maven.apache.org)
>>>>>>>>>>>> For additional commands, e-mail:
>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> (mailto:dev-help@maven.apache.org)
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>> ------------------------------------------------------------
>>>>> ---------
>>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> (mailto:dev-unsubscribe@maven.apache.org)
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> (mailto:dev-unsubscribe@maven.apache.org)
>>>>>>>>>>> For additional commands, e-mail:
>>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> (mailto:dev-help@maven.apache.org)
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>>> 
>>> 
>> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
Up,

Encountered a few bugs related to this regression, wonder how we want to
tackle it.
My 2cts would be to drop cdi-api and replace the single used
annotation from there by a maven one.
If we don't want to break plugins (not sure any use that) we can rewrite it
with asm or equivalent at load time since we own the classloading.

Anyone having an opinion on that?

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>


Le mar. 6 févr. 2018 à 18:02, Romain Manni-Bucau <rm...@gmail.com> a
écrit :

> commented inline
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
> 2018-02-06 17:51 GMT+01:00 Stuart McCulloch <mc...@gmail.com>:
>
>> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
>> > 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mcculls@gmail.com (mailto:
>> mcculls@gmail.com)>:
>> >
>> > > The "javax.enterprise.inject" CDI package was explicitly exported as
>> part
>> > > of the ongoing effort to migrate legacy Plexus components onto more
>> modern
>> > > standard annotations.
>> > >
>> >
>> >
>> > Hmm, this looks wrong from my window cause Maven doesn't support CDI
>> API -
>> > guice doesn't. So it is an interpretation of a well defined API which
>> is by
>> > defintion a bad public API, no?
>> >
>> >
>>
>> Raw Guice supports JSR330, but requires programmatic configuration (ie.
>> bindings defined in modules)
>>
>> Sisu builds on Guice to add support for things like annotation scanning
>> and wiring, injectable collections that dynamically update as plugins come
>> and go, property placeholders, etc.
>>
>> If the CDI annotations are on the classpath then it also honours the
>> @Typed annotation. This was a feature request to help migrate certain
>> components from other Plexus-based applications over to JSR330 + @Typed. At
>> the time there was a consideration that the rest of the CDI annotations
>> would eventually be supported, as another compatibility layer.
>>
>> Sisu also provides a Plexus compatibility layer that supports Plexus
>> annotations and XML
>>
>> Maven 3.x switched to Sisu so old Plexus-based components can still be
>> used, while modern components can be written with JSR330. At the time of
>> the switch it was decided to enable support for @Typed in Maven
>> plugins/extensions (because there was a developer need for this feature,
>> but that may well have changed and no longer be relevant).
>>
>> So Maven currently honours using @Typed on components - if it’s decided
>> that Maven doesn’t want to support @Typed in plugins then just remove the
>> export and exclude the cdi-api jar. As mentioned previously support for
>> @Typed is used by other downstream non-Maven applications so it will always
>> be something the container supports, but it's totally optional so if you
>> don’t want it then you don’t need to ship the cdi-api jar.
>>
>
> Yes but having the full API for one class is luxury (see later comment for
> the detail)
>
>
>> >
>> > >
>> > > Specifically, the @javax.enterprise.inject.Typed annotation lets
>> > > components state they are only visible for injection under a specific
>> type,
>> > > rather than any type in their hierarchy.
>> > >
>> > > There’s no annotation to control binding visibility in JSR330,
>> because it
>> > > deliberately avoids configuration concerns, which is why we went with
>> the
>> > > closest standard annotation (@Typed from JSR299 aka CDI). While we
>> could
>> > > have decided to use our own annotation - and the container does in
>> fact
>> > > support using @org.eclipse.sisu.Typed - this is not standardised or
>> > > portable. Also note the container will continue to support this
>> (optional)
>> > > feature for other downstream users, regardless of what’s decided here
>> - the
>> > > question is whether Maven still wants to use this feature and whether
>> it
>> > > wants to use the standard annotation or not.
>> > >
>> > > Another point is that whichever annotation is chosen must be
>> > > visible/defined from the same classloader to both core and plugins.
>> If the
>> > > annotation is not exported then core and each plugin will end up with
>> a
>> > > different @Typed class, defined by different classloaders. Any use of
>> > > @Typed in plugins would then effectively be invisible to the
>> container,
>> > > because the JVM’s AnnotatedElement API (getAnnotation,
>> isAnnotationPresent,
>> > > etc.) work off classes and not name equivalence.
>> > >
>> > > Similarly shading won’t work because neither the plugin’s components
>> nor
>> > > the container would know about the shaded package.
>> > >
>> >
>> >
>> > Hmm, not sure. I mean it works in most projects and it is easy to expose
>> > the shaded API so not a big deal *technically*. Agree it would be a bad
>> > solution to use a misused API publicly.
>> >
>> >
>>
>> By “not work” I really meant “not practical”. It’s not enough to just
>> shade the CDI jar, you’d also need to shade the container - being careful
>> that its reflective calls were properly updated (since it uses reflection
>> to decide whether to load the feature or not). TBH all that work is
>> overkill, since the container already supports an alternative annotation:
>> @org.eclipse.sisu.Typed
>>
>
> works for me
>
>
>> >
>> >
>> > >
>> > > As you can see from the thread in http://maven.40175.n5.nabble.
>> > > com/Linkage-error-td5784411.html a number of alternative solutions
>> have
>> > > been discussed before, including narrowing the export to:
>> > >
>> > > "javax.enterprise.inject.Typed"
>> > >
>> > > as that’s the only annotation we’re currently interested in. Since
>> @Typed
>> > > hasn’t changed between 1.x and 2.x that should be a workable solution,
>> > > assuming you wanted to keep using the standard annotation.
>> > >
>> > > Removing the export (and thereby removing the feature to limit
>> injection
>> > > visibility to a specific type) was also discussed, and at the time
>> Igor
>> > > asked for it to be kept:
>> > >
>> > > “Please keep @Typed annotation available outside of core.
>> > >
>> > > I use @Typed annotation in one of my (private) core extensions where I
>> > > need a class to implement an interface but not make that interface
>> > > visible for injection in other components.”
>> > >
>> >
>> >
>> > Issue is I can say the opposite "I use this in my plugin cause I use
>> CDI to
>> > impl my plugin, please ignore it for all Maven usage". Both are valid
>> and
>> > therefore the Maven API shouldn't have any overlapping.
>> >
>> >
>>
>> Whenever you embed a container inside any kind of plugin you’re at the
>> mercy of what’s exposed to that plugin - whether that plugin is running in
>> Maven, Jenkins, or an IDE. If you want full control/sanity then use a
>> custom classloader to isolate the embedded container from the plugin’s
>> environment, and just let through those packages you expect to be provided.
>>
>> For example, say we did fully support CDI 1.x components inside plugins
>> (as in the entire API was supported). You’d still have an issue embedding a
>> CDI 2.x container, because of the API clash, unless you used a custom
>> classloader between the plugin and the embedded container.
>>
>
> Yes but would have complained way earlier ;)
>
>
>> >
>> >
>> > >
>> > > Assuming Igor still needs this feature then the only other option
>> would be
>> > > to ask him if he can move to the non-standard
>> @org.eclipse.sisu.Typed. The
>> > > existing CDI export could then be replaced by exporting
>> “org.eclipse.sisu”.
>> > > Once that was done then the cdi-api dependency could be excluded from
>> the
>> > > distribution, as the container will still work without it on the
>> classpath
>> > > (it’s only required if you want to use the standard CDI annotation).
>> > >
>> > > So to summarise, the options are:
>> > >
>> > > a) Continue to support the standard API, but narrow the entry in
>> > > META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
>> > >
>> > > b) Switch to support @org.eclipse.sisu.Typed
>> > >
>> > > c) Remove this feature completely from Maven
>> >
>> > From what I'm concerned b and c would solve it but I guess sisu users
>> can
>> > have the same issue - not sure how likely it is.
>> >
>> >
>>
>> Sisu users typically have control over the container classpath and can
>> choose whether to include CDI or not (and at which level)
>> > There is a d) option: add in @Mojo a list of imported API. ClassRealm
>> can
>> > support filtering from the parent classloader and therefore I could use:
>> >
>> > @Mojo(name ="...", pluginPackages={"javax", ...})
>> >
>> > This would allow to keep current setup and let mojo to override it.
>> > Compared to a) it is defined in plugin.xml and not extension.xml.
>> >
>> >
>>
>> At the moment there’s a single Maven API realm, which imports all the
>> packages listed in the core extension.xml from the core classloader.
>> Plugins then import that realm wholesale, so they automatically get all
>> exported packages. However, it should be possible to be more selective,
>> whether that’s using a whitelisting or blacklisting approach.
>>
>> That said, it would be much simpler to either remove the export or switch
>> to @org.eclipse.inject.Typed (since use of the annotation in Maven is
>> currently very limited)
>>
>
> A last alternative is to still support @Typed without providing it.
> Concretely it means maven drops cdi (sadly not inject jar) and use asm to
> check if @Typed if here. Sounds the less breaking compromise even if not
> the most sexy in terms of impl.
>
>
>> >
>> > >
>> > > --
>> > > Cheers, Stuart
>> > >
>> > >
>> > > On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
>> > >
>> > > > 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org
>> (mailto:tibordigana@apache.org) (mailto:
>> > > tibordigana@apache.org (mailto:tibordigana@apache.org))>:
>> > > >
>> > > > > Personally I would like to see a new Git branch with CDI 2.0 and
>> the
>> > > > > integration test results on Jenkins.
>> > > > > This would give us more confidence.
>> > > > > Question: Does the CDI 2.0 have any NEW mandatory descriptive
>> methods
>> > > > > without default value already introduced in OLD annotations CDI
>> > > > >
>> > > >
>> > > >
>> > >
>> > > 1.0/1.1?
>> > > > >
>> > > >
>> > > >
>> > > >
>> > > > It is more a change in the hierarchy. It doesn't break the user API
>> since
>> > > > cdi is designed to be provided but it is broken if new code uses
>> old API.
>> > > >
>> > > > Side note: if the idea behind this answer is to ensure the default
>> > > provided
>> > > > API is the last one then it doesn't work cause an API has a few
>> logic
>> > >
>> > > which
>> > > > can require to be overriden (like the SPI and defaults handling).
>> > > > Maven uses its own API and exposing CDI is a leaking abuse IMHO.
>> > > >
>> > > > Note that this is an old bug which should be fixed now IMO before
>> maven
>> > > > considers CDI being exposed as part of the contract.
>> > > >
>> > > > For reference, older threads:
>> > > >
>> > > > http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
>> > > td5828015.html
>> > > >
>> http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
>> > > >
>> > > >
>> > > > There is no risk removing it, worse case plugins would add the API
>> as
>> > > > compile instead of provided which should likely already be the case.
>> > > >
>> > > >
>> > > > > On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
>> > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>> > > > > wrote:
>> > > > >
>> > > > > >
>> > > > > > For the reproducer here it is https://github.com/
>> > > > > > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
>> > > > > >
>> > > > > > 2018-02-06 8:05 GMT+01:00 Tibor Digana <tibordigana@apache.org
>> (mailto:tibordigana@apache.org)
>> > > (mailto:tibordigana@apache.org)>:
>> > > > > >
>> > > > > > > Changing the package would not be possible in 3.x.
>> > > > > >
>> > > > > > Why? In particular since it is an old regression already
>> reported on
>> > > the
>> > > > > > list due to guice introduction it shouldn't be delayed for this
>> kind
>> > > > >
>> > > >
>> > >
>> > > of
>> > > > > > reason IMHO.
>> > > > > > Was less visible until CDI 2 was released cause the API
>> difference
>> > > > > >
>> > > > >
>> > > >
>> > >
>> > > was
>> > > > > >
>> > > > >
>> > > > >
>> > > > > not
>> > > > > > triggered but now there are new entries it breaks immediately.
>> > > > > >
>> > > > > >
>> > > > > > > Guessing the version 4.0.0.
>> > > > > > > WDYT?
>> > > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > Would stay a blocker until 4 is out which is that soon so not
>> sure
>> > > it is
>> > > > > > an option.
>> > > > > >
>> > > > > >
>> > > > > > >
>> > > > > > > On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
>> > > tibordigana@apache.org (mailto:tibordigana@apache.org)>
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > The question is maybe about what is realistic for Maven
>> devs.
>> > > > > > > > Shading the CPI package (to something like
>> > > > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> > > org.apache.maven.cdi.*)
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > >
>> > > > > would
>> > > > > > > > be maybe the case instead of removing the original CDI and
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > >
>> > > >
>> > >
>> > > reinventing
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > >
>> > > > > the
>> > > > > > > > wheel.
>> > > > > > > >
>> > > > > > > > On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
>> > > herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
>> > > > > > > > wrote:
>> > > > > > > >
>> > > > > > > > > and does the MNG issue contain a reproducible test case
>> for us
>> > > to
>> > > > > > > > > investigate
>> > > > > > > > > more precisely?
>> > > > > > > > >
>> > > > > > > > > Regards,
>> > > > > > > > >
>> > > > > > > > > Hervé
>> > > > > > > > >
>> > > > > > > > > Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a
>> écrit :
>> > > > > > > > > > Is there a MNG[1] issue?
>> > > > > > > > > >
>> > > > > > > > > > Robert
>> > > > > > > > > >
>> > > > > > > > > > [1] https://issues.apache.org/jira/browse/MNG
>> > > > > > > > > >
>> > > > > > > > > > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>> > > > > > > > > >
>> > > > > > > > > > <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>> > > wrote:
>> > > > > > > > > > > Up?
>> > > > > > > > > > >
>> > > > > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
>> > > > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
>> > > > > > > > > a
>> > > > > > > > > > >
>> > > > > > > > > > > écrit :
>> > > > > > > > > > > > Hi guys,
>> > > > > > > > > > > >
>> > > > > > > > > > > > cdi-api is still in maven lib and breaks any plugin
>> > > using it
>> > > > > since
>> > > > > > > > > it is
>> > > > > > > > > > > > an old version, can it be dropped or at least
>> isolated
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > >
>> > > >
>> > >
>> > > from
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > >
>> > > > > plugin
>> > > > > > > > > > > > classloaders?
>> > > > > > > > > > > >
>> > > > > > > > > > > > Thanks,
>> > > > > > > > > > > > Romain Manni-Bucau
>> > > > > > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> |
>> Blog
>> > > > > > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
>> > > > > > > > > > > > <http://rmannibucau.wordpress.com> | Github
>> > > > > > > > > > > > <https://github.com/rmannibucau> | LinkedIn
>> > > > > > > > > > > > <https://www.linkedin.com/in/rmannibucau>
>> > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> ------------------------------------------------------------
>> > > > > ---------
>> > > > > > > > > > To unsubscribe, e-mail:
>> dev-unsubscribe@maven.apache.org (mailto:dev-unsubscribe@maven.apache.org
>> )
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > >
>> > > >
>> > >
>> > > (mailto:dev-unsubscribe@maven.apache.org)
>> > > > > > > > > > For additional commands, e-mail:
>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> > > (mailto:dev-help@maven.apache.org)
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> ------------------------------------------------------------
>> > > ---------
>> > > > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> (mailto:dev-unsubscribe@maven.apache.org)
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> > > (mailto:dev-unsubscribe@maven.apache.org)
>> > > > > > > > > For additional commands, e-mail:
>> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> > > (mailto:dev-help@maven.apache.org)
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> > >
>> >
>> >
>> >
>>
>>
>>
>

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
commented inline


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>

2018-02-06 17:51 GMT+01:00 Stuart McCulloch <mc...@gmail.com>:

> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
> > 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mcculls@gmail.com (mailto:
> mcculls@gmail.com)>:
> >
> > > The "javax.enterprise.inject" CDI package was explicitly exported as
> part
> > > of the ongoing effort to migrate legacy Plexus components onto more
> modern
> > > standard annotations.
> > >
> >
> >
> > Hmm, this looks wrong from my window cause Maven doesn't support CDI API
> -
> > guice doesn't. So it is an interpretation of a well defined API which is
> by
> > defintion a bad public API, no?
> >
> >
>
> Raw Guice supports JSR330, but requires programmatic configuration (ie.
> bindings defined in modules)
>
> Sisu builds on Guice to add support for things like annotation scanning
> and wiring, injectable collections that dynamically update as plugins come
> and go, property placeholders, etc.
>
> If the CDI annotations are on the classpath then it also honours the
> @Typed annotation. This was a feature request to help migrate certain
> components from other Plexus-based applications over to JSR330 + @Typed. At
> the time there was a consideration that the rest of the CDI annotations
> would eventually be supported, as another compatibility layer.
>
> Sisu also provides a Plexus compatibility layer that supports Plexus
> annotations and XML
>
> Maven 3.x switched to Sisu so old Plexus-based components can still be
> used, while modern components can be written with JSR330. At the time of
> the switch it was decided to enable support for @Typed in Maven
> plugins/extensions (because there was a developer need for this feature,
> but that may well have changed and no longer be relevant).
>
> So Maven currently honours using @Typed on components - if it’s decided
> that Maven doesn’t want to support @Typed in plugins then just remove the
> export and exclude the cdi-api jar. As mentioned previously support for
> @Typed is used by other downstream non-Maven applications so it will always
> be something the container supports, but it's totally optional so if you
> don’t want it then you don’t need to ship the cdi-api jar.
>

Yes but having the full API for one class is luxury (see later comment for
the detail)


> >
> > >
> > > Specifically, the @javax.enterprise.inject.Typed annotation lets
> > > components state they are only visible for injection under a specific
> type,
> > > rather than any type in their hierarchy.
> > >
> > > There’s no annotation to control binding visibility in JSR330, because
> it
> > > deliberately avoids configuration concerns, which is why we went with
> the
> > > closest standard annotation (@Typed from JSR299 aka CDI). While we
> could
> > > have decided to use our own annotation - and the container does in fact
> > > support using @org.eclipse.sisu.Typed - this is not standardised or
> > > portable. Also note the container will continue to support this
> (optional)
> > > feature for other downstream users, regardless of what’s decided here
> - the
> > > question is whether Maven still wants to use this feature and whether
> it
> > > wants to use the standard annotation or not.
> > >
> > > Another point is that whichever annotation is chosen must be
> > > visible/defined from the same classloader to both core and plugins. If
> the
> > > annotation is not exported then core and each plugin will end up with a
> > > different @Typed class, defined by different classloaders. Any use of
> > > @Typed in plugins would then effectively be invisible to the container,
> > > because the JVM’s AnnotatedElement API (getAnnotation,
> isAnnotationPresent,
> > > etc.) work off classes and not name equivalence.
> > >
> > > Similarly shading won’t work because neither the plugin’s components
> nor
> > > the container would know about the shaded package.
> > >
> >
> >
> > Hmm, not sure. I mean it works in most projects and it is easy to expose
> > the shaded API so not a big deal *technically*. Agree it would be a bad
> > solution to use a misused API publicly.
> >
> >
>
> By “not work” I really meant “not practical”. It’s not enough to just
> shade the CDI jar, you’d also need to shade the container - being careful
> that its reflective calls were properly updated (since it uses reflection
> to decide whether to load the feature or not). TBH all that work is
> overkill, since the container already supports an alternative annotation:
> @org.eclipse.sisu.Typed
>

works for me


> >
> >
> > >
> > > As you can see from the thread in http://maven.40175.n5.nabble.
> > > com/Linkage-error-td5784411.html a number of alternative solutions
> have
> > > been discussed before, including narrowing the export to:
> > >
> > > "javax.enterprise.inject.Typed"
> > >
> > > as that’s the only annotation we’re currently interested in. Since
> @Typed
> > > hasn’t changed between 1.x and 2.x that should be a workable solution,
> > > assuming you wanted to keep using the standard annotation.
> > >
> > > Removing the export (and thereby removing the feature to limit
> injection
> > > visibility to a specific type) was also discussed, and at the time Igor
> > > asked for it to be kept:
> > >
> > > “Please keep @Typed annotation available outside of core.
> > >
> > > I use @Typed annotation in one of my (private) core extensions where I
> > > need a class to implement an interface but not make that interface
> > > visible for injection in other components.”
> > >
> >
> >
> > Issue is I can say the opposite "I use this in my plugin cause I use CDI
> to
> > impl my plugin, please ignore it for all Maven usage". Both are valid and
> > therefore the Maven API shouldn't have any overlapping.
> >
> >
>
> Whenever you embed a container inside any kind of plugin you’re at the
> mercy of what’s exposed to that plugin - whether that plugin is running in
> Maven, Jenkins, or an IDE. If you want full control/sanity then use a
> custom classloader to isolate the embedded container from the plugin’s
> environment, and just let through those packages you expect to be provided.
>
> For example, say we did fully support CDI 1.x components inside plugins
> (as in the entire API was supported). You’d still have an issue embedding a
> CDI 2.x container, because of the API clash, unless you used a custom
> classloader between the plugin and the embedded container.
>

Yes but would have complained way earlier ;)


> >
> >
> > >
> > > Assuming Igor still needs this feature then the only other option
> would be
> > > to ask him if he can move to the non-standard @org.eclipse.sisu.Typed.
> The
> > > existing CDI export could then be replaced by exporting
> “org.eclipse.sisu”.
> > > Once that was done then the cdi-api dependency could be excluded from
> the
> > > distribution, as the container will still work without it on the
> classpath
> > > (it’s only required if you want to use the standard CDI annotation).
> > >
> > > So to summarise, the options are:
> > >
> > > a) Continue to support the standard API, but narrow the entry in
> > > META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
> > >
> > > b) Switch to support @org.eclipse.sisu.Typed
> > >
> > > c) Remove this feature completely from Maven
> >
> > From what I'm concerned b and c would solve it but I guess sisu users can
> > have the same issue - not sure how likely it is.
> >
> >
>
> Sisu users typically have control over the container classpath and can
> choose whether to include CDI or not (and at which level)
> > There is a d) option: add in @Mojo a list of imported API. ClassRealm can
> > support filtering from the parent classloader and therefore I could use:
> >
> > @Mojo(name ="...", pluginPackages={"javax", ...})
> >
> > This would allow to keep current setup and let mojo to override it.
> > Compared to a) it is defined in plugin.xml and not extension.xml.
> >
> >
>
> At the moment there’s a single Maven API realm, which imports all the
> packages listed in the core extension.xml from the core classloader.
> Plugins then import that realm wholesale, so they automatically get all
> exported packages. However, it should be possible to be more selective,
> whether that’s using a whitelisting or blacklisting approach.
>
> That said, it would be much simpler to either remove the export or switch
> to @org.eclipse.inject.Typed (since use of the annotation in Maven is
> currently very limited)
>

A last alternative is to still support @Typed without providing it.
Concretely it means maven drops cdi (sadly not inject jar) and use asm to
check if @Typed if here. Sounds the less breaking compromise even if not
the most sexy in terms of impl.


> >
> > >
> > > --
> > > Cheers, Stuart
> > >
> > >
> > > On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
> > >
> > > > 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org
> (mailto:tibordigana@apache.org) (mailto:
> > > tibordigana@apache.org (mailto:tibordigana@apache.org))>:
> > > >
> > > > > Personally I would like to see a new Git branch with CDI 2.0 and
> the
> > > > > integration test results on Jenkins.
> > > > > This would give us more confidence.
> > > > > Question: Does the CDI 2.0 have any NEW mandatory descriptive
> methods
> > > > > without default value already introduced in OLD annotations CDI
> > > > >
> > > >
> > > >
> > >
> > > 1.0/1.1?
> > > > >
> > > >
> > > >
> > > >
> > > > It is more a change in the hierarchy. It doesn't break the user API
> since
> > > > cdi is designed to be provided but it is broken if new code uses old
> API.
> > > >
> > > > Side note: if the idea behind this answer is to ensure the default
> > > provided
> > > > API is the last one then it doesn't work cause an API has a few logic
> > >
> > > which
> > > > can require to be overriden (like the SPI and defaults handling).
> > > > Maven uses its own API and exposing CDI is a leaking abuse IMHO.
> > > >
> > > > Note that this is an old bug which should be fixed now IMO before
> maven
> > > > considers CDI being exposed as part of the contract.
> > > >
> > > > For reference, older threads:
> > > >
> > > > http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
> > > td5828015.html
> > > > http://maven.40175.n5.nabble.com/Linkage-error-td5784411.
> html#a5784470
> > > >
> > > >
> > > > There is no risk removing it, worse case plugins would add the API as
> > > > compile instead of provided which should likely already be the case.
> > > >
> > > >
> > > > > On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
> > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > > > > wrote:
> > > > >
> > > > > >
> > > > > > For the reproducer here it is https://github.com/
> > > > > > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
> > > > > >
> > > > > > 2018-02-06 8:05 GMT+01:00 Tibor Digana <tibordigana@apache.org
> (mailto:tibordigana@apache.org)
> > > (mailto:tibordigana@apache.org)>:
> > > > > >
> > > > > > > Changing the package would not be possible in 3.x.
> > > > > >
> > > > > > Why? In particular since it is an old regression already
> reported on
> > > the
> > > > > > list due to guice introduction it shouldn't be delayed for this
> kind
> > > > >
> > > >
> > >
> > > of
> > > > > > reason IMHO.
> > > > > > Was less visible until CDI 2 was released cause the API
> difference
> > > > > >
> > > > >
> > > >
> > >
> > > was
> > > > > >
> > > > >
> > > > >
> > > > > not
> > > > > > triggered but now there are new entries it breaks immediately.
> > > > > >
> > > > > >
> > > > > > > Guessing the version 4.0.0.
> > > > > > > WDYT?
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > Would stay a blocker until 4 is out which is that soon so not
> sure
> > > it is
> > > > > > an option.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
> > > tibordigana@apache.org (mailto:tibordigana@apache.org)>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > The question is maybe about what is realistic for Maven devs.
> > > > > > > > Shading the CPI package (to something like
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > > org.apache.maven.cdi.*)
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > would
> > > > > > > > be maybe the case instead of removing the original CDI and
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > >
> > >
> > > reinventing
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > the
> > > > > > > > wheel.
> > > > > > > >
> > > > > > > > On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
> > > herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > and does the MNG issue contain a reproducible test case
> for us
> > > to
> > > > > > > > > investigate
> > > > > > > > > more precisely?
> > > > > > > > >
> > > > > > > > > Regards,
> > > > > > > > >
> > > > > > > > > Hervé
> > > > > > > > >
> > > > > > > > > Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a
> écrit :
> > > > > > > > > > Is there a MNG[1] issue?
> > > > > > > > > >
> > > > > > > > > > Robert
> > > > > > > > > >
> > > > > > > > > > [1] https://issues.apache.org/jira/browse/MNG
> > > > > > > > > >
> > > > > > > > > > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
> > > > > > > > > >
> > > > > > > > > > <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > > wrote:
> > > > > > > > > > > Up?
> > > > > > > > > > >
> > > > > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
> > > > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > > > > > > > > a
> > > > > > > > > > >
> > > > > > > > > > > écrit :
> > > > > > > > > > > > Hi guys,
> > > > > > > > > > > >
> > > > > > > > > > > > cdi-api is still in maven lib and breaks any plugin
> > > using it
> > > > > since
> > > > > > > > > it is
> > > > > > > > > > > > an old version, can it be dropped or at least
> isolated
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > >
> > >
> > > from
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > plugin
> > > > > > > > > > > > classloaders?
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > Romain Manni-Bucau
> > > > > > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> |
> Blog
> > > > > > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > > > > > > > > <http://rmannibucau.wordpress.com> | Github
> > > > > > > > > > > > <https://github.com/rmannibucau> | LinkedIn
> > > > > > > > > > > > <https://www.linkedin.com/in/rmannibucau>
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > ------------------------------
> ------------------------------
> > > > > ---------
> > > > > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > >
> > >
> > > (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > > > > For additional commands, e-mail:
> dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > > (mailto:dev-help@maven.apache.org)
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > ------------------------------
> ------------------------------
> > > ---------
> > > > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > > (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > > > For additional commands, e-mail: dev-help@maven.apache.org
> (mailto:dev-help@maven.apache.org)
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > > (mailto:dev-help@maven.apache.org)
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> >
> >
> >
>
>
>

Re: cdi-api leaking?

Posted by Stuart McCulloch <mc...@gmail.com>.
On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote:
> 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mcculls@gmail.com (mailto:mcculls@gmail.com)>:
>  
> > The "javax.enterprise.inject" CDI package was explicitly exported as part
> > of the ongoing effort to migrate legacy Plexus components onto more modern
> > standard annotations.
> >  
>  
>  
> Hmm, this looks wrong from my window cause Maven doesn't support CDI API -
> guice doesn't. So it is an interpretation of a well defined API which is by
> defintion a bad public API, no?
>  
>  

Raw Guice supports JSR330, but requires programmatic configuration (ie. bindings defined in modules)

Sisu builds on Guice to add support for things like annotation scanning and wiring, injectable collections that dynamically update as plugins come and go, property placeholders, etc.

If the CDI annotations are on the classpath then it also honours the @Typed annotation. This was a feature request to help migrate certain components from other Plexus-based applications over to JSR330 + @Typed. At the time there was a consideration that the rest of the CDI annotations would eventually be supported, as another compatibility layer.

Sisu also provides a Plexus compatibility layer that supports Plexus annotations and XML

Maven 3.x switched to Sisu so old Plexus-based components can still be used, while modern components can be written with JSR330. At the time of the switch it was decided to enable support for @Typed in Maven plugins/extensions (because there was a developer need for this feature, but that may well have changed and no longer be relevant).

So Maven currently honours using @Typed on components - if it’s decided that Maven doesn’t want to support @Typed in plugins then just remove the export and exclude the cdi-api jar. As mentioned previously support for @Typed is used by other downstream non-Maven applications so it will always be something the container supports, but it's totally optional so if you don’t want it then you don’t need to ship the cdi-api jar.
>  
> >  
> > Specifically, the @javax.enterprise.inject.Typed annotation lets
> > components state they are only visible for injection under a specific type,
> > rather than any type in their hierarchy.
> >  
> > There’s no annotation to control binding visibility in JSR330, because it
> > deliberately avoids configuration concerns, which is why we went with the
> > closest standard annotation (@Typed from JSR299 aka CDI). While we could
> > have decided to use our own annotation - and the container does in fact
> > support using @org.eclipse.sisu.Typed - this is not standardised or
> > portable. Also note the container will continue to support this (optional)
> > feature for other downstream users, regardless of what’s decided here - the
> > question is whether Maven still wants to use this feature and whether it
> > wants to use the standard annotation or not.
> >  
> > Another point is that whichever annotation is chosen must be
> > visible/defined from the same classloader to both core and plugins. If the
> > annotation is not exported then core and each plugin will end up with a
> > different @Typed class, defined by different classloaders. Any use of
> > @Typed in plugins would then effectively be invisible to the container,
> > because the JVM’s AnnotatedElement API (getAnnotation, isAnnotationPresent,
> > etc.) work off classes and not name equivalence.
> >  
> > Similarly shading won’t work because neither the plugin’s components nor
> > the container would know about the shaded package.
> >  
>  
>  
> Hmm, not sure. I mean it works in most projects and it is easy to expose
> the shaded API so not a big deal *technically*. Agree it would be a bad
> solution to use a misused API publicly.
>  
>  

By “not work” I really meant “not practical”. It’s not enough to just shade the CDI jar, you’d also need to shade the container - being careful that its reflective calls were properly updated (since it uses reflection to decide whether to load the feature or not). TBH all that work is overkill, since the container already supports an alternative annotation: @org.eclipse.sisu.Typed
>  
>  
> >  
> > As you can see from the thread in http://maven.40175.n5.nabble.
> > com/Linkage-error-td5784411.html a number of alternative solutions have
> > been discussed before, including narrowing the export to:
> >  
> > "javax.enterprise.inject.Typed"
> >  
> > as that’s the only annotation we’re currently interested in. Since @Typed
> > hasn’t changed between 1.x and 2.x that should be a workable solution,
> > assuming you wanted to keep using the standard annotation.
> >  
> > Removing the export (and thereby removing the feature to limit injection
> > visibility to a specific type) was also discussed, and at the time Igor
> > asked for it to be kept:
> >  
> > “Please keep @Typed annotation available outside of core.
> >  
> > I use @Typed annotation in one of my (private) core extensions where I
> > need a class to implement an interface but not make that interface
> > visible for injection in other components.”
> >  
>  
>  
> Issue is I can say the opposite "I use this in my plugin cause I use CDI to
> impl my plugin, please ignore it for all Maven usage". Both are valid and
> therefore the Maven API shouldn't have any overlapping.
>  
>  

Whenever you embed a container inside any kind of plugin you’re at the mercy of what’s exposed to that plugin - whether that plugin is running in Maven, Jenkins, or an IDE. If you want full control/sanity then use a custom classloader to isolate the embedded container from the plugin’s environment, and just let through those packages you expect to be provided.

For example, say we did fully support CDI 1.x components inside plugins (as in the entire API was supported). You’d still have an issue embedding a CDI 2.x container, because of the API clash, unless you used a custom classloader between the plugin and the embedded container.
>  
>  
> >  
> > Assuming Igor still needs this feature then the only other option would be
> > to ask him if he can move to the non-standard @org.eclipse.sisu.Typed. The
> > existing CDI export could then be replaced by exporting “org.eclipse.sisu”.
> > Once that was done then the cdi-api dependency could be excluded from the
> > distribution, as the container will still work without it on the classpath
> > (it’s only required if you want to use the standard CDI annotation).
> >  
> > So to summarise, the options are:
> >  
> > a) Continue to support the standard API, but narrow the entry in
> > META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
> >  
> > b) Switch to support @org.eclipse.sisu.Typed
> >  
> > c) Remove this feature completely from Maven
>  
> From what I'm concerned b and c would solve it but I guess sisu users can
> have the same issue - not sure how likely it is.
>  
>  

Sisu users typically have control over the container classpath and can choose whether to include CDI or not (and at which level)
> There is a d) option: add in @Mojo a list of imported API. ClassRealm can
> support filtering from the parent classloader and therefore I could use:
>  
> @Mojo(name ="...", pluginPackages={"javax", ...})
>  
> This would allow to keep current setup and let mojo to override it.
> Compared to a) it is defined in plugin.xml and not extension.xml.
>  
>  

At the moment there’s a single Maven API realm, which imports all the packages listed in the core extension.xml from the core classloader. Plugins then import that realm wholesale, so they automatically get all exported packages. However, it should be possible to be more selective, whether that’s using a whitelisting or blacklisting approach.

That said, it would be much simpler to either remove the export or switch to @org.eclipse.inject.Typed (since use of the annotation in Maven is currently very limited)
>  
> >  
> > --
> > Cheers, Stuart
> >  
> >  
> > On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
> >  
> > > 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org (mailto:tibordigana@apache.org) (mailto:
> > tibordigana@apache.org (mailto:tibordigana@apache.org))>:
> > >  
> > > > Personally I would like to see a new Git branch with CDI 2.0 and the
> > > > integration test results on Jenkins.
> > > > This would give us more confidence.
> > > > Question: Does the CDI 2.0 have any NEW mandatory descriptive methods
> > > > without default value already introduced in OLD annotations CDI
> > > >  
> > >  
> > >  
> >  
> > 1.0/1.1?
> > > >  
> > >  
> > >  
> > >  
> > > It is more a change in the hierarchy. It doesn't break the user API since
> > > cdi is designed to be provided but it is broken if new code uses old API.
> > >  
> > > Side note: if the idea behind this answer is to ensure the default
> > provided
> > > API is the last one then it doesn't work cause an API has a few logic
> >  
> > which
> > > can require to be overriden (like the SPI and defaults handling).
> > > Maven uses its own API and exposing CDI is a leaking abuse IMHO.
> > >  
> > > Note that this is an old bug which should be fixed now IMO before maven
> > > considers CDI being exposed as part of the contract.
> > >  
> > > For reference, older threads:
> > >  
> > > http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
> > td5828015.html
> > > http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
> > >  
> > >  
> > > There is no risk removing it, worse case plugins would add the API as
> > > compile instead of provided which should likely already be the case.
> > >  
> > >  
> > > > On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
> > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > > > wrote:
> > > >  
> > > > >  
> > > > > For the reproducer here it is https://github.com/
> > > > > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
> > > > >  
> > > > > 2018-02-06 8:05 GMT+01:00 Tibor Digana <tibordigana@apache.org (mailto:tibordigana@apache.org)
> > (mailto:tibordigana@apache.org)>:
> > > > >  
> > > > > > Changing the package would not be possible in 3.x.
> > > > >  
> > > > > Why? In particular since it is an old regression already reported on
> > the
> > > > > list due to guice introduction it shouldn't be delayed for this kind
> > > >  
> > >  
> >  
> > of
> > > > > reason IMHO.
> > > > > Was less visible until CDI 2 was released cause the API difference
> > > > >  
> > > >  
> > >  
> >  
> > was
> > > > >  
> > > >  
> > > >  
> > > > not
> > > > > triggered but now there are new entries it breaks immediately.
> > > > >  
> > > > >  
> > > > > > Guessing the version 4.0.0.
> > > > > > WDYT?
> > > > > >  
> > > > >  
> > > > >  
> > > > >  
> > > > > Would stay a blocker until 4 is out which is that soon so not sure
> > it is
> > > > > an option.
> > > > >  
> > > > >  
> > > > > >  
> > > > > > On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
> > tibordigana@apache.org (mailto:tibordigana@apache.org)>
> > > > > > wrote:
> > > > > >  
> > > > > > > The question is maybe about what is realistic for Maven devs.
> > > > > > > Shading the CPI package (to something like
> > > > > > >  
> > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > >  
> >  
> > org.apache.maven.cdi.*)
> > > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > > >  
> > > > would
> > > > > > > be maybe the case instead of removing the original CDI and
> > > > > >  
> > > > >  
> > > >  
> > > >  
> > >  
> >  
> > reinventing
> > > > > >  
> > > > >  
> > > >  
> > > >  
> > > > the
> > > > > > > wheel.
> > > > > > >  
> > > > > > > On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
> > herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
> > > > > > > wrote:
> > > > > > >  
> > > > > > > > and does the MNG issue contain a reproducible test case for us
> > to
> > > > > > > > investigate
> > > > > > > > more precisely?
> > > > > > > >  
> > > > > > > > Regards,
> > > > > > > >  
> > > > > > > > Hervé
> > > > > > > >  
> > > > > > > > Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
> > > > > > > > > Is there a MNG[1] issue?
> > > > > > > > >  
> > > > > > > > > Robert
> > > > > > > > >  
> > > > > > > > > [1] https://issues.apache.org/jira/browse/MNG
> > > > > > > > >  
> > > > > > > > > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
> > > > > > > > >  
> > > > > > > > > <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > wrote:
> > > > > > > > > > Up?
> > > > > > > > > >  
> > > > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
> > > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > > > > > > > a
> > > > > > > > > >  
> > > > > > > > > > écrit :
> > > > > > > > > > > Hi guys,
> > > > > > > > > > >  
> > > > > > > > > > > cdi-api is still in maven lib and breaks any plugin
> > using it
> > > > since
> > > > > > > > it is
> > > > > > > > > > > an old version, can it be dropped or at least isolated
> > > > > > > > > >  
> > > > > > > > >  
> > > > > > > >  
> > > > > > > >  
> > > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > > >  
> > >  
> >  
> > from
> > > > > > > > > >  
> > > > > > > > >  
> > > > > > > >  
> > > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > > >  
> > > > plugin
> > > > > > > > > > > classloaders?
> > > > > > > > > > >  
> > > > > > > > > > > Thanks,
> > > > > > > > > > > Romain Manni-Bucau
> > > > > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > > > > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > > > > > > > <http://rmannibucau.wordpress.com> | Github
> > > > > > > > > > > <https://github.com/rmannibucau> | LinkedIn
> > > > > > > > > > > <https://www.linkedin.com/in/rmannibucau>
> > > > > > > > > > >  
> > > > > > > > > >  
> > > > > > > > >  
> > > > > > > > >  
> > > > > > > > >  
> > > > > > > > > ------------------------------------------------------------
> > > > ---------
> > > > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > >  
> > > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > > >  
> > >  
> >  
> > (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > > > For additional commands, e-mail: dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
> > > > > > > >  
> > > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > >  
> >  
> > (mailto:dev-help@maven.apache.org)
> > > > > > > > >  
> > > > > > > >  
> > > > > > > >  
> > > > > > > >  
> > > > > > > >  
> > > > > > > >  
> > > > > > > > ------------------------------------------------------------
> > ---------
> > > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > >  
> >  
> > (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > > For additional commands, e-mail: dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
> > > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > >  
> >  
> > (mailto:dev-help@maven.apache.org)
> > > > > > > >  
> > > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > >  
> >  
> >  
>  
>  
>

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mc...@gmail.com>:

> The "javax.enterprise.inject" CDI package was explicitly exported as part
> of the ongoing effort to migrate legacy Plexus components onto more modern
> standard annotations.
>

Hmm, this looks wrong from my window cause Maven doesn't support CDI API -
guice doesn't. So it is an interpretation of a well defined API which is by
defintion a bad public API, no?


>
> Specifically, the @javax.enterprise.inject.Typed annotation lets
> components state they are only visible for injection under a specific type,
> rather than any type in their hierarchy.
>
> There’s no annotation to control binding visibility in JSR330, because it
> deliberately avoids configuration concerns, which is why we went with the
> closest standard annotation (@Typed from JSR299 aka CDI). While we could
> have decided to use our own annotation - and the container does in fact
> support using @org.eclipse.sisu.Typed - this is not standardised or
> portable. Also note the container will continue to support this (optional)
> feature for other downstream users, regardless of what’s decided here - the
> question is whether Maven still wants to use this feature and whether it
> wants to use the standard annotation or not.
>
> Another point is that whichever annotation is chosen must be
> visible/defined from the same classloader to both core and plugins. If the
> annotation is not exported then core and each plugin will end up with a
> different @Typed class, defined by different classloaders. Any use of
> @Typed in plugins would then effectively be invisible to the container,
> because the JVM’s AnnotatedElement API (getAnnotation, isAnnotationPresent,
> etc.) work off classes and not name equivalence.
>
> Similarly shading won’t work because neither the plugin’s components nor
> the container would know about the shaded package.
>

Hmm, not sure. I mean it works in most projects and it is easy to expose
the shaded API so not a big deal *technically*. Agree it would be a bad
solution to use a misused API publicly.


>
> As you can see from the thread in http://maven.40175.n5.nabble.
> com/Linkage-error-td5784411.html a number of alternative solutions have
> been discussed before, including narrowing the export to:
>
> "javax.enterprise.inject.Typed"
>
> as that’s the only annotation we’re currently interested in. Since @Typed
> hasn’t changed between 1.x and 2.x that should be a workable solution,
> assuming you wanted to keep using the standard annotation.
>
> Removing the export (and thereby removing the feature to limit injection
> visibility to a specific type) was also discussed, and at the time Igor
> asked for it to be kept:
>
> “Please keep @Typed annotation available outside of core.
>
> I use @Typed annotation in one of my (private) core extensions where I
> need a class to implement an interface but not make that interface
> visible for injection in other components.”
>

Issue is I can say the opposite "I use this in my plugin cause I use CDI to
impl my plugin, please ignore it for all Maven usage". Both are valid and
therefore the Maven API shouldn't have any overlapping.


>
> Assuming Igor still needs this feature then the only other option would be
> to ask him if he can move to the non-standard @org.eclipse.sisu.Typed. The
> existing CDI export could then be replaced by exporting “org.eclipse.sisu”.
> Once that was done then the cdi-api dependency could be excluded from the
> distribution, as the container will still work without it on the classpath
> (it’s only required if you want to use the standard CDI annotation).
>
> So to summarise, the options are:
>
> a)  Continue to support the standard API, but narrow the entry in
> META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”
>
> b)  Switch to support @org.eclipse.sisu.Typed
>
> c)  Remove this feature completely from Maven
>

From what I'm concerned b and c would solve it but I guess sisu users can
have the same issue - not sure how likely it is.
There is a d) option: add in @Mojo a list of imported API. ClassRealm can
support filtering from the parent classloader and therefore I could use:

@Mojo(name ="...", pluginPackages={"javax", ...})

This would allow to keep current setup and let mojo to override it.
Compared to a) it is defined in plugin.xml and not extension.xml.


>
> --
> Cheers, Stuart
>
>
> On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:
>
> > 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org (mailto:
> tibordigana@apache.org)>:
> >
> > > Personally I would like to see a new Git branch with CDI 2.0 and the
> > > integration test results on Jenkins.
> > > This would give us more confidence.
> > > Question: Does the CDI 2.0 have any NEW mandatory descriptive methods
> > > without default value already introduced in OLD annotations CDI
> 1.0/1.1?
> > >
> >
> >
> > It is more a change in the hierarchy. It doesn't break the user API since
> > cdi is designed to be provided but it is broken if new code uses old API.
> >
> > Side note: if the idea behind this answer is to ensure the default
> provided
> > API is the last one then it doesn't work cause an API has a few logic
> which
> > can require to be overriden (like the SPI and defaults handling).
> > Maven uses its own API and exposing CDI is a leaking abuse IMHO.
> >
> > Note that this is an old bug which should be fixed now IMO before maven
> > considers CDI being exposed as part of the contract.
> >
> > For reference, older threads:
> >
> > http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-
> td5828015.html
> > http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
> >
> >
> > There is no risk removing it, worse case plugins would add the API as
> > compile instead of provided which should likely already be the case.
> >
> >
> > > On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <
> rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > > wrote:
> > >
> > > >
> > > > For the reproducer here it is https://github.com/
> > > > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
> > > >
> > > > 2018-02-06 8:05 GMT+01:00 Tibor Digana <tibordigana@apache.org
> (mailto:tibordigana@apache.org)>:
> > > >
> > > > > Changing the package would not be possible in 3.x.
> > > >
> > > > Why? In particular since it is an old regression already reported on
> the
> > > > list due to guice introduction it shouldn't be delayed for this kind
> of
> > > > reason IMHO.
> > > > Was less visible until CDI 2 was released cause the API difference
> was
> > > >
> > >
> > > not
> > > > triggered but now there are new entries it breaks immediately.
> > > >
> > > >
> > > > > Guessing the version 4.0.0.
> > > > > WDYT?
> > > > >
> > > >
> > > >
> > > > Would stay a blocker until 4 is out which is that soon so not sure
> it is
> > > > an option.
> > > >
> > > >
> > > > >
> > > > > On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <
> tibordigana@apache.org (mailto:tibordigana@apache.org)>
> > > > > wrote:
> > > > >
> > > > > > The question is maybe about what is realistic for Maven devs.
> > > > > > Shading the CPI package (to something like
> org.apache.maven.cdi.*)
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> > > would
> > > > > > be maybe the case instead of removing the original CDI and
> reinventing
> > > > >
> > > >
> > >
> > > the
> > > > > > wheel.
> > > > > >
> > > > > > On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <
> herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
> > > > > > wrote:
> > > > > >
> > > > > > > and does the MNG issue contain a reproducible test case for us
> to
> > > > > > > investigate
> > > > > > > more precisely?
> > > > > > >
> > > > > > > Regards,
> > > > > > >
> > > > > > > Hervé
> > > > > > >
> > > > > > > Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
> > > > > > > > Is there a MNG[1] issue?
> > > > > > > >
> > > > > > > > Robert
> > > > > > > >
> > > > > > > > [1] https://issues.apache.org/jira/browse/MNG
> > > > > > > >
> > > > > > > > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
> > > > > > > >
> > > > > > > > <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> wrote:
> > > > > > > > > Up?
> > > > > > > > >
> > > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
> > > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > > > > > > a
> > > > > > > > >
> > > > > > > > > écrit :
> > > > > > > > > > Hi guys,
> > > > > > > > > >
> > > > > > > > > > cdi-api is still in maven lib and breaks any plugin
> using it
> > > since
> > > > > > > it is
> > > > > > > > > > an old version, can it be dropped or at least isolated
> from
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > > plugin
> > > > > > > > > > classloaders?
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > > Romain Manni-Bucau
> > > > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > > > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > > > > > > <http://rmannibucau.wordpress.com> | Github
> > > > > > > > > > <https://github.com/rmannibucau> | LinkedIn
> > > > > > > > > > <https://www.linkedin.com/in/rmannibucau>
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > ------------------------------------------------------------
> > > ---------
> > > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > > For additional commands, e-mail: dev-help@maven.apache.org
> (mailto:dev-help@maven.apache.org)
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > ------------------------------------------------------------
> ---------
> > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > For additional commands, e-mail: dev-help@maven.apache.org
> (mailto:dev-help@maven.apache.org)
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> >
> >
> >
>
>
>

Re: cdi-api leaking?

Posted by Stuart McCulloch <mc...@gmail.com>.
The "javax.enterprise.inject" CDI package was explicitly exported as part of the ongoing effort to migrate legacy Plexus components onto more modern standard annotations.

Specifically, the @javax.enterprise.inject.Typed annotation lets components state they are only visible for injection under a specific type, rather than any type in their hierarchy.

There’s no annotation to control binding visibility in JSR330, because it deliberately avoids configuration concerns, which is why we went with the closest standard annotation (@Typed from JSR299 aka CDI). While we could have decided to use our own annotation - and the container does in fact support using @org.eclipse.sisu.Typed - this is not standardised or portable. Also note the container will continue to support this (optional) feature for other downstream users, regardless of what’s decided here - the question is whether Maven still wants to use this feature and whether it wants to use the standard annotation or not.

Another point is that whichever annotation is chosen must be visible/defined from the same classloader to both core and plugins. If the annotation is not exported then core and each plugin will end up with a different @Typed class, defined by different classloaders. Any use of @Typed in plugins would then effectively be invisible to the container, because the JVM’s AnnotatedElement API (getAnnotation, isAnnotationPresent, etc.) work off classes and not name equivalence.

Similarly shading won’t work because neither the plugin’s components nor the container would know about the shaded package.

As you can see from the thread in http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html a number of alternative solutions have been discussed before, including narrowing the export to:

"javax.enterprise.inject.Typed"

as that’s the only annotation we’re currently interested in. Since @Typed hasn’t changed between 1.x and 2.x that should be a workable solution, assuming you wanted to keep using the standard annotation.

Removing the export (and thereby removing the feature to limit injection visibility to a specific type) was also discussed, and at the time Igor asked for it to be kept:

“Please keep @Typed annotation available outside of core.  

I use @Typed annotation in one of my (private) core extensions where I  
need a class to implement an interface but not make that interface  
visible for injection in other components.”

Assuming Igor still needs this feature then the only other option would be to ask him if he can move to the non-standard @org.eclipse.sisu.Typed. The existing CDI export could then be replaced by exporting “org.eclipse.sisu”. Once that was done then the cdi-api dependency could be excluded from the distribution, as the container will still work without it on the classpath (it’s only required if you want to use the standard CDI annotation).

So to summarise, the options are:

a)  Continue to support the standard API, but narrow the entry in META-INF/maven/extension.xml to “javax.enterprise.inject.Typed”

b)  Switch to support @org.eclipse.sisu.Typed

c)  Remove this feature completely from Maven

--  
Cheers, Stuart


On Tuesday, 6 February 2018 at 09:09, Romain Manni-Bucau wrote:

> 2018-02-06 9:41 GMT+01:00 Tibor Digana <tibordigana@apache.org (mailto:tibordigana@apache.org)>:
>  
> > Personally I would like to see a new Git branch with CDI 2.0 and the
> > integration test results on Jenkins.
> > This would give us more confidence.
> > Question: Does the CDI 2.0 have any NEW mandatory descriptive methods
> > without default value already introduced in OLD annotations CDI 1.0/1.1?
> >  
>  
>  
> It is more a change in the hierarchy. It doesn't break the user API since
> cdi is designed to be provided but it is broken if new code uses old API.
>  
> Side note: if the idea behind this answer is to ensure the default provided
> API is the last one then it doesn't work cause an API has a few logic which
> can require to be overriden (like the SPI and defaults handling).
> Maven uses its own API and exposing CDI is a leaking abuse IMHO.
>  
> Note that this is an old bug which should be fixed now IMO before maven
> considers CDI being exposed as part of the contract.
>  
> For reference, older threads:
>  
> http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-td5828015.html
> http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470
>  
>  
> There is no risk removing it, worse case plugins would add the API as
> compile instead of provided which should likely already be the case.
>  
>  
> > On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > wrote:
> >  
> > >  
> > > For the reproducer here it is https://github.com/
> > > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
> > >  
> > > 2018-02-06 8:05 GMT+01:00 Tibor Digana <tibordigana@apache.org (mailto:tibordigana@apache.org)>:
> > >  
> > > > Changing the package would not be possible in 3.x.
> > >  
> > > Why? In particular since it is an old regression already reported on the
> > > list due to guice introduction it shouldn't be delayed for this kind of
> > > reason IMHO.
> > > Was less visible until CDI 2 was released cause the API difference was
> > >  
> >  
> > not
> > > triggered but now there are new entries it breaks immediately.
> > >  
> > >  
> > > > Guessing the version 4.0.0.
> > > > WDYT?
> > > >  
> > >  
> > >  
> > > Would stay a blocker until 4 is out which is that soon so not sure it is
> > > an option.
> > >  
> > >  
> > > >  
> > > > On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <tibordigana@apache.org (mailto:tibordigana@apache.org)>
> > > > wrote:
> > > >  
> > > > > The question is maybe about what is realistic for Maven devs.
> > > > > Shading the CPI package (to something like org.apache.maven.cdi.*)
> > > > >  
> > > >  
> > > >  
> > >  
> > >  
> >  
> > would
> > > > > be maybe the case instead of removing the original CDI and reinventing
> > > >  
> > >  
> >  
> > the
> > > > > wheel.
> > > > >  
> > > > > On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <herve.boutemy@free.fr (mailto:herve.boutemy@free.fr)>
> > > > > wrote:
> > > > >  
> > > > > > and does the MNG issue contain a reproducible test case for us to
> > > > > > investigate
> > > > > > more precisely?
> > > > > >  
> > > > > > Regards,
> > > > > >  
> > > > > > Hervé
> > > > > >  
> > > > > > Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
> > > > > > > Is there a MNG[1] issue?
> > > > > > >  
> > > > > > > Robert
> > > > > > >  
> > > > > > > [1] https://issues.apache.org/jira/browse/MNG
> > > > > > >  
> > > > > > > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
> > > > > > >  
> > > > > > > <rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)> wrote:
> > > > > > > > Up?
> > > > > > > >  
> > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
> > rmannibucau@gmail.com (mailto:rmannibucau@gmail.com)>
> > > > > > a
> > > > > > > >  
> > > > > > > > écrit :
> > > > > > > > > Hi guys,
> > > > > > > > >  
> > > > > > > > > cdi-api is still in maven lib and breaks any plugin using it
> > since
> > > > > > it is
> > > > > > > > > an old version, can it be dropped or at least isolated from
> > > > > > > >  
> > > > > > >  
> > > > > >  
> > > > > >  
> > > > >  
> > > >  
> > >  
> >  
> > plugin
> > > > > > > > > classloaders?
> > > > > > > > >  
> > > > > > > > > Thanks,
> > > > > > > > > Romain Manni-Bucau
> > > > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > > > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > > > > > <http://rmannibucau.wordpress.com> | Github
> > > > > > > > > <https://github.com/rmannibucau> | LinkedIn
> > > > > > > > > <https://www.linkedin.com/in/rmannibucau>
> > > > > > > > >  
> > > > > > > >  
> > > > > > >  
> > > > > > >  
> > > > > > > ------------------------------------------------------------
> > ---------
> > > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > > For additional commands, e-mail: dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
> > > > > > >  
> > > > > >  
> > > > > >  
> > > > > >  
> > > > > >  
> > > > > > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org (mailto:dev-unsubscribe@maven.apache.org)
> > > > > > For additional commands, e-mail: dev-help@maven.apache.org (mailto:dev-help@maven.apache.org)
> > > > > >  
> > > > >  
> > > >  
> > >  
> >  
> >  
>  
>  
>

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
2018-02-06 9:41 GMT+01:00 Tibor Digana <ti...@apache.org>:

> Personally I would like to see a new Git branch with CDI 2.0 and the
> integration test results on Jenkins.
> This would give us more confidence.
> Question: Does the CDI 2.0 have any NEW mandatory descriptive methods
> without default value already introduced in OLD annotations CDI 1.0/1.1?
>

It is more a change in the hierarchy. It doesn't break the user API since
cdi is designed to be provided but it is broken if new code uses old API.

Side note: if the idea behind this answer is to ensure the default provided
API is the last one then it doesn't work cause an API has a few logic which
can require to be overriden (like the SPI and defaults handling).
Maven uses its own API and exposing CDI is a leaking abuse IMHO.

Note that this is an old bug which should be fixed now IMO before maven
considers CDI being exposed as part of the contract.

For reference, older threads:

http://maven.40175.n5.nabble.com/libs-in-mavens-lib-folder-td5828015.html
http://maven.40175.n5.nabble.com/Linkage-error-td5784411.html#a5784470


There is no risk removing it, worse case plugins would add the API as
compile instead of provided which should likely already be the case.


> On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
> >
> > For the reproducer here it is https://github.com/
> > rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
> >
> > 2018-02-06 8:05 GMT+01:00 Tibor Digana <ti...@apache.org>:
> >
> >> Changing the package would not be possible in 3.x.
> >>
> >
> > Why? In particular since it is an old regression already reported on the
> > list due to guice introduction it shouldn't be delayed for this kind of
> > reason IMHO.
> > Was less visible until CDI 2 was released cause the API difference was
> not
> > triggered but now there are new entries it breaks immediately.
> >
> >
> >> Guessing the version 4.0.0.
> >> WDYT?
> >>
> >
> > Would stay a blocker until 4 is out which is that soon so not sure it is
> > an option.
> >
> >
> >>
> >> On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <ti...@apache.org>
> >> wrote:
> >>
> >>> The question is maybe about what is realistic for Maven devs.
> >>> Shading the CPI package (to something like org.apache.maven.cdi.*)
> would
> >>> be maybe the case instead of removing the original CDI and reinventing
> the
> >>> wheel.
> >>>
> >>> On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <he...@free.fr>
> >>> wrote:
> >>>
> >>>> and does the MNG issue contain a reproducible test case for us to
> >>>> investigate
> >>>> more precisely?
> >>>>
> >>>> Regards,
> >>>>
> >>>> Hervé
> >>>>
> >>>> Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
> >>>> > Is there a MNG[1] issue?
> >>>> >
> >>>> > Robert
> >>>> >
> >>>> > [1] https://issues.apache.org/jira/browse/MNG
> >>>> >
> >>>> > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
> >>>> >
> >>>> > <rm...@gmail.com> wrote:
> >>>> > > Up?
> >>>> > >
> >>>> > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <
> rmannibucau@gmail.com>
> >>>> a
> >>>> > >
> >>>> > > écrit :
> >>>> > >> Hi guys,
> >>>> > >>
> >>>> > >> cdi-api is still in maven lib and breaks any plugin using it
> since
> >>>> it is
> >>>> > >> an old version, can it be dropped or at least isolated from
> plugin
> >>>> > >> classloaders?
> >>>> > >>
> >>>> > >> Thanks,
> >>>> > >> Romain Manni-Bucau
> >>>> > >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >>>> > >> <https://rmannibucau.metawerx.net/> | Old Blog
> >>>> > >> <http://rmannibucau.wordpress.com> | Github
> >>>> > >> <https://github.com/rmannibucau> | LinkedIn
> >>>> > >> <https://www.linkedin.com/in/rmannibucau>
> >>>> >
> >>>> > ------------------------------------------------------------
> ---------
> >>>> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>>> > For additional commands, e-mail: dev-help@maven.apache.org
> >>>>
> >>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>>> For additional commands, e-mail: dev-help@maven.apache.org
> >>>>
> >>>>
> >>>
> >>
> >
>

Re: cdi-api leaking?

Posted by Tibor Digana <ti...@apache.org>.
Personally I would like to see a new Git branch with CDI 2.0 and the
integration test results on Jenkins.
This would give us more confidence.
Question: Does the CDI 2.0 have any NEW mandatory descriptive methods
without default value already introduced in OLD annotations CDI 1.0/1.1?

On Tue, Feb 6, 2018 at 8:57 AM, Romain Manni-Bucau <rm...@gmail.com>
wrote:

>
> For the reproducer here it is https://github.com/
> rmannibucau/test-maven-plugin - pretty trivial you'll see ;).
>
> 2018-02-06 8:05 GMT+01:00 Tibor Digana <ti...@apache.org>:
>
>> Changing the package would not be possible in 3.x.
>>
>
> Why? In particular since it is an old regression already reported on the
> list due to guice introduction it shouldn't be delayed for this kind of
> reason IMHO.
> Was less visible until CDI 2 was released cause the API difference was not
> triggered but now there are new entries it breaks immediately.
>
>
>> Guessing the version 4.0.0.
>> WDYT?
>>
>
> Would stay a blocker until 4 is out which is that soon so not sure it is
> an option.
>
>
>>
>> On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <ti...@apache.org>
>> wrote:
>>
>>> The question is maybe about what is realistic for Maven devs.
>>> Shading the CPI package (to something like org.apache.maven.cdi.*) would
>>> be maybe the case instead of removing the original CDI and reinventing the
>>> wheel.
>>>
>>> On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <he...@free.fr>
>>> wrote:
>>>
>>>> and does the MNG issue contain a reproducible test case for us to
>>>> investigate
>>>> more precisely?
>>>>
>>>> Regards,
>>>>
>>>> Hervé
>>>>
>>>> Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
>>>> > Is there a MNG[1] issue?
>>>> >
>>>> > Robert
>>>> >
>>>> > [1] https://issues.apache.org/jira/browse/MNG
>>>> >
>>>> > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>>>> >
>>>> > <rm...@gmail.com> wrote:
>>>> > > Up?
>>>> > >
>>>> > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <rm...@gmail.com>
>>>> a
>>>> > >
>>>> > > écrit :
>>>> > >> Hi guys,
>>>> > >>
>>>> > >> cdi-api is still in maven lib and breaks any plugin using it since
>>>> it is
>>>> > >> an old version, can it be dropped or at least isolated from plugin
>>>> > >> classloaders?
>>>> > >>
>>>> > >> Thanks,
>>>> > >> Romain Manni-Bucau
>>>> > >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>>>> > >> <https://rmannibucau.metawerx.net/> | Old Blog
>>>> > >> <http://rmannibucau.wordpress.com> | Github
>>>> > >> <https://github.com/rmannibucau> | LinkedIn
>>>> > >> <https://www.linkedin.com/in/rmannibucau>
>>>> >
>>>> > ---------------------------------------------------------------------
>>>> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> > For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>>
>>>
>>
>

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
For the reproducer here it is
https://github.com/rmannibucau/test-maven-plugin - pretty trivial you'll
see ;).

2018-02-06 8:05 GMT+01:00 Tibor Digana <ti...@apache.org>:

> Changing the package would not be possible in 3.x.
>

Why? In particular since it is an old regression already reported on the
list due to guice introduction it shouldn't be delayed for this kind of
reason IMHO.
Was less visible until CDI 2 was released cause the API difference was not
triggered but now there are new entries it breaks immediately.


> Guessing the version 4.0.0.
> WDYT?
>

Would stay a blocker until 4 is out which is that soon so not sure it is an
option.


>
> On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <ti...@apache.org>
> wrote:
>
>> The question is maybe about what is realistic for Maven devs.
>> Shading the CPI package (to something like org.apache.maven.cdi.*) would
>> be maybe the case instead of removing the original CDI and reinventing the
>> wheel.
>>
>> On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <he...@free.fr>
>> wrote:
>>
>>> and does the MNG issue contain a reproducible test case for us to
>>> investigate
>>> more precisely?
>>>
>>> Regards,
>>>
>>> Hervé
>>>
>>> Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
>>> > Is there a MNG[1] issue?
>>> >
>>> > Robert
>>> >
>>> > [1] https://issues.apache.org/jira/browse/MNG
>>> >
>>> > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>>> >
>>> > <rm...@gmail.com> wrote:
>>> > > Up?
>>> > >
>>> > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <rm...@gmail.com>
>>> a
>>> > >
>>> > > écrit :
>>> > >> Hi guys,
>>> > >>
>>> > >> cdi-api is still in maven lib and breaks any plugin using it since
>>> it is
>>> > >> an old version, can it be dropped or at least isolated from plugin
>>> > >> classloaders?
>>> > >>
>>> > >> Thanks,
>>> > >> Romain Manni-Bucau
>>> > >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>>> > >> <https://rmannibucau.metawerx.net/> | Old Blog
>>> > >> <http://rmannibucau.wordpress.com> | Github
>>> > >> <https://github.com/rmannibucau> | LinkedIn
>>> > >> <https://www.linkedin.com/in/rmannibucau>
>>> >
>>> > ---------------------------------------------------------------------
>>> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> > For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>
>

Re: cdi-api leaking?

Posted by Tibor Digana <ti...@apache.org>.
Changing the package would not be possible in 3.x.
Guessing the version 4.0.0.
WDYT?

On Tue, Feb 6, 2018 at 8:01 AM, Tibor Digana <ti...@apache.org> wrote:

> The question is maybe about what is realistic for Maven devs.
> Shading the CPI package (to something like org.apache.maven.cdi.*) would
> be maybe the case instead of removing the original CDI and reinventing the
> wheel.
>
> On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <he...@free.fr>
> wrote:
>
>> and does the MNG issue contain a reproducible test case for us to
>> investigate
>> more precisely?
>>
>> Regards,
>>
>> Hervé
>>
>> Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
>> > Is there a MNG[1] issue?
>> >
>> > Robert
>> >
>> > [1] https://issues.apache.org/jira/browse/MNG
>> >
>> > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
>> >
>> > <rm...@gmail.com> wrote:
>> > > Up?
>> > >
>> > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <rm...@gmail.com>
>> a
>> > >
>> > > écrit :
>> > >> Hi guys,
>> > >>
>> > >> cdi-api is still in maven lib and breaks any plugin using it since
>> it is
>> > >> an old version, can it be dropped or at least isolated from plugin
>> > >> classloaders?
>> > >>
>> > >> Thanks,
>> > >> Romain Manni-Bucau
>> > >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> > >> <https://rmannibucau.metawerx.net/> | Old Blog
>> > >> <http://rmannibucau.wordpress.com> | Github
>> > >> <https://github.com/rmannibucau> | LinkedIn
>> > >> <https://www.linkedin.com/in/rmannibucau>
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> > For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
>

Re: cdi-api leaking?

Posted by Tibor Digana <ti...@apache.org>.
The question is maybe about what is realistic for Maven devs.
Shading the CPI package (to something like org.apache.maven.cdi.*) would be
maybe the case instead of removing the original CDI and reinventing the
wheel.

On Tue, Feb 6, 2018 at 7:52 AM, Hervé BOUTEMY <he...@free.fr> wrote:

> and does the MNG issue contain a reproducible test case for us to
> investigate
> more precisely?
>
> Regards,
>
> Hervé
>
> Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
> > Is there a MNG[1] issue?
> >
> > Robert
> >
> > [1] https://issues.apache.org/jira/browse/MNG
> >
> > On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
> >
> > <rm...@gmail.com> wrote:
> > > Up?
> > >
> > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <rm...@gmail.com> a
> > >
> > > écrit :
> > >> Hi guys,
> > >>
> > >> cdi-api is still in maven lib and breaks any plugin using it since it
> is
> > >> an old version, can it be dropped or at least isolated from plugin
> > >> classloaders?
> > >>
> > >> Thanks,
> > >> Romain Manni-Bucau
> > >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > >> <https://rmannibucau.metawerx.net/> | Old Blog
> > >> <http://rmannibucau.wordpress.com> | Github
> > >> <https://github.com/rmannibucau> | LinkedIn
> > >> <https://www.linkedin.com/in/rmannibucau>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: cdi-api leaking?

Posted by Hervé BOUTEMY <he...@free.fr>.
and does the MNG issue contain a reproducible test case for us to investigate 
more precisely?

Regards,

Hervé

Le lundi 5 février 2018, 22:11:56 CET Robert Scholte a écrit :
> Is there a MNG[1] issue?
> 
> Robert
> 
> [1] https://issues.apache.org/jira/browse/MNG
> 
> On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau
> 
> <rm...@gmail.com> wrote:
> > Up?
> > 
> > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <rm...@gmail.com> a
> > 
> > écrit :
> >> Hi guys,
> >> 
> >> cdi-api is still in maven lib and breaks any plugin using it since it is
> >> an old version, can it be dropped or at least isolated from plugin
> >> classloaders?
> >> 
> >> Thanks,
> >> Romain Manni-Bucau
> >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> <https://rmannibucau.metawerx.net/> | Old Blog
> >> <http://rmannibucau.wordpress.com> | Github
> >> <https://github.com/rmannibucau> | LinkedIn
> >> <https://www.linkedin.com/in/rmannibucau>
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org

Re: cdi-api leaking?

Posted by Robert Scholte <rf...@apache.org>.
Is there a MNG[1] issue?

Robert

[1] https://issues.apache.org/jira/browse/MNG

On Sat, 03 Feb 2018 16:29:49 +0100, Romain Manni-Bucau  
<rm...@gmail.com> wrote:

> Up?
>
> Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <rm...@gmail.com> a
> écrit :
>
>> Hi guys,
>>
>> cdi-api is still in maven lib and breaks any plugin using it since it is
>> an old version, can it be dropped or at least isolated from plugin
>> classloaders?
>>
>> Thanks,
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
Up?

Le 19 janv. 2018 13:18, "Romain Manni-Bucau" <rm...@gmail.com> a
écrit :

> Hi guys,
>
> cdi-api is still in maven lib and breaks any plugin using it since it is
> an old version, can it be dropped or at least isolated from plugin
> classloaders?
>
> Thanks,
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau>
>

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
Created https://issues.apache.org/jira/browse/MNG-6354
A more advanced solution would be to enable plugins to customize what they
import.




Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>

2018-02-06 7:15 GMT+01:00 Olivier Lamy <ol...@apache.org>:

> On 6 February 2018 at 06:08, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
> > I do and it works in several cases but it is a lot of work to handle the
> > classloader right and found several plugins just broken cause they use a
> > parent first strategy. This API being so easily pulled - spring/jaxrs can
> > easily pull it - it shouldnt sit here by default IMHO.
> >
> > Dont think there is a ticket yet but would like to know if dropping it is
> > an option? Not sure if guice+api should be shaded or equivalent.
> >
>
> could be solved easily by changing lines here
> https://github.com/apache/maven/blob/master/maven-core/
> src/main/resources/META-INF/maven/extension.xml#L95
>
> but I guess this "simple" change could/will have a LOT of impacts :-(
>
>
>
> >
> > Le 5 févr. 2018 23:52, "Tibor Digana" <ti...@apache.org> a écrit :
> >
> > > This must be a big problem for you actually.
> > > Can you use isolated ClassLoader, set Context ClassLoader in current
> > Thread
> > > and back the origin, reflectively create a new instance of a factory
> and
> > > build an object which runs your code during the plugin execution? When
> > the
> > > plugin's job has finally finished replace CL in current Thread.
> > >
> > > On Fri, Jan 19, 2018 at 1:18 PM, Romain Manni-Bucau <
> > rmannibucau@gmail.com
> > > >
> > > wrote:
> > >
> > > > Hi guys,
> > > >
> > > > cdi-api is still in maven lib and breaks any plugin using it since it
> > is
> > > an
> > > > old version, can it be dropped or at least isolated from plugin
> > > > classloaders?
> > > >
> > > > Thanks,
> > > > Romain Manni-Bucau
> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > > > rmannibucau> |
> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau>
> > > >
> > >
> >
>
>
>
> --
> Olivier Lamy
> http://twitter.com/olamy | http://linkedin.com/in/olamy
>

Re: cdi-api leaking?

Posted by Olivier Lamy <ol...@apache.org>.
On 6 February 2018 at 06:08, Romain Manni-Bucau <rm...@gmail.com>
wrote:

> I do and it works in several cases but it is a lot of work to handle the
> classloader right and found several plugins just broken cause they use a
> parent first strategy. This API being so easily pulled - spring/jaxrs can
> easily pull it - it shouldnt sit here by default IMHO.
>
> Dont think there is a ticket yet but would like to know if dropping it is
> an option? Not sure if guice+api should be shaded or equivalent.
>

could be solved easily by changing lines here
https://github.com/apache/maven/blob/master/maven-core/src/main/resources/META-INF/maven/extension.xml#L95

but I guess this "simple" change could/will have a LOT of impacts :-(



>
> Le 5 févr. 2018 23:52, "Tibor Digana" <ti...@apache.org> a écrit :
>
> > This must be a big problem for you actually.
> > Can you use isolated ClassLoader, set Context ClassLoader in current
> Thread
> > and back the origin, reflectively create a new instance of a factory and
> > build an object which runs your code during the plugin execution? When
> the
> > plugin's job has finally finished replace CL in current Thread.
> >
> > On Fri, Jan 19, 2018 at 1:18 PM, Romain Manni-Bucau <
> rmannibucau@gmail.com
> > >
> > wrote:
> >
> > > Hi guys,
> > >
> > > cdi-api is still in maven lib and breaks any plugin using it since it
> is
> > an
> > > old version, can it be dropped or at least isolated from plugin
> > > classloaders?
> > >
> > > Thanks,
> > > Romain Manni-Bucau
> > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > > rmannibucau> |
> > > LinkedIn <https://www.linkedin.com/in/rmannibucau>
> > >
> >
>



-- 
Olivier Lamy
http://twitter.com/olamy | http://linkedin.com/in/olamy

Re: cdi-api leaking?

Posted by Romain Manni-Bucau <rm...@gmail.com>.
I do and it works in several cases but it is a lot of work to handle the
classloader right and found several plugins just broken cause they use a
parent first strategy. This API being so easily pulled - spring/jaxrs can
easily pull it - it shouldnt sit here by default IMHO.

Dont think there is a ticket yet but would like to know if dropping it is
an option? Not sure if guice+api should be shaded or equivalent.

Le 5 févr. 2018 23:52, "Tibor Digana" <ti...@apache.org> a écrit :

> This must be a big problem for you actually.
> Can you use isolated ClassLoader, set Context ClassLoader in current Thread
> and back the origin, reflectively create a new instance of a factory and
> build an object which runs your code during the plugin execution? When the
> plugin's job has finally finished replace CL in current Thread.
>
> On Fri, Jan 19, 2018 at 1:18 PM, Romain Manni-Bucau <rmannibucau@gmail.com
> >
> wrote:
>
> > Hi guys,
> >
> > cdi-api is still in maven lib and breaks any plugin using it since it is
> an
> > old version, can it be dropped or at least isolated from plugin
> > classloaders?
> >
> > Thanks,
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau>
> >
>

Re: cdi-api leaking?

Posted by Tibor Digana <ti...@apache.org>.
This must be a big problem for you actually.
Can you use isolated ClassLoader, set Context ClassLoader in current Thread
and back the origin, reflectively create a new instance of a factory and
build an object which runs your code during the plugin execution? When the
plugin's job has finally finished replace CL in current Thread.

On Fri, Jan 19, 2018 at 1:18 PM, Romain Manni-Bucau <rm...@gmail.com>
wrote:

> Hi guys,
>
> cdi-api is still in maven lib and breaks any plugin using it since it is an
> old version, can it be dropped or at least isolated from plugin
> classloaders?
>
> Thanks,
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau>
>