Remove JS Legacy configurations (updated) #296

mvicsokolova · 2023-04-25T15:10:18Z

No description provided.

…f atomicfu is compiled with Kotlin 1.9+

In addition to removal of JS/Legacy configurations, JS compiler type should also be changed from BOTH to IR when switching to Kotlin 1.9. The removal condition is also based on Kotlin language version now instead of compiler release version (it allows to adopt newer compiler releases without removing JS/Legacy until language version is changed to 1.9+).

This test only fails during testing of Kotlin compiler dev builds at the moment, and thus should probably be muted in `kotlin-community`-branches instead of `develop`.

Tapchicoma · 2023-04-25T15:25:43Z

atomicfu/build.gradle

+// (right now it implements a toggle between IR and BOTH JS compiler types for testing of Kotlin dev builds)
+def fromVersion = { String arg -> KotlinVersion.values().find { it.version == arg } }
+def kotlinLanguageVersion = fromVersion(KotlinAggregateBuild.getOverriddenKotlinLanguageVersion(project) ?: "1.4")
+def isJsLegacyCompilerEnabled = kotlinLanguageVersion < KotlinVersion.KOTLIN_1_9


LanguageVersion has nothing with JS target deprecation. Should be checked using KGP version.

I made these changes based on the fact that CLI-level deprecation of JS/Legacy seems to work based on the language version at the moment, so we might want to take a closer look at it in the compiler as well then.

Hmm 👀
For now I rolled this back to KGP version check

Tapchicoma · 2023-04-25T15:26:57Z

atomicfu-gradle-plugin/build.gradle

+def kotlinVersion = ext.kotlin_version.split('\\.').take(2).collect { it.toInteger() }
+if (kotlinVersion[0] == 1 && kotlinVersion[1] == 9) {
+    test {
+        exclude "**/JsLegacyTransformationTest*", "**/MppLegacyTransformationTest*"
+    }
+}


I would propose to move kotlin version parsing into a method inside buildSrc module. So it could be used in all build files.

Tapchicoma · 2023-04-25T15:28:56Z

atomicfu/build.gradle

-    if (it instanceof IvyArtifactRepository) {
-        metadataSources {
-            artifact()
+    tasks.withType(compileJsLegacy.getClass()) {


compileJsLegacy should be already a Class.

after .withType .configureEach { ... } should be used

With gradle 6.8.3 compilation fails if I remove getClass()

I mean in the lines above it could be:

def compileJsLegacy = tasks.hasProperty("compileKotlinJsLegacy") ? compileKotlinJsLegacy.getClass() : compileKotlinJs.getClass()

mvicsokolova · 2023-04-25T16:45:33Z

buildSrc/src/main/kotlin/KotlinAggregateBuild.kt

    }
 }
+
+internal fun isKotlinVersionAtLeast(project: Project, atLeastMajor: Int, atLeastMinor: Int): Boolean =


Probably it should be moved to another file

atomicfu/build.gradle

FenstonSingel · 2023-04-25T19:54:04Z

I've tried compiling this branch with Kotlin 1.9.0-dev-6688 and various LVs. The results are:

for LV 1.4 (project LV), only one test fails (it is tracked by JVM legacy transformation: both original and transformed classes stay in the test classpath #295 and is muted in relevant TC configurations)
for LV 1.9, the results are the same as for LV 1.4
for LV 2.0, tests for post-compilation JVM transformers (transformedTestXX) fail (it should be fixed when the next kotlinx.metadata release is published and migrated to) and 11 out of 11 completed atomicfu-gradle-plugin tests fail (they are muted in relevant TC configurations so I haven't taken a closer look)

From testing of Kotlin dev-builds's perspective, everything is great for 1.9.0-dev TC configurations and good enough for K2 TC configurations (we will probably mute all tests in relevant TC configurations eventually regardless, so red tests are not a big problem).

mvicsokolova · 2023-04-25T21:49:03Z

Thank you!

I'll have a look what is wrong with atomicfu-gradle-plugin tests for lv=2.0

mvicsokolova · 2023-04-26T08:20:40Z

@FenstonSingel Am I right, that we can merge these change into develop now? And for dev-builds you will be able to mute failing tests for now?

* moved check function to buildsrc

mvicsokolova · 2023-04-26T11:26:54Z

I can locally reproduce the failure of atomicfu-gradle-plugin tests with lv = 1.9. They fail with Could not initialize class kotlinx.atomicfu.plugin.gradle.Platform. Probably that is somehow related to EnumEntries (e.g. KT-57317).

buildSrc/src/main/kotlin/KotlinVersion.kt

FenstonSingel · 2023-04-26T11:32:49Z

@FenstonSingel Am I right, that we can merge these change into develop now? And for dev-builds you will be able to mute failing tests for now?

Yes, any failing tests are not a blocker for dev-builds testing.

* JS Legacy build configurations as well as JS Legacy test projects were conditionally removed for KGP version >= 1.9.0 Co-authored-by: Stanislav Ruban <[email protected]> (cherry picked from commit 4dd0a6c)

mvicsokolova and others added 4 commits April 21, 2023 15:39

Remove js legacy compile configurations and js legacy test projects i…

78a0ea8

…f atomicfu is compiled with Kotlin 1.9+

Enable JvmProjectTest::testClasspath test back

968a268

This test only fails during testing of Kotlin compiler dev builds at the moment, and thus should probably be muted in `kotlin-community`-branches instead of `develop`.

Update metadata version to 0.6.0

135d328

mvicsokolova mentioned this pull request Apr 25, 2023

Removing JS Legacy compilation configurations #294

Closed

mvicsokolova requested a review from FenstonSingel April 25, 2023 15:14

Tapchicoma reviewed Apr 25, 2023

View reviewed changes

mvicsokolova commented Apr 25, 2023

View reviewed changes

FenstonSingel reviewed Apr 25, 2023

View reviewed changes

atomicfu/build.gradle Outdated Show resolved Hide resolved

mvicsokolova force-pushed the remove-js-legacy-tweaked branch from 23db8f0 to b831000 Compare April 26, 2023 11:01

Fixed: isJsCompilerEnabled == KGP < 1.9

6966e9d

* moved check function to buildsrc

mvicsokolova force-pushed the remove-js-legacy-tweaked branch from b831000 to 6966e9d Compare April 26, 2023 11:06

mvicsokolova requested a review from Tapchicoma April 26, 2023 11:06

FenstonSingel reviewed Apr 26, 2023

View reviewed changes

buildSrc/src/main/kotlin/KotlinVersion.kt Outdated Show resolved Hide resolved

buildSrc/src/main/kotlin/KotlinVersion.kt Outdated Show resolved Hide resolved

Tweak in kotlin version check

f6490f4

FenstonSingel approved these changes Apr 26, 2023

View reviewed changes

Minor fix for gradle script

9812645

mvicsokolova merged commit 4dd0a6c into develop Apr 26, 2023

mvicsokolova deleted the remove-js-legacy-tweaked branch April 26, 2023 13:52

Remove JS Legacy configurations (updated) #296

Remove JS Legacy configurations (updated) #296

Uh oh!

Conversation

mvicsokolova commented Apr 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FenstonSingel commented Apr 25, 2023

Uh oh!

mvicsokolova commented Apr 25, 2023

Uh oh!

mvicsokolova commented Apr 26, 2023

Uh oh!

mvicsokolova commented Apr 26, 2023

Uh oh!

Uh oh!

Uh oh!

FenstonSingel commented Apr 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants