gnodet commented on code in PR #950:
URL: https://github.com/apache/maven/pull/950#discussion_r1387840616
##########
maven-core/pom.xml:
##########
@@ -144,6 +144,12 @@ under the License.
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
+ <dependency>
Review Comment:
> Yes exactly, I think it is a case which becomes not rare to ignore.
My point is that I mitigated the problem by externalising ASM. Whenever a
new ASM release comes, we can easily bump it and release maven. But not sure
it actually works, see the last point below.
> About the interface thing it is mainly a matter of registering the scoped
bean with an interface instead of its class so means
`@SessionScoped(targetInterface = MyApi.class) class MyApiImpl implements Api
{}` or something like that - once again we can relax it for internal usage but
not for external ones - and `T` becomes `MyApi` instead of taking the actual
instance. Could it be a compromise to try to not shout in users foot?
I've reworked the PR in this way. The injection is limited to beans which
have a `Typed` annotation (sisu or inject) and the proxies now use JDK proxies.
Related to sisu, I investigated a bit, and ASM is not used to actually
discover classes, that's correctly done with the generated META-INF files, but
it's used to discover the injection points without having to load the classes,
see [sisu
SpaceScanner](https://github.com/eclipse/sisu.inject/blob/master/org.eclipse.sisu.inject/src/main/java/org/eclipse/sisu/space/SpaceScanner.java).
Unfortunately, the sisu code does need [some small
migration](https://github.com/eclipse/sisu.inject/commit/a9752361662edac045f16129a49e1a8b625aa604)
when upgrading to a new ASM version to support reading new code...
Another point I just found is that the `java.lang.reflect.Proxy`... [is
using
ASM](https://github.com/openjdk/jdk/blob/dd9eab15c832c20e65681c21c5f91df11f4cddf9/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java#L56)
to create the proxies ! So not sure we can really do much here... However, I
assume the JDK uses an ASM version which support the JDK itself somehow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]