Backport kxDT-as-KUP infrastructure and migration changes #560
Conversation
JDK 21 support is required for builds as a Kotlin user project.
* the deprecated `kotlin.js.compiler` Gradle property is removed from `gradle.properties` * the deprecated `kotlin.native.ignoreIncorrectDependencies` Gradle property is removed from `gradle.properties` * `:kotlinx-datetime:compileCommonJsMainKotlinMetadata` task is enabled back to preserve compilation correctness (see KT-66382 for more details).
FenstonSingel
force-pushed
the
kotlin-community/k2/dev
branch
from
6007329 to
8efcd34
Compare
|
(corrected a faulty commit description) |
| networkTimeout=10000 | ||
| validateDistributionUrl=true |
There was a problem hiding this comment.
What is the purpose of this line? https://docs.gradle.org/current/dsl/org.gradle.api.tasks.wrapper.Wrapper.html#org.gradle.api.tasks.wrapper.Wrapper:validateDistributionUrl says it's true by default.
There was a problem hiding this comment.
AFAIR, this line was added automatically by ./gradlew wrapper --gradle-version 8.5.
I decided that it knew better than me. 😅
| @@ -88,6 +88,9 @@ internal fun readTzFile(data: ByteArray): TzFile { | |||
| override fun toString(): String = "Ttinfo(utoff=$utoff, isdst=$isdst, abbrind=$abbrind)" | |||
| } | |||
|
|
|||
| fun abbreviationForIndex(abbreviations: List<Byte>, startIndex: UByte): String = abbreviations.drop(startIndex.toInt()) | |||
| .takeWhile { byte -> byte != 0.toByte() }.toByteArray().decodeToString() | |||
There was a problem hiding this comment.
The commit message says that this change was introduced because of https://youtrack.jetbrains.com/issue/KT-77986/, but I don't see the connection. This is just a function, not a local class. Could you please elaborate?
There was a problem hiding this comment.
AFAIU, the KT-77986 fix addressed a more general problem of the compiler not running relevant checks in nested local scopes (regardless of whether the subject is a class or a function), and this function was previously located in a local function inside a local function.
I see that we have a slightly more applicable ticket (created precisely as a result of this kxDT-as-KUP hit), though: https://youtrack.jetbrains.com/issue/KT-78976/, – so I can link it instead.
| rootProject.properties["kotlin_snapshot_version"] as? String ?: error("kotlin_snapshot_version must be specified") | ||
| } else { | ||
| rootProject.properties["defaultKotlinVersion"] as String | ||
| } |
There was a problem hiding this comment.
I like this file: it looks like it could some day become a Gradle plugin, a single one for all projects, reducing the maintenance load and providing a unified experience. For example, here's the corresponding file in kotlinx.coroutines: https://github.com/Kotlin/kotlinx.coroutines/blob/f4f519b36734238ec686dfaec1e174086691781e/buildSrc/src/main/kotlin/CommunityProjectsBuild.kt It's fairly similar, and one can compare them to see the discrepancies/omissions.
In fact, even if just for conveniently searching for discrepancies, this file should not only exist but encompass more of the logic: if Xpartial-linkage-loglevel=error truly is a KUP requirement, kotlinx.coroutines doesn't fulfill it currently, and it would have been easier to notice were this configuration part of this file.
Still, since you're the one who has to interact with this part of the project the most, if you think this file is unnecessary, to be it. I'm not really asking for this file to be restored but am simply sharing an opinion.
There was a problem hiding this comment.
We have previously attempted to develop an in-house Gradle plugin that would do all things KUP automatically, but it was too high of a maintenance load in and of itself (thanks Gradle) and ended up abandoned and deprecated. Your idea sounds like a good middle ground between that and fully manual maintenance, though; definitely something to consider. Thanks! 👍🏼
On a more relevant note, AFAIK, there were talks of reassigning maintenance of kotlinx libraries as KUPs to the Libraries team in the near future; TBH, this is a main reason I have decided to finally address this piece of our infra debt.
So moving KUP infra back to buildSrc seems to be a good idea, yeah; will do.
| @@ -1,3 +1,5 @@ | |||
| import org.jetbrains.kotlin.gradle.tasks.KotlinCompilationTask | |||
|
|
|||
| /* | |||
There was a problem hiding this comment.
Please move the import below the copyright header so that it stays a header.
No description provided.