-
Notifications
You must be signed in to change notification settings - Fork 470
Fix classpath when source set has no metadata compilations #4296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix classpath when source set has no metadata compilations #4296
Conversation
|
JIC: my attempt to fix |
12c2bb1 to
4dd9e43
Compare
4cddab4 to
685f09b
Compare
d2b6dd7 to
747573f
Compare
685f09b to
62d4bff
Compare
Workaround for intermediate source sets missing dependencies (whether stdlib or user-defined). KT-80551
…actual' declarations
DGP can still produce output even when the code cannot be compiled. To avoid creating invalid examples, add a test.
d249aa3 to
ab74561
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main change of this PR is in KotlinAdapter.
whyoleg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look on the comment in KotlinAdapter
| } | ||
|
|
||
| @OptIn(org.jetbrains.kotlin.gradle.ExperimentalWasmDsl::class) | ||
| wasmJs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add wasmWasi target and declarations in wasmMain, so that we can be sure, that we test the original issue from kotlinx-io?
webMain and wasmMain might behave differently I believe.
Note: that wasmMain is not created automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a functional test that adds a custom shared WasmJS/WasmWASI source set. I think it's best to avoid adding custom source sets into the example projects, so as to keep them simpler.
There should be no differences between a shared source set that's provided by KGP and one that's defined by users.
dokka-runners/dokka-gradle-plugin/src/main/kotlin/adapters/KotlinAdapter.kt
Show resolved
Hide resolved
|
|
||
| // This KotlinSourceSet has no primary compilations, therefore it must be an intermediate source set | ||
| // e.g. mingwMain or webMain. | ||
| // If there are no metadata compilations then we must include the classpaths of the actual targets, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, in case of webMain or wasmMain, which compilations will be there?
For some reason, I think that at least in the case of wasmMain we will have both wasmJs and wasmWasi compilations, and so we will add classpath from both of them, potentially having conflicting klibs.
For webMain, on the other hand, I believe that a shared compilation already exists, but I might be wrong.
All of these points indicate that this PR will address #3386, and not #4116.
I do think that we need a functional test here, which will check the classpaths of source sets.
This recreates the issue reported in #4116
452538e to
55e4830
Compare
|
I've discussed more with @abdulowork and we think we've found a better workaround. We can use IdeMultiplatformImport to get the same data that IJ uses. I'll have another attempt... |
When an intermediate KotlinSourceSet has no metadata compilation (e.g. webMain, mingwMain), the classpath misses the required dependencies (e.g. kotlin-stdlib, or those defined in buildscripts).
This PR updates the logic for determining the classpath. If a source set has no primary compilations, and it has no metadata compilations, then the classpath should be aggregated from the 'leaf' source sets.
Additionally, update kotlin-multiplatform-example to compile(!), and test webMain.
Workaround for: