Make the compatibility metadata variant check more specific #3103
Conversation
IgnatBeresnev
force-pushed
the
improve-compatibility-metadata-variant-check
branch
from
5a077d0 to
80077a8
Compare
runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/kotlin/kotlinClasspathUtils.kt
Show resolved
Hide resolved
| sourceSet in compilation.kotlinSourceSets || sourceSet == compilation.defaultSourceSet | ||
| } | ||
|
|
||
| val hasCompatibilityMetadataVariant = compilations.size >= 2 |
There was a problem hiding this comment.
Did I understand correctly that compilations.size >= 2 is only about a non-single target? Does it mean that a single target does not have compatibility metadata at all?
There was a problem hiding this comment.
Does it mean that a single target does not have compatibility metadata at all?
I see how the name of the variable might be confusing. I've changed it to hasAdditionalCommonCompatibilityMetadataVariant to be more specific, and corrected the comment a little.
The original filterNot check was introduced so that ktor works. One of its modules has both main and commonMain compilations, and it fails the build with the error I mentioned in the code comment. Our fix was to filter out the main compilation, so only commonMain is used and there is no error. However, we did not account for the fact that main might be the only compilation available.
Single-target projects in <= 1.8.20 returned both main and commonMain, so everything worked. But in 1.9.0 it stopped returning commonMain, and only returned main (no other compilations). But we filtered it out regardless, so we were left with an empty list -> got the error.
So this PR essentially fixes the bug of the previous bugfix - this is what the ktor fix should've been: only filter out main if commonMain is also present (in other words, if compatibility metadata is enabled), otherwise return whatever compilations are available.
This transitively fixes single-target KMP projects, because they have the main compilation now, but there's no guarantee that it will exist in the future. So it's not really a fix for single-target KMP projects, more like a happy coincidence that makes it work for the time being.
The check for compilations.size >= 2 is kind of like a prerequisite / fast path for checking if compatibility metadata is enabled. If the size is 1 (an often occurrence), from what I understand, it must mean it's disabled.
There was a problem hiding this comment.
I've changed it to hasAdditionalCommonCompatibilityMetadataVariant
I think it is clearer.
Also, I mean if we use main compilation, can we have the same problem as with ktor for a single target?
Anyway, it is better than supporting no single target at all.
There was a problem hiding this comment.
can we have the same problem as with ktor for a single target?
I'm actually not sure, but quite possibly, if one of the dependencies leads to the same problem as in ktor (stacktrace here).
Anyway, it is better than supporting no single target at all.
Indeed, this is why I don't want to call the PR "Fix single-target multiplatform projects" - it doesn't really fix the problem. This PR introduces a better check for ktor, which happens to help with single-target projects, but there are no guarantees - such projects are still officially not supported
Oh yeah, I wanted to do that, but forgot 🤦♂️
|
(cherry picked from commit b559131)
This transitively fixes the support for single-target multiplatform projects.
Background
During some refactoring in KGP, a bug was noticed that led to the creation of a redundant commonMain compilation in single-target multiplatform projects. This bug was fixed in KGP 1.9, so single-target multi-platform projects do not have this accidental compilation anymore.
Dokka had an additional check that filtered out the compatibility / legacy
maincompilation.This led to the situation where single-target multiplatform projects using Kotlin 1.9.0 had no viable compilations from Dokka's perspective (because one was removed by KGP, and another one filtered out by Dokka), so it failed with an error (#3063).
However, if you looked at which compilations were returned by KGP for single-target multiplatform projects, you would see that it actually had the
maincompilation, which could be used to generate documentation successfully, but Dokka was filtering it out.If you removed the check that filtered out the
maincompilation, it would fix the issue for single-target multiplatform projects, but it would lead to problems in other projects that have setkotlin.mpp.enableCompatibilityMetadataVariant=true(like ktor).The solution
An additional condition was added to the check: the compatibility compilation should only be filtered out if this source set has an additional proper compilation which would be used instead. Otherwise, we have a chance of filtering out all available compilations.
Now, single-target multiplatform projects should work as they (for some reason) have the
maincompilation, and Dokka doesn't filter it out, so it is used to generate documentation.I've stressed "transitively" specifically because there are no guarantees that single-target projects will have that
maincompilation in the future - it might be removed (because such projects are still not supported officially), and the support for single-target projects would be broken again.Maybe by that time we'll have migrated to a different API provided by KGP, so it wouldn't be a problem anymore - we'll see.