dokka
dokka copied to clipboard
Use buildSrc convention plugins to configure the Dokka subprojects
Basic groundwork for #2703
I made some good progress but then I got stuck when I reached the integration test config, because it's a bit of a beast.
https://github.com/Kotlin/dokka/blob/83174361becb2af227159834cdf6e14db9300c53/integration-tests/build.gradle.kts
I think most of the config in here should be converted to a specific 'integration test' buildSrc plugin.
@IgnatBeresnev I see you've added the 'Gradle Plugin' label, and while this PR would help with that, it's more about the improving the general Gradle build config of the entire project :)
Yeah, true. I don't want to create too many labels, but I definitely want to mark all Gradle-related PRs in case we ask for anyone's help with review and whatnot
If it makes anyone uneasy, we can certainly create a separate one or rename the existing. We'll discuss it with the team this week, just in case
With regards to labels, what about a 'build config' or 'ci/cd' label that would cover the build scripts, and also GitHub Actions, Qodana, TeamCity etc?
I've made some good progress! All shared build-config now comes from buildSrc convention plugins, not from subprojects{} or allprojects{} blocks. This makes the build much more modular, because it's not restricted by inheritance, and easier to configure because a build script is more self contained - it doesn't have config applied from some arbitrary parent.
After researching #2652, I think I now finally understand why the proposed changes are good and what they do!
However, if we first merge #2652 - it would introduce conflicts. @aSemy would you be OK with that? Looks like you would be able to just move the files over into build-logic.
Either merging this PR or #2652 first would work, but since there are fewer changes in #2652, and converting buildSrc to an included build is a more 'logical' upgrade, I think it would be best if this PR were merged first, and either #2652 is updated, or we create a new similar PR
@aSemy Rebasing?
@Goooler thanks for the review! I'm also taking a look now
Found a minor problem when comparing two versions:
- 1.8.10 RC - 1.8.10-dev-203
- This branch - 1.8.20-dev-204
The configuration for subprojects {} was extracted into the kotlin-jvm convention. Previously, it was applied to the gradle plugin as well, whereas now it's not. This leads to:
- KotlinCompile configuration is not applied for the Gradle plugin, so if you compare the
.modulefiles for the plugin you'll see that"org.gradle.jvm.version": 8was changed to11 - The sources for the gradle plugin are not published. I'll leave a comment on the line that I think causes it
The rest seems to work fine :)
[Optional] Dokka used for publishing used to be 1.7.20, now it's 1.7.10 - got a bit confused when comparing published javadocs :) I'll update the versions after the release anyway, so it's ok to skip it
Hi! Not sure if you noticed that the integration tests for coroutines are unable to be run because of the toolchain being set to 1.8 globally. This refactoring has uncovered quite a number of problems already, which is good, I guess :)
A problem occurred evaluating root project 'kotlinx.coroutines'.
> Failed to apply plugin 'jdk-convention'.
> Project required JDK 11+, but found 1.8
Can you please fix it when you have the time, so that the checks pass? I believe that's the last thing holding up the merge
Not sure what the best solution for the fix is, but I'd say any that builds the same bytecode as before and makes the tests pass. We can revisit our requirements for the used java/kotlin versions later and separately
thanks for checking @IgnatBeresnev, I didn't realise that was the problem.
I set the toolchain for Dokka to use Java 8 because that matches the existing jvmTarget
https://github.com/Kotlin/dokka/blob/ded804e5772399f1495016d598573cb20b673b58/build.gradle.kts#L33
If I set the toolchain to 11, then the :runners:maven-plugin build fails:
> Task :runners:maven-plugin:helpMojo
[INFO] Error stacktraces are turned on.
[INFO] Scanning for projects...
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building dokka-maven-plugin 1.8.20-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-plugin-plugin:3.5.2:helpmojo (default-cli) @ dokka-maven-plugin ---
> Task :plugins:base:frontend:nodeSetup
> Task :plugins:base:frontend:npmSetup SKIPPED
> Task :runners:maven-plugin:helpMojo
[WARNING] Using platform encoding (UTF-8 actually) to read mojo source files, i.e. build is platform dependent!
[INFO] java-javadoc mojo extractor found 0 mojo descriptor.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.425 s
[INFO] Finished at: 2023-03-07T16:44:34+01:00
[INFO] Final Memory: 12M/68M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-plugin-plugin:3.5.2:helpmojo (default-cli) on project dokka-maven-plugin: Execution default-cli of goal org.apache.maven.plugins:maven-plugin-plugin:3.5.2:helpmojo failed: Unsupported class file major version 55 -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-plugin-plugin:3.5.2:helpmojo (default-cli) on project dokka-maven-plugin: Execution default-cli of goal org.apache.maven.plugins:maven-plugin-plugin:3.5.2:helpmojo failed: Unsupported class file major version 55
And there's a warning for :runners:gradle-plugin
> Task :runners:gradle-plugin:compileTestKotlin
'compileTestJava' task (current target is 11) and 'compileTestKotlin' task (current target is 1.8) jvm target compatibility should be set to the same Java version.
By default will become an error since Gradle 8.0+! Read more: https://kotl.in/gradle/jvm/target-validation
Consider using JVM toolchain: https://kotl.in/gradle/jvm/toolchain
It looks like kotlinx.coroutines has a custom check for the JDK version...
https://github.com/Kotlin/kotlinx.coroutines/blob/4116d4a178f32b1241db8247a38cd2823fb8b03e/buildSrc/src/main/kotlin/jdk-convention.gradle.kts#L7-L14
I'll look into it a little more. Perhaps the kotlinx.coroutines int. test requires a little more config.
I set the toolchain for Dokka to use Java 8 because that matches the existing jvmTarget
Yeah, that's the most important part - we cannot break compatibility with Java 8 for now.
We can set jvmTarget to 8 in the kotlin convention + the gradle plugin, but run the whole project with java 11 - I don't mind that. Adding the toolchain is definitely a good idea, even if jvmTarget stays at 8
But whatever works is fine :) We can revisit the used versions later
Tell me once it's ready to be looked at, I'll trigger the teamcity build
@IgnatBeresnev Do you know why the jvm-version is set to 12 in the example workflows?
https://github.com/Kotlin/dokka/blob/2e78b7b79579ee6ed502e2985cbfdf5bb701abee/.github/workflows/gh-actions-artifacts-snapshots.yml#L21
We can set
jvmTargetto 8 in the kotlin convention + the gradle plugin, but run the whole project with java 11 - I don't mind that. Adding the toolchain is definitely a good idea, even ifjvmTargetstays at 8
Toolchains don't let you separately define a language version; the JVM target is set to that of the Java version of the Toolchain. So if the project needs to target Java 8, the Toolchain needs to be Java 8. Don't ask me why jvmTarget can't be set separately...
Fortunately Toolchains are very flexible, so I set the integrationTest task launcher to be Java 11.
https://github.com/Kotlin/dokka/pull/2704/commits/1bbbd9b67a86ec81106f4f64f9c3c5d4e082c663
@IgnatBeresnev Tests have passed, it's ready for the TeamCity build!
@IgnatBeresnev Do you know why the jvm-version is set to 12 in the example workflows?
It was copied from s3-snapshots.yml, but why it's set there to 12 - zero clue :( I think it's fine to set it to 11 like you did
@IgnatBeresnev Tests have passed, it's ready for the TeamCity build!
Thank you once again! Triggered the builds, will merge once they pass :)
@IgnatBeresnev By the way, good job on the GitHub labels! They look really smart, and they're helpful.
@aSemy yeah, going through all of the issues :) Part of the reason is actually for the dokkatoo DSL. In particular, the configuration and the Gradle plugin tags could be of interest to go along with the re-write since we're breaking compatibility anyway
Hi @aSemy.
I've noticed that after this change the artifact dokka-analysis started publishing an additional shadowRuntimeElements variant with the following attributes:
{
"name": "shadowRuntimeElements",
"attributes": {
"org.gradle.category": "library",
"org.gradle.dependency.bundling": "shadowed",
"org.gradle.libraryelements": "jar",
"org.gradle.usage": "java-runtime"
},
"files": [
{
"name": "dokka-analysis-1.8.20-local-1-all.jar",
"url": "dokka-analysis-1.8.20-local-1-all.jar",
"size": 100899229,
...
}
]
}
This is unfortunate, because it causes variant resolution ambiguity with the runtimeElements variant in ...Plugin configurations of manually created DokkaTaskPartial tasks in the stdlib docs build.
Is this an expected consequence of this change?
- if no, could you investigate why it has happened and how to stop publishing this variant?
- if yes, could you provide a guidance how to overcome this variant ambiguity in a project?
I will take a look! I'm not very familiar with the Shadow plugin, but it looks like there are a few issues relating to shadowRuntimeElements https://github.com/johnrengelman/shadow/issues?q=shadowRuntimeElements
What might be a quick fix is disabling publication of this specific configuration, with a similar way to how Test Fixtures publication can be disabled.
val javaComponent = components["java"] as AdhocComponentWithVariants
javaComponent.withVariantsFromConfiguration(configurations["testFixturesApiElements"]) { skip() }
javaComponent.withVariantsFromConfiguration(configurations["testFixturesRuntimeElements"]) { skip() }