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

Add support for Kotlin K2 compiler plugin #716

Merged
merged 6 commits into from
Jun 4, 2024

Conversation

markushi
Copy link
Member

@markushi markushi commented May 28, 2024

📜 Description

As of now:

  • tests are broken, seems to be some configuration/version issue with the compiletesting library
DEFAULT
java.lang.NoSuchFieldError: DEFAULT
	at com.tschuchort.compiletesting.KotlinCompilation.<init>(KotlinCompilation.kt:143)
	at io.sentry.compose.JetpackComposeInstrumentationTest$Fixture.compileFile(JetpackComposeInstrumentationTest.kt:83)
  • when plugin is applied, it no-ops with the following error. This seems to stem from some compiler side API changes, it's probably best to peek into the JPC plugin source and align with their way of doing it.
w: No definition of androidx.compose.ui.Modifier found, Sentry Kotlin Compiler plugin won't run. Please ensure you're applying to plugin to a compose-enabled project.

💡 Motivation and Context

Support K2, fixes #698

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

Copy link
Contributor

github-actions bot commented May 28, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 4f92f66

@romtsn
Copy link
Member

romtsn commented May 28, 2024

there's a better maintained fork of kotlin-comple-testing, maybe we should try that one? https://github.com/ZacSweers/kotlin-compile-testing

markushi added 2 commits June 3, 2024 12:27
Avoid creating potentially unused variables, wrap modifiers in-place
instead
@markushi
Copy link
Member Author

markushi commented Jun 3, 2024

there's a better maintained fork of kotlin-comple-testing, maybe we should try that one? ZacSweers/kotlin-compile-testing

Yeah I tried, but unfortunately that fork suffers from the same limitation:

Because the internal APIs of the Kotlin compiler often change between versions, we can only support one kotlin-compiler-embeddable version at a time.
(link)

@markushi markushi marked this pull request as ready for review June 3, 2024 10:40
@markushi markushi changed the title [WIP] Add support for Kotlin K2 compiler plugin Add support for Kotlin K2 compiler plugin Jun 3, 2024
Copy link
Member

@romtsn romtsn left a comment

Choose a reason for hiding this comment

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

LGTM, good stuff! I assume it's still backward-compatible with older kotlin compiler versions, right?

@romtsn
Copy link
Member

romtsn commented Jun 3, 2024

looks like the test is failing because we're still running against an older kotlin version. Should we introduce another dimension to the matrix for kotlin versions?

markushi added 2 commits June 3, 2024 15:40
fqName is discouraged in 2.0.0, and packageFqName didn't exist before
2.0.0
@kahest kahest mentioned this pull request Jun 3, 2024
@markushi
Copy link
Member Author

markushi commented Jun 4, 2024

looks like the test is failing because we're still running against an older kotlin version. Should we introduce another dimension to the matrix for kotlin versions?

Yeah I tried to do that, but upgrading to Kotlin 2.0 breaks in several areas:

  • kapt breaks in the android-instrumentation-sample example
  • the -Xjvm-default=enable flag is not supported in 2.0 anymore

Hate to say it, but I'll create a follow up ticket for this. In the meantime I tested this change with different Kotlin versions locally.

@markushi markushi merged commit dfa635f into main Jun 4, 2024
26 checks passed
@markushi markushi deleted the feat/k2-kotlin-compiler-plugin-support branch June 4, 2024 07:08
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.

Support K2 compiler
2 participants