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

Implicit task dependency when using Gradle 8 + KMP custom cinterop #2977

Closed
chrisbanes opened this issue Apr 24, 2023 · 9 comments
Closed

Implicit task dependency when using Gradle 8 + KMP custom cinterop #2977

chrisbanes opened this issue Apr 24, 2023 · 9 comments
Labels
bug runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin

Comments

@chrisbanes
Copy link

chrisbanes commented Apr 24, 2023

Describe the bug
You can see the issue here: cashapp/redwood#823. We're trying to upgrade from Gradle 7.6 -> 8.x and see implicit task dependency issues:

- Gradle detected a problem with the following location: '/Users/runner/work/redwood/redwood/.gradle/kotlin/kotlinTransformedCInteropMetadataLibraries/app.cash.zipline-zipline-0.9.17-nativeMain-cinterop/app.cash.zipline_zipline-cinterop-quickjs-wkj8jQ.klib'.
    
    Reason: Task ':redwood-treehouse:dokkaHtmlPartial' uses this output of task ':redwood-protocol-widget:transformIosMainCInteropDependenciesMetadataForIde' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed.
    
    Possible solutions:
      1. Declare task ':redwood-protocol-widget:transformIosMainCInteropDependenciesMetadataForIde' as an input of ':redwood-treehouse:dokkaHtmlPartial'.
      2. Declare an explicit dependency on ':redwood-protocol-widget:transformIosMainCInteropDependenciesMetadataForIde' from ':redwood-treehouse:dokkaHtmlPartial' using Task#dependsOn.
      3. Declare an explicit dependency on ':redwood-protocol-widget:transformIosMainCInteropDependenciesMetadataForIde' from ':redwood-treehouse:dokkaHtmlPartial' using Task#mustRunAfter.

To Reproduce

git clone https://github.com/cashapp/redwood.git -b renovate/gradle-8.x
cd redwood
./gradlew dokkaHtmlMultiModule

If we remove kotlin.mpp.enableCInteropCommonization=true from gradle.properties, dokkaHtmlMultiModule then builds successfully

Dokka configuration
https://github.com/cashapp/redwood/blob/trunk/build.gradle#L119

Installation

  • Operating system: Linux and MacOS
  • Build tool: Gradle v8.0.2
  • Dokka version: 1.8.10
@chrisbanes chrisbanes added the bug label Apr 24, 2023
chrisbanes added a commit to cashapp/redwood that referenced this issue Apr 25, 2023
@aSemy
Copy link
Contributor

aSemy commented Apr 25, 2023

I think this might be a KGP issue - more info (plus a workaround) in cashapp/redwood#823 (comment)

@IgnatBeresnev IgnatBeresnev added the runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin label Apr 25, 2023
aSemy added a commit to adamko-dev/dokkatoo that referenced this issue Apr 25, 2023
aSemy added a commit to adamko-dev/dokkatoo that referenced this issue Apr 27, 2023
…droidAdapter (#59)

* fix getName() in DokkaSourceSetIdSpec.kt

* add DokkatooAndroidAdapter

* align Kotlin version to match Gradle's embedded Kotlin version

* add KotlinPlatform.AndroidJVM, refactor to rename 'key' to 'displayName'

* rm deprecated helper in TestFixtures

* update test after KotlinPlatform refactor

* tidy build config

* add kotlinx-coroutines dependency to DokkaGenerator classpath

* big ol' refactor of DokkatooKotlinAdapter

- tidy up the complication/source set details extractors
- move extractors into specific companion objects
- update logic to better match DGP code

* update API dump

* add link to Kotlin/dokka#2977

* refactor DokkatooKotlinAdapter to better organise logic into specific builder utility classes

* update conventions for Android/JDK documentation links, so they're enabled dynamically based on the analysis platform

* add cacheRoot comment

* sort KotlinPlatform alphabetically

* tidy up KotlinAdapter, update AndroidAdapter to fetch android.jar, update the configuration-files-fetch logic to fetch JAVA_RUNTIME files

* lots of changes, so bump the minor version

* remove conditional default for enableJdkDocumentationLink - it seems to result in the wrong Java version in the KotlinMultiplatformExample test...?
jeffdgr8 added a commit to jeffdgr8/kotbase that referenced this issue May 17, 2023
Partially workaround Kotlin/dokka#2977
Still need to run transformCommonMainDependenciesMetadata and fix missing files in paging modules before publishToMavenLocal succeeds
Nek-12 added a commit to respawn-app/KMPUtils that referenced this issue May 29, 2023
@Nek-12
Copy link

Nek-12 commented May 29, 2023

Solution I found for Kotlin DSL approach

subprojects {
    apply(plugin = rootProject.libs.plugins.dokka.id)
    tasks {
        val taskClass =
            "org.jetbrains.kotlin.gradle.targets.native.internal.CInteropMetadataDependencyTransformationTask"
        withType(Class.forName(taskClass) as Class<Task>) {
            onlyIf { gradle.taskGraph.allTasks.none { it is AbstractDokkaTask } }
        }
    }
}

Nek-12 added a commit to respawn-app/KMPUtils that referenced this issue May 29, 2023
@jeffdgr8
Copy link

jeffdgr8 commented May 31, 2023

I've been testing the Kotlin 1.9.0 Beta and RC builds and have found some issues this workaround doesn't fully address.

With a build-logic composite build, I get this error:

Could not determine the dependencies of task ':lib-A:dokkaHtml'.
> Cannot change dependencies of dependency configuration ':lib-A:nativeMainImplementationDependenciesMetadata' after it has been resolved.

If I change the build-logic composite build to buildSrc instead, it works in this case.

Also, running Dokka tasks on a clean build now results in this error (where lib-B depends on lib-A):

Execution failed for task ':lib-B:dokkaHtml'.
> java.io.FileNotFoundException: .../lib/lib-B/build/kotlinTransformedCInteropMetadataLibraries/nativeCommonMain/.lib-B-nativeCommonMain.cinteropLibraries (No such file or directory)

Running transformCommonMainDependenciesMetadata manually before then running the task tree that includes Dokka tasks fixes this in some cases. Although I found a module where it didn't work (lib-C, which depends on lib-B and lib-A transitively). Thebuild/kotlinTransformedCInteropMetadataLibraries directory was still missing after running transformCommonMainDependenciesMetadata. I found the module contained an ios intermediate source set that the other modules did not. After removing the intermediate source set (which was unneeded), it worked like lib-B.

jeffdgr8 added a commit to jeffdgr8/kotbase that referenced this issue Jun 1, 2023
Composite build doesn't work with Dokka workaround: Kotlin/dokka#2977 (comment)
@Nek-12
Copy link

Nek-12 commented Jun 1, 2023

This approach also has other consequences. We've started seeing failed signature verification and duplicated klib file deployments while we were trying to render javadocs during publishing. The only solution was to disable dokka javadoc generation to get signing back and running.

@jeffdgr8
Copy link

jeffdgr8 commented Jun 1, 2023

Building on the workarounds discussed and the issues I encountered with Kotlin 1.9.0, I came up with this workaround, which makes all Dokka tasks depend on all CInteropMetadataDependencyTransformationTask tasks for all sibling modules (these tasks seem to share output directories within other modules, causing the dependency errors for other modules' tasks).

tasks.withType<AbstractDokkaTask> {
    val className = "org.jetbrains.kotlin.gradle.targets.native.internal.CInteropMetadataDependencyTransformationTask"
    @Suppress("UNCHECKED_CAST")
    val taskClass = Class.forName(className) as Class<Task>
    parent?.subprojects?.forEach {
        dependsOn(it.tasks.withType(taskClass))
    }
}

I've been able to run Dokka tasks successfully from a clean build with Kotlin 1.9.0 with this workaround.

Note, this workaround still doesn't work with a build-logic composite build (which still gets this error), but does with buildSrc.

jeffdgr8 added a commit to jeffdgr8/kotbase that referenced this issue Jun 3, 2023
Partially workaround Kotlin/dokka#2977
Still need to run transformCommonMainDependenciesMetadata and fix missing files in paging modules before publishToMavenLocal succeeds
jeffdgr8 added a commit to jeffdgr8/kotbase that referenced this issue Jun 3, 2023
Composite build doesn't work with Dokka workaround: Kotlin/dokka#2977 (comment)
hoc081098 added a commit to hoc081098/kmp-viewmodel that referenced this issue Jul 11, 2023
@JakeWharton
Copy link

Do you know what is the interaction with an includeBuild that changes the behavior so dramatically? I'm trying to upgrade Redwood again and stuck trying to figure out the right incantation to unblock our upgrade without losing all docs.

@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Aug 9, 2023

@JakeWharton we haven't had time to investigate the includBuild problem specifically, but there have been a number of similar problems with KGP 1.9.0, and I believe this one falls under the same category.

Long story short, Dokka's tasks try to resolve the project's classpath by calling KGP's API, which transitively triggers some KGP tasks, and they fail in one way or another, depending on the project.

We have a workaround for Dokka 1.9.0 (#3081), which we have just begun testing and should release some time next week, but I don't know if it addresses this particular problem. If you want, I can publish a dev version that you can try, or you can wait for us to test and release 1.9.0. Either way, the problem is significant enough, and we'll do our best to address it if not in 1.9.0, then in 1.9.10.

I'll provide more details and try to reduce the spread of information about 1.9.0 in the upcoming days.

As a side note: really sorry to hear this is happening. People struggling with Dokka is the last thing that we want. We're just having a rough patch where a lot of things are happening at the same time, and we can't address everything, so we have to prioritise. We do have a long-term solution that will address these sorts of problems with KGP hopefully for good (use the same stable API that IDEA uses to resolve classpath), so don't lose hope in Dokka 😅

@JakeWharton
Copy link

I will gladly test snapshots, dev versions, anything you got! We just moved to be EAP champions as well. Hopefully we can catch problems with EAP/beta/RC releases (whether Dokka-specific or not) earlier than the stable release.

@IgnatBeresnev IgnatBeresnev added this to the Gradle runner 2.0 milestone Aug 17, 2023
oldergod added a commit to square/wire that referenced this issue Aug 30, 2023
Probably related to Kotlin/dokka#2977
oldergod added a commit to square/wire that referenced this issue Aug 30, 2023
Probably related to Kotlin/dokka#2977
@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Sep 1, 2023

Dokka 1.9.0 has been released, and it includes some workarounds for this problem. See the following issue for details:


To avoid the spread of information and have updates in a single place, I'll close this issue and a few others as duplicates in favour of #3153.

@IgnatBeresnev IgnatBeresnev closed this as not planned Won't fix, can't repro, duplicate, stale Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin
Projects
None yet
Development

No branches or pull requests

6 participants