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

JPMS modules #42

Open
kb-1000 opened this issue Feb 4, 2024 · 5 comments
Open

JPMS modules #42

kb-1000 opened this issue Feb 4, 2024 · 5 comments

Comments

@kb-1000
Copy link

kb-1000 commented Feb 4, 2024

While the artifacts currently contain Automatic-Module-Name declarations in the manifest, which should make them usable from other JPMS modules, they don't have an actual module-info, which is required to use them with jlink for example.
Instead of being automatic modules, at least the library artifacts should have support for being loaded as actual named modules.
(Unfortunately, the unmaintained patched ANTLR fork as well as the collections library you use appear to be incompatible with this though)

@holzensp
Copy link
Contributor

holzensp commented Feb 5, 2024

We're looking into moving off of both ANTLR and said collections library. Especially a collections lib is tricky.

It would really help rolling this in if you could flesh out (links to issues etc) this description somewhat (or go straight ahead and submit a PR) It doesn't even seem unreasonable to me to keep a Draft PR going, pending replacement of ANTLR & Paguro removal.

@kb-1000
Copy link
Author

kb-1000 commented Feb 5, 2024

Sure, I could make a PR. Though, shaded JARs could be a problem there (especially pkl-cli-java, afaict), you kinda have to either make them not modules, or relocate the packages of dependencies to avoid any conflicts.
Regarding issue links - upstream ANTLR does have an automatic module name as well which would make depending on it from a named module possible, but no module-info by itself yet (see antlr/antlr4#2946), while the fork doesn't. Paguro seems to be inactive, but I guess I could try to open an issue/send a PR there as well.

@sk-ilya
Copy link

sk-ilya commented Feb 13, 2024

The org.pkl.core.parser.antlr.pklLexer class conflicts with other artifacts using the original version of the antlr-runtime and fails in runtime, due to fully incompatible versions of the ATNDeserializer (at least that). It would be great to either migrate to the original antlr or to move off of antlr.

Meanwhile, can someone think of a workaround to such a conflict?

@sgammon
Copy link
Contributor

sgammon commented Mar 1, 2024

@holzensp I've built a version of Paguro and Antlr4 locally which uses modules, allowing Pkl to build with a full modulepath compile. Since Pkl intends to move off these libraries eventually, they probably won't get version bumped (and see little activity anyway). If you'd like, I can post a PR which uses these libs.

@kb-1000
Copy link
Author

kb-1000 commented Jun 5, 2024

Oh for the record since I don't want to hold up progress here or anything, I was looking into it for a few hours but I'm no longer working on this since the project I wanted pkl for (ideally as JPMS modules) is on hold for now

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

4 participants