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

Apply atomicfu compiler plugin directly in buildscript classpath. #377

Merged
merged 5 commits into from
Dec 7, 2023

Conversation

mvicsokolova
Copy link
Collaborator

  • 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 that leaked into user projects are removed

This commit is part of the solution to the problem described in #370

@mvicsokolova mvicsokolova requested review from shanshin and removed request for shanshin December 6, 2023 22:15
@mvicsokolova mvicsokolova marked this pull request as draft December 6, 2023 22:22
* 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 mvicsokolova force-pushed the remove-transitive-stdlib-dependency branch from d0179bd to 53de063 Compare December 7, 2023 10:09
@mvicsokolova mvicsokolova marked this pull request as ready for review December 7, 2023 10:12
@mvicsokolova
Copy link
Collaborator Author

Here I also suggest to remove old atomicfu-gradle-plugin tests 👀: #378

@mvicsokolova
Copy link
Collaborator Author

I've added a separate commit with removal of old atomicfu-gradle-plugin tests to these changes.

  • Those tests did not honestly publish the current versin of atomicfu-gradle-plugin to the local repo
  • The current integration tests cover their functionality

@@ -1,10 +1,14 @@
buildscript {
repositories {
mavenLocal()
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that in the future it is better to use custom repositories for functional tests, rather than mavenLocal()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fzhinkin fzhinkin self-requested a review December 7, 2023 15:59
@mvicsokolova mvicsokolova merged commit 45ca0bd into develop Dec 7, 2023
1 check passed
@mvicsokolova mvicsokolova deleted the remove-transitive-stdlib-dependency branch December 7, 2023 16:04
mvicsokolova added a commit to Kotlin/kotlinx.coroutines that referenced this pull request Dec 11, 2023
…ript: when atomicfu-gradle-plugin is not added to the classpath first, it is not applied at all.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

The problem was revealed while running a train build before kotlinx-atomicfu 0.23.2 release.
mvicsokolova added a commit to Kotlin/kotlinx.coroutines that referenced this pull request Dec 11, 2023
…ript: when atomicfu-gradle-plugin is not added to the classpath first, it is not applied at all.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

The problem was revealed while running a train build before kotlinx-atomicfu 0.23.2 release. The coroutines build failed with error: "e: Module kotlinx.atomicfu cannot be found in the module graph".
woainikk pushed a commit to Kotlin/kotlinx.coroutines that referenced this pull request Dec 12, 2023
…ript: when atomicfu-gradle-plugin is not added to the classpath first, it is not applied at all.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

The problem was revealed while running a train build before kotlinx-atomicfu 0.23.2 release.
mvicsokolova added a commit to Kotlin/kotlinx.coroutines that referenced this pull request Dec 12, 2023
…ugin to the classpath.

 in the global buildscript: when atomicfu-gradle-plugin is not added to the classpath first, it is not applied at all.
 When kotlin-gradle-plugin is added first, atomicfu-gradle-plugin is not applied at all and this error is thrown: "e: Module kotlinx.atomicfu cannot be found in the module graph".

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

The problem was revealed while running a train build against kotlin-community/dev branches before kotlinx-atomicfu 0.23.2.
mvicsokolova added a commit to Kotlin/kotlinx.coroutines that referenced this pull request Dec 12, 2023
…ugin to the classpath.

 in the global buildscript: when atomicfu-gradle-plugin is not added to the classpath first, it is not applied at all.
 When kotlin-gradle-plugin is added first, atomicfu-gradle-plugin is not applied at all and this error is thrown: "e: Module kotlinx.atomicfu cannot be found in the module graph".

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

The problem was revealed while running a train build against kotlin-community/dev branches before kotlinx-atomicfu 0.23.2.
mvicsokolova added a commit to Kotlin/kotlinx.coroutines that referenced this pull request Dec 14, 2023
This commit changes the order of adding atomicfu and kotlin gradle plugin to the classpath.

When `kotlin-gradle-plugin` is added first, `atomicfu-gradle-plugin` is not applied at all and this error is thrown: "e: Module kotlinx.atomicfu cannot be found in the module graph". The change of order fixes the issue. Should be properly investigated.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

Here is the corresponding issue in atomicfu: Kotlin/kotlinx-atomicfu#384
woainikk pushed a commit to Kotlin/kotlinx.coroutines that referenced this pull request Jan 4, 2024
This commit changes the order of adding atomicfu and kotlin gradle plugin to the classpath.

When `kotlin-gradle-plugin` is added first, `atomicfu-gradle-plugin` is not applied at all and this error is thrown: "e: Module kotlinx.atomicfu cannot be found in the module graph". The change of order fixes the issue. Should be properly investigated.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

Here is the corresponding issue in atomicfu: Kotlin/kotlinx-atomicfu#384

(cherry picked from commit 605ec56)
woainikk pushed a commit to Kotlin/kotlinx.coroutines that referenced this pull request Jan 4, 2024
This commit changes the order of adding atomicfu and kotlin gradle plugin to the classpath.

When `kotlin-gradle-plugin` is added first, `atomicfu-gradle-plugin` is not applied at all and this error is thrown: "e: Module kotlinx.atomicfu cannot be found in the module graph". The change of order fixes the issue. Should be properly investigated.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

Here is the corresponding issue in atomicfu: Kotlin/kotlinx-atomicfu#384

(cherry picked from commit 605ec56)
woainikk pushed a commit to Kotlin/kotlinx.coroutines that referenced this pull request Jan 9, 2024
This commit changes the order of adding atomicfu and kotlin gradle plugin to the classpath.

When `kotlin-gradle-plugin` is added first, `atomicfu-gradle-plugin` is not applied at all and this error is thrown: "e: Module kotlinx.atomicfu cannot be found in the module graph". The change of order fixes the issue. Should be properly investigated.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

Here is the corresponding issue in atomicfu: Kotlin/kotlinx-atomicfu#384

(cherry picked from commit 605ec56)
woainikk pushed a commit to Kotlin/kotlinx.coroutines that referenced this pull request Jan 9, 2024
This commit changes the order of adding atomicfu and kotlin gradle plugin to the classpath.

When `kotlin-gradle-plugin` is added first, `atomicfu-gradle-plugin` is not applied at all and this error is thrown: "e: Module kotlinx.atomicfu cannot be found in the module graph". The change of order fixes the issue. Should be properly investigated.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

Here is the corresponding issue in atomicfu: Kotlin/kotlinx-atomicfu#384

(cherry picked from commit 605ec56)
mvicsokolova added a commit that referenced this pull request Jan 15, 2024
…ject (#386)

* Do not require to add atomicfu compiler plugin dependency manually to the buildscript of the project. This change reverts #377. To prevent resolution conflicts of the compiler plugin version, atomicfu-gradle-plugin adds dependency to the earliest plugin version 1.6.20 and gradle should resolve it to the newest version present in the classpath of the user project. 

Fixes #384

* Old atomicfu-gradle-plugin tests: remove tasks that used to log classpath of the plugin for atomicfu-gradle-plugin tests, as those tests are already removed.

* Resolved the warning for compileOnly dependency to kotlin-stdlib present in Native sourceSets. kotlin-stdlib dependency is added as an implementation dependency now, though it's version is marked as prefered to avoid resolution in the user project.
woainikk pushed a commit to Kotlin/kotlinx.coroutines that referenced this pull request Jan 18, 2024
This commit changes the order of adding atomicfu and kotlin gradle plugin to the classpath.

When `kotlin-gradle-plugin` is added first, `atomicfu-gradle-plugin` is not applied at all and this error is thrown: "e: Module kotlinx.atomicfu cannot be found in the module graph". The change of order fixes the issue. Should be properly investigated.

This behaviour started to reproduce after this change in kotlinx-atomicfu (Kotlin/kotlinx-atomicfu#377).

Here is the corresponding issue in atomicfu: Kotlin/kotlinx-atomicfu#384

(cherry picked from commit 605ec56)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants