-
Notifications
You must be signed in to change notification settings - Fork 412
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
Mark tasks as not compatible with the Gradle configuration cache #3070
Conversation
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.
Thank you for the contribution, very much appreciated!
I'll start Gradle integration tests now, which should take a couple of hours, and if everything's alright - will merge it.
@@ -232,6 +232,7 @@ abstract class AbstractDokkaTask : DefaultTask() { | |||
|
|||
init { | |||
group = JavaBasePlugin.DOCUMENTATION_GROUP | |||
super.notCompatibleWithConfigurationCache("Dokka tasks are not yet compatible with the Gradle configuration cache. See https://github.com/Kotlin/dokka/issues/1217") |
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.
Unfortunately, the integration tests are failing :(
Caused by: java.lang.NoSuchMethodError: 'void org.gradle.api.DefaultTask.notCompatibleWithConfigurationCache(java.lang.String)
The used method (notCompatibleWithConfigurationCache
) was introduced in Gradle 7.4, whereas we still have to keep compatibility with Gradle 6.9+ (for as long as Kotlin itself supports it).
We probably need to use something like plugin variants to fix this properly, but there's little point in investing time into the current Gradle plugin - it will be completely re-written anyway.
However, it would still be nice to have this change for the time being. I think we can add a dirty hack - check with reflection that this method exists, and only then call it. What do you think?
Something along the lines of
val containsNotCompatibleWithConfigurationCache = this::class.memberFunctions
.any { it.name == "notCompatibleWithConfigurationCache" && it.parameters.firstOrNull()?.name == "reason" }
if (containsNotCompatibleWithConfigurationCache) {
super.notCompatibleWithConfigurationCache("Dokka tasks are not yet compatible with the Gradle configuration cache. See https://github.com/Kotlin/dokka/issues/1217")
}
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.
Yes I think using reflection is a fair workaround :) Feel free to amend the PR (or I can do it if you prefer). 👍
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 don't want to rob you of the valuable contribution 😅 If you have the time and the desire, I'd be happy if you did it :) I also didn't test the suggestion - I mean, it should work if this function is returned in memberFunctions
, but who knows
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.
Thanks! I've just pushed the change. It seems to work on my side but let's see if the CI agrees :)
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.
Looking good 👍
I had to run the integration tests locally because this branch is quite old. All of the tests are green, so it should be good to go
Once again, thank you for addressing the issue!
Since, as for now, Dokka is not compatible with the Gradle configuration cache (see #1217), using it in a project that has it enabled will fail the build.
Marking the tasks as incompatible avoids these fails.