> 1. Exclude the library on the plugin side (e.g. how Token Macro excludes ASM) 2. Mask the library's classes (e.g. how JaCoCo masks ASM classes)2 -> 3. Shade the library into the plugin
1 -> Which is fine until the library evolves in a binary incompatible way.. and the ASM library is KNOWN to do this. 2-> has many pitfalls and this only works if no one else depends on your plugin otherwise they also get those classes (ref the jacoco guice inject mess) 3 -> requires a larger restructuring of plugins to enable that and I look forward to the PRs. > Note that even if stapler/stapler#244 is reverted, a plugin that uses ASM but fails to follow one of the three approaches outlined above is going to have problems when invoking a core API that invokes JNR: JNR will invoke the (recent, unshaded) ASM (just as Stapler is doing post-stapler/stapler#244), which will fail. In other words, reverting the Stapler change will reduce the surface area of the problem, but it will not eliminate the problem. I do not believe this to be correct, the core class will resolve ASM using the core classes' classloader not the plugins classloader even when invoked from the plugin. If the core class took an ASM class/object as a parameter then it would blow up but meerly calling `jenkins.foo()` where `foo` calls some ASM method will work correctly due to the hierachical nature of Jenkins classloaders. Thus based on the above this really is making things worse. On Thursday, June 10, 2021 at 5:18:44 PM UTC+1 m...@basilcrow.com wrote: > ASM has been shipped by core, unshaded, as a transitive dependency of > JNR (_not_ JNA) since JNR was introduced in 2013. Removing core's > dependency on JNR (and therefore its transitive dependency on unshaded > ASM) is a large and yet unscoped project; similarly, hiding core > dependencies from plugins is another large and yet unscoped project. > > JNR was updated in 2.277 in December 2020, which also updated core's > (transitive) ASM dependency. This broke Token Macro. The Token Macro > breakage was resolved by excluding ASM from Token Macro, allowing it > to use the copy provided by core. > > In general, when a plugin depends on a library already provided by > core, I have seen three approaches in the short term: > > 1. Exclude the library on the plugin side (e.g. how Token Macro excludes > ASM) > 2. Mask the library's classes (e.g. how JaCoCo masks ASM classes) > 3. Shade the library into the plugin > > None of these are ideal compared to the larger projects of removing > (or hiding) Guice/JNR (and by extension Guava/ASM) from core, but all > three approaches work in the short term. > > As long as core continues to expose JNR (and therefore unshaded ASM) > in its public API, plugins that use ASM (directly or indirectly) must > follow one of these three approaches in the short term. Similarly, as > long as core continues to expose Guice (and therefore unshaded Guava) > in its public API, plugins that use Guava (directly or indirectly) > also must follow one of these three approaches in the short term. > > Whether we like it or not, core has been in the business of providing > unshaded ASM since 2013, so being explicit about it (by adding ASM to > the list of core dependencies and the core BOM as I did in > jenkinsci/jenkins#5525) at least allows us to manage it carefully. > This is not as good as ripping out JNR (and by extension ASM) from > core, but it is better than having ASM get updated accidentally with > unrelated JNR upgrades, as happened in December 2020. > > If a plugin that uses ASM or Guava fails to follow one of these three > approaches (as is currently the case with Subversion), it is going to > have problems in the short term: either with JNR alone (prior to 2.296 > thus stapler/stapler#244), or with JNR and Stapler (after 2.296 thus > stapler/stapler#244). > > Note that even if stapler/stapler#244 is reverted, a plugin that uses > ASM but fails to follow one of the three approaches outlined above is > going to have problems when invoking a core API that invokes JNR: JNR > will invoke the (recent, unshaded) ASM (just as Stapler is doing > post-stapler/stapler#244), which will fail. In other words, reverting > the Stapler change will reduce the surface area of the problem, but it > will not eliminate the problem. > > For this reason, I recommend that all plugins that use Guava or ASM > follow one of the three approaches outlined above in the short term. > This is the only guaranteed way for plugins to avoid Guava and ASM > problems at present. > -- You received this message because you are subscribed to the Google Groups "Jenkins Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/4376f434-41de-4fbb-a5f7-481504861581n%40googlegroups.com.
