[FIXED JENKINS-44898] Add findResource to PluginFirstClassLoader #2916
Conversation
This fixes GroovyClassLoader.loadClass for a .groovy file in a plugin with a PluginFirstClassLoader, specifically by fixing fast-loading via the UberClassLoader.
|
|
||
| try { | ||
| Field $pathComponents = AntClassLoader.class.getDeclaredField("pathComponents"); | ||
| $pathComponents.setAccessible(true); |
There was a problem hiding this comment.
Seems like you need to introduce a common ancestor with AntClassLoader2 to avoid code duplication.
There was a problem hiding this comment.
(Or we should just stop using AntClassLoader at all. I am not sure why we ever did.)
There was a problem hiding this comment.
Yeah, I'm generally mystified but wanted to go with the least-disruptive-change-possible in case there was some legitimate reason somewhere for it. I'll add a common ancestor.
|
@jglick Any suggestions/thoughts/ideas on how to get said test coverage? I definitely agree, I just can't figure out what the right way to do that would be. |
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
There are some tests which install plugins from |
| * As of 1.8.0, {@link org.apache.tools.ant.AntClassLoader} doesn't implement {@link #findResource(String)} | ||
| * in any meaningful way, which breaks fast lookup. Implement it properly. | ||
| */ | ||
| public class AntWithFindResourceClassLoader extends AntClassLoader implements Closeable { |
There was a problem hiding this comment.
Prefer @Restricted(NoExternalUse.class). Or just move into hudson package and leave it as default access.
| private final class AntClassLoader2 extends AntClassLoader implements Closeable { | ||
| private final Vector pathComponents; | ||
|
|
||
| private final class AntClassLoader2 extends AntWithFindResourceClassLoader implements Closeable { |
| @@ -42,10 +42,14 @@ | |||
| * @since 1.371 | |||
| */ | |||
| public class PluginFirstClassLoader | |||
| extends AntClassLoader | |||
| extends AntWithFindResourceClassLoader | |||
| implements Closeable | |||
|
@abayer @daniel-beck I propose the following changelog entry:
@daniel-beck Maybe you've already moved this page to jenkins.io |
oleg-nenashev
added
the
ready-for-merge
|
Well the bug is generic and could have broader effects but OK. |
Description
See JENKINS-44898.
Details: This fixes
GroovyClassLoader.loadClassfor a .groovy file in a plugin with aPluginFirstClassLoader, specifically by fixing fast-loading via theUberClassLoader.Still need to figure out the right way to do a test for this - I have verified that the issue described in jenkinsci/kubernetes-plugin#127 doesn't occur with this PR but the
PluginFirstClassLoaderexpected behavior is still good.Changelog entries
Proposed changelog entries:
Submitter checklist
Desired reviewers
@reviewbybees