Skip to content

Atomicfu Js/Ir compiler plugin supported in the gradle plugin #215

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Apr 14, 2022
Merged

Atomicfu Js/Ir compiler plugin supported in the gradle plugin #215

merged 6 commits into from
Apr 14, 2022

Conversation

mvicsokolova
Copy link
Collaborator

  • Added support of the compiler plugin transformation to the AtomicfuGradlePlugin.
  • atomicfu-gradle-plugin test projects are ignored for a while. I'm going to create a PR with new test projects soon.
  • Kotlin version is for now 1.7.0-dev-565, built from the 1.6.20-M1-release branch

@mvicsokolova mvicsokolova force-pushed the atomicfu-compiler-plugin-support branch from d0554a2 to 2fd0ad2 Compare January 25, 2022 10:45
@mvicsokolova mvicsokolova requested a review from qwwdfsad January 25, 2022 10:46
@ghost
Copy link

ghost commented Jan 25, 2022

@mvicsokolova just for curiosity, what is the main difference between JS and IR variants?

@mvicsokolova
Copy link
Collaborator Author

Hi, @olme04!
In case of JS option the transformation is applied after the compilation to the generated *.js files and JavaScript transformer is quite fragile as it depends on the JS syntax.

Whereas, for IR option the transformation is applied to the IR and relies to the compiler API of the IR tree, that is a more stable approach. So, IR option is preferable if the Kotlin/JS IR backend is used for compilation.

@mvicsokolova
Copy link
Collaborator Author

Atomicfu compiler plugin for Kotlin/JS IR backend will be supported since Kotlin 1.6.20

@mvicsokolova mvicsokolova changed the base branch from master to develop January 25, 2022 11:30
@ghost
Copy link

ghost commented Jan 25, 2022

@mvicsokolova, thx for the answer!
One more question - what should be the best variant, when using js(BOTH)? (my used case is library, to run tests on both legacy and IR)

@mvicsokolova mvicsokolova force-pushed the atomicfu-compiler-plugin-support branch from 2fd0ad2 to 9bdc603 Compare February 4, 2022 09:28
@mvicsokolova mvicsokolova force-pushed the atomicfu-compiler-plugin-support branch from 9bdc603 to 33317bb Compare April 11, 2022 14:54
@mvicsokolova mvicsokolova force-pushed the atomicfu-compiler-plugin-support branch from 33317bb to 173220b Compare April 11, 2022 15:05
@qwwdfsad qwwdfsad removed their request for review April 11, 2022 18:02
@mvicsokolova mvicsokolova requested a review from ilgonmic April 11, 2022 19:49
@mvicsokolova mvicsokolova requested a review from qwwdfsad April 12, 2022 11:17
@mvicsokolova mvicsokolova force-pushed the atomicfu-compiler-plugin-support branch from c008910 to 4cebe94 Compare April 12, 2022 22:30
mvicsokolova
mvicsokolova commented Apr 12, 2022
View reviewed changes
atomicfu-gradle-plugin/src/main/kotlin/kotlinx/atomicfu/plugin/gradle/AtomicFUGradlePlugin.kt Outdated
// apply the compiler plugin, if IR transformation should be applied to KotlinJsIrTarget compilations
plugins.apply(AtomicfuKotlinGradleSubplugin::class.java)
}
plugins.apply(AtomicfuKotlinGradleSubplugin::class.java)
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 figured, that we can only obtain values passed to the atomic {} extension after project evaluation, though jsVariant option was supposed to define whether to apply the compiler plugin (AtomicfuKotlinGradleSubplugin) and compiler plugin should be applied before project evaluation is completed.

For now, compiler plugin is applied by default.

Maybe there is some other way to create a user flag, that can be resolved before the project is evaluated?

qwwdfsad
qwwdfsad reviewed Apr 13, 2022
View reviewed changes
atomicfu-gradle-plugin/src/main/kotlin/kotlinx/atomicfu/plugin/gradle/AtomicFUGradlePlugin.kt Outdated
val pluginVersion = rootProject.buildscript.configurations.findByName("classpath")
?.allDependencies?.find { it.name == "atomicfu-gradle-plugin" }?.version
extensions.add(EXTENSION_NAME, AtomicFUPluginExtension(pluginVersion))
if (rootProject.findProperty(ENABLE_IR_TRANSFORMATION).toString().toBoolean()) {
Copy link
Member

Choose a reason for hiding this comment

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

You probably need toBooleanStrict here to prevent typos coercing into false

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Gradle seems to force stdlib version to 1.4, the same as bundled in it's runtime. So using toBooleanStrict() fails with unresolved reference. And forcing stdlib version in atomicfu-gradle-plugin does not help. Maybe we should update gradle version?)

Copy link
Member

Choose a reason for hiding this comment

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

I believe it's safer to copy-paster toBooleanStrict implementation here

mvicsokolova and others added 2 commits April 13, 2022 18:10
@mvicsokolova @qwwdfsad
Fix README.md
e43743c 
Co-authored-by: Vsevolod Tolstopyatov <qwwdfsad@gmail.com>
@mvicsokolova @qwwdfsad
Fix README.md
a647688 
Co-authored-by: Vsevolod Tolstopyatov <qwwdfsad@gmail.com>
@mvicsokolova mvicsokolova force-pushed the atomicfu-compiler-plugin-support branch 2 times, most recently from 80b75a3 to a2b3270 Compare April 14, 2022 11:46
@mvicsokolova mvicsokolova force-pushed the atomicfu-compiler-plugin-support branch from a2b3270 to 4fc4d1e Compare April 14, 2022 11:59
@mvicsokolova mvicsokolova merged commit 8e1f8b3 into develop Apr 14, 2022
@mvicsokolova mvicsokolova deleted the atomicfu-compiler-plugin-support branch April 14, 2022 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qwwdfsad qwwdfsad qwwdfsad left review comments

@ilgonmic ilgonmic ilgonmic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mvicsokolova @qwwdfsad @ilgonmic