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

Fix Freshmark compatibility with JDK 15+ (fixes #803) #1304

Merged

Conversation

beegee1
Copy link

@beegee1 beegee1 commented Sep 2, 2022

No description provided.

@beegee1 beegee1 force-pushed the bugfix/fix-freshmark-on-jdk15-and-higher branch from 273a832 to 225d992 Compare September 4, 2022 15:24
@beegee1
Copy link
Author

beegee1 commented Sep 5, 2022

Hi @nedtwigg, I've implemented the functionality as you've described in #803 , but unfortunately it is not working. The Nashorn library is provided as a JPMS module with a module descriptor file. The service provider infrastructure does not automatically pick up the library's implementation of the script engine. Furthermore, I guess JDK9+ rejects loading the classes from the lib as the --module-path and --add-modules arguments are missing.

Some more information:
https://stackoverflow.com/questions/65265629/how-to-use-nashorn-in-java-15-and-later
https://stackoverflow.com/questions/46288170/is-it-possible-to-mix-class-path-and-module-path-in-javac-jdk-9/46289257#46289257
https://github.com/openjdk/nashorn/blob/main/src/org.openjdk.nashorn/share/classes/module-info.java#L163

This describes how a possible solution could look like: https://stackoverflow.com/questions/48742354/best-approach-to-dynamically-load-modules-classes-in-java Modifying the module path layer, could work, but I cannot directly access these classes as the library must be compatible with Java 8 compilation. Reflection could be used, but hard to read.

@beegee1
Copy link
Author

beegee1 commented Sep 5, 2022

This is a good answer how to load a JPMS module dynamically:
https://stackoverflow.com/questions/62742150/how-do-i-dynamically-load-modules-from-a-directory-in-java-9

@beegee1
Copy link
Author

beegee1 commented Sep 5, 2022

Should loading the Nashorn library/module be implemented with reflection or should another small lib be implemented which uses Java9+ and is loaded dynamically into the classpath and then a static method is called to load the Nashorn module?

@nedtwigg
Copy link
Member

Thanks so much for tackling this!

should... be implemented with reflection or should another small lib

Another possible approach is to look at

They are two PR's but should have been one.

nedtwigg added a commit to diffplug/blowdryer-diffplug that referenced this pull request Jan 28, 2023
nedtwigg added a commit to diffplug/blowdryer-diffplug that referenced this pull request Jan 28, 2023
@nedtwigg
Copy link
Member

nedtwigg commented Feb 5, 2023

Thanks very much @beegee1 for getting this started! Fixing this required making a silly little change in an upstream project.

@nedtwigg nedtwigg merged commit 523a601 into diffplug:main Feb 5, 2023
@nedtwigg
Copy link
Member

nedtwigg commented Feb 5, 2023

Released in plugin-gradle 6.14.1.

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

Successfully merging this pull request may close these issues.

3 participants