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

Package name utils breaks modularized projects that doesn't employ jnigen #60

Open
GustavBW opened this issue Mar 3, 2024 · 5 comments

Comments

@GustavBW
Copy link

GustavBW commented Mar 3, 2024

Hi,
I opened an issue in libgdx/libgdx while trying to find a work around to a split package issue (this one: libgdx/libgdx#7355) and would like to ask if it was possible to change the gdx-jnigen-loader package com.badlogic.gdx.utils name to anything else?

TL:DR; Since gdx (v1.12.1) depends on jnigen (v. 2.4.1), and gdx itself is not modularized, even if you're not using jnigen yourself, jnigen becomes a transitive dependency (I think that's the word for it) and causes problems for your project either way.
Nothing prevents gdx from being used in a modular application as far as I can tell, as java can automatically infer it as a module if put on the module path. However, gdx requires jnigen to be present as well, which is why I haven't been able to find a workaround.

Admittedly, I don't have a full comprehension of what this will cause of problems for libgdx itself, but I'm hoping it could be a part of the next release cycle (https://github.com/libgdx/libgdx/milestone/6) and I should personally like to help in any way I can.

Edit: I went through the closed issues to see when if this had been brought up before, and it kind of has: #2 <- the SharedLibraryLoader-merger (the class that causes a ClassNotFoundException if jnigen isn't included as a module in a modular project). I'm not sure the split package issue didn't exist then, but it sounds like what I'm experiencing is an accident following that merger.

@Berstanio
Copy link
Contributor

The only way I am aware of to fix this, is to move the SharedLibraryLoader to a different package in jnigen. However, this would break every project that uses jnigen and I wouldn't know a way to do it backwards compatible. It's true that modularizing libGDX is inherently broken currently, however modules seem to be very unpopular in libGDX (and maybe in general).

For a workaround for your project, you can either fork jnigen or I assume you can just remove the "jnigen" dependency from your project, and copy over these classes into your project in a favorable package: https://github.com/libgdx/gdx-jnigen/tree/master/gdx-jnigen-loader/src/main/java/com/badlogic/gdx/utils

@GustavBW
Copy link
Author

GustavBW commented Mar 12, 2024

The only way I am aware of to fix this, is to move the SharedLibraryLoader to a different package in jnigen. However, this would break every project that uses jnigen and I wouldn't know a way to do it backwards compatible. It's true that modularizing libGDX is inherently broken currently, however modules seem to be very unpopular in libGDX (and maybe in general).

Yeah, I guess moving / renaming existing is essentially the same from a start-of-runtime perspective, and would break anything using the next version if done. Java's JPMS is not very popular that's true, but for my purposes, it would bring me more value than pain to include and is a must for larger projects due to inherent limitations with Java's standard visibility modifiers.

For a workaround for your project, you can either fork jnigen or I assume you can just remove the "jnigen" dependency from your project, and copy over these classes into your project in a favorable package: https://github.com/libgdx/gdx-jnigen/tree/master/gdx-jnigen-loader/src/main/java/com/badlogic/gdx/utils

With the way that the latest release of libgdx is build, jnigen is brought in "implicitly" - as in, I declare no dependency on jnigen myself, but my libgdx dependency does which I can't change. Good idea, putting the SharedLibraryLoader on the classpath myself, I did manage to exclude jnigen explicitly (which ran into the ClassNotFound), so maybe just maybe.
As for shading generally, I'd really rather not - as it would bring no potential value to anyone but myself, and would require me to shade the entirety of libgdx itself as well (updating all jnigen imports).

I do have an alternative, which resolves this issue for a while without breaking any projects: You could pull the contents of jnigen....utils into libgdx...utils when building a jar for a release of libgdx. Maven has a plugin to do this automatically, and gradle probably does too, but this would prevent the dependency from "bubbling up" and causing issues because there would be no dependency on jnigen from libgdx in the first place. I don't know if I could do this from my side, as I don't understand gradle or know exactly what the statements in libgdx' build.gradle "apply plugin: jnigen" does (https://github.com/libgdx/libgdx/blob/master/gdx/build.gradle). But if its some kind of static pre-processing (during build or compile) it shouldn't interfere.

Edit: Clarity, rendundancy.

@Berstanio
Copy link
Contributor

There do be pretty good search and replace tools, and I reckon you could run a simple bash script updating all import statements.

The problem is more complicated in transitive dependency. If jnigen updates the package name, every project that uses jnigen would need to do changes to be compatible again. It would also add a clear compatibilty cut, where two dependencies in different versions are just not compatible. It is preferable to avoid that.

but this would prevent the dependency from "bubbling up" and causing issues because there would be no dependency on jnigen from libgdx in the first place.

We could package the jnigen-loader classes into libGDX and rename the package in jnigen at the same time, which should ensure backwards compatibilty and non-breaking. I don't hate it, but I also don't like this added complexity/duplication in the jar. Not sure what others opinions are on that.

@GustavBW
Copy link
Author

The problem is more complicated in transitive dependency. If jnigen updates the package name, every project that uses jnigen would need to do changes to be compatible again. It would also add a clear compatibilty cut, where two dependencies in different versions are just not compatible. It is preferable to avoid that.

I'm aware. Its indeed not preferable. I mean, people could always choose to stay on an earlier version for their own purposes, so nothing is forced upon them - also those using it and libgdx if libgdx swapped to a breaking version. Do you have any knowledge about how many projects would potentially get broken?

We could package the jnigen-loader classes into libGDX and rename the package in jnigen at the same time, which should ensure backwards compatibilty and non-breaking. I don't hate it, but I also don't like this added complexity/duplication in the jar. Not sure what others opinions are on that.

I would appreciate it so much if you did and I'd love to help. It would mean the world to me and the hundreds of people forced to use a very old version of libgdx (way before #2 which is JPMS compatible) during a course I had at university.
If a breaking change is on the horizon, I reckon one could use the opportunity to modularize, or partially modularize libgdx itself. (Which isn't a breaking change necessarily, but a structural change which could cascade).

@GustavBW
Copy link
Author

GustavBW commented Apr 9, 2024

Update:
I've finally come back around to this after forking jnigen and trying some different things.
First attempt:
Setting some manifest attributes to attempt to have jnigen be picked up by the java runtime as declared module. This did not fool the automatic module resolution, and made no difference.

Second attempt:
Forking and adding a module-info file. This actually worked to a degree and the gdx-jnigen-loader was picked up as a declared module:

PS E:\GitHub\MELANGE\jnigen> jar --describe-module --file=gdx-jnigen-loader-2.5.1-SNAPSHOT.jar
gdx.jnigen.amtest.gdx.jnigen.loader.main jar:file:///E:/GitHub/MELANGE/jnigen/gdx-jnigen-loader-2.5.1-SNAPSHOT.jar!/module-info.class
exports com.badlogic.gdx.utils
requires java.base mandated

But ran straight into the the same issue as gdx itself is not modularized either:

java: the unnamed module reads package com.badlogic.gdx.utils from both gdx and gdx.jnigen.amtest.gdx.jnigen.loader.main
java: module melange.example reads package com.badlogic.gdx.utils from both gdx and gdx.jnigen.amtest.gdx.jnigen.loader.main

Admittedly I've got a lot of dependencies going in the project where I ran this (mono branch of Melange), but not only did it cause issues for me, but also spring and reflections which where also present.

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