Migrate from kaml to yamlkt

[FloEdelmann]

• Fixes Multiplatform config file parsing #5415
• Replaces and closes Change config data from YAML to JSON #5444

Using (multiplatform) YamlKt instead of converting the files to JSON turned out to be much easier than trying to convert all files to JSON for now.

We can still change config files from YAML to JSON in the future to eventually get rid of the YAML parser completely. We might want to wait for Kotlin/kotlinx.serialization#2221 to allow comments in JSON files (and use the .jsonc file extension to reflect that). Trailing commas are already allowed with the allowTrailingComma option.

[westnordost]

Looks good to me! Do all the tests pass and did you test it? (I.e. look in the app at places where data parsed from Yaml should appear)

[FloEdelmann]

Yesterday, I checked it in the app manually and everything seemed to work fine. But I didn't run the tests.

Now I did and the unit tests all pass, but the Android tests revealed that yamlkt has a problem with unquoted strings like BEKB | BCBE, which do indeed occur in country metadata (in this case it's a Swiss ATM operator).

So as predicted, the YAML format proves to add some difficulties 😅

The library we use to write the country metadata files (YamlBeans) has an option to quote strings, but it also quotes boolean values etc. then. So I'll look into writing the files with yamlkt now.

[FloEdelmann]

YamlKt is now also used (instead of YamlBeans) to generate the country metadata files, which works fine if the strings are always quoted. Otherwise, it splits e.g. Tesla, Inc. into two list items instead of one quoted one (see e.g. AE.yml). I opened Him188/yamlkt#68.

Also, YamlKt has a stricter parser, which requires us to wrap e.g. regexes (which contain special characters like [) in single quotes, see streetcomplete/countrymetadata#23.

Now the Android tests pass, except for the (unrelated) ones that are currently failing on master, too.

I also changed the GetTranslatorCreditsTask to use YamlKt instead of YamlBeans, but I am unable to test this because I don't have the access rights on POEditor. @westnordost, could you please run that task and check if it works fine?

[westnordost]

I've run the jobs as requested, didn't look at your other changes yet.

[westnordost]

Seeing that the library has some pretty obvious bugs and that the library hasn't seen a commit since 10 months despite some open bug reports, I think, let's just let this PR for a while wait and see if the issues will be fixed. If not, maybe the charleskorn library will gain proper multiplatform support. And finally if nothing is an option, the make-everything-JSON PR could be revived.

[FloEdelmann]

I opened another issue in their repository, let's see what happens: Him188/yamlkt#69.

[westnordost]

Meanwhile, the author of snakeyaml-engine-kmp, a Kotlin multiplatform port of snakeyaml, stopped working on the PR that would use snakeyaml-engine-kmp for all platforms of kaml.

It looks like only very little is missing to actually complete that PR and the PR in general is quite small. On the other hand, if the author of snakeyaml-engine-kmp had to give up on finishing the PR, it looks to me as if either it is more work than it appears to finish it or he overall has no time to spare, in which case it's unknown if we can expect snakeyaml-engine-kmp to continue to be maintained.

[krzema12]

Hi, the co-owner of snakeyaml-engine-kmp here. Together with @aSemy we're committed to maintaining the KMP port of snakeyaml-engine. I stopped working on the mentioned PR mainly because I got a bit stuck with how okio works and how to port the JVM-specific API of kaml + busy personal time. It would be ideal if someone could pick up my effort on kaml side, then we'll be happy to keep snakeyaml-engine-kmp alive 🚀

[westnordost]

Oh, sorry, didn't want to come across as rude (in case I did).

So, it is the former :-)

On this project (Android + soon iOS multiplatform), we are kind of on the edge whether to migrate to just everything json, to yamlkt (but see previous comments) or wait for kaml to become fully multiplatform. This is why I posted new development here.

[krzema12]

Sure, got it! I actually have another idea that may unblock you. My PR in kaml makes the JVM target use the KMP port of snakeyaml-engine. It may be better to make another step first: enable more targets in kaml thanks to snakeyaml-engine-kmp, and let the JVM target still use snakeyaml-engine. Kaml already supports JS this way. It's not ideal long-term as little discrepancies may appear in behavior between JVM and the rest, but looking at how rarely snakeyaml-engine releases appear, I think it shouldn't be a big issue. Optionally, to mitigate this risk, we could add a mechanism to kaml's build logic that would help in keeping the two libraries in sync.

[westnordost]

Hmm, but this would mean to mostly duplicate the code currently in jsMain to iosMain. I think it is cleaner to move the code from jsMain to commonMain instead. Anyway, we are not in any particular hurry, we got a good amount of other things to do on our path to an eventual iOS port, so at the moment for me the most convenient action is to wait. (Not going to stop other contributors to jump in, though.)

(Anyway, IIRC correctly, I was a bit surprised that the kotlinx-serialization-json interface only works with from and to String, rather than from and to InputStream(java) / Source(okio) / ByteArray(kotlinx-io-core). I explained the apparent lack of API by that there is currently no standard multiplatform replacement of InputStream in Kotlin to myself.
So, from that point of view, I am surprised that kaml does offer an API for that, and depends on okio, apparently, which subsequently seems a bit un-clean to me. I.e. a parsing library should do parsing only, not do anything file-related)

[FloEdelmann]

Meanwhile, official kotlinx-serialization-json has started working on allowing comments: Kotlin/kotlinx.serialization/#2221 (comment)

[westnordost]

No movement whatsoever on yamlkt repo, I guess it is unmaintained.

In the meantime, the mentioned PR in kaml was picked up by the other maintainer of snakeyaml-engine-kmp, merged and released. The only thing that seems to be missing now are to enable iOS (etc.) as a target over at snakeyaml-engine-kmp (which apparently also needs some tests/fixeing) and then also in kaml.

So, I would say this PR should definitely be closed, for it is clear that we will not use yamlkt, instead most likely we will keep using kaml or if there are unforeseen issues, kotlinx-serialization-json with comments.

[westnordost]

(If we settle on kaml, the stuff you did in this PR would still be nice - replacing the use of those other yaml-libraries in the buildscripts with just one, but I figure it would be most safe until we are 100% sure that we stay with yaml)