search

Jannyboy11 / ScalaPluginLoader

PluginLoader for Bukkit that provides Scala runtime classes and enhanced APIs.
https://www.spigotmc.org/resources/scalaloader.59568/
GNU Lesser General Public License v3.0
26 stars 10 forks source link

Suggestion - Library download #4

Open EpiCanard opened 3 years ago

EpiCanard commented 3 years ago

Hello :)

I was thinking about a feature. Currently ScalaPluginLoader download and load Scala in classpath. Would it be possible to do the same with libraries, to reduce the size of scala plugins ? For example, if I want to use Cats in a plugin. Ask to ScalaPluginLoader to download Cats with a specific version and load it into the plugin's classpath.

Hope you will find this idea useful and achievable ^^

Have a good day :)

Jannyboy11 commented 3 years ago

Hi EpiCanard.

Yes this feature is very useful and yes it is also achievable. ScalaLoader can make use of Eclipse Aether to resolve and download maven artifacts. But I really want to do this right, that means the following:

This feature might also cause some source-incompatible changes since libraries need to be downloaded and loaded before the plugin's main class load, but I can make it binary-backwards compatible since ScalaLoader already does bytecode transformations at class-load time.

All the TASTy-related stuff will be the hardest to implement because I might have to depend on the dotty compiler or create a tasty reader in Java myself.

So yes, the feature is on the TODO list (it literally has been on the roadmap in the readme for years) and I do have plans for what I want it to look like, but I haven't been able do implement this because of real life work and the ConfigurationSerializable feature that is still not completely done. The reason I decided to work on that first is that I would rather wait till Scala 3 is fully released before I start on auto-downloading maven artefacts. It is likely that I will start working on this somewhere in the summer so in the meantime you'll have to shade your third-party dependencies, I'm sorry.

Thanks anyway for the suggestion and for thinking along with the project!

Kind regards, Jan

EpiCanard commented 3 years ago

Hi :)

Thanks a lot for all the details. It's awesome to see you thinking so far the design and the quality of work for your features.

No problem I will continue to shade the libraries I need.

Best Regards EpiCanard

ps: Sorry I didn't see the line in Readme for this feature, but I don't regret you gave so many interesting details.

Jannyboy11 commented 3 years ago

Don't worry about the Readme. This issue has allowed me to really dive deep into this and strucuture my thoughts. I'm leaving the issue open as it can serve as documentation for a future me when I start to implement this, and for others who stumble upon this project.

Jannyboy11 commented 3 years ago

Hey there, I recently came across this thread on SpigotMC where one person linked to two other projects which can load dependencies at runtime. They seem to work at the 'javaplugin-level' meaning that the dependencies are loaded as javaplugins and so when used in combination with ScalaLoader they are accessible to all ScalaPlugins. This can be a problem if the library is specific to a certain version of Scala and your ScalaPlugin is a publicly available, but if your plugin is private or the library is a java library then you could already use it. Here's the thread: Suggestion - Including Scala Runtime

EpiCanard commented 3 years ago

It looks like you can use a relocation system. So if you relocate it in your plugin package, it shouldn't have problem of conflict with other plugins. But I don't know how it works exactly. Will it work with ScalaPugin and with ScalaLibrary?

Jannyboy11 commented 3 years ago

But I don't know how it works exactly. Will it work with ScalaPugin and with ScalaLibrary? Both projects seem to work in different ways:

  • PDM loads dependencies by calling "addUrl" reflectively on the classloader of the plugin. This will work for ScalaPlugins because ScalaPluginClassLoader subclasses URLClassLoader. The dependency will thus be able to find the scala library too, because the classloader that loads the scala standard library is a parent of the ScalaPluginClassLoader.
  • HDL works differently; it downloads the dependency as a jar and loads it as a JavaPlugin. JavaPlugins cannot find the scala standard library classes loaded by ScalaLoader, so this method only works if your dependency does not use the Scala standard library. Thus you can't use this method to download Cats if you use my ScalaLoader.

Edit: now that I've taken a deeper look at PDM, it checks whether the classloader is actually a PluginClassLoader from bukkit, so PDM isn't usable either without modifications. I'm sorry.

EpiCanard commented 3 years ago

A contribution to remove this restriction or change it to support ScalaPluginClassLoader can be a quick win. :)

Jannyboy11 commented 3 years ago

well yes but you could just do the same thing yourself in your plugin. In your plugin's main class constructor/initializer:

val url: Url = new URL("https://location/of/cats_2.13.jar")
val classLoader: ScalaPluginClassLoader = getClassLoader();
val method: Method = classOf[URLClassLoader].getDeclaredMethod("addUrl", classOf[URL])
method.setAccessible(true)
method.invoke(classLoader, url)

The only caveat being that you must make sure none of the library classes are classloaded before that point or you will get a NoClassDefFoundError.

Admittedly you don't get all the caching and other niceties that PDM offers, but I don't have the time to fork PDM right now and adjust its behaviour, test that it actually works for ScalaPlugins and create a pull request.

Jannyboy11 commented 3 years ago

I made it work using standard PDM, a demo can be found here: https://github.com/Jannyboy11/CatsPlugin. The only reason why this works is that the classloader check is only used in the factory method SpigotDependencyManager.of(Class<? extends Plugin>), but not in the factory method SpigotDependencyManager.of(Plugin). This was probably not intentional, but the author gave us a nice escape hatch that way (see https://github.com/knightzmc/pdm/blob/master/pdm/src/main/java/me/bristermitten/pdm/SpigotDependencyManager.java#L55). I had to adjust the ScalaPluginClassLoader such that it would always use the scala standard library that was downloaded by ScalaLoader, and not the one downloaded by PDM, so to use this you need ScalaLoader version 0.14.5 or newer.

Note that this only works on java 8 or 9 till 16 with illegal access enabled (can be enabled on jdk9-16 using --illegal-acces=warn or --illegal-acces=permit). As of Java 17, this method will no longer work without a patch to PDM!

Edit2: I had a little talk with the author of PDM their Discord: afbeelding And they suggested to instantiate a custom DependencyManager using the builder. This is probably the better way to do it because the factory method that I used in the CatsPlugin might break in the future.

EpiCanard commented 3 years ago

It sounds awesome. I didn't found the time to try yet but it seems to be a perfect solution to resolve the issue of dependencies and keep my plugin light.

Note that this only works on java 8 or 9 till 16 with illegal access enabled (can be enabled on jdk9-16 using --illegal-acces=warn or --illegal-acces=permit). As of Java 17, this method will no longer work without a patch to PDM!

It will no longer work because he use reflection ?

Jannyboy11 commented 3 years ago

It will no longer work because he use reflection ?

Indeed. Though in my testing it only went wrong for transitive dependencies, so I think it is possible to patch pdm to address this. You'll be fine for a while though. Hardly anybody is running his minecraft server on Java 16, and Illegal access is still enabled by default on all prior versions. Java 17 will only be released next september, at which point I hope to have a preliminary dependency api.

EpiCanard commented 3 years ago

As you said I think I have the time before everyone is on Java 17 or higher. It's already hard to make them move from Java 8 ^^" (bstats javaversion)

Jannyboy11 commented 3 years ago

I thought I'd just share this here since it's related to loading libraries at runtime: https://hub.spigotmc.org/jira/browse/SPIGOT-6419?focusedCommentId=38865&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-38865 md_5 says he's working on Bukkit being able to load plugins from Maven Central. So in the future you could just publish a 'dependency'-plugin that itself depends on all your runtime dependencies to Maven Central and let your main plugin depend on that.

Since this will likely mean changes to bukkit's class-loading scheme, I am going to see this feature as a blocker for my own dependency loader - meaning I won't work on it before bukkit's own dependency loader is implemented. I might not even need to implement a dependency loader after all in the best case.

EpiCanard commented 3 years ago

Interesting. That could be a part of the API that you can reuse to inject depenency as a library for scala plugin and not a plugin bukkit load.

Jannyboy11 commented 3 years ago

That could be a part of the API that you can reuse to inject depenency as a library for scala plugin and not a plugin bukkit load.

Yes that's what I was thinking.

I'll have to query this feature for availability in a backwards-compatible way though. Currently ScalaLoader works on all CraftBukkit versions since Minecraft 1.8 and I don't intend to break compatibility because of new features in Bukkit. That won't mean I won't utilize new features from Bukkit, it just means the behaviour will be slightly different. Take for example bukkit's bytecode transformation compatibility layer; The ScalaPluginClassLoader will use it if it's available (1.13 and up), and on lower versions a transformation is only attempted once, and once it fails it's never even attempted again.

EpiCanard commented 3 years ago

Even if the dependency download feature will only be available in a future version (maybe 1.17, who knows). Would you like to use this as inspiration to make a similar system for lower versions ?

Jannyboy11 commented 3 years ago

Even if the dependency download feature will only be available in a future version (maybe 1.17, who knows). Would you like to use this as inspiration to make a similar system for lower versions ?

Yes, in the ideal case I would, but it's got lower priority. I'm just currently very stressed by work on my master thesis so I find it hard to make any long term planning right now. If you need your plugin to work on older versions you can still use PDM or just use shading.

EpiCanard commented 3 years ago

Ok :) Oh no problem take your time I already planned to use PDM first ^^ And if later there is a native solution on ScalaPluginLoader, I will switch on it.

Jannyboy11 commented 3 years ago

Although it's not the fully-fledged solution I sketched earler, it's now possible to define dependencies in the plugin.yml because I want to keep feature parity with bukkit's JavaPluginLoader. Some differences in my implementation:

See https://hub.spigotmc.org/javadocs/bukkit/org/bukkit/plugin/PluginDescriptionFile.html#getLibraries(). The Scala3Example plugin now uses this new api to load ZIO.

EpiCanard commented 3 years ago

It's so cool ! Thank you a lot, you are awesome !

I guess that since the library loader is really new, it will not be available for previous versions of Minecraft.

Jannyboy11 commented 3 years ago

I guess that since the library loader is really new, it will not be available for previous versions of Minecraft.

That is correct, however for ScalaPlugins it will work on older versions of minecraft. I tested the Scala 3 example plugin on Spigot 1.8.8 and there were no issues.

EpiCanard commented 3 years ago

Awesome 😁