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

The problem of 2 atomicfu gradle plugins #370

Closed
mvicsokolova opened this issue Nov 23, 2023 · 4 comments
Closed

The problem of 2 atomicfu gradle plugins #370

mvicsokolova opened this issue Nov 23, 2023 · 4 comments

Comments

@mvicsokolova
Copy link
Collaborator

There are 2 atomicfu gradle plugins: kotlinx.atomicfu:atomicfu-gradle-plugin provided by the library and kotlin:atomicfu compiler plugin, the latter is not supposed to be applied directly and is applied by the atomicfu-gradle-plugin.

They not only create confusion among users but also serve as the cause of an important issue: atomicfu-gradle-plugin, when applied to the user project, should apply compiler plugin of the kotlin version used in the user project, not the kotlin version that was used to build atomicfu-gradle-plugin itself. That is not the case currently.

The reason: compiler plugin is found in the classpath of the user project, because it is stated as the implementation dependency of the atomicfu-gradle-plugin and hence leaks into the user project transitively, though with the kotlin version used in kotlinx.atomicfu.

The fast solution: for now the behaviour is correct when kotlin versions used in a user project and kotlinx.atomicfu are equal. Otherwise, atomicfu-gradle-plugin should ask a user to add the compiler plugin with the kotlin version used in the project directly:

buildscript {
    repositories {
        mavenCentral()
    }

    dependencies {
        classpath("org.jetbrains.kotlinx:atomicfu-gradle-plugin:$atomicfu_version}")
        classpath("org.jetbrains.kotlin:atomicfu:$my_kotlin_version")
    }
}

The right solution: getting rid of the gradle plugin on the side of the compiler and leaving only one gradle plugin in the library may help. To be further investigated.

@mvicsokolova mvicsokolova added gradle major An important problem that will require some effort labels Nov 23, 2023
mvicsokolova added a commit that referenced this issue Dec 6, 2023
* Atomicfu compiler plugin should be manually added to the classpath by the user, otherwise the error is thrown by the plugin
* All kotlin-stdlib transitive dependencies are removed

This commit is part of the solution to the problem described in #370
mvicsokolova added a commit that referenced this issue Dec 7, 2023
* Atomicfu compiler plugin should be manually added to the classpath by the user, otherwise the error is thrown by the plugin
* All kotlin-stdlib transitive dependencies are removed

This commit is part of the solution to the problem described in #370
mvicsokolova added a commit that referenced this issue Dec 7, 2023
* Atomicfu compiler plugin should be manually added to the classpath by the user, otherwise the error is thrown by the plugin
* All kotlin-stdlib transitive dependencies are removed
* Old atomicfu-gradle-plugin tests are removed, as they are now replaced with integration tests

This commit is part of the solution to the problem described in #370
@mvicsokolova
Copy link
Collaborator Author

mvicsokolova commented Dec 19, 2023

I see 2 possible options to address this problem:

I'll further refer to the plugins as the compiler plugin (AtomicfuGradleSubplugin in kotlin repo) and the gradle plugin (AtomicfuGradlePlugin in kotlinx-atomicfu repo).

1 Solution. Remove the compiler plugin from kotlin repo and stick to the gradle plugin:

Pros:

  • The release cycle will not be tied to Kotlin releases, hot-fixes may be published quickly if necessary.

Counterarg: most of the bugs that have recently required hot-fix releases were caused by the obscure logic in the gradle plugin itself. Moreover, fixes in IR transformations (which are on the way to become the only option of transformations) are tied to the Kotlin release anyway.

Cons:

  • This will require to publish the compiler module with actual transformation (kotlinx-atomicfu-compiler-plugin) to some custom repo (it can not be exposed at mavenCentral, it contains just internals of the compiler plugin) and provide dependency to this module by the gradle plugin somehow. That’s bad because it will cause the same problem described in this issue above: the user will get the dependency to the compiler plugin of the wrong version.

2 Solution. Remove the gradle plugin and migrate all the logic to the compiler plugin:

Pros:

  • compiler plugin is the regular gradle plugin, it's logic will be very clear and simple, plus it will be easier to provide gradle extension for plugin configuration (Improve atomicfu gradle extension  #341). And we can still manage the atomicfu dependencies provided to the user in this plugin if necessary.

Cons:

  • I can not move the logic for legacy transformations to the compiler plugin.

Counterarg: it not required, since I can have 2 plugins for a while: the legacy atomicfu-gradle-plugin that will be deprecated and still provide legacy transformations. And the preferred option of using the compiler plugin.

After these thoughts I decided to stick to the 2 solution, and the migration plan may be as follows:

  1. I move the logic to the compiler plugin and prepare integration tests in kotlinx.atomicfu
  2. As soon as Kotlin with these changes is released, I start to show a deprecation message in the legacy gradle plugin asking users to migrate to the new compiler plugin. This will solve this migration issue as well (Deprecate JVM bytecode transformer #338)
  3. The end result: no gradle plugin exists, users only apply the compiler plugin ~ like this:
plugins {
    kotlin("jvm") version "1.9.21"
    kotlin("plugin.atomicfu“) version "1.9.21"
}

atomicfu {
   isJsIrTransformationEnabled = false
   isJvmIrTransformationEnabled = true
   isNativeIrTransformationEnabled = true
}

// if necessary and the transformation is disabled users provide a runtime dependency to atomicfu
dependencies {
    implementation "org.jetbrains.kotlinx:atomicfu:0.25.0"
}

🤔

@mvicsokolova
Copy link
Collaborator Author

mvicsokolova commented Feb 26, 2024

After some discussion, it was decided to postpone moving atomicfu-gradle-plugin to the kotlin repo.

The reason is that we’ll get into the situation when 2 different entities are deprecated simultaneously: atomicfu-gradle-plugin itself and legacy bytecode transformations.
If a user runs into a bug with atomicfu compiler plugin, then there is no other choice but to fallback to bytecode transformations provided by the atomicfu-gradle-plugin and get the message that the plugin is deprecated.

To avoid a situation, when a new non-stable solution is proposed as default and the old one already is deprecated, it's decided to keep up with this plan:

  1. Enable compiler plugin transformations by default.
  2. Remove legacy bytecode transformations completely together with all related parts in atomicfu-gradle-plugin.
  3. Then move the atomicfu compiler plugin.

To pay attention 👀:

  • kotlinx-atomicfu library is required as a runtime dependency for wasm targets, for native targets to provide kotlinx.atomicfu.locks.* API, for tests. As atomicfu compiler plugin will add this dependency, there may potentially be problems with compatibility of plugin and library versions.

Regarding the problem that this solution was initially supposed to solve:

confusion caused by existence of 2 published atomicfu plugins

may be resolved with a warning message in case the wrong plugin is applied.

the wrong kotlin version of the compiler plugin provided by the atomicfu-gradle-plugin

should already be resolved with gradle version resolution in favor of the newer version present on the classpath (#386), though some problems are still present (#399).

@mvicsokolova mvicsokolova added postponed and removed major An important problem that will require some effort labels Feb 27, 2024
@mgroth0
Copy link

mgroth0 commented Feb 27, 2024

Hello, thank you for these updates. Can you please clarify in a few words, which atomicfu dependency should new users use? In particular, is there going to be one "main" atomicfu dependency when K2 stabilizes, and which one will it be?

@mvicsokolova
Copy link
Collaborator Author

For now, kotlinx-atomicfu should still be applied as stated in the README by adding dependency to org.jetbrains.kotlinx:atomicfu-gradle-plugin

buildscript {
    repositories {
        mavenCentral()
    }

    dependencies {
      classpath("org.jetbrains.kotlinx:atomicfu-gradle-plugin:0.23.2")
    }
}

apply(plugin = "kotlinx-atomicfu")

And then atomicfu-gradle-plugin will automatically add the dependency to the atomicfu compiler plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants