ethauvin / urlencoder

A simple defensive library to encode/decode URL components.
Apache License 2.0
38 stars 4 forks source link

Support Java 8 #18

Open krzema12 opened 1 month ago

krzema12 commented 1 month ago

Hi! We use the library in https://github.com/krzema12/snakeyaml-engine-kmp. There's a request to support Java 8 (https://github.com/krzema12/snakeyaml-engine-kmp/issues/202), however urlencoder is compatible with Java 11+:

https://github.com/ethauvin/urlencoder/blob/dd3d500496b8768a89ff94d1cd0343e739a72940/buildSrc/src/main/kotlin/buildsrc/conventions/lang/kotlin-multiplatform-base.gradle.kts#L46-L50

Would it be possible to support Java 8 as well? Technically it's an LTS release that is still supported by some major vendors.

ethauvin commented 1 month ago

Hi @krzema12!

I don't recall specifically, but I believe we couldn't support Java 8 because of some KMP restrictions.

@aSemy might be able to clarify some more.

aSemy commented 1 month ago

The config specifying Java 11 was already defined before I started updating the project to KMP :)

https://github.com/ethauvin/urlencoder/blob/34b69a7d1f3570aa056285253376ed7a7bde03d8/lib/build.gradle.kts#L50-L54

ethauvin commented 1 month ago

@aSemy Ah. Thanks for looking into it.

It's up to you if you want to support Java 8. It's over 10 years old and no-longer receiving public updates.

krzema12 commented 1 month ago

@ethauvin so here's the full story.

The dependency chain looks as follows: kaml -> snakeyaml-engine-kmp -> urlencoder. Some time ago, kaml was converted to a Kotlin Multiplatform library, previously only the JVM was supported. This conversion involved switching from snakeyaml-engine to snakeyaml-engine-kmp. Apparently as a result, a regression was introduced and kaml no longer supports Java 8: https://github.com/charleskorn/kaml/discussions/580. We asked the user why he needs Java 8, and it's a limitation of some tool related to Minecraft: https://github.com/krzema12/snakeyaml-engine-kmp/issues/202#issuecomment-2211733386

I'd say, if supporting Java 8 is as trivial as adjusting the two values mentioned by @aSemy, I think we can do it for the sake of fixing the regression in the upstream project.

ethauvin commented 1 month ago

@krzema12 It's a little more complicated than that.

If @aSemy is willing to make the changes and test them, I'll publish a new version. I just don't have the bandwidth for much more than that right now.

aSemy commented 1 month ago

It'd be nice to be able to build with Java 21, but set the Java target to 8, and then use Java 8 to run the tests.

I've been trying to learn how to do that with KGP, but I've always struggled. So this would be a good opportunity to learn...

But yes, I also think that Java 8 is too old and should die 😄

I'll see if I have time this week to make a PR.