Skip to content

Conversation

whyoleg
Copy link
Collaborator

@whyoleg whyoleg commented Sep 15, 2025

Depends on / Blocked by #4263

Found in #4240 (comment)

Coil, and maybe some other projects, have a custom shared source set which is shared between jvm and android targets. Sometimes, in that case, JDK classes could be used in those source sets. While Kotlin doesn't officially support this use case:

  • KT-76097 Add experimental support for shared JVM and Android source set
  • KT-42466 HMPP: JVM & Android intermediate source set
  • KT-42468 HMPP: JVM and Android intermediate source set publication
  • and maybe more...

Still, IDEA does support this use case, and both types and KDoc links are resolved against the JDK classes:

image

WDYT about replicating this behavior in DGPv2, so that if there is a shared source set between jvm and android, we will use JVM instead of common platform during analysis?

As mentioned in #4240 (comment), the same could be done on the user's side, which solves this issue.

@whyoleg whyoleg self-assigned this Sep 15, 2025
@atyrin
Copy link
Contributor

atyrin commented Sep 15, 2025

Could you please add a test for the scenario? At least a simple integration.

@adam-enko
Copy link
Member

I think we'll also need to adjust this code to support webMain (or, more generally, any shared WasmJS/JS source sets) - see #4116.

I'm just sharing it as it comes to mind. We don't have to update the code in this PR, but maybe there's an elegant way of fixing the problem here too?

@vmishenev
Copy link
Contributor

I think we'll also need to adjust this code to support webMain

As I remember, there is the same issue with JS+WasmJS sourceset in Kotlin Wrappers

@whyoleg
Copy link
Collaborator Author

whyoleg commented Sep 16, 2025

I think we'll also need to adjust this code to support webMain (or, more generally, any shared WasmJS/JS source sets)

I don't think it's the same case — those are two different issues, and they have two underlying mechanisms/problems.

Kotlin, at this moment, has the ability to have a shared source set, which is also analyzed not as common, but as a platform-specific one only for native. But, in case of shared source set between jvm and android-jvm, we can still try to analyze it as a jvm, because in most of the cases, it will work, as Android SDK has most of the JDK APIs. This is probably how it works in IDEA. And it's still just a workaround, which is nice to have, because, as I said, Kotlin doesn't properly support this, and so this intermediate source-set might have problems with publication/consumption (one of the issues in the description).

The case with a shared source set between js and wasm-js is totally different because in that situation, we will have a source set with different platforms. So, the only platform with which we could analyze that source set is common. And it's correct, and it's how klib backends and compilers work, AFAIK.

The case with webMain, shared between js and wasm-js or wasm-js and wasm-wasi, should work correctly if KGP provides correct classpaths, or we will somehow propagate those manually (#4116 (comment))

@whyoleg
Copy link
Collaborator Author

whyoleg commented Sep 16, 2025

Could you please add a test for the scenario? At least a simple integration.

The test is added in 1252dc9, and thanks to it, a bug was discovered (1a205ec), which is required to be fixed for the original changes to work correctly, as otherwise, allPlatforms will always have common platform coming from metadata target.

@whyoleg
Copy link
Collaborator Author

whyoleg commented Sep 17, 2025

blocked by #4263

@whyoleg whyoleg marked this pull request as draft September 17, 2025 09:07
@whyoleg whyoleg force-pushed the whyoleg/default-jvm-platform branch from 79d20ef to ffdd9f5 Compare September 18, 2025 16:00
@whyoleg whyoleg changed the base branch from master to whyoleg/dgp-isMetadata September 18, 2025 16:01
@whyoleg whyoleg added the blocked: AA Changes required in Analysis API label Sep 18, 2025
@whyoleg whyoleg removed the blocked: AA Changes required in Analysis API label Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants