dokka
dokka copied to clipboard
Fix compatibility with Gradle's configuration cache
Fixes #1217 Fixes #2231
I've tested locally with success.
The changes are fairly simple, but because the configurations are now resolved when executing certain tests, those tests fail if the dependencies aren't available (this includes project dependencies as well as external dependencies).
The way to properly achieve this would be to publish the required artifacts to either mavenLocal()
or a custom local file repository and adding that repository so they're made available to the tests.
Publishing for review first though in case there's another approach to follow.
@3flex hi, what is the state of the pull request? Are you waiting for a review from the team or is it still in development? Do you plan to finish your work?
I intend to finish it - but I have the open question re: testing, see https://github.com/Kotlin/dokka/pull/2499#discussion_r962271895
There are options but I'd like the maintainers to weigh in on what they'd prefer.
Hi! Yeah, sorry it's taking so long to get to: I haven't had the pleasure of dealing with this in any other project, so I also need time (and, more importantly, brain fuel) to research it.
Right off the bat I don't see any other solution apart from publishing it to mavenLocal()
or some file repository :( It is strange though that in order to run unit tests you need to do something like this
I've also worked on an alternative for detekt, based on TestKit and the way it injects plugin classpaths to builds under test when testing Gradle plugins.
https://github.com/detekt/detekt/pull/5149
So that's a third option.
Is there anything I can do to move this along? There's still more code to write and review once a direction is chosen for the testing side of things, so it would be good to get some direction.
Gradle is moving towards making configuration cache stable, possibly for Gradle 8 or soon after. It would be great to get this plugin ready before then.
Hi! We have not forgotten about this PR, but our resources have been spread thin for quite some time, and dealing with Gradle would occupy quite a bit of it, sorry.
I'll be making progress with Gradle-related PRs in the upcoming weeks (as part of #2700), and I'll have a closer look at this PR and your question after the first wave is complete.
@aSemy I don't think any of your PRs are related to or resolved this? If not, I'd be happy if you helped with brainstorming ideas for the question @3flex had - you definitely know better than me, and it'll take some time for me to research it all myself to propose an adequate solution.
@aSemy I don't think any of your PRs are related to or resolved this? If not, I'd be happy if you helped with brainstorming ideas for the question @3flex had - you definitely know better than me, and it'll take some time for me to research it all myself to propose an adequate solution.
Hey thanks for asking.
So as I understand, this PR will updates the task properties to use types that Gradle can correctly serialize. However, my guess is that there will still be some recurring issues with config cache and/or build cache (because of the cross-project task dependencies #2822), and that requires a much more hefty fix.
So I think this PR is good and should be merged.... but the updates might become outdated soon. (What I'm hoping is that the refactored plugin #2839 will be an almost drop-in replacement for the current plugin, and it solves config cache/build cache problems.)
What I think would help is improving the tests.
- updating Dokka's Gradle config (buildSrc, and then included-build plugins)
- refactoring the integration tests (e.g. use Gradle TestKit and JVM Test Suite to structure them)
- then enable the configuration cache (and build cache) in all test, running the tests twice, and making sure that the tests still work, and the logs indicate that the config cache/build cache were used
- https://docs.gradle.org/7.6/userguide/configuration_cache.html#config_cache:testing
- https://docs.gradle.org/7.6/userguide/test_kit.html#sub:test-kit-build-cache
Which are all reasonable, but they would take time (which is why I think this PR should be merged, because it will help a little)
Configuration cache is being promoted to stable in Gradle 8.1 which will be released within weeks.
What can I do to move this forward?
@3flex thanks for the reminder! I believe we've merged all of the major refactorings from #2700 (which introduced a lot of conflicts, sorry), and we still have a few weeks before Dokka 1.8.20, so we can try to squeeze it in
Could you please rebase this branch onto master? Or feel free to open a new PR if you find it easier than resolving conflicts
As for possible blockers, I think pretty much any solution that enables the use of Gradle's configuration cache should be fine, as we're aiming to eventually migrate to the re-written Gradle plugin from #2839, and it doesn't make much sense to spend a lot of time on making the current one beautiful - just have to make sure it works in the meantime.
The only thing to pay attention to are the tests - they should be runnable and should fail if breaking changes are introduced (like inserting a random throw IllegalStateException()
in one of them or deleteing some important code inside dokka-base
). I'll check this manually once all GitHub checks are green
I'm happy to rebase, but still need a response or approach to deal with this that's considered acceptable: https://github.com/Kotlin/dokka/pull/2499#issuecomment-1240177371
The simplest way to address it would be to publish the required artifacts to a local repository and then access them in the tests. Let me know if this is acceptable and I'll work on closing this out.
(discussed this in one of the comments, posting a summary here in case it was missed)
Publishing dependencies to Maven Local seems to be a good enough solution to unblock this PR :+1: We can revisit this part later once there's a better way to go about it
This isn't closed, so maybe there's still some value, but I'd like to know if dokkatoo will be used as the new Gradle plugin? I understand that already supports the configuration cache so I would close this if the plan is to use that or rewrite the plugin.
I have renewed interest in this since Gradle 8.6 supports encryption of the configuration cache and the setup-gradle
action integrates nicely with it, meaning CI builds including doc builds will greatly benefit from CC support.
This isn't closed, so maybe there's still some value, but I'd like to know if dokkatoo will be used as the new Gradle plugin?
hey @3flex, I can answer, because I work for JetBrains now!
Dokka Gradle Plugin will be updated to fully support Configuration Cache and Build Cache, and this will be based on Dokkatoo in some fashion. However, I can't say how long this will take. In the meantime I'm still actively maintaining and improving Dokkatoo, and I'd be happy to help out if anyone needs support.
I'd like to note as well that it's a good opportunity for anyone to get involved with the new Gradle plugin. For example, what should the new DSL look like? And what features should it have?
To confirm, does that mean the updated Gradle plugin will be:
- Built using the existing plugin as the base, with learnings from dokkatoo
- Built using dokkatoo as the base
- Written from scratch?
It sounds like it's 2?
If it's 1 I'll update this PR (eventually!), if it's 2 or 3 then I'll close it.
If it's not clear yet then I'll hold off.
(replying with my official JB hat since this is more about the future work rather than in my of-the-clock Dokkatoo work)
To confirm, does that mean the updated Gradle plugin will be:
- Built using the existing plugin as the base, with learnings from dokkatoo
- Built using dokkatoo as the base
- Written from scratch?
It sounds like it's 2?
The idea is: written from scratch, but copy-pasting a lot from Dokkatoo. So a mix of 3 and 2.
- The DSL used to configure the plugin in build-logic is still being considered, but it will probably be closer to Dokkatoo (a DSL-based config) than DGP (task-based config).
- The underlying plugin workings will need to be informed by the DSL, but again it will probably more similar to Dokkatoo.
But it's not been started yet and this could change.
In any case, I still see value in this PR because it does improve DGP, which is still widely used.
WDYT @IgnatBeresnev?