jwtk / jjwt

Java JWT: JSON Web Token for Java and Android
Apache License 2.0
10.13k stars 1.32k forks source link

Enable JDK9+ module-info #519

Open skjolber opened 4 years ago

skjolber commented 4 years ago

I'm trying to use modules in a support library, but there seems to be no module-info for jjwt. I suggest following either Jackson's or SLF4J's approach.

skjolber commented 4 years ago

First add a default module name, then later module info (a bigger job).

stale[bot] commented 4 years ago

This issue has been automatically marked as stale due to inactivity for 60 or more days. It will be closed in 7 days if no further activity occurs.

skjolber commented 4 years ago

Bump

lhazlewood commented 4 years ago

@skjolber now that 0.11.0 is out and builds/works on JDK 11, is this still needed?

skjolber commented 4 years ago

@lhazlewood can't seem to find any module descriptor or automatic module name entry in the manifest? Ref https://repo1.maven.org/maven2/io/jsonwebtoken/jjwt-impl/0.11.0/

lhazlewood commented 4 years ago

@skjolber correct, we didn't add them to the release - I was curious however if they are still necessary to use in JDK11 given that our builds run and work on JDK11.

skjolber commented 4 years ago

So disabling the module system is possible, and then all will run 'as before'. But that is hardly proper support. You might be interested in this: https://dzone.com/articles/automatic-module-name-calling-all-java-library-maintainers

I have best experience with using the moditech plugin, see this example.

andrewhrytsiv commented 2 years ago

Hi @lhazlewood. Is there a due date for the 0.12.0 milestone?

lhazlewood commented 2 years ago

@andrewhrytsiv Hi! All I can say as an unpaid team of volunteers, we move as quickly as we're able, so we cannot commit to any dates or timelines.

I will say however that there has been a tremendous amount of active work in the last 3 months on the jwe branch and other PRs, so all I can say is that 1) we're working as hard as we can and 2) there are a lot of awesome things about to drop in the next release as soon as we can get it out. I hope that helps!

andrewhrytsiv commented 2 years ago

@lhazlewood I appreciate your work. Thank you for the feedback.

bmarwell commented 2 years ago

@skjolber @lhazlewood I could create such a file using the moditect plugin if you are interested. I did the same for snakeyaml: https://blog.bmarwell.de/2020/12/08/use-snakeyaml-in-a-modular-jlink-distribution.html

WDYT? We would even still be Java 7 compatible if you needed that.

lhazlewood commented 2 years ago

@bmarwell that would be great! Any help here would be much appreciated.

bmarwell commented 2 years ago

@lhazlewood I do have a local sketch-up now, but various things will fail. Some Groovy tests are now restricted to the same level as Java is, e.g. DecodersTest will instantiate new Decoders() which will not be possible anymore. Another one is access to Base64.DEFAULT (which is package-private and fails with 'no such property exception').

Or the call to this package-private constructor in another test:

def decoder = new ExceptionPropagatingDecoder(new Decoder() { … }

There are 42 (of course!) of those in the api project alone.

Anyway, moditect is Java 8 and the jsonb-extension is also java8-only. That means that you wouldn’t be able to verify every feature using Java 7 anymore and release can only be done on Java 8 and above.

I will look further into it, but I am still far away from creating a branch with helpful commits.

lhazlewood commented 2 years ago

@bmarwell makes sense. I'm fine waiting for 1.0 to introduce these changes. I don't want to cause any more churn than is necessary. jwe is by far the highest priority, so anything else that requires more than trivial changes to the codebase or project configuration is best left for after that release.

cowwoc commented 5 months ago

I'm not sure why you guys made this more complex than it has to be. You just need to add a few lines to pom.xml to turn your releases into multirelease JAR files and you're done. Here is the code I lifted from https://medium.com/@ankitagrahari.rkgit/multi-release-functionality-8fc423b6c94e:

<execution>
    <id>java11</id>
    <goals><goal>compile</goal></goals>
    <configuration>
        <release>11</release>
        <compileSourceRoots>${project.basedir}/src/main/java11</compileSourceRoots>
        <multiReleaseOutput>true</multiReleaseOutput>
    </configuration>
</execution>

No need to mess around with Moditect or any 3rd-party plugin. Don't get me wrong, Moditect is a great plugin, but there is an easier way to get this done in this case...

bmarwell commented 5 months ago

No need to mess around with Moditect or any 3rd-party plugin. Don't get me wrong, Moditect is a great plugin, but there is an easier way to get this done in this case...

Yes, but back then it was probably necessary because jjwt did support very old Java versions (at least 7, maybe even 6) and needed to be compile on exactly java 8. Using MRJars would have meant introducing toolchains which is even more configuration compared to moditect which runs on 8. 😃

But yes,, the MR is now probably the better solution.

lhazlewood commented 5 months ago

@cowwoc JJWT still supports JDK 7, so if you'd like to open a PR that supports JDK 7 through 21, we'd appreciate it! 😉

cowwoc commented 5 months ago

@lhazlewood Sorry. No time at the moment... I'll revisit this eventually but it could be months away.