ocpsoft / prettytime

Social Style Date and Time Formatting for Java
http://ocpsoft.org/prettytime/
Apache License 2.0
1.29k stars 252 forks source link

Missing module-info.java #262

Open webracer999 opened 1 year ago

webracer999 commented 1 year ago

Prettytime does not have a module-info.java (-> Java jigsaw project). So there is no way to use jlink to create a custom java runtime image.

jlink fails with "Error: automatic module cannot be used with jlink: prettytime from [...]/.m2/repository/org/ocpsoft/prettytime/prettytime/5.0.7.Final/prettytime-5.0.7.Final.jar"

For example, two projects that work with jlink: https://github.com/google/gson and https://github.com/remkop/picocli

lincolnthree commented 1 year ago

@webracer999 Thanks for the issue! Would you mind sending a PR to fix this? I'd be happy to review and incorporate it, if possible. I appreciate the heads up.

webracer999 commented 1 year ago

It is easy for Java >= 9 (simply create a module-info.java). But you also support Java 1.8. In Java 1.8 a module-info.java would make trouble as far as I know. I personally don't have the problem as I only use the newest Java version for my private projects. :)

The example projects above (gson and picocli) also support both: Java <= 1.8 and Java >= 9. I need to have a look, how they solved it. I am a bit busy and I am not doing very well at the moment but I will try to investigate and create a PR.

lincolnthree commented 1 year ago

Sorry to hear you aren't doing too well right now. I hope things get better. Thanks for setting the intention to help out. I know very little about JigSaw, so your assistance would make a huge difference!

bowbahdoe commented 12 months ago

The easiest way to solve this is to use moditect. If @webracer999 doesn't get well soon I can make a PR

webracer999 commented 12 months ago

Thank you @lincolnthree and @bowbahdoe.

I started with a PR some days ago (inspired by Gson). Now I have tested the changes and submitted the PR. My OCD really challenged me with testing/submitting the PR and write the comments on GitHub. But it is also a good feeling to have made it.

@bowbahdoe: Feel free to improve the PR.

PS: The BasicJavaApp.java on the website https://www.ocpsoft.org/prettytime/ is missing the line: import java.time.LocalDateTime;

lincolnthree commented 12 months ago

PS: The BasicJavaApp.java on the website https://www.ocpsoft.org/prettytime/ is missing the line: import java.time.LocalDateTime;

Fixed. Thank you!

Is the PR ready for review? It looks pretty clean and simple so far to me. Just an extra Maven plugin/config. I left one comment/question in the PR for you.

Since I am only somewhat familiar with Java 9 Modules (I am familiar with module systems in general) could you give me a quick overview of how this changes the build process?

  1. What are the new commands to build the project, if any?
  2. What new build-outputs are created (or does this modify the existing build output JAR)?
  3. What version of Java / JDK is required to build & compile? (Looks like it's still 8, correct?)

Thanks so much for your hard work on this!

lincolnthree commented 12 months ago

One more question. Why is the module-info.java file checked in to the repo, if Moditect plugin is generating it? Am I misunderstanding something?

webracer999 commented 12 months ago
  1. What are the new commands to build the project, if any?

The Moditect plugin is bound to the package phase - so there is no new command. For my tests I used the command mvn package -DskipTests=true -Darguments="- DskipTests=true -Dmaven.test.skip=true" to build the jar (loosely based on your README.md)

  1. What new build-outputs are created (or does this modify the existing build output JAR)?

No new build-outputs - the existing build output JAR is modified.

  1. What version of Java / JDK is required to build & compile? (Looks like it's still 8, correct?)

I assume you need Java 9+ because Java 1.8 can not compile the module-info.java. But could it be that Moditect compiles it directly?! -> I tried to set my JAVA_HOME environment variable to Java 1.8 and run Maven. The MRJAR gets built with the module_info.class in it. But I get a compilation error regarding samples/jstl/IndexServlet.java:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project sample-jstl: Compilation failure
[ERROR] [...]/prettytime/samples/jstl/src/main/java/org/ocpsoft/prettytime/samples/jstl/IndexServlet.java:[6,21] cannot access javax.servlet.ServletException
[ERROR]   bad class file: [...]/.m2/repository/org/eclipse/jetty/toolchain/jetty-servlet-api/4.0.6/jetty-servlet-api-4.0.6.jar(javax/servlet/ServletException.class)
[ERROR]     class file has wrong version 55.0, should be 52.0
[ERROR]     Please remove or make sure it appears in the correct subdirectory of the classpath.

I get the same error when I try to build the master branch with Java 1.8. Maybe my Java setup is broken there? Could you please try if you can build it with Java 1.8 on your side?

Thanks so much for your hard work on this!

You are welcome. And thank you for prettytime. :)

webracer999 commented 12 months ago

One more question. Why is the module-info.java file checked in to the repo, if Moditect plugin is generating it? Am I misunderstanding something?

I tried it and yes Moditect plugin could also generate the module-info.java. I originally checked it in because Gson does it also. ;) It is a matter of taste like @blutorange wrote in https://github.com/ocpsoft/rewrite/issues/370#issuecomment-1672241325

With the module-info.javachecked in you can easily modify it. So I would prefer this way. BTW: Do you know which packages of prettytime should be visible from outside? Then we could adapt/remove some of the exports-lines in the module-info.java.

But if you wish I could change the Moditect configuration to auto generate the module-info.java.