-
Notifications
You must be signed in to change notification settings - Fork 409
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
base: master
Are you sure you want to change the base?
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
…/feat/KT-70336/pass-common-src-to-dependents
…ndents # Conflicts: # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/index.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/navigation.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/org.dokka.it.android.kmp.core/-menu-item/-menu-item.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/org.dokka.it.android.kmp.core/-menu-item/image-vector.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/org.dokka.it.android.kmp.core/-menu-item/index.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/org.dokka.it.android.kmp.core/-menu-item/is-important.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/org.dokka.it.android.kmp.core/-menu-item/label.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/org.dokka.it.android.kmp.core/-menu-item/on-click.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/core/org.dokka.it.android.kmp.core/index.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/material3/index.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/material3/navigation.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/material3/org.dokka.it.android.kmp.material3/-top-app-bar-action.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/material3/org.dokka.it.android.kmp.material3/index.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/navigation.html # dokka-integration-tests/gradle/projects/it-android-compose/expectedData/html/scripts/pages.json # dokka-integration-tests/gradle/projects/it-android/expectedData/html/it-android-0/it.android/-integration-test-activity/index.html # dokka-integration-tests/gradle/projects/it-android/expectedData/html/it-android-0/it.android/index.html # dokka-integration-tests/gradle/projects/it-android/expectedData/html/it-android-0/package-list # dokka-integration-tests/gradle/projects/it-android/expectedData/html/scripts/pages.json # dokka-integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/junit/DokkaGradlePluginTestExtension.kt # dokka-integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/junit/GradleTestProjectInitializer.kt # dokka-integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/junit/TestedVersions.kt # dokka-integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/junit/testTags.kt # dokka-integration-tests/gradle/src/main/kotlin/org/jetbrains/dokka/it/gradle/utils/SemVer.kt # dokka-integration-tests/gradle/src/test/kotlin/AndroidComposeIT.kt # dokka-integration-tests/gradle/src/test/kotlin/AndroidProjectIT.kt # dokka-integration-tests/gradle/src/testTemplateProjectAndroid/kotlin/Android0GradleIntegrationTest.kt # dokka-runners/dokka-gradle-plugin/src/main/kotlin/tasks/DokkaGenerateTask.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 :)
if (compilation.target.platformType == androidJvm) { | ||
attributes { artifactType(artifactType) } | ||
|
||
// Setting lenient=true is not ideal, because it might hide problems. |
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.
just curious: is it possible to somehow log
those problems in case of lenient=true
or are they just silently ignored? just for troubleshooting?
@@ -91,7 +91,7 @@ private fun describeFileDifferences( | |||
/* revisedFileName = */ actualFile.relativeTo(actualDir).invariantSeparatorsPathString, | |||
/* originalLines = */ expectedLines, | |||
/* patch = */ patch, | |||
/* contextSize = */ 3, | |||
/* contextSize = */ 0, |
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.
is it an expected change?
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 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