kotlinx.coroutines
kotlinx.coroutines copied to clipboard
Add explicit module-info.java for JPMS compatibility
Note that there is still a highlighting issue with IntelliJ IDEA when using Java modules in a multi-platform Kotlin project.
The relevant issues are https://youtrack.jetbrains.com/issue/KTIJ-15498 and https://youtrack.jetbrains.com/issue/KTIJ-19614.
Right now this results in highlighting errors like these:
Unfortunately I cannot use Idea.isActive
because whatever the Gradle script configures is ignored by IDEA.
It just immediately starts analyzing the module graph if it detects a module-info.java
in the source root
(if I understood the logic in https://github.com/JetBrains/intellij-community/blob/master/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java
correctly).
For now I just moved the module-info.java
files to a separate java9
source folder which is included explicitly when compile the module descriptor, similar to how it's done in the kotlinx.serialization
project.
AFAIU, module declarations must also declare all the services with uses
and provides
declarations or the corresponding functionality (like the main dispatcher) will not work. For this purse, it also makes sense to start running all the tests with a module path. This way, these kinds of omissions will be caught during testing.
A short update: the corresponding support of JPMS should land in IDEA 2022.3, so we are ready to proceed with the change.
Could you please rebase it on the most recent develop
when you have time to spare?
I just rebased and can confirm that the highlighting issues described in https://youtrack.jetbrains.com/issue/KTIJ-19614 are resolved in IDEA 2022.3 (EAP)
Also, it seems like some modules cannot be compiled: https://teamcity.jetbrains.com/viewLog.html?buildId=3984907&tab=buildResultsDiv&buildTypeId=KotlinTools_KotlinxCoroutines_Build
AFAIU, module declarations must also declare all the services with
uses
andprovides
declarations or the corresponding functionality (like the main dispatcher) will not work. For this purse, it also makes sense to start running all the tests with a module path. This way, these kinds of omissions will be caught during testing.
I also added the necessary uses
and provides
declarations to the module descriptors. I haven't touched any tests yet, so I have no guarantee that it works as intended though.
@lion7 Is it ready for review or are you still working on it? (I'm not sure if "re-request review" button is available for external contributors, thus the question)
Hey @lion7 -- is there any holdup on this? Having proper JPMS support would really be fantastic.
@qwwdfsad I think this is ready enough for review, I'm not working on it anymore and just rebased it on the latest develop. The only thing that still bothers me is that there are currently no tests executed with a module path, especially after re-reading the comment from @elizarov. I did do some manual testing and it should all work, but there might be regressions or that I missed something. Should that kind of testing also be part of this PR or is it ok to do that in a separate PR?
Thanks! Will take care of it during this week
I've started the internal testing (mostly our internal projects, Ktor and some Android apps) process and, as soon as all perturbations are resolved, will publish the dev build for everybody interested in trying it out.
For now, it seems like JavaFx ahs some non-trivial setup issues: none of the tests is able to start and fails with java.lang.ClassNotFoundException: kotlinx.coroutines.javafx.JavaFxStressTest
(or any other tests).
@lion7 could you please provide any additional pointers here while I'm looking at other modules?
I've applied all the proposed suggestions in #3629, will keep you posted regarding further actions
I've run a few out integration tests, so far so good. Our JPMS-related testing capabilities are limited (because we are not really using JPMS ourselves anywhere), so I've published 1.7.0-jpms-preview
to https://maven.pkg.jetbrains.space/public/p/kotlinx-coroutines/maven
repository; if you are relying on JPMS in your project, consider checking it out.
Merged manually. Thanks!