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

WA: Disable JS legacy transformation manually until it is completely removed from the atomicfu plugin. #4223

Merged

Conversation

mvicsokolova
Copy link
Contributor

Fixes KT-71203

Through an oversight atomicfu-gradle-plugin kept tranformJs option (corresponding to the legacy JS transformation) enabled by default, and with no *.js files to transform it just copied the compileJs output directory, which led to an error described in KT-71203.

I suggest to manually disable JS transformation in all subprojects, which apply atomicfu plugin (I couldn't do it more gracefully in the root build script 😔 ).

This fix can be removed as soon as JS transformation is removed completely in atomicfu library. I plan to prepare a corresponding commit and provide it with the next kotlinx-atomicfu release.

…removed from the atomicfu plugin.

Fixes KT-71203

atomicfu {
transformJs = false
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

But kotlinx-coroutines-reactive doesn't build any JS artifacts. Of the subprojects listed, only -core, -test, and test-utils do. Is this really needed?


atomicfu {
transformJs = false
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't this be put into buildSrc/src/main/kotlin/configure-compilation-conventions.gradle.kts? The atomicfu plugin is applied there, and it would be nice to keep the related logic close together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I applied this config in configure-compilation-conventions in the last commit)

@@ -22,3 +22,7 @@ kotlin {
}
}
}

atomicfu {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
atomicfu {
// workaround for KT-71203. Can be removed after https://github.com/Kotlin/kotlinx-atomicfu/issues/431
atomicfu {

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this comment 🙏


configure(subprojects) {
val project = this
if (name in sourceless) return@configure
apply(plugin = "org.jetbrains.kotlinx.atomicfu")
// Workaround for KT-71203. Can be removed after https://github.com/Kotlin/kotlinx-atomicfu/issues/431
afterEvaluate {
Copy link
Contributor Author

@mvicsokolova mvicsokolova Sep 13, 2024

Choose a reason for hiding this comment

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

The problem is that AtomicFUPluginExtnesion type is not accessible in the root build script yet, and seems that the only way to dynamically set the property in this extension class is via reflection 🥲
(At least the config is not scattered across the build scripts of all the subprojects)

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about something like this? 60c0707

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool! Thanks, that's much better and solves the problem)

@mvicsokolova mvicsokolova merged commit 2636159 into develop Sep 13, 2024
1 check passed
@mvicsokolova mvicsokolova deleted the mvicsokolova/disable-js-legacy-transformation-manually branch September 13, 2024 14:06
tbogdanova pushed a commit that referenced this pull request Sep 16, 2024
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
tbogdanova pushed a commit that referenced this pull request Sep 18, 2024
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
tbogdanova added a commit that referenced this pull request Sep 19, 2024
…pletely removed from the atomicfu plugin. (#4223)"

This reverts commit 863575a.
woainikk added a commit that referenced this pull request Sep 25, 2024
…pletely removed from the atomicfu plugin. (#4223)"

This reverts commit 2636159.
woainikk added a commit that referenced this pull request Sep 25, 2024
…pletely removed from the atomicfu plugin. (#4223)"

This reverts commit 2636159.
woainikk added a commit that referenced this pull request Sep 25, 2024
…pletely removed from the atomicfu plugin. (#4223)"

This reverts commit 2636159.
woainikk pushed a commit that referenced this pull request Oct 1, 2024
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
woainikk pushed a commit that referenced this pull request Oct 2, 2024
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
woainikk pushed a commit that referenced this pull request Oct 2, 2024
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
tbogdanova pushed a commit that referenced this pull request Oct 8, 2024
…pletely removed from the atomicfu plugin. (#4223)"

This reverts commit 2636159.
tbogdanova pushed a commit that referenced this pull request Oct 8, 2024
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
tbogdanova pushed a commit that referenced this pull request Oct 25, 2024
…pletely removed from the atomicfu plugin. (#4223)"

This reverts commit 2636159.
tbogdanova pushed a commit that referenced this pull request Oct 25, 2024
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
woainikk added a commit that referenced this pull request Jan 7, 2025
…pletely removed from the atomicfu plugin. (#4223)"

This reverts commit 2636159.
woainikk pushed a commit that referenced this pull request Jan 7, 2025
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
woainikk added a commit that referenced this pull request Jan 7, 2025
…pletely removed from the atomicfu plugin. (#4223)"

This reverts commit 2636159.
woainikk pushed a commit that referenced this pull request Jan 7, 2025
…removed from the atomicfu plugin. (#4223)

Fixes KT-71203

---------

Co-authored-by: Dmitry Khalanskiy <dmitry.khalanskiy@jetbrains.com>
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.

2 participants