-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Choose the inner-most scalac-plugin.xml resource during plugin resolution #8322
Conversation
/nothingtoseehere (travis build timed out, jenkins is happy) |
loader.getResource(PluginXML) match { | ||
case null => Failure(new MissingPluginException(path)) | ||
case url => | ||
loader.getResources(PluginXML).asScala.toList.lastOption match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a second line of defence, in case a Global
subclass overrides findPluginClassloader
and doesn't special case parent delegation for plugin.xml
as I've done in Plugins.scala
.
Are you planning to write tests that exercise this? This change should be easily portable to do but I'm not comfortable doing that without tests |
@smarter I did attempt to replicate the classloader setup in a test case but couldn't find a decent way to do it without creating new test infrastructure. |
This commit reverts part of the scala#8322 I recently changed to using the last entry from getResources to avoid parent classloader delegation, but this leads to unwanted results if the plugin classpath itself contains multiple `scalac-plugin.xml` resources and wants the compiler to choose the first. I have _not_ reverted the other change in scala#8322 which overrides `getResource` to avoid parent delegation for `scalac-plugin.xml`. I have documented the `findPluginClassLoader` extension point with instructions to do the same.
I've had to tweak this in #8345 to cope with plugin classpaths that themselves contain multiple |
Fixes scala/bug#11666