-
Notifications
You must be signed in to change notification settings - Fork 413
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
Fix DGP/KMP integration, so Dokka can 'see' code from shared source sets in target source sets #3814
Fix DGP/KMP integration, so Dokka can 'see' code from shared source sets in target source sets #3814
Conversation
…ndents # Conflicts: # dokka-runners/dokka-gradle-plugin/src/main/kotlin/internal/uriUtils.kt
- Filter out legacy KMP metadata compilations (they were retained to support Dokka v1). - Fetch both `jar` and `android-classes-jar` from AGP (probably not necessary, but why not?) - Only use `compilation.compileDependencyFiles` in non-AGP projects. - Add more docs
…ntion set in KotlinAdapter
- implement new JUnit extension to help set up and re-run DGP integration tests with different versions
…ses, tidy Android projects, fix suppress/analysisPlatform defaults.
`fileTree.kt` contains a utility for logging pretty file trees, which is useful in providing human-readable context for test failures. This PR updates the utility to only use NIO Path, rather than the older `java.io.File`. This change was split off from #3814 to make the PR smaller
`fileTree.kt` contains a utility for logging pretty file trees, which is useful in providing human-readable context for test failures. This PR updates the utility to only use NIO Path, rather than the older `java.io.File`. This change was split off from #3814 to make the PR smaller
…ndents # Conflicts: # .gitattributes # build.gradle.kts # dokka-integration-tests/gradle/build.gradle.kts # dokka-runners/dokka-gradle-plugin/src/main/kotlin/tasks/DokkaGenerateTask.kt # dokka-runners/dokka-gradle-plugin/src/testFixtures/kotlin/GradleTestKitUtils.kt # dokka-runners/dokka-gradle-plugin/src/testFixtures/kotlin/fileTree.kt
- Filter out metadata compilations when determining the source set target. - Remove KotlinPlatform warning - it's always triggered for commonMain. - Fetch _all_ dependent source sets, including transitives.
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.
Nice!
Changes to defaults are looks logical for me, only the test placement worries me :)
dokka-runners/dokka-gradle-plugin/src/main/kotlin/adapters/KotlinAdapter.kt
Show resolved
Hide resolved
dokka-runners/dokka-gradle-plugin/src/testFixtures/kotlin/kotestFiles.kt
Show resolved
Hide resolved
import kotlin.io.path.walk | ||
|
||
@Ignored("KMP: References is not linked if they are in shared code and there is an intermediate level between them https://github.com/Kotlin/dokka/issues/3382") | ||
class KmpCommonSourceSharedWithDependentsTest : FunSpec({ |
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 believe this test should be placed into dokka-integration-tests
so that we will be able to check it with different Kotlin versions.
In my understanding, functional tests
in DGPv2 are more about plugin behavior while this test is more about overall Dokka behavior and so should be in IT
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 would probably except here a test which checks that we set correct defaults for DokkaSourceSetSpec platform, dependsOn and etc
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 believe this test should be placed into
dokka-integration-tests
Good idea, done.
Could you check the expected data in dokka-integration-tests/gradle/projects/it-kotlin-multiplatform/expectedData/html
? There are errors, but I'm not sure if any are DGP's responsibility, or are caused by #3382
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.
Hmm, it doesn't look like TeamCity likes the KMP IT. Some of the tests are failing.
https://ge.jetbrains.com/s/ndo3qeb66whvy/tests/overview?outcome=FAILED
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.
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.
Hmmmmm it looks like locally I get Error type: Unresolved type for NativeMainCls
, but not on TeamCity?? Why??
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 difference in OS (linux vs macOS) and so probably the difference is in dependencies passed inside KGP. E.g iOS
targets are not compilable on linux and so probably the classpath (or some other configuration) is somehow different, which causes this behavior.
In this case, I think that for this test probably having just linux
targets should be enough to reproduce the issue, correct?
We could also try to add mingw
target (or androidNative
targets), or add additional intermediate sourceSets somewhere, so that all targets will be cross-compiled on all OSs.
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 could understand it if locally, on my Mac, I would get no errors and on Linux the macOS targets would cause Unresolved type
errors, but I don't see why it's the other way around. I'd like to understand the problem more before trying to work around it, but I'm not sure how.
I'll check with my team.
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 think that it's better to compare configurations on CI (linux) vs on macOS and then think about what could go wrong.
Also, I would propose in any case add second targets for iOS and linux, as Dokka doesn't work well with single target source sets - may be it could be also cause difference in produced configuration in this case.
E.g, we have an issue with K2: #3386, may be something similar also exists in K1.
Again, the first step will be to obtain configuration used to run Dokka and compare them
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.
From looking at the Dokka configuration, it looks correct (both locally and on TeamCity), so I'm going to chalk the failures up to issues with K1 analysis and disable the HTML output validation tests. They're not critical for this PR anyway.
…src-to-dependents
…to integration test, to support testing different KGP versions
31d28b3
to
b58d42f
Compare
| 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 ([#​3701](Kotlin/dokka#3701)) and intersecting sample roots ([#​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 ([#​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 ([#​3590](Kotlin/dokka#3590)). - Enum entries: anonymous and overridden methods are no longer rendered ([#​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
There are 3 main fixes in this PR:
KotlinAdapter#determineKotlinPlatform
). Filter out metadata targets (they're not relevant for target determination), and only fall back toKotlinPlatform.Common
if necessary.compilation.konanTarget.name
instead ofcompilation.target.name
.jar
andandroid-classes-jar
dependencies. For KGP projects, usecompileDependencyFiles
.The remaining changes are to support these fixes:
convention()
to set default values.Should fix KT-70857