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

Avoid applying compiler plugin to unsupported native targets #3657

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

natario1
Copy link

Otherwise, we get:

e: Compilation failed: Cannot find the Composer class in the classpath

@natario1
Copy link
Author

Hi team, in addition to the change above, I'd be interested in adding targets - even among those that are supported - and NOT enable compose for them. Currently this is not possible because of the error above, and the root issue is that compiler plugin application and dependencies are intentionally out of sync, one is done by the plugin, the other by the user. This can be a source of problems for some.

How could we enable this scenario? Some ideas:

  1. Global gradle.properties flag: if found, we ignore the Cannot find the Composer class in the classpath error. But I'm not sure if possible to ignore errors in the plugins pipeline.
  2. Gradle extension function, like needsCompilerPlugin((KotlinTarget) -> Bool), defaults to { true }. Flexible, maybe it may suffer from some race conditions.
  3. Change compiler plugin so that instead of throwing Cannot find the Composer class in the classpath, it just returns. If you think about it long enough, I think it makes sense. If there's no Composer class, there can't possibly be any @Composable? so there's no need for the compiler plugin to run at all, but also no need to throw.

Note: if I implement any of the above, it would be in another PR and only after your feedback. So feel free to merge this.


import org.jetbrains.kotlin.konan.target.KonanTarget

internal val SUPPORTED_NATIVE_TARGETS = setOf(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, Compose Compiler plugin supports more k/native targets than listed here. Compose Compile plugin can support any kotlin target which has compose-runtime compiled for it. Here is a list of compose-runtime targets: https://github.com/JetBrains/compose-multiplatform-core/blob/jb-main/compose/runtime/runtime/build.gradle#L75

private val SUPPORTED_NATIVE_TARGETS = setOf( - this is a recent change. I'll ask my team if linux,watchos and others were intentiomally excluded here.

Copy link
Author

Choose a reason for hiding this comment

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

I see, please let me know. I can make the list match the list you linked, and add a comment there to remind that new targets should be added to this set.

@eymar
Copy link
Collaborator

eymar commented Sep 13, 2023

I'm wondering in what case you see e: Compilation failed: Cannot find the Composer class in the classpath.

This erros probably means that you applied the compose plugin but didn't add implementation(compose.runtime) dependency to your source sets.

@natario1
Copy link
Author

To reproduce, you can just add one of the androidNative() targets. Which don't have a runtime available.

In fact, now that coroutines have androidNative support, Compose Multiplatform could support these too, but that's a separate issue.

@eymar
Copy link
Collaborator

eymar commented Sep 13, 2023

In fact, now that coroutines have androidNative support, Compose Multiplatform could support these too, but that's a separate issue.

Nice. We'll consider that. Besides coroutines, we need atomic-fu as well.


Let me ask more details about your intentions.

So you have multiple targets in your module and you apply compose gradle plugin to that module. Right?
Do you have a common source set in that module? If so, you don't plan to use anything Compose related in it, right?
Do you want to use Compose (I mean declare and call Composable functions) only in platform-specific source sets?


We didn't consider a use case with mixing supported and not supported targets within one module.
If you can't or don't want to use Compose for one of the targets while using it for others, the current advise is to split your module into 2+ modules.

You mentioned androidNative(), so currently you can't use Compose in it, but you want to have it as a target. Right? But do you want to use Compose in it if it was possible? Or you just want to have it as a target and make Compose plugin not complain about it?

@natario1
Copy link
Author

natario1 commented Sep 13, 2023

Besides coroutines, we need atomic-fu as well.

Yeah, they are available too. All kotlinx libraries are AFAIK.

You mentioned androidNative(), so currently you can't use Compose in it, but you want to have it as a target. Right?

Right!

But do you want to use Compose in it if it was possible? Or you just want to have it as a target and make Compose plugin not complain about it?

Both.
I have a module where compose runtime is used in a non-UI way. There, I would really like to use it in androidNative.
But then I have other modules which use compose UI, where it would make no sense in androidNative, and I'd just like the plugin to not break the androidNative build.

KMP is smart enough to let me choose when to add the dependency:

kotlin {
    val composeMain by sourceSets.creating {
        dependencies { api(compose.runtime) }
    }
    
    // Then, select targets to depend on composeMain...
    iosArm64 { compilations["main"].defaultSourceSet.dependsOn(composeMain) } // compose
    androidNativeArm64() // no compose
}

So I'd like the same level of flexibility from the plugin application. Could be solved in many ways I think -- downgrade Cannot find the Composer class in the classpath to a warning is my favourite. No runtime implies no @Composable which implies no need for plugin to run at all.

@eymar eymar added discussion Need further discussion to understand if it actually needed to be considered labels Sep 14, 2023
@eymar
Copy link
Collaborator

eymar commented Sep 14, 2023

Thank you @natario1
We'll take some time to discuss how to approach such use cases and provide the flexibility to configure the plugin.

@natario1
Copy link
Author

natario1 commented Sep 14, 2023

Thanks guys. @eymar let me just add something which came to mind, maybe it will simplify things.

If a target is supported by compose (say iosArm64), but I don't want to enable compose in it, there's a simple trick to avoid the error: just use compileOnly(compose.runtime) as a dependency. It's not a perfect solution, ideally the compiler plugin should not even run, but it is a workaround at least.

So if we had the androidNative runtime, all the use cases would have a solution/workaround:

  • for androidNative modules where I don't want to use compose, add compileOnly(compose.runtime)
  • for androidNative modules where I'd like to use compose, add implementation(compose.runtime)

androidNative has been recently added to all kotlinx libs, because of the tiers definition. https://kotlinlang.org/docs/native-target-support.html

Edit: I take that back, Kotin/Native does not support compileOnly. Using compileOnly results in a warning and the dep is added as implementation anyway.

@eymar
Copy link
Collaborator

eymar commented Sep 22, 2023

@natario1 This #3695 can be interesting to you. There is a workaround.
Anyway, we're working on a proper solution for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Need further discussion to understand if it actually needed to be considered
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants