Skip to content
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

Suggestion - Library download #4

Open
EpiCanard opened this issue Mar 15, 2021 · 22 comments
Open

Suggestion - Library download #4

EpiCanard opened this issue Mar 15, 2021 · 22 comments
Labels
enhancement New feature or request

Comments

@EpiCanard
Copy link
Contributor

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
Copy link
Owner

Jannyboy11 commented Mar 16, 2021

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:

  • ScalaLoader will only download libraries once per major version of Scala. Multiple plugins can make use of the same library. ScalaPlugins should be able to opt-out of this on a per-dependency basis. This can be done by changing the classloader hierarchy.
  • Java libraries only have to be downloaded once since they are binary compatible with any version of Scala.
  • Both Maven coordinates and direct URLs should be supported. Integrety checks using hashes must be included.
  • Libraries that are already included in the server (such as Guava, Apache Commons, Gson, Netty, jetbrains-annotations) should not have to be donwloaded if version of those libraries used by the server is in the range of the accepted versions required by the ScalaPlugin.
  • Transitive dependencies should not have to be specified in the ScalaPlugin's description. They should resolved and downloaded automatically. I need to investigate whether there are existing version resolution libraries that can deal with SemVer and PVP, otherwise I need to implement this logic myself.
  • Server owners must be able to override the versions of libraries used.
  • Plugins must be able to configure whether to expose their dependencies to other plugins, or not.
  • Plugins must be able to configure whether the dependencies undergo bytecode transformations.
  • When Scala 3 arrives, Scala 2.13.x plugins should be able to make use of Scala 3 libraries (using the -tastyreader option) and Scala 3 plugins should be able to use Scala 2.13 libraries (with the exception of breaking changes such as macros). Ideally, it is architected in such a way that ScalaLoader can recompile plugins from tastyfiles, so that in the future I can force plugins to be compatible even though their major versions of Scala differ (e.g. Scala 3.0 and Scala 3.1). This will allow plugins to upgrade their Scala versions independently and still depend on one another.

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

@Jannyboy11 Jannyboy11 added the enhancement New feature or request label Mar 16, 2021
@EpiCanard
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

Jannyboy11 commented Mar 29, 2021

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
Copy link
Contributor Author

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

@Jannyboy11
Copy link
Owner

Jannyboy11 commented Mar 29, 2021

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
Copy link
Owner

Jannyboy11 commented Mar 29, 2021

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
Copy link
Contributor Author

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
Copy link
Owner

Jannyboy11 commented Mar 30, 2021

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
Copy link
Contributor Author

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
Copy link
Owner

Jannyboy11 commented Apr 15, 2021

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
Copy link
Contributor Author

EpiCanard commented Apr 15, 2021

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
Copy link
Owner

Jannyboy11 commented Apr 15, 2021

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

Jannyboy11 commented Jun 5, 2021

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:

  • Bytecode transformations will apply to dependencies too (I'm surprised Bukkit itself doesn't do this, this might be a bug in Bukkit's implementation)
  • Dependencies that depend on the scala standard library will find the scala standard library classes loaded by ScalaLoader, ensuring compatilibility.
  • In addition to Maven Central, the CodeMC maven-public repository can also be used to look up dependencies.

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

Awesome 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants