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

Increase Kotlinx Serialization version used by the Compose Gradle Plugin #3479

Closed
bitspittle opened this issue Aug 10, 2023 · 3 comments · Fixed by #3607
Closed

Increase Kotlinx Serialization version used by the Compose Gradle Plugin #3479

bitspittle opened this issue Aug 10, 2023 · 3 comments · Fixed by #3607
Assignees
Labels
enhancement New feature or request

Comments

@bitspittle
Copy link

According to this: BuildProperties.kt#L14, the Compose plugin depends on kotlinx-serialization 1.2.1.

I'm writing my own Gradle plugin for a project built on top of Compose, and I'm using another library which depends on serialization. I'd like to upgrade that library to the most recent version (to fix a reported security issue), but it is using something introduced in a later version of Kotlinx serialization.

When the Compose plugin is applied first, the older version of serialization is pulled in, which then causes my plugin to crash when it applies and tries parsing something indirectly via that library.

Since it's pretty common for Kotlin serialization to be used across multiple artifacts / plugins, and since JVM classpath resolution works by using an existing artifact if already found on the classpath (as far as I can tell), it would be really nice if the team could keep the serialization library up to date.

@bitspittle bitspittle added enhancement New feature or request submitted labels Aug 10, 2023
@bitspittle
Copy link
Author

bitspittle commented Aug 10, 2023

I'm also not sure how the Compose Gradle plugin is pulling in that dependency.

I was trying to force things to resolve to a newer version in my Gradle plugin, using Gradle configuration resolution tricks, but it's weird - it doesn't seem to find the older serialization library, even though in the end, that's the one that gets used.

I'm wondering if that's because of the embedded trick used here? I'm not really familiar with what's going on here and if this is confusing Gradle somehow.

 fun embedded(dep: Any) {
        compileOnly(dep)
        testCompileOnly(dep)
        embeddedDependencies(dep)
    }

 embedded("org.jetbrains.kotlinx:kotlinx-serialization-json:${BuildProperties.serializationVersion}")
 embedded("org.jetbrains.kotlinx:kotlinx-serialization-core:${BuildProperties.serializationVersion}")
 embedded("org.jetbrains.kotlinx:kotlinx-serialization-core-jvm:${BuildProperties.serializationVersion}")

@AlexeyTsvetkov
Copy link
Collaborator

I'm wondering if that's because of the embedded trick used here

Most likely. Generally, Gradle's dependency resolution considers all dependencies and chooses the highest version.
However, we embed our dependencies into one uber jar, because by default Gradle resolves plugins using Gradle plugin portal only. And Gradle plugin portal hosts only plugin jars themselves.
Since Gradle also does not have any mechanism of classloader isolation between different plugins, it is possible that kotlinx-serialization from our plugin would be loaded based on plugin declaration order (actual apply order does not matter).

I see the following options:

  1. We can stop embedding kotlinx-serialization (replace embedded with implementation). This would break projects that don't have mavenCentral() (or Compose Dev maven repo) in plugin repositories.
  2. We could try to relocate kotlinx-serialization to a different package. I don't know if kotlinx-serialization would survive relocation. I asked kotlinx-serialization maintainers, so we could wait for an answer or just try.
  3. Our usage of kotlinx-serialization is limited to our built-in iOS simulator/devices management. However, since it was written we decided that trying to support xcode/xcodebuild functionality is hopeless, so our current recommended way of interacting with iOS tooling is via actual XCode. I think we will drop this code sooner or later. Maybe we could do it sooner?
  4. Finally, we could try to keep kotlinx-serialization up-to-date. I like this option the least, because in some cases we would still end up overriding kotlinx-serialization for other plugins, but now in the other direction. It's not catastrophic, but I would prefer to avoid effectively overriding anyone's dependencies.

@dima-avdeev-jb what do you think?

@dima-avdeev-jb
Copy link
Contributor

dima-avdeev-jb commented Aug 11, 2023

@AlexeyTsvetkov
I think, we can drop support of simulator/devices management in our Compose Multiplatform Gradle plugin and remove kotlinx-serialization dependency. This feature was experimental and we provide better way to launch from IDE.
Also, I think that it is better to apply after Compose 1.5.0 release.

@dima-avdeev-jb dima-avdeev-jb self-assigned this Aug 11, 2023
AlexeyTsvetkov added a commit that referenced this issue Aug 31, 2023
Gradle plugin:
* Update Gradle to 8.3;
* Enable Configuration Cache;
* Update Gradle plugins;
* Start using version catalogs;
* Update dependencies;
* Relocate Kotlinx Serialization (#3479)

Resolves #3479

Intellij plugin:
* Update Gradle to 8.3;
* Enable Configuration Cache;
* Start using version catalogs;
AlexeyTsvetkov added a commit that referenced this issue Sep 1, 2023
AlexeyTsvetkov added a commit that referenced this issue Sep 2, 2023
AlexeyTsvetkov added a commit that referenced this issue Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants