ktorio / ktor

Framework for quickly creating connected applications in Kotlin with minimal effort
https://ktor.io
Apache License 2.0
13.09k stars 1.07k forks source link

Add modulepath support for Java >= 9 #1137

Closed remal closed 2 years ago

remal commented 5 years ago

Ktor application can't be compiled with JDK >=9. I'm getting a lot of such errors:

error: module kotlinx.io.jvm reads package io.ktor.http from both ktor.server.core and ktor.http.jvm
error: module kotlinx.io.jvm reads package io.ktor.http.content from both ktor.server.core and ktor.http.jvm
error: module kotlinx.io.jvm reads package io.ktor.util from both ktor.server.core and ktor.utils.jvm
error: module kotlinx.coroutines.jdk8 reads package io.ktor.http from both ktor.server.core and ktor.http.jvm
error: module kotlinx.coroutines.jdk8 reads package io.ktor.http.content from both ktor.server.core and ktor.http.jvm
error: module kotlinx.coroutines.jdk8 reads package io.ktor.util from both ktor.server.core and ktor.utils.jvm
error: module kotlinx.coroutines.core reads package io.ktor.http from both ktor.server.core and ktor.http.jvm
error: module kotlinx.coroutines.core reads package io.ktor.http.content from both ktor.server.core and ktor.http.jvm
error: module kotlinx.coroutines.core reads package io.ktor.util from both ktor.server.core and ktor.utils.jvm
...................

Support of Java 8 ends quite soon, so it makes sense to start supporting Jigsaw. IMHO, this should be prioritized, as it will require breaking backward compatibility. The less Ktor is popular, the easier would be to break backward compatibility. And Ktor will definitely become more popular over time.

Ktor Version

1.2.0

JVM Version, Operating System and Relevant Context

JDK 12

remal commented 5 years ago

I'd like to add that Java 8 support ends in 3 years. No serious project can rely on such short period of platform support.

robstoll commented 5 years ago

I have the same problem here, I see multiple solutions to circumvent the split package problematic:

In the long run I would suggest to go with the first approach, IMO the second is more a workaround. Of course, this implies a breaking change and is therefore only valid for 2.x (migrating to it/deprecate old packages could already be done in 1.x though)

alkoclick commented 5 years ago

This issue also affects any other kotlin web framework that uses ktor as a dependency, such as kweb. I think that @robstoll's suggestions both make sense - a fat jar is an immediate fix that will unlock Java 9+ users, while solving the split packages is the eventual mid/long term solution

chris-hatton commented 5 years ago

This is surely a critical issue to solve; with Ktor being such a new and forward looking framework and others trying to create similarly forward looking libraries and Applications that reference it - we can't afford to be stuck in Java 8 land any more.

Ktor is already well modularised by artifact; that just needs to be formalised for Java, with a Java Module per Ktor module. There will have to be a brief 'breaking' pain of renaming the packages-per-module - but considering the number of dependents on Ktor is set to grow fast - the sooner the better!

Dico200 commented 5 years ago

Perhaps it would be better for the compiler to provide a workaround by moving some definitions between packages and delegating from the original? There wouldn't be any breaking change. I don't know what these package restrictions are about though...

rajohns08 commented 4 years ago

Does Ktor still not support anything above Java 8?

marshallpierce commented 4 years ago

FWIW I've been running ktor on java 11 for years now (via the classpath, not the module path), and ./gradlew jvmTest passes locally for me on 11.0.7, albeit with one test occasionally failing from a timeout. It won't run on 14 because the version of gradle is too old.

rajohns08 commented 4 years ago

Is there an official Ktor page somewhere that mentions supported java versions?

e5l commented 4 years ago

We're continuously testing against JDK 11 for a year and everything works well!

I think that issue is obsolete. Closed.

robstoll commented 4 years ago

@e5l and do you use module-path or classpath?

alkoclick commented 4 years ago

I think the underlying problem remains unaddressed, unless I missed something? Does Ktor run in the modulepath? If not we'll have to immediately open up a new issue and closing this one feels a bit off

e5l commented 4 years ago

Sure. I will setup the CI test with that check

Patresss commented 4 years ago

FWIW I've been running ktor on java 11 for years now (via the classpath, not the module path),

@marshallpierce Could you provide an example? Did you use the build.gradle file for this?

marshallpierce commented 4 years ago

@Patresss I maintain a demo of some patterns I've used successfully with ktor at https://bitbucket.org/marshallpierce/ktor-demo/src/master/. It's pretty similar to a number of services that I work with that are serving real traffic.

Patresss commented 4 years ago

Thanks! I thought that you have added the Ktor library via the classpath and the rest of all via the module path. That's why I was curious about how you could do it.

remal commented 4 years ago

Guys, isn't it a good time for 2.* version with renamed packages? I understand that nobody wants to implement a change that breaks backward compatibility, but sooner or later it should be fixed anyway. And the sooner it's fixed, the less people and projects are affected.

Despite a lot of hacks with compiler arguments can be found on the Internet, they are still hacks and are very inconvenient to use.

Patresss commented 4 years ago

@remal I am also waiting for this fix, but I am not sure that it is possible at a library level. I found the same issue in project Kotest. Please look at this comment: https://github.com/kotest/kotest/issues/1495#issuecomment-640303895

with Kotlin MPP you can't just rename them because you implement platform specific code by use of actual / expect keywords for functions in the same package.

remal commented 4 years ago

@Patresss looks like a design issue for me. In this case I think it should be fixed ASAP to minimize affected projects count.

robstoll commented 4 years ago

IMO should be possible also in MPP project, you usually don't consume two platforms of the same artifact. I.e no split package problematic there. I did not run into issues so far with Atrium (https://github.com/robstoll/atrium) though I have never checked it with jlink. so far I relied on the checks of the Kotlin compiler and unfortunately it is buggy (so :crossed_fingers:).

oleg-larshin commented 4 years ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

WIPShriharsh commented 3 years ago

Hello team, I am working on a project where Ktor is implemented already. Now, we are trying to move to modular approach with Java11. I am getting following error while building the project

error: the unnamed module reads package io.ktor.network.sockets from both ktor.network.jvm and ktor.client.core.jvm
error: the unnamed module reads package io.ktor.network.tls.certificates from both ktor.network.tls.certificates.jvm and ktor.network.tls.jvm
error: the unnamed module reads package io.ktor.util from both ktor.server.core.jvm and ktor.utils.jvm
error: the unnamed module reads package io.ktor.util.date from both ktor.server.core.jvm and ktor.utils.jvm
error: the unnamed module reads package io.ktor.http from both ktor.server.core.jvm and ktor.http.jvm
error: the unnamed module reads package io.ktor.http.content from both ktor.server.core.jvm and ktor.http.jvm
error: module ktor.server.core.jvm reads package io.ktor.network.sockets from both ktor.network.jvm and ktor.client.core.jvm
error: module ktor.server.core.jvm reads package io.ktor.network.tls.certificates from both ktor.network.tls.certificates.jvm and ktor.network.tls.jvm
+120 more lines

Since, I have been studying Jigsaw implementation for long now and have faced this issue in my projects as well, I can tell u few things.

I believe ktor is not build to support java modules. That is why gradle is creating unnamed modules while building. That is why, we are getting an "unnamed module" Now, starting from java9, we should "not "have 2 packages of same name in 2 modules. Comes the second part of error: ktor.network.jvm and ktor.client.core.jvm both have a package io.ktor.network.sockets. You need to rename at least one package.

Please fix this ASAP. Java modularity approach is being implemented in lot of places now.

david-gang commented 3 years ago

Hello team, I am working on a project where Ktor is implemented already. Now, we are trying to move to modular approach with Java11. I am getting following error while building the project

error: the unnamed module reads package io.ktor.network.sockets from both ktor.network.jvm and ktor.client.core.jvm
error: the unnamed module reads package io.ktor.network.tls.certificates from both ktor.network.tls.certificates.jvm and ktor.network.tls.jvm
error: the unnamed module reads package io.ktor.util from both ktor.server.core.jvm and ktor.utils.jvm
error: the unnamed module reads package io.ktor.util.date from both ktor.server.core.jvm and ktor.utils.jvm
error: the unnamed module reads package io.ktor.http from both ktor.server.core.jvm and ktor.http.jvm
error: the unnamed module reads package io.ktor.http.content from both ktor.server.core.jvm and ktor.http.jvm
error: module ktor.server.core.jvm reads package io.ktor.network.sockets from both ktor.network.jvm and ktor.client.core.jvm
error: module ktor.server.core.jvm reads package io.ktor.network.tls.certificates from both ktor.network.tls.certificates.jvm and ktor.network.tls.jvm
+120 more lines

Since, I have been studying Jigsaw implementation for long now and have faced this issue in my projects as well, I can tell u few things.

I believe ktor is not build to support java modules. That is why gradle is creating unnamed modules while building. That is why, we are getting an "unnamed module" Now, starting from java9, we should "not "have 2 packages of same name in 2 modules. Comes the second part of error: ktor.network.jvm and ktor.client.core.jvm both have a package io.ktor.network.sockets. You need to rename at least one package.

Please fix this ASAP. Java modularity approach is being implemented in lot of places now.

hi @WIPShriharsh i am akso waiting for this issue, the main activity of ktor issues moved to youtrack. you an add the info there https://youtrack.jetbrains.com/issue/KTOR-619

vidicunt commented 3 years ago

I'm waiting for this as well. It's been so long, I really need ktor to support module support.

e5l commented 2 years ago

Fixed with 2.0.0