+1 for dropping. Also already had problems with it. LieGrue, strub
> Am 01.12.2020 um 17:50 schrieb Romain Manni-Bucau <rmannibu...@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 <rmannibu...@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 <mccu...@gmail.com>: >> >>> On Tuesday, 6 February 2018 at 13:25, Romain Manni-Bucau wrote: >>>> 2018-02-06 12:25 GMT+01:00 Stuart McCulloch <mccu...@gmail.com (mailto: >>> mccu...@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 <tibordig...@apache.org >>> (mailto:tibordig...@apache.org) (mailto: >>>>> tibordig...@apache.org (mailto:tibordig...@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 < >>>>> rmannibu...@gmail.com (mailto:rmannibu...@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 <tibordig...@apache.org >>> (mailto:tibordig...@apache.org) >>>>> (mailto:tibordig...@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 < >>>>> tibordig...@apache.org (mailto:tibordig...@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.bout...@free.fr (mailto:herve.bout...@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 >>>>>>>>>>>> >>>>>>>>>>>> <rmannibu...@gmail.com (mailto:rmannibu...@gmail.com)> >>>>> wrote: >>>>>>>>>>>>> Up? >>>>>>>>>>>>> >>>>>>>>>>>>> Le 19 janv. 2018 13:18, "Romain Manni-Bucau" < >>>>>>> rmannibu...@gmail.com (mailto:rmannibu...@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-unsubscr...@maven.apache.org (mailto:dev-unsubscr...@maven.apache.org >>> ) >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> (mailto:dev-unsubscr...@maven.apache.org) >>>>>>>>>>>> For additional commands, e-mail: >>> dev-h...@maven.apache.org (mailto:dev-h...@maven.apache.org) >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> (mailto:dev-h...@maven.apache.org) >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>> ------------------------------------------------------------ >>>>> --------- >>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org >>> (mailto:dev-unsubscr...@maven.apache.org) >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> (mailto:dev-unsubscr...@maven.apache.org) >>>>>>>>>>> For additional commands, e-mail: >>> dev-h...@maven.apache.org (mailto:dev-h...@maven.apache.org) >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> (mailto:dev-h...@maven.apache.org) >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>>> >>> >>> >>> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
