2018-02-06 12:25 GMT+01:00 Stuart McCulloch <[email protected]>: > 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 <[email protected] (mailto: > [email protected])>: > > > > > 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 < > [email protected] (mailto:[email protected])> > > > 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 <[email protected] > (mailto:[email protected])>: > > > > > > > > > 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 < > [email protected] (mailto:[email protected])> > > > > > 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 < > [email protected] (mailto:[email protected])> > > > > > > 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 > > > > > > > > > > > > > > > > <[email protected] (mailto:[email protected])> > wrote: > > > > > > > > > Up? > > > > > > > > > > > > > > > > > > Le 19 janv. 2018 13:18, "Romain Manni-Bucau" < > > > [email protected] (mailto:[email protected])> > > > > > > > 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: [email protected] > (mailto:[email protected]) > > > > > > > > For additional commands, e-mail: [email protected] > (mailto:[email protected]) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------ > --------- > > > > > > > To unsubscribe, e-mail: [email protected] > (mailto:[email protected]) > > > > > > > For additional commands, e-mail: [email protected] > (mailto:[email protected]) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
