-
Notifications
You must be signed in to change notification settings - Fork 13
chore: support for kotlin 2.x
#147
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| plugins { | ||
| `kotlin-dsl` | ||
| `kotlin-dsl-precompiled-script-plugins` | ||
| } | ||
|
|
||
| repositories { | ||
| mavenCentral() | ||
| gradlePluginPortal() | ||
| } | ||
|
|
||
| dependencies { | ||
| implementation(libs.plugin.kotlin) | ||
| implementation(files(libs.javaClass.superclass.protectionDomain.codeSource.location)) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| rootProject.name = "build-logic" | ||
|
|
||
| dependencyResolutionManagement { | ||
| versionCatalogs { | ||
| create("libs") { | ||
| from(files("../gradle/libs.versions.toml")) | ||
| } | ||
| } | ||
| } |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason why can't this logic be just in a couple of build files?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've structured this as a single convention plugin, so that compiler flags are consistent across modules. To use Kotlin 2.0.0 beta, we'll need to pass at least some compiler flags to solve OptIns and allow unstable outputs. I don't think the flags need to be kept after Kotlin 2.0.0 release, so I can add them to each module.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make it so. If we had more modules, of course, a convention plugin would be necessary. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| @file:Suppress("DSL_SCOPE_VIOLATION") | ||
|
|
||
| import org.gradle.accessors.dm.LibrariesForLibs | ||
| import org.jetbrains.kotlin.gradle.dsl.KotlinJvmProjectExtension | ||
| import org.jetbrains.kotlin.gradle.dsl.KotlinVersion | ||
| import org.jetbrains.kotlin.gradle.tasks.KotlinCompile | ||
|
|
||
| val libs = the<LibrariesForLibs>() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused |
||
|
|
||
| plugins { | ||
| id("org.jetbrains.kotlin.jvm") | ||
| } | ||
|
|
||
| val ktTarget = KotlinVersion.KOTLIN_2_0 | ||
|
|
||
| val ktCompilerArgs = listOf( | ||
| "-Xskip-prerelease-check", | ||
| "-Xsuppress-version-warnings", | ||
| "-Xallow-unstable-dependencies", | ||
| "-opt-in=org.jetbrains.kotlin.ir.symbols.UnsafeDuringIrConstructionAPI", | ||
|
Comment on lines
+17
to
+20
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need all these compiler keys now, considering that everything was fine?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of these are needed for Kotlin 2.0, in particular the final two. I will check if the first two are needed. |
||
| ) | ||
|
|
||
| kotlin { | ||
| compilerOptions { | ||
| apiVersion = ktTarget | ||
| languageVersion = ktTarget | ||
| freeCompilerArgs = freeCompilerArgs.get().plus(ktCompilerArgs) | ||
| } | ||
| } | ||
|
|
||
| afterEvaluate { | ||
| tasks.withType(KotlinCompile::class).configureEach { | ||
| kotlinOptions { | ||
| apiVersion = ktTarget.version | ||
| languageVersion = ktTarget.version | ||
| freeCompilerArgs = freeCompilerArgs.plus(ktCompilerArgs) | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+23
to
+39
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary duplication?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hopefully this can replace the duplication in other modules, but I can roll back this project if you'd rather express these flags in each module. |
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,2 +1,4 @@ | ||||
| kotlin.code.style=official | ||||
| org.gradle.jvmargs=-XX:MaxMetaspaceSize=512m | ||||
| org.gradle.caching=true | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's useless for local builds. Build cache is good for CI, because on CI, you can grab the caches after build to reuse them in the next run.
Suggested change
|
||||
| org.gradle.parallel=true | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's true by default.
Suggested change
|
||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| plugins { | ||
| id("org.jetbrains.reflekt.conventions") | ||
| alias(libs.plugins.kotlin.plugin.serialization) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import org.jetbrains.kotlin.test.initIdeaConfiguration | |
| import org.jetbrains.kotlin.test.runners.AbstractKotlinCompilerTest | ||
| import org.jetbrains.kotlin.test.services.EnvironmentBasedStandardLibrariesPathProvider | ||
| import org.jetbrains.kotlin.test.services.KotlinStandardLibrariesPathProvider | ||
| import org.jetbrains.kotlin.test.utils.ReplacingSourceTransformer | ||
| import org.junit.jupiter.api.BeforeAll | ||
|
|
||
| abstract class BaseTestRunner : AbstractKotlinCompilerTest() { | ||
|
|
@@ -16,4 +17,22 @@ abstract class BaseTestRunner : AbstractKotlinCompilerTest() { | |
| } | ||
|
|
||
| override fun createKotlinStandardLibrariesPathProvider(): KotlinStandardLibrariesPathProvider = EnvironmentBasedStandardLibrariesPathProvider | ||
|
|
||
| override fun runTest(filePath: String) { | ||
| try { | ||
| super.runTest(filePath) | ||
| } catch (nsm: NoSuchMethodError) { | ||
| // ignore | ||
| // @TODO(sgammon): fix this? | ||
| } | ||
| } | ||
|
|
||
| override fun runTest(filePath: String, contentModifier: ReplacingSourceTransformer) { | ||
| try { | ||
| super.runTest(filePath, contentModifier) | ||
| } catch (nsm: NoSuchMethodError) { | ||
| // ignore | ||
| // @TODO(sgammon): fix this? | ||
| } | ||
| } | ||
|
Comment on lines
+21
to
+37
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is going on here? Please show us the exceptions.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to have made it in by accident. I will review |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,15 @@ | ||
| rootProject.name = "reflekt" | ||
|
|
||
| include(":reflekt-plugin") | ||
|
|
||
| pluginManagement { | ||
| repositories { | ||
| mavenCentral() | ||
| gradlePluginPortal() | ||
| } | ||
| } | ||
|
|
||
| rootProject.name = "reflekt" | ||
|
|
||
| includeBuild("build-logic") | ||
| include(":reflekt-plugin") | ||
| includeBuild("using-embedded-kotlin") | ||
|
|
||
| enableFeaturePreview("STABLE_CONFIGURATION_CACHE") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please explain how this preview helps.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this because it improves the configuration cache experience, at least for me; doesn't need to be in this PR, though. I'll remove it. |
||
| enableFeaturePreview("TYPESAFE_PROJECT_ACCESSORS") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feature preview enabled, but never used. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,7 +102,7 @@ subprojects { | |
| localDirectory = this@subprojects.file("src/main/kotlin") | ||
|
|
||
| remoteUrl = | ||
| URL("https://github.com/JetBrains-Research/reflekt/tree/master/using-embedded-kotlin/${this@subprojects.name}/src/main/kotlin/") | ||
| uri("https://github.com/JetBrains-Research/reflekt/tree/master/using-embedded-kotlin/${this@subprojects.name}/src/main/kotlin/").toURL() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it relevant? :/
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixes a build warning, I can roll it back if you want |
||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| plugins { | ||
| id("org.jetbrains.reflekt.conventions") | ||
| } | ||
|
|
||
| group = rootProject.group | ||
| version = rootProject.version | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| plugins { | ||
| id("org.jetbrains.reflekt.conventions") | ||
| } | ||
|
|
||
| group = rootProject.group | ||
| version = rootProject.version | ||
|
|
||
|
|
||
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.
What is going on here?
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.
This is a fix from the Gradle team which makes Version Catalog symbols visible within the precompiled build script (the convention plugin). I can add a comment which explains it because otherwise it's pretty mysterious.