Remove ASM dependency #396
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of my goals for reducing core dependency debt is to remove ASM from core (possibly by detaching it into a library plugin), which means first removing any usages of ASM from core. A big one was JNR, which I have already detached. Stapler is probably the second-biggest one.
This PR replaces the ASM dependency with an inlined copy of https://github.com/paul-hammant/paranamer/blob/master/paranamer/src/java/com/thoughtworks/paranamer/BytecodeReadingParanamer.java which is basically just the portions of ASM that we need, inlined into a single file. This removes the risk of linkage errors because we're just invoking a regular static method on a regular Java class, and it brings us one step forward to removing ASM from the core WAR.
One might be concerned about the fact that we're unlikely to get updates to this code, but it doesn't need to be able to deal with class files from new Java versions: this is a fallback code path for plugins that were compiled before our plugin parent POM enabled parameter names in the compiler options, which is only the case for very old plugins compiled for Java 8 or earlier. Hopefully in some hypothetical glorious future this will be part of the Java Platform and the need for ASM will be a thing of the past.
BTW I loaded this up on my Jenkins installation with 150 plugins and set breakpoints on the fallback ASM code path and clicked around in the UI. The only two plugins I had installed that triggered the fallback codepath were
docker-commonsanddocker-workflow, which were using older parent POMs. I think that demonstrates the low risk of this change: almost all critical plugins are compiled with a recent parent POM and not using this legacy code path. It is effectively just there for backward compatibility with ancient plugins.