fix: keep the Kotlin plugin extension, which CodeQL's extractor depends on - #523
Merged
Conversation
…deQL CodeQL's java-kotlin analysis has failed on every build since Dokka moved to 2.2.0 in the Ktor modules. Its autobuild runs mvnw clean package -Dmaven.javadoc.skip -DskipTests ... and the reactor reaches storm-ktor, where Dokka 2.2.0 loads a Kotlin 2.2 analysis into the Maven JVM, and then storm-kotlin-test, whose kapt runs on Kotlin 2.0.21. CodeQL's extractor is injected into that compilation as a compiler plugin and is now bound against the wrong generation: The provided plugin com.github.codeql.KotlinExtractorComponentRegistrar is not compatible with this version of compiler. java.lang.AbstractMethodError: ... does not define or inherit an implementation of the resolved method registerProjectComponents ... The green build before it ran Dokka 2.0.0 at the same point, in the same order, under the same CodeQL, and the same kapt succeeded. Dokka is this project's javadoc, so it now honours the switch that says a build wants none. The scan asked for no documentation and gets none, which keeps a documentation engine's Kotlin out of the JVM that is compiling Kotlin. Releases and the docs workflow do not set the property, so they still produce the javadoc jar Maven Central requires. Only the two Ktor modules opt in, since they are the only ones whose Dokka runs a different Kotlin generation than the reactor compiles with.
…with The tidy-up in the previous change broke CodeQL's java-kotlin analysis on every build since, and skipping Dokka's output was not enough: the mojo returns early but its plugin realm is still loaded, and the failure reproduced unchanged. Rather than leave the analysis red while narrowing which half of that tidy-up is responsible, both halves go back. The Ktor modules return to Dokka 2.0.0, the generation the rest of the framework runs, and the three modules that declared the Kotlin plugin as a build extension declare it that way again. Both carry a comment saying what they cost and why they stay: Dokka 2.0 reports metadata errors it cannot read for a Kotlin 2.3 module, and the extension synthesises duplicate compile and kapt executions. What stays from the tidy-up is what cannot affect the extractor: Dokka's source root is still named explicitly, so a module with no src/main/java does not report a missing directory, and Dokka now honours maven.javadoc.skip, so a build that asks for no documentation gets none.
zantvoort
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ds on Isolating the two halves of the reverted tidy-up shows Dokka is innocent: Dokka 2.2.0 with the extension left in place analyses green. So the extension flag comes back and Dokka 2.2.0 stays, keeping the metadata fix for the two modules Dokka 2.0 cannot read. The flag now says why it cannot be removed, since the duplicate compile and kapt executions it synthesises look like an obvious cleanup and are not one.
zantvoort
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CodeQL's
java-kotlinanalysis has failed on every run since #521, onmainand on every PR. This restores it, and records why the change that broke it cannot simply be redone.The failure
CodeQL default setup autobuilds with
mvnw clean package -Dmaven.javadoc.skip -DskipTests ...and the reactor dies two modules from the end:CodeQL injects its extractor into each Kotlin compilation as a compiler plugin. Not the CodeQL version, which is 2.26.3 in the last green run and in every failing one, and not the reactor order, which is identical. Mixed Kotlin versions are not inherently the problem either:
kotlin:2.0.21at[27/34]andkotlin:2.3.10at[31/34]both extract fine in the failing run.Which half
#521's build tidy-up did two things, and the culprit is the one that looked most obviously safe.
<extensions>true</extensions>onkotlin-maven-plugincontributes the plugin's lifecycle mapping, which synthesisesdefault-compileanddefault-kaptexecutions beside the declared ones. Removing it stopped the reactor compiling everything twice, which is why I removed it. It also changes what CodeQL's extractor binds against, and the analysis fails on a later module.Isolated in #524: Dokka 2.2.0 with the extension left in place analyses green. So Dokka is innocent and stays at 2.2.0, keeping the metadata fix for the two modules Dokka 2.0 cannot read. The extension flag comes back, and now carries a comment saying it is load-bearing, because the duplicate compilation it causes reads as an obvious cleanup and is not one.
An earlier attempt here assumed the opposite and made Dokka honour
maven.javadoc.skipso the scan would not run it. CI showedDokka skip parameter is true so no dokka output will be producedand failed identically: the mojo returns early but its plugin realm loads anyway, so it never tested the hypothesis. That commit is in the history; the wiring it added is kept below on its own merits.What this leaves
Restored:
<extensions>true</extensions>on the three modules that had it, with the reason recordedKept from the tidy-up:
src/main/javastops reporting a missing directory on every buildmaven.javadoc.skip, so a build that asked for no documentation gets none, which is right on its own terms and takes documentation generation out of the security scanReleases and
docs.ymldo not set that property, so the javadoc jar Maven Central requires is unaffected; verified locally that the default build still produces it with all its API pages.The configuration here is byte-identical to #524, which CodeQL analysed green.