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

CompileOnly for the KGP in wire-gradle-plugin #3100

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

Conversation

oldergod
Copy link
Member

No description provided.

@JakeWharton
Copy link
Member

Is this actually used in a way that works when KGP is not on the classpath?

@oldergod
Copy link
Member Author

It's more so that the version isn't transitively passed downstream. The cash-server monorepo has collateral damage with Wire being on 2.x.
I'm not 100% sure this will fix it but that's the try

@martinbonnin
Copy link
Collaborator

Is this actually used in a way that works when KGP is not on the classpath?

The wire plugin could fail with a nice message if no KGP is on the classpath (either using reflection or pluginManager.withId("org.jetbrains.kotlin...") {}).
compileOnly gives more freedom to consumers to chose their KGP version.

@JakeWharton
Copy link
Member

  1. Wire should not require KGP on the classpath since it can generate only Java
  2. The Cash server not being able to upgrade Kotlin is not Wire's problem. They can stay on old versions of Wire, force a lower KGP version, or use a KGP exclude. Yes the dependency should be migrated to compileOnly, but in a way that's tested for point 1 and not to band aid someone's project being stuck on old Kotlin.

@martinbonnin
Copy link
Collaborator

Wire should not require KGP on the classpath since it can generate only Java

Very good point 👍

@JakeWharton
Copy link
Member

  1. If we are concerned about people needing to select their own versions then consider depending on minimum-supported versions rather than the latest. This will also draw issues from people complaining about dependencies with CVEs because they rely on your transitive version, so it's a trade-off.

@oldergod oldergod force-pushed the bquenaudon.2024-09-11.compileonly branch from a0cbfa0 to be78550 Compare September 11, 2024 13:35
@oldergod
Copy link
Member Author

Thanks for the explanation. The PR as is sounds good to me.

@martinbonnin
Copy link
Collaborator

in a way that's tested for point 1

That's also a very good point. There's a very good chance that changing to compileOnly breaks the java-only consumers because linking the WirePlugin class now fails due to missing KGP symbols. Ideally the plugin is refactored so that "kotlin-only" portions are not accessed unless KGP is present but that's a much bigger lift.

@oldergod oldergod force-pushed the bquenaudon.2024-09-11.compileonly branch from be78550 to d1aaed0 Compare September 12, 2024 10:27
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