lavalink-devs
Created module-info.java file
I have created a module-info.java file, which currently exports all packages (as I do not know which should be exposed). I have done so to allow projects that require modules to be able to access this project via the module system.
The best way to do it, if your just gonna expose everything anyways, is just to put this in the manifest file
'Automatic-Module-Name: com.sedmelluq.discord.lavaplayer'
module-info.java isnt needed, its only if you do plan to restrict packages/etc from being accessed by another module...
And there are ways to get it working just fine, as Ive done in my Discord Bot which uses JPMS
https://github.com/MangoRageBot/ https://github.com/MangoRageBot/MangoBot
Also natives being a seperate jar doesnt quite help, as it makes making a proper Module Aware Classloader abit more tricky...
I just made it so Modules can be considered having "children", of which they are allowed to access, for resource files only...
https://github.com/MangoRageBot/MangoBotPlugin/blob/8e067e43c00afccb2fd9b84e1557cc28fbc6316a/src/main/java/org/mangorage/mangobotplugin/module/ModuleConfigurator.java#L14
As can be seen here on how I handle that -> https://github.com/MangoRageBot/MangoBotBootstrap/blob/fce1347861f8d7ac1c02e3bf373a66cb24a9e5d0/src/main/java/org/mangorage/bootstrap/internal/MangoLoader.java#L61
And here ->
https://github.com/MangoRageBot/MangoBotBootstrap/blob/fce1347861f8d7ac1c02e3bf373a66cb24a9e5d0/src/main/java/org/mangorage/bootstrap/internal/LoadedModule.java#L45
Hope this info helps 👍
@RealMangorage Thanks, but I suggested using a module-info.java because I was hoping the maintainers would decide in the future what packages would be exported, I only wrote it so that there would be such a file.
@topi314 Hello, sorry to mention you out of the blue, but seeing as you were the last to commit to this repository and that you responded to another pull request recently (albeit to close it), what are your thoughts on this addition and would you be willing to merge it?
The reason I haven't acted on this is that I have never used java modules before. I also don't see the need to restrict usage of anything so what would having all exposed archive?
it's just another thing to keep maintained
The reason I haven't acted on this is that I have never used java modules before.
I also don't see the need to restrict usage of anything so what would having all exposed archive?
it's just another thing to keep maintained
Because the project uses Java 10 it has the ability to be written with a module file, and having such a module file greatly simplifies things for projects that do use the module system as the library can be accessed through the module path rather than classpath. Accessing the library through the classpath on a modular project requires a few extra steps, or even some dodgy hacks to get tooling/IDEs working well with it.
Also it would be a convenience feature as Java 24 introduces the ability to just directly import modules, which imports all exported packages.
As for the matter of maintenance, a module-info file is extremely low maintenance as the only time it would need to be updated is if a new library needs to be accessed by this library (i.e. a new dependency is introduced), or if a new package is created in the library that needs to be visible. Either way, in my opinion the benefits greatly outweigh the drawbacks.
@devoxin Hi, I do not know if you have more experience with Java modules, but given the above reasoning, would you be willing to merge this pull request?
@topi314 I decided to look more deeply into what packages should be exported, and I suspect that com.sedmelluq.discord.lavaplayer.natives.* and all the sub-packages within that are not meant to be exported? As in, they are not designed for public use but rather only for the library itself. In that case I would remove that from module-info.java so that it is not exported. I also wonder if anything in com.sedmelluq.discord.lavaplayer.tools.* is meant to be public.
So in total the ones I am wondering if should be public are
-
com.sedmelluq.discord.lavaplayer.natives.* -
com.sedmelluq.discord.lavaplayer.tools.*(though I am less sure of this)
If this is the case I'll remove them from the file. The point of module-info.java is after all to preserve encapsulation as much as possible so that internal APIs are not exported to the client.
An automatic module name inside the manifest and call it good, jda already does that for its library. There is no need for module info file.
And having the natives to be included in the jar, instead of an extra jar.
Then it will be good enough
unexporting anything would be considered a breaking change now.
Okay, that's fine - I'll just simply continue to export all packages. Nonetheless, it's much easier to access the library through the module path than classpath on a modular project, which is why I would like to be able to do so for this project.
An automatic module name inside the manifest and call it good, jda already does that for its library. There is no need for module info file.
JDA is written in Java 8 and thus obviously cannot have a module-info.java file.
Java doesn't require that anyways for module support....
It does if you want to access the library through module path, and also being able to import the entire library with a single statement in Java 24 would simplify a lot of code
It lets you use either or, both put it onto the module path, Ive already been there and done that.
It lets you use either or, both put it onto the module path, Ive already been there and done that.
I'm not familiar with how to do it in a simple way or without using complex/non-straightforward hacks, but regardless I don't see any downsides to having a module-info file. It does not slow the library at runtime or introduce any real complication during development, but does still offer benefits.
The simplest method is to use auto module name inside of the manifest by far since topi does not want to use the module-info anyways, that's the only choice here.
Well that is the reason for this pull request, to convince the devs that a module-info file would be beneficial. Of course if they don't want to merge it they are free to close this PR.
@topi314 Hi, I'm sorry to bother you again. I understand you do not have much experience working with the Java module system, but I would like to ask if there is any circumstance in which your team would be willing to merge this request, and if so what changes you would like made to my edits, and are there any things you would like clarified about my edits?
I once again reiterate my reasons that a module-info.java file is low-maintenance while greatly simplifying the way in which one is able to access the library through module path, does not break projects (modules were introduced in Java 9 and the project uses Java 10, also non-modular projects can still access modular libraries), and the ability to import the entire library through a single import module com.sedmelluq.discord.lavaplayer; statement as of Java 24 could potentially useful for many.
If not, I will just close this request to avoid bothering you again.
@topi314 Hi, I'm sorry to bother you again. I understand you do not have much experience working with the Java module system, but I would like to ask if there is any circumstance in which your team would be willing to merge this request, and if so what changes you would like made to my edits, and are there any things you would like clarified about my edits?
If not, I will just close this request to avoid bothering you again.
I believe topi has made it very clear already.. not sure what you did not understand from what he said
@topi314 Hi, I'm sorry to bother you again. I understand you do not have much experience working with the Java module system, but I would like to ask if there is any circumstance in which your team would be willing to merge this request, and if so what changes you would like made to my edits, and are there any things you would like clarified about my edits? If not, I will just close this request to avoid bothering you again.
I believe topi has made it very clear already.. not sure what you did not understand from what he said
There's no need to be confrontational. The reasons I was given in a previous message is that they haven't used the module system before, there is no reason to hide any symbol, and that it would be additional overhead to maintain. While I understand there is a reluctance, at least to me it came off as coming from a place of not previously working with them rather than definitively believing they would be a detriment. That's why I'm asking again.
Once again, I'm not trying to force them to merge the request, but I do want the feature to be added. I want to at least gauge what would need to change so that it could be added.
We have (briefly) discussed this and I believe we came to the conclusion that it's not something we're really interested in at this point in time. Neither Topi or I have experience with the module system and it seems like yet another thing to maintain which, for me, I don't really want to deal with currently.
