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

Proposed rewrite/changes #37

Open
BomBardyGamer opened this issue Apr 2, 2021 · 3 comments
Open

Proposed rewrite/changes #37

BomBardyGamer opened this issue Apr 2, 2021 · 3 comments

Comments

@BomBardyGamer
Copy link
Contributor

I know there is already a sort of issue open for this, but I thought I would detail a bit more and make it clear. And also add my propositions as well.

I propose that we completely change the project's structure, splitting each supported platform for PDM into its own module.
For example, we could have a module called "platforms", and then have a module for Bukkit, BungeeCord, Sponge, Velocity, Krypton, etc.

I also think that to maintain compatibility with Java 16, we should implement a system very similar to Luck's jar in jar system that he uses for LuckPerms, which allows us to control the loading of the plugin's classes ourselves, and also allows us to make our own URL class loader extension that exposes adding URLs to classpath, to avoid messy and hacky reflection that Java 16 doesn't allow by default.

In addition, I also believe that it the relocation PR is well overdue to be merged into the upstream, and we might be able to integrate that in when we do the rewrite.

I would like to mention as well that anyone is welcome to participate in this, and that I intend to be at the heart of this.

Let me know if I missed anything!

@bristermitten
Copy link
Owner

I don't really like the idea of needing a separate module for each platform. As I've mentioned before, PDM is already designed in a way that it should work on any platform that provides an URLClassLoader -- ideally, you'd only need a single class to make the API a bit more convenient for a single platform (see BungeeDependencyManager). But perhaps this'll need to be changed for the Java 16 workaround.

I'd like to try and look into a more simple solution for Java 16 as jar in jar will be pretty convoluted to implement dynamically. My current idea is to just create a child classloader which exposes addURLs (or an analogous method). This means the only thing we need to do at build time is generate a dependencies.json and potentially relocate definitions.

Relocation PR hasn't been merged because it's not yet functional (as far as I know) but I'll give it another look at some point.

@BomBardyGamer
Copy link
Contributor Author

Sounds fair. I only suggested the different platform idea because, for example, Sponge, Velocity and Krypton all provide ways for plugins to directly add paths to their class loaders, meaning we don't require control over our class loader there. Whereas, for Bukkit and BungeeCord, they don't, so we need control of the class loader.

And I'll look into the relocation PR a bit more and ask @PiggyPiglet if he can do any more work on it.

@bristermitten
Copy link
Owner

Yes, modules may be a good idea for the more obscure platforms, but with the current design they kinda seem like overkill.

Looking at the relocation PR might not be the best idea given the impending rewrite (and the fact that it doesn't work). There's still a few implementation details that need ironing out before following up on relocation anyway,

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

No branches or pull requests

2 participants