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

Non-JVM targets are not supported #232

Open
brizzbuzz opened this issue Jan 19, 2022 · 26 comments
Open

Non-JVM targets are not supported #232

brizzbuzz opened this issue Jan 19, 2022 · 26 comments
Labels
enhancement New feature or request frozen help wanted Extra attention is needed

Comments

@brizzbuzz
Copy link

Describe the bug

Hey, TBH I'm not sure if this is a bug or just not implemented yet, I'm very new to Kotlin Multiplatform :) I'm trying to leverage Kaml to deserialize some YAML files in a MPP project, but it seems like trying to leverage Kaml in commonMain is not implemented yet when targeting non-JVM applications.

FAILURE: Build failed with an exception.

* What went wrong:
Could not determine the dependencies of task ':satisfaketion-generators:jsTestPackageJson'.
> Could not resolve all dependencies for configuration ':satisfaketion-generators:jsTestNpm'.
   > Could not resolve com.charleskorn.kaml:kaml:0.40.0.
     Required by:
         project :satisfaketion-generators
      > No matching variant of com.charleskorn.kaml:kaml:0.40.0 was found. The consumer was configured to find a usage of 'kotlin-runtime' of a library, preferably optimized for non-jvm, as well as attribute 'org.jetbrains.kotlin.platform.type' with value 'js', attribute 'org.jetbrains.kotlin.js.compiler' with value 'ir' but:
          - Variant 'jvmApiElements-published' capability com.charleskorn.kaml:kaml:0.40.0 declares a library:
              - Incompatible because this component declares an API of a component, as well as attribute 'org.jetbrains.kotlin.platform.type' with value 'jvm' and the consumer needed a usage of 'kotlin-runtime' of a component, as well as attribute 'org.jetbrains.kotlin.platform.type' with value 'js'
              - Other compatible attributes:
                  - Doesn't say anything about its target Java environment (preferred optimized for non-jvm)
                  - Doesn't say anything about org.jetbrains.kotlin.js.compiler (required 'ir')
          - Variant 'jvmRuntimeElements-published' capability com.charleskorn.kaml:kaml:0.40.0 declares a runtime of a library:
              - Incompatible because this component declares a component, as well as attribute 'org.jetbrains.kotlin.platform.type' with value 'jvm' and the consumer needed a component, as well as attribute 'org.jetbrains.kotlin.platform.type' with value 'js'
              - Other compatible attributes:
                  - Doesn't say anything about its target Java environment (preferred optimized for non-jvm)
                  - Doesn't say anything about org.jetbrains.kotlin.js.compiler (required 'ir')
          - Variant 'metadataApiElements' capability com.charleskorn.kaml:kaml:0.40.0 declares a library:
              - Incompatible because this component declares a usage of 'kotlin-api' of a component, as well as attribute 'org.jetbrains.kotlin.platform.type' with value 'common' and the consumer needed a usage of 'kotlin-runtime' of a component, as well as attribute 'org.jetbrains.kotlin.platform.type' with value 'js'
              - Other compatible attributes:
                  - Doesn't say anything about its target Java environment (preferred optimized for non-jvm)
                  - Doesn't say anything about org.jetbrains.kotlin.js.compiler (required 'ir')

Reproduction repo

So, this is the repo I am currently trying to convert to multiplatform https://github.com/unredundant/satisfaketion. More specifically, the generators module. I can't actually push the code b/c I have some (clearly too lofty) git hooks that prevent me from pushing a broken gradle build and I'm too lazy to delete them 😅 If it would really help I can temporarily remove them and push the code. But effectively, I have a gradle build file like this

plugins {
  kotlin("multiplatform")
  // ...
}

// ...

kotlin {
  sourceSets {
    val commonMain by getting {
      dependencies {
        implementation(kotlin("stdlib"))
        api(projects.satisfaketionCore)  // project dependency (also multiplatform)
        implementation("org.jetbrains.kotlinx:kotlinx-serialization-json:1.3.2")
        implementation("com.charleskorn.kaml:kaml:0.40.0")
      }
    }
    val commonTest by getting
    val jvmMain by getting 
    val jvmTest by getting
    val jsMain by getting
    val jsTest by getting
    val nativeMain by getting
    val nativeTest by getting
  }
}

which leads to the error posted above.

Steps to reproduce

  1. Import kaml dependency into MPP project which targets at least JS (presumably native has same issue?)
  2. Try to build
  3. Weep Profit?

Expected behaviour

I'm not sure this is expected behavior, but it would be nice if this was supported :)

Actual behaviour

Stacktrace sadness :(

Version information

Kaml 0.40.0
Kotlin 1.6.10

Any other information

No response

@charleskorn
Copy link
Owner

Hi @unredundant, yep, unfortunately kaml currently only supports JVM targets. I'd love to support other targets, but that's a long way down my current list of priorities.

If you (or anyone else reading this) is interested in submitting a PR, there are three possible options I see for implementing this:

  1. Find an existing pure-Kotlin YAML parser and use that as a replacement for SnakeYAML
  2. Write our own pure-Kotlin YAML parser and use that as a replacement for SnakeYAML
  3. Find an existing C YAML parser and use that for Kotlin/Native targets and a JS one for Kotlin/JS targets

Options 1 or 2 are preferable over option 3 as they would give a consistent experience across all targets. And option 1 is preferable over option 2 as it would mean less code to maintain in this library.

@charleskorn charleskorn changed the title Support for Non-JVM Targets is Broken Non-JVM targets are not supported Jan 20, 2022
@charleskorn charleskorn added enhancement New feature or request help wanted Extra attention is needed labels Jan 20, 2022
@westnordost
Copy link

westnordost commented Feb 19, 2022

Any idea how Him188/yamlkt does it? According to https://github.com/Kotlin/kotlinx.serialization/tree/master/formats#yaml-multiplatform it is YAML multiplatform but looking through their build config, I haven't found any hint of what they are using as parser on JS/Native.

@charleskorn
Copy link
Owner

Any idea how Him188/yamlkt does it?

They've written their own YAML parser, but the feature set is limited. From their readme:

The features that aren't yet supported:

  • Anchors (*, &)
  • Explicit types (e.g. !!map)
  • Multiline string (|, >, )

@akuleshov7
Copy link

They've written their own YAML parser, but the feature set is limited. From their readme:

Omg, instead of contributing to this awesome project and help with writing a parser, they have written their own serializer.
Now we have two incomplete projects...

@rohdef
Copy link

rohdef commented Jul 18, 2022

Stumbled over this, that might be a help: https://github.com/jurgc11/yakl according to the description it's a pure port of SnakeYAML

(And also the classic +1 for this feature)

@charleskorn
Copy link
Owner

Thanks for sharing @rohdef. Looks interesting but sadly also looks abandoned (there's been no changes for over two years). If nothing else, it's at least a starting point or source of inspiration.

@sschuberth
Copy link
Contributor

Omg, instead of contributing to this awesome project and help with writing a parser, they have written their own serializer. Now we have two incomplete projects...

While https://github.com/Him188/yamlkt was created after https://github.com/charleskorn/kaml, I'd argue that yamlkt follows the technically more attractive approach of a self-contained Kotlin-only library, instead of wrapping existing platform-specific libraries like kaml does. That allows yamlkt to share much more code across platforms.

In any case, it's indeed a bit unfortunate that engineering resources of people interested in YAML support for kotlinx-serialization is now divided across two projects.

So, one should consider what's more effort: Adding more platforms to kaml by wrapping more platform-specific libraries (including the native platform), or adding the remaining features to yamlkt. My personal hunch is that the latter is maybe a bit less work, with more gain in the mid-term.

@westnordost
Copy link

@sschuberth I partly agree. I think the best approach would be to have a pure Kotlin YAML parser and separately from that, a YAML plugin for kotlinx-serialization that uses that YAML parser.

kaml suffers from that a pure Kotlin YAML parser does not exist (yet) and yamlkt suffers from that they are trying to write a YAML parser from scratch (and mashed it into the same project as the kotlinx-serialization plugin).

@stale
Copy link

stale bot commented Nov 2, 2022

This issue has been automatically marked as stale because it has not had any activity in the last 60 days. It will automatically be closed if no further activity occurs in the next seven days to enable maintainers to focus on the most important issues.
If this issue is still affecting you, please comment below within the next seven days.
Thank you for your contributions.

@stale stale bot added the stale label Nov 2, 2022
@stale
Copy link

stale bot commented Nov 9, 2022

This issue has been automatically closed because it has not had any recent activity.

@stale stale bot closed this as completed Nov 9, 2022
@russellbanks
Copy link
Contributor

@charleskorn Can we keep this open?

@charleskorn charleskorn added frozen and removed stale labels Apr 29, 2023
@charleskorn charleskorn reopened this Apr 29, 2023
@krzema12
Copy link
Contributor

I'm considering porting snakeyaml-engine to multiplatform. Started a thread in https://kotlinlang.slack.com/archives/C7A1U5PTM/p1684498259191979?thread_ts=1684498259.191979&cid=C7A1U5PTM

@charleskorn
Copy link
Owner

I'm considering porting snakeyaml-engine to multiplatform. Started a thread in https://kotlinlang.slack.com/archives/C7A1U5PTM/p1684498259191979?thread_ts=1684498259.191979&cid=C7A1U5PTM

That would be really cool. I don't have much time to help out with the porting, but let me know once you've got something ready and I can give it a go in kaml.

@sschuberth
Copy link
Contributor

I'm considering porting snakeyaml-engine to multiplatform.

For those who want to follow the port, see https://github.com/krzema12/snakeyaml-engine-kmp.

@krzema12
Copy link
Contributor

Here's a WIP integration with the multiplatform port of snakeyaml-engine #437. I need to pause working on it, but it potentially requires minimal effort to push to a mergeable state.

@krzema12
Copy link
Contributor

krzema12 commented Jul 30, 2023

kaml 0.55.0 with experimental Kotlin/JS support has just been released! Feel free to give it a spin and give feedback.
Kudos @aSemy for rewriting most of snakeyaml-engine to KMP, as https://github.com/krzema12/snakeyaml-engine-kmp! It wouldn't be possible without your support.

@russellbanks
Copy link
Contributor

Could we get Kotlin Native support?

@krzema12
Copy link
Contributor

krzema12 commented Aug 24, 2023

@russellbanks definitely doable, do you feel like contributing to https://github.com/krzema12/snakeyaml-engine-kmp? Should be as simple as adding more Kotlin targets, without actual source code change. CC @aSemy

Edit: actually, the snakeyaml KMP port already supports some native targets, so just kaml needs an adjustment.

@aSemy
Copy link
Contributor

aSemy commented Aug 24, 2023

K/N support is definitely possible. In principle all that's needed is to move the jsMain code that was added in #437 into commonMain. Hopefully it just works, with minimal changes.

@krzema12
Copy link
Contributor

krzema12 commented Jan 21, 2024

@charleskorn I think we can try switching kaml fully to https://github.com/krzema12/snakeyaml-engine-kmp, also for the JVM. Then it will be simple to handle virtually all possible Kotlin targets, and we'll be able to close this issue.
@aSemy since you know the lib well - do you think it's a good idea?

@aSemy
Copy link
Contributor

aSemy commented Jan 21, 2024

Yes, I think snakeyaml-engine-kmp will work well.

Some notes

  • Since Kaml added a JS target, snakeyaml-engine-kmp added the YAML test suite for all targets, and the behaviour is 100% consistent on all platforms.
  • Performance hasn't been evaluated, but since the Java code was just simply converted to Kotlin I don't expect a significant change.
  • afaik nested Kotest tests still aren't supported on JS Implement JS target (experimental) #437 (comment)

@charleskorn
Copy link
Owner

@krzema12 @aSemy I'd be happy to review a PR that switches all targets to use snakeyaml-engine-kmp.

If any changes to the tests are required, it'll be easiest for me to review those separately - so please open a separate PR if this is required.

@krzema12
Copy link
Contributor

krzema12 commented Feb 2, 2024

WIP in #507

@krzema12
Copy link
Contributor

krzema12 commented Mar 4, 2024

I have to drop working on this, sorry! Feel free to take over.

@krzema12
Copy link
Contributor

krzema12 commented Mar 20, 2024

Looking good, switching to the KMP implementation of YAML engine (#507) has been merged and released, kudos to @aSemy for picking it up! We're now waiting for feedback from the JVM consumers to ensure there was no regression, and in parallel working on introducing more targets. Starting with enabling more targets in snakeyaml-engine-kmp (krzema12/snakeyaml-engine-kmp#141).

aSemy added a commit to krzema12/snakeyaml-engine-kmp that referenced this issue Mar 29, 2024
The main motivation for this is pushing
charleskorn/kaml#232 further. I'd like this
library to be truly multiplatform, and work with all targets supported
by Kotlin.

---------

Co-authored-by: Adam <897017+aSemy@users.noreply.github.com>
@krzema12
Copy link
Contributor

krzema12 commented Mar 29, 2024

snakeyaml-engine-kmp 2.7.5 is released that supports all available Kotlin targets, apart from some unpopular ones.

The next step is to make kaml build and release with these targets as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request frozen help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

9 participants