Skip to content
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

[K2] KMP: Support intersecting source roots #3701

Open
atyrin opened this issue Jul 19, 2024 · 6 comments
Open

[K2] KMP: Support intersecting source roots #3701

atyrin opened this issue Jul 19, 2024 · 6 comments
Labels
blocked: AA Changes required in Analysis API bug topic: K2 Issues / PRs that are related to the K2 migration. See #2888
Milestone

Comments

@atyrin
Copy link
Contributor

atyrin commented Jul 19, 2024

Sometimes projects may use the same folder as source root in different modules. Like the same dir for several targets in KMP.
It can be done with such syntax:

kotlin {
  sourceSets {
    jsMain {
      kotlin.srcDir('src/browserMain/kotlin')
    }
    wasmJsMain {
      kotlin.srcDir('src/browserMain/kotlin')
    }
  }
}

Possible scenarios for that: Android and JVM source sets or Js and WasmJs. Sharing source sources for them using KMP hierarchies is just painful.

Projects that already use the pattern:

Kotlin with K2 works fine for them.
IJ with Kotlin K2 Analysis also works: it just uses one of the platforms for analysis. And report warning:

Path [/Users/Andrei.Tyrin/IdeaProjects/_external/molecule/molecule-runtime/src/browserMain/kotlin] of module [molecule.molecule-runtime.jsMain] was removed from modules [molecule.molecule-runtime.wasmJsMain]

Dokka at the same time has the special treatment for that (#3239):

Pre-generation validity check failed: Source sets 'android' and 'jvm' have the common source roots: /Users/Andrei.Tyrin/IdeaProjects/_gates/firebase-kotlin-sdk_mpp/firebase-config/src/androidMain/kotlin. Every Kotlin source file should belong to only one source set (module).

There is a ticket to forbid such setup in KGP: https://youtrack.jetbrains.com/issue/KT-62829/Add-a-check-that-two-source-sets-cannot-have-the-same-file-system-directory. But the priority is low.

@atyrin atyrin added bug topic: K2 Issues / PRs that are related to the K2 migration. See #2888 labels Jul 19, 2024
@vmishenev
Copy link
Contributor

Related: #3373

@vmishenev
Copy link
Contributor

There are 3 solutions here:

  1. Support it on AA's side, e.g. https://youtrack.jetbrains.com/issue/IJPL-339/Multi-context-resolve-support-in-IJ-platform
  2. Forbid such setup in KGP - https://youtrack.jetbrains.com/issue/KT-62829/Add-a-check-that-two-source-sets-cannot-have-the-same-file-system-directory
  3. Add a workaround on Dokka's side

Workarounds:

  1. Clone Documentable for common files
    For example,

    sourceSetA {  1.kt, 2.kt; platform: JVM; libsA}
    sourceSetB {  1.kt, 2.kt; platform: Android; libsB}
    

    can be mapped to the following project structure of AA:

    sourceSetA {  1.kt, 2.kt; platform: JVM; libsA}
    sourceSetB {  1.kt; platform: Android; libsB}
    

    2.kt should passed twice in the documentable translator.
    Pros:

    • Relative simplicity of implementation

    Cons:

    • Quite spread logic. It requires changes in the documentable translator and the session builder
  2. Fake sourcesets
    The above-mentioned example can be mapped into:

    sourceSetA {  1.kt,  platform: JVM; libsA}
    sourceSetB {  1.kt; platform: Android; libsB}
    sourceSetAB {  2.kt; platform: JVM???; libsA}
    
    sourceSetA, sourceSetB depends on sourceSetAB (friend dependency)
    

    Disadvantages:

    • The target platform of a fake source set is under question: common can be insufficient. A different platform in a dependent source set can be unsupported(???) (or might be in the future).
  3. Independent AA sessions
    Create an AA session for each source set or only for intersecting source sets.

    Pros:

    • The same symbol can be resolved into different types depending on the platform (is there a real case?)

    Cons:

    • Works only for independent source sets that do not depend on each other
    • Currently, an AA session is resource-consuming

@whyoleg whyoleg added the blocked: AA Changes required in Analysis API label Aug 23, 2024
@whyoleg whyoleg added this to the Dokka 2.1.0 milestone Aug 23, 2024
@whyoleg
Copy link
Collaborator

whyoleg commented Aug 23, 2024

Action point:

  • add logging when running into this situation to create an issue on Dokka GitHub

@vmishenev
Copy link
Contributor

Current tatus: We are awaiting feedback from Dokka 2.0.0.

svc-squareup-copybara pushed a commit to cashapp/misk that referenced this issue Dec 17, 2024
| Package | Type | Package file | Manager | Update | Change |
|---|---|---|---|---|---|
|
[org.jetbrains.dokka:dokka-gradle-plugin](https://github.com/Kotlin/dokka)
| dependencies | misk/gradle/libs.versions.toml | gradle | major |
`1.9.20` -> `2.0.0` |

---

### Release Notes

<details>
<summary>Kotlin/dokka
(org.jetbrains.dokka:dokka-gradle-plugin)</summary>

### [`v2.0.0`](https://github.com/Kotlin/dokka/releases/tag/v2.0.0):
2.0.0

Dokka 2.0.0 focuses on simplifying configuration and preparing for
support of future Kotlin language features. Key highlights include:

- **Dokka Gradle Plugin v2**: revamped Gradle plugin based on
[Dokkatoo](https://github.com/adamko-dev/dokkatoo).
-   **Experimental K2 Analysis**: support for K2 compiler frontend.
- **HTML Improvements**: better accessibility and easier customization.

More details about each of the changes below. See [Dokka
2.0.0](https://github.com/Kotlin/dokka/milestone/31?closed=1) milestone
for the list of all changes.

#### Dokka's Gradle plugin v2 based on
[Dokkatoo](https://github.com/adamko-dev/dokkatoo)

> Please check the [migration
guide](https://kotl.in/dokka-gradle-migration) for the update procedure
and give feedback!

Dokka 2.0.0 introduces significant improvements to Dokka Gradle plugin,
aligning more closely with Gradle best practices:

-   Adopts Gradle types, which leads to better performance.
- Uses an intuitive top-level DSL configuration instead of a low-level
task-based setup, which simplifies the build scripts and their
readability.
- Takes a more declarative approach to documentation aggregation, which
makes multi-project documentation easier to manage.
- Uses a type-safe plugin configuration, which improves the reliability
and maintainability of your build scripts.
- Fully supports Gradle [configuration
cache](https://docs.gradle.org/current/userguide/configuration_cache.html)
and [build
cache](https://docs.gradle.org/current/userguide/build_cache.html),
which improves performance and simplifies build work.

##### Changes from Dokka 2.0.0-Beta

- Forward Dokka Generator messages to Gradle logger
(Kotlin/dokka#3833)
- Add deprecated property to help migrate from JSON Dokka plugin
configuration (Kotlin/dokka#3877)
- Rename `dokkaPublicationDirectory` to `basePublicationsDirectory`
(Kotlin/dokka#3876)
- Rename `dokkaModuleDirectory`, and mark as internal
(Kotlin/dokka#3880)
- Update and add KDoc for DGPv2
(Kotlin/dokka#3842)
- Remove DokkaPluginParametersBuilder
(Kotlin/dokka#3872)
- Increase default max heap of Worker to 2G
(Kotlin/dokka#3913)
- Remove the dependency of `dokkaGenerate` on `dokkaGenerateModule*`
(Kotlin/dokka#3920)
- KT-71784 Fix classpath for KMP shared source sets
(Kotlin/dokka#3942)
- Fix DGP/KMP integration, so Dokka can 'see' code from shared source
sets in target source sets (Kotlin/dokka#3814)

#### Dokka's K2 analysis

Dokka 2.0.0 introduces **K2 analysis**, which is currently in an
experimental stage. Dokka's K2 analysis leverages Kotlin's K2 compiler
frontend for analysing code and uses the same shared Analysis API, which
is used in [IntelliJ IDEA’s K2
mode](https://blog.jetbrains.com/idea/2024/11/k2-mode-becomes-stable/).
The output with K2 analysis should mostly match that of K1 analysis, but
there may be minor differences. We are actively working towards
stabilizing K2 analysis and are planning to enable K2 analysis by
default in future.

To opt in to Dokka's K2 analysis, add the following flag to your
project's `gradle.properties` file:

```properties
org.jetbrains.dokka.experimental.tryK2=true
```

##### Known limitations:

- Intersecting source roots
([#&#8203;3701](Kotlin/dokka#3701)) and
intersecting sample roots
([#&#8203;3373](Kotlin/dokka#3373)) may cause
issues.
- Kotlin/JS libraries compiled with the [legacy Kotlin/JS
compiler](https://kotlinlang.org/docs/js-ir-compiler.html) are not
supported ([#&#8203;3751](Kotlin/dokka#3751)).

We would greatly value your feedback if you encounter any of these
limitations.

##### Potential differences between the outputs of K1 and K2:

While the output of K2 analysis aims to align with K1, there are some
differences to be aware of:

- Java synthetic properties: rendering may vary
([details](Kotlin/dokka#2888 (comment))).
- KDoc links: resolution and rendering differences
([details](Kotlin/dokka#2888 (comment))).
- Inconsistent documentable rendering order
([#&#8203;3590](Kotlin/dokka#3590)).
- Enum entries: anonymous and overridden methods are no longer rendered
([#&#8203;3129](Kotlin/dokka#3129)).
- Other differences may arise due to variations in type inference
between K1 and K2.

#### HTML format

Dokka 2.0.0 introduces some changes to HTML output. We updated the
structure of some elements and classes, particularly in the navigation
and sidebar, to improve accessibility and simplify maintenance. These
changes only affect you if you previously
[customized](https://kotlinlang.org/docs/dokka-html.html#customization)
Dokka styles.

- We removed redundant wrappers like `navigation--inner` and
`navigation-title`.
- We reworked blocks such as `versions-dropdown` to make them more
accessible.
- We renamed classes like `navigation-controls--homepage` to improve
consistency. For example, it’s now called
`navigation-controls--btn_homepage`.

For more information, see the [pull
request](https://github.com/Kotlin/dokka/pull/3917/files).

The easiest way to update your current templates is to adjust the [new
templates](https://github.com/Kotlin/dokka/tree/master/dokka-subprojects/plugin-base/src/main/resources/dokka/templates)
to your needs.

#### Bugfixes

- Fix merging expect-actual declarations
(Kotlin/dokka#3875)

#### Other changes

- Update the external link URL for stdlib
(Kotlin/dokka#3938)

#### Feedback

We would appreciate your feedback!

-   Please report any feedback or problems https://kotl.in/dokka-issues
- Chat with the community visit #dokka in https://kotlinlang.slack.com/
(To sign up visit https://kotl.in/slack)

**Full Changelog**:
Kotlin/dokka@v1.9.20...v2.0.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 6pm every weekday,before 2am
every weekday" in timezone Australia/Melbourne, Automerge - At any time
(no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

GitOrigin-RevId: 585ec9cbb39b481920aae89e0c6884b1e682110a
@sureshg
Copy link

sureshg commented Dec 20, 2024

Dokka 2.0 task is failing (configuration cache enabled) when we have a custom hierarchy templates

 applyDefaultHierarchyTemplate {
    common {
      group("posix") {
        // Using group will add the intermediate source sets
        group("linux")
        group("apple")
      }

      group("jsCommon") {
        withJs()
        withWasmJs()
      }
    }
  }
t/kotlin. Every Kotlin source file should belong to only one source set (module).
     Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701,Source sets 'jsCommon' and 'js' have the common sample roots: ../web/src/test/kotlin. Every Kotlin source file should belong to only one source set (module).
     Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701,Source sets 'jsCommon' and 'wasmJs' have the common sample roots: ...web/src/test/kotlin. Every Kotlin source file should belong to only one source set (module).
     Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701,Source sets 'js' and 'wasmJs' have the common sample roots:.../web/src/test/kotlin. Every Kotlin source file should belong to only one source set (module).
     Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701

Even common dependencies between modules in a multi module kotlin multiplatform projects fails (Here jvm and wasjs modules depend on common)

    Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701,Source sets 'common' and 'jvm' have the common sample roots:../shared/src/test/kotlin. Every Kotlin source file should belong to only one source set (module).
     Also, please consider reporting your user case: https://github.com/Kotlin/dokka/issues/3701,Source sets 'common' and 'wasmJs' have the common sample roots:.. shared/src/test/kotlin. Every Kotlin source file should belong to only one source set (module).

@whyoleg
Copy link
Collaborator

whyoleg commented Jan 14, 2025

Dokka 2.0 task is failing (configuration cache enabled) when we have a custom hierarchy templates

Hey! is it possible to provide a reproducer (GitHub repo or zip)?
As message says have the common sample roots, most likely you provide the same sources root for different source sets in samples.
Do you think it's possible at your side to change it, so that there will be no intersected source sets ?(e.g like in this change)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked: AA Changes required in Analysis API bug topic: K2 Issues / PRs that are related to the K2 migration. See #2888
Projects
None yet
Development

No branches or pull requests

4 participants