-
Notifications
You must be signed in to change notification settings - Fork 11
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
chore: support for kotlin 2.x
#147
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")) | ||
} | ||
} | ||
} |
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? 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. 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>() | ||
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
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? 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
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? 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 | ||||
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 | ||||
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
|
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
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. 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") | ||
enableFeaturePreview("TYPESAFE_PROJECT_ACCESSORS") | ||
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. 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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,7 +102,7 @@ subprojects { | |
localDirectory = [email protected]("src/main/kotlin") | ||
|
||
remoteUrl = | ||
URL("https://github.com/JetBrains-Research/reflekt/tree/master/using-embedded-kotlin/${[email protected]}/src/main/kotlin/") | ||
uri("https://github.com/JetBrains-Research/reflekt/tree/master/using-embedded-kotlin/${[email protected]}/src/main/kotlin/").toURL() | ||
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? :/ 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.