Skip to content
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

Move the logic from koltin repo atomicfu gradle plugin to the gradle plugin in the library. #406

Merged
merged 18 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions atomicfu-gradle-plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ dependencies {
// Atomicfu compiler plugin dependency will be loaded to kotlinCompilerPluginClasspath
// Atomicfu plugin will only be applied if the flag is set kotlinx.atomicfu.enableJsIrTransformation=true
compileOnly "org.jetbrains.kotlin:atomicfu:$kotlin_version"
// This runtimeOnly dependency is added as a temporary WA for the problem #384.
// The version 1.6.21 was chosen as the lowest valid version of this artifact,
// and it's expected to be resolved to the highest existing version from the user's classpath.
runtimeOnly "org.jetbrains.kotlin:atomicfu:1.6.21"

testImplementation gradleTestKit()
testImplementation 'org.jetbrains.kotlin:kotlin-test'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ private const val TEST_IMPLEMENTATION_CONFIGURATION = "testImplementation"
// If the project uses KGP <= 1.6.20, only JS IR compiler plugin is available, and it is turned on via setting this property.
// The property is supported for backwards compatibility.
private const val ENABLE_JS_IR_TRANSFORMATION_LEGACY = "kotlinx.atomicfu.enableIrTransformation"
private const val ENABLE_JS_IR_TRANSFORMATION = "kotlinx.atomicfu.enableJsIrTransformation"
private const val ENABLE_JVM_IR_TRANSFORMATION = "kotlinx.atomicfu.enableJvmIrTransformation"
private const val ENABLE_NATIVE_IR_TRANSFORMATION = "kotlinx.atomicfu.enableNativeIrTransformation"
internal const val ENABLE_JS_IR_TRANSFORMATION = "kotlinx.atomicfu.enableJsIrTransformation"
internal const val ENABLE_JVM_IR_TRANSFORMATION = "kotlinx.atomicfu.enableJvmIrTransformation"
internal const val ENABLE_NATIVE_IR_TRANSFORMATION = "kotlinx.atomicfu.enableNativeIrTransformation"
private const val MIN_SUPPORTED_GRADLE_VERSION = "7.0"
private const val MIN_SUPPORTED_KGP_VERSION = "1.7.0"

Expand Down Expand Up @@ -83,28 +83,18 @@ private fun Project.checkCompatibility() {

private fun Project.applyAtomicfuCompilerPlugin() {
val kotlinVersion = getKotlinVersion()
// for KGP >= 1.7.20:
// compiler plugin for JS IR is applied via the property `kotlinx.atomicfu.enableJsIrTransformation`
// compiler plugin for JVM IR is applied via the property `kotlinx.atomicfu.enableJvmIrTransformation`
if (kotlinVersion.atLeast(1, 7, 20)) {
plugins.apply(AtomicfuKotlinGradleSubplugin::class.java)
extensions.getByType(AtomicfuKotlinGradleSubplugin.AtomicfuKotlinGradleExtension::class.java).apply {
isJsIrTransformationEnabled = rootProject.getBooleanProperty(ENABLE_JS_IR_TRANSFORMATION)
isJvmIrTransformationEnabled = rootProject.getBooleanProperty(ENABLE_JVM_IR_TRANSFORMATION)
if (kotlinVersion.atLeast(1, 9, 20)) {
// Native IR transformation is available since Kotlin 1.9.20
isNativeIrTransformationEnabled = rootProject.getBooleanProperty(ENABLE_NATIVE_IR_TRANSFORMATION)
}
}
if (kotlinVersion.atLeast(1, 9, 0)) {
// Since Kotlin 1.9.0 the logic of the Gradle plugin from the Kotlin repo (AtomicfuKotlinGradleSubplugin)
// may be moved to the Gradle plugin in the library. The sources of the compiler plugin
// are published as `kotlin-atomicfu-compiler-plugin-embeddable` since Kotlin 1.9.0 and may be accessed out of the Kotlin repo.
plugins.apply(AtomicfuKotlinCompilerPluginInternal::class.java)
} else {
// for KGP >= 1.6.20 && KGP <= 1.7.20:
// compiler plugin for JS IR is applied via the property `kotlinx.atomicfu.enableIrTransformation`
// compiler plugin for JVM IR is not supported yet
if (kotlinVersion.atLeast(1, 6, 20)) {
if (rootProject.getBooleanProperty(ENABLE_JS_IR_TRANSFORMATION_LEGACY)) {
plugins.apply(AtomicfuKotlinGradleSubplugin::class.java)
}
}
error("You are applying `kotlinx-atomicfu` plugin of version 0.23.3 or newer. " +
Copy link
Collaborator Author

@mvicsokolova mvicsokolova Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, in case of KGP version < 1.9.0, I asked users to add org.jetbrains.kotlin:atomicfu dependency manually to the buildscript. However, this caused further problems, that already were solved in higher Kotlin versions.

E.g. the problem below would occur if you try to apply the current kotlinx-atomicfu version with Kotlin 1.8.20 and just add the kotlin:atomicfu:1.8.20 dependency manually (this problem was solved in 0.23.0 and Kotlin 1.9.10)

e: Could not find ".gradle/caches/modules-2/files-2.1/org.jetbrains.kotlinx/atomicfu-linuxarm64/0.23.2/7b4270560c2723dc55b1d3c8925b7b93201c437b/atomicfu.klib" in [IdeaProjects/kotlinx-atomicfu-new/integration-testing/examples/mpp-sample, .konan/klib, .konan/kotlin-native-prebuilt-macos-aarch64-1.8.20/klib/common, .konan/kotlin-native-prebuilt-macos-aarch64-1.8.20/klib/platform/linux_arm64]

So, I decided to write this error message about incompatibility instead.
I've checked that the sample mpp project can be built with kotlinx-atomicfu 0.22.0 and Kotlin down to 1.6.0, though I believe these are very marginal use cases.

@fzhinkin WDYT? 👀

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version 0.23.3 or newer

Can we print the exact version here?

I think it should be enough to:

  • mention that Kotlin version should be upgraded
  • add a document (like readme) describing an alternative solution for those who can't upgrade the Kotlin for some reason

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a note about downgrading atomicfu version as an alternative solution in the README. And in the message left just the recommendation to upgrade the Kotlin version.

"However, this version of the plugin is only compatible with Kotlin versions newer than 1.9.0.\n" +
"If you wish to use this version of the plugin, please upgrade your Kotlin version to 1.9.0 or newer.\n" +
"The alternative solution is to downgrade `kotlinx-atomicfu` plugin version to 0.22.0.\n" +
"Please note, that using the latest version of the plugin and upgrading the Kotlin version is a more preferable option.\n\n" +
"If you encounter any problems, please submit the issue: https://github.com/Kotlin/kotlinx-atomicfu/")
fzhinkin marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -186,7 +176,7 @@ private fun KotlinVersion.atLeast(major: Int, minor: Int, patch: Int) =
// kotlinx-atomicfu compiler plugin is available for KGP >= 1.6.20
private fun Project.isCompilerPluginAvailable() = getKotlinVersion().atLeast(1, 6, 20)

private fun Project.getBooleanProperty(name: String) =
internal fun Project.getBooleanProperty(name: String) =
rootProject.findProperty(name)?.toString()?.toBooleanStrict() ?: false

private fun String.toBooleanStrict(): Boolean = when (this) {
Expand All @@ -195,15 +185,15 @@ private fun String.toBooleanStrict(): Boolean = when (this) {
else -> throw IllegalArgumentException("The string doesn't represent a boolean value: $this")
}

private fun Project.needsJsIrTransformation(target: KotlinTarget): Boolean =
internal fun Project.needsJsIrTransformation(target: KotlinTarget): Boolean =
(rootProject.getBooleanProperty(ENABLE_JS_IR_TRANSFORMATION) || rootProject.getBooleanProperty(ENABLE_JS_IR_TRANSFORMATION_LEGACY))
&& target.isJsIrTarget()

private fun Project.needsJvmIrTransformation(target: KotlinTarget): Boolean =
internal fun Project.needsJvmIrTransformation(target: KotlinTarget): Boolean =
rootProject.getBooleanProperty(ENABLE_JVM_IR_TRANSFORMATION) &&
(target.platformType == KotlinPlatformType.jvm || target.platformType == KotlinPlatformType.androidJvm)

private fun Project.needsNativeIrTransformation(target: KotlinTarget): Boolean =
internal fun Project.needsNativeIrTransformation(target: KotlinTarget): Boolean =
rootProject.getBooleanProperty(ENABLE_NATIVE_IR_TRANSFORMATION) &&
(target.platformType == KotlinPlatformType.native)

Expand All @@ -215,7 +205,7 @@ private fun KotlinTarget.isJsIrTarget() =
private fun Project.isTransitiveAtomicfuDependencyRequired(target: KotlinTarget): Boolean {
val platformType = target.platformType
return !config.transformJvm && (platformType == KotlinPlatformType.jvm || platformType == KotlinPlatformType.androidJvm) ||
!config.transformJs && platformType == KotlinPlatformType.js ||
(!config.transformJs && platformType == KotlinPlatformType.js) ||
platformType == KotlinPlatformType.wasm ||
// Always add the transitive atomicfu dependency for native targets, see #379
platformType == KotlinPlatformType.native
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package kotlinx.atomicfu.plugin.gradle
fzhinkin marked this conversation as resolved.
Show resolved Hide resolved

import org.gradle.api.provider.Provider
import org.jetbrains.kotlin.gradle.plugin.*

/**
* This Gradle plugin applies compiler transformations to the project, it was copied from the kotlin repo (org.jetbrains.kotlinx.atomicfu.gradle.AtomicfuKotlinGradleSubplugin).
*
* As the sources of the compiler plugin are published as `org.jetbrains.kotlin.kotlin-atomicfu-compiler-plugin-embeddable` starting from Kotlin 1.9.0,
* the Gradle plugin can access this artifact from the library and apply the transformations.
*
* NOTE: The version of KGP may differ from the version of Kotlin compiler, and kotlin.native.version may override the version of Kotlin native compiler.
fzhinkin marked this conversation as resolved.
Show resolved Hide resolved
* So, the right behavior for the Gradle plugin would be to obtain compiler versions and apply compiler transformations separately to JVM/JS and Native targets.
* This was postponed as a separate task (#408).
*/
internal class AtomicfuKotlinCompilerPluginInternal : KotlinCompilerPluginSupportPlugin {

companion object {
const val ATOMICFU_ARTIFACT_NAME = "kotlin-atomicfu-compiler-plugin-embeddable"
}

mvicsokolova marked this conversation as resolved.
Show resolved Hide resolved

override fun isApplicable(kotlinCompilation: KotlinCompilation<*>): Boolean {
val target = kotlinCompilation.target
val project = target.project
return project.needsJvmIrTransformation(target) || project.needsJsIrTransformation(target) || project.needsNativeIrTransformation(target)

}

override fun applyToCompilation(
kotlinCompilation: KotlinCompilation<*>
): Provider<List<SubpluginOption>> =
kotlinCompilation.target.project.provider { emptyList() }

override fun getCompilerPluginId() = "org.jetbrains.kotlinx.atomicfu"

// Gets "org.jetbrains.kotlin:kotlin-atomicfu-compiler-plugin-embeddable:{KGP version}"
override fun getPluginArtifact(): SubpluginArtifact {
return JetBrainsSubpluginArtifact(ATOMICFU_ARTIFACT_NAME)
}
}
13 changes: 13 additions & 0 deletions integration-testing/examples/mpp-version-catalog/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import org.jetbrains.kotlin.gradle.dsl.KotlinCompile

plugins {
// this is necessary to avoid the plugins to be loaded multiple times
// in each subproject's classloader
alias(libs.plugins.kotlinMultiplatform) apply false
}

tasks.withType<KotlinCompile<*>>().configureEach {
kotlinOptions {
freeCompilerArgs += listOf("-Xskip-prerelease-check")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=2g
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[versions]
junit = "4.13.2"
kotlin = "1.9.21"
atomicfu = "0.23.2-SNAPSHOT"
ktor = "2.3.8"
logback = "1.5.0"

[libraries]
kotlin-test = { module = "org.jetbrains.kotlin:kotlin-test", version.ref = "kotlin" }
kotlin-test-junit = { module = "org.jetbrains.kotlin:kotlin-test-junit", version.ref = "kotlin" }
junit = { group = "junit", name = "junit", version.ref = "junit" }
logback = { module = "ch.qos.logback:logback-classic", version.ref = "logback" }
ktor-server-core = { module = "io.ktor:ktor-server-core-jvm", version.ref = "ktor" }
ktor-server-netty = { module = "io.ktor:ktor-server-netty-jvm", version.ref = "ktor" }
ktor-server-tests = { module = "io.ktor:ktor-server-tests-jvm", version.ref = "ktor" }
atomicfuGradlePlugin = { module = "org.jetbrains.kotlinx:atomicfu-gradle-plugin", version.ref = "atomicfu" }

[plugins]
kotlinJvm = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin" }
ktor = { id = "io.ktor.plugin", version.ref = "ktor" }
kotlinMultiplatform = { id = "org.jetbrains.kotlin.multiplatform", version.ref = "kotlin" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
rootProject.name = "mpp-version-catalog"
enableFeaturePreview("TYPESAFE_PROJECT_ACCESSORS")

pluginManagement {
repositories {

google()
gradlePluginPortal()
mavenCentral()
mavenLocal()
}
}

dependencyResolutionManagement {
repositories {
google()
mavenCentral()
mavenLocal()
}
}

include(":shared")
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
buildscript {
dependencies {
classpath(libs.atomicfuGradlePlugin)
}
}

repositories {
mavenCentral()
mavenLocal()
}

plugins {
alias(libs.plugins.kotlinMultiplatform)
}

apply (plugin = "kotlinx-atomicfu")

kotlin {

jvm()

js(IR)

macosArm64()
macosX64()
linuxArm64()
linuxX64()
mingwX64()

sourceSets {
commonMain{
dependencies {
implementation(kotlin("stdlib"))
implementation(kotlin("test-junit"))
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import kotlinx.atomicfu.*
import kotlinx.atomicfu.locks.*
import kotlin.test.*

public class AtomicSampleClass {
private val _x = atomic(0)
val x get() = _x.value

public fun doWork(finalValue: Int) {
assertEquals(0, x)
assertEquals(0, _x.getAndSet(3))
assertEquals(3, x)
assertTrue(_x.compareAndSet(3, finalValue))
}

private val lock = reentrantLock()

public fun synchronizedFoo(value: Int): Int {
return lock.withLock { value }
}
}
8 changes: 8 additions & 0 deletions integration-testing/examples/plugin-order-bug/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,11 @@ tasks.withType(KotlinCompile).configureEach {
freeCompilerArgs += ["-Xskip-prerelease-check"]
}
}

// Workaround for https://youtrack.jetbrains.com/issue/KT-58303:
// the `clean` task can't delete the expanded.lock file on Windows as it's still held by Gradle, failing the build
tasks.clean {
setDelete(layout.buildDirectory.asFileTree.matching {
exclude("tmp/.cache/expanded/expanded.lock")
})
}
8 changes: 8 additions & 0 deletions integration-testing/examples/user-project/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,11 @@ tasks.withType<KotlinCompile<*>>().configureEach {
freeCompilerArgs += listOf("-Xskip-prerelease-check")
}
}

// Workaround for https://youtrack.jetbrains.com/issue/KT-58303:
// the `clean` task can't delete the expanded.lock file on Windows as it's still held by Gradle, failing the build
tasks.clean {
setDelete(layout.buildDirectory.asFileTree.matching {
exclude("tmp/.cache/expanded/expanded.lock")
})
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package kotlinx.atomicfu.gradle.plugin.test.cases

import kotlinx.atomicfu.gradle.plugin.test.framework.checker.*
import kotlinx.atomicfu.gradle.plugin.test.framework.runner.*
import kotlin.test.*

/**
* This test checks the build of mpp-version-catalog project that uses a versions catalog and was a reproducer for this error (#399).
*/
class MppVersionCatalogTest {
private val mppWithVersionCatalog: GradleBuild = createGradleBuildFromSources("mpp-version-catalog")

@Test
fun testBuildWithKotlinNewerThan_1_9_0() {
mppWithVersionCatalog.enableJvmIrTransformation = true
mppWithVersionCatalog.enableNativeIrTransformation = true
val buildResult = mppWithVersionCatalog.cleanAndBuild()
assertTrue(buildResult.isSuccessful, buildResult.output)
mppWithVersionCatalog.buildAndCheckBytecode()
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package kotlinx.atomicfu.gradle.plugin.test.cases

import kotlinx.atomicfu.gradle.plugin.test.framework.checker.getProjectClasspath
import kotlinx.atomicfu.gradle.plugin.test.framework.runner.*
import kotlinx.atomicfu.gradle.plugin.test.framework.runner.GradleBuild
import kotlinx.atomicfu.gradle.plugin.test.framework.runner.cleanAndBuild
Expand All @@ -19,15 +18,6 @@ class PluginOrderBugTest {
assertTrue(buildResult.isSuccessful, buildResult.output)
}

/**
* Ensures that the version of atomicfu compiler plugin in the project's classpath equals the version of KGP used in the project.
*/
@Test
fun testResolvedCompilerPluginDependency() {
val classpath = pluginOrderBugProject.getProjectClasspath()
assertTrue(classpath.contains("org.jetbrains.kotlin:atomicfu:${pluginOrderBugProject.getKotlinVersion()}"))
}

/**
* kotlin-stdlib is an implementation dependency of :atomicfu module,
* because compileOnly dependencies are not applicable for Native targets (#376).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ internal const val ENABLE_JVM_IR_TRANSFORMATION = "kotlinx.atomicfu.enableJvmIrT
internal const val ENABLE_JS_IR_TRANSFORMATION = "kotlinx.atomicfu.enableJsIrTransformation"
internal const val ENABLE_NATIVE_IR_TRANSFORMATION = "kotlinx.atomicfu.enableNativeIrTransformation"
internal const val LOCAL_REPOSITORY_URL_PROPERTY = "localRepositoryUrl"
internal const val KOTLIN_VERSION_PROPERTY = "localRepositoryUrl"
internal const val DUMMY_VERSION = "DUMMY_VERSION"
internal const val LOCAL_REPO_DIR_PREFIX = "build/.m2/"

Expand Down