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]

Reply via email to