Fix serialization using standalone kotlinx compiler #2690

peternewman · 2021-03-28T19:11:21Z

Based on fix from Kotlin/kotlinx.serialization#1105

Resync with master

…lete into data-actions

Based on fix from Kotlin/kotlinx.serialization#1105

peternewman · 2021-03-28T19:14:43Z

I'm assuming this works on GitHub actions too, but it doesn't run as it only goes on master.

FloEdelmann

It works correctly on my Ubuntu machine, so it should also work fine on GitHub actions.

It works fine without a change, too, though 😅
How is the linked issue triggered for you? When you run this from the command line?

kotlinc -script -Xplugin=/snap/kotlin/current/lib/kotlinx-serialization-compiler-plugin.jar .github/generate-quest-list.main.kts

peternewman · 2021-03-29T21:09:23Z

It works correctly on my Ubuntu machine, so it should also work fine on GitHub actions.

Cool.

It works fine without a change, too, though sweat_smile

~~Odd. Other changes to the code fix it for me too I think (not sure exactly what)~~ I think that was just some poor testing.

How is the linked issue triggered for you? When you run this from the command line?

Indeed. Well nearly the same as that:

kotlinc -script -Xplugin=/snap/kotlin/current/lib/kotlinx-serialization-compiler-plugin.jar .github/generate-quest-list.main.kts

Like so (using kotlin-compiler-1.4.31.zip):

~/Downloads/kotlinc/bin/kotlinc -script -Xplugin=/home/user/Downloads/kotlinc/lib/kotlinx-serialization-compiler-plugin.jar .github/generate-quest-list.main.kts
kotlinx.serialization.SerializationException: Serializer for class 'ApiResult' is not found.
Mark the class as @Serializable or provide the serializer explicitly.
	at kotlinx.serialization.internal.Platform_commonKt.serializerNotRegistered(Platform.common.kt:91)
	at kotlinx.serialization.internal.PlatformKt.platformSpecificSerializerNotRegistered(Platform.kt:29)
	at kotlinx.serialization.SerializersKt__SerializersKt.serializer(Serializers.kt:59)
	at kotlinx.serialization.SerializersKt.serializer(Unknown Source)
	at Generate_quest_list_main.getWikiTableContent(generate-quest-list.main.kts:269)
	at Generate_quest_list_main.main(generate-quest-list.main.kts:36)
	at Generate_quest_list_main.<init>(generate-quest-list.main.kts:27)

Is this some weird path/include thing as I'm just running it from the zip?

FloEdelmann · 2021-03-29T21:17:41Z

Is this some weird path/include thing as I'm just running it from the zip?

Hmm, not sure.

Can we completely avoid the compiler plugin by calling the serializer function explicitly? That probably also means dropping the @Serializable annotations.

Then calling the script would be much easier:

- kotlinc -script -Xplugin=/path/to/kotlinx-serialization-compiler-plugin.jar .github/generate-quest-list.main.kts
+ kotlinc -script .github/generate-quest-list.main.kts

smichel17 · 2021-03-29T21:58:51Z

I recently encountered a similar error when trying to build StreetComplete. For me it was about the system Java version (openjdk 11.0.10), which did not match the version Android was expecting. I fixed it by pointing gradle to Android Studio's version of Java (openjdk version "1.8.0_242-release") instead. I added this arg when calling gradle (your path will be different): -Dorg.gradle.java.home="/var/lib/flatpak/app/com.google.AndroidStudio/current/active/files/extra/android-studio/jre/"

peternewman · 2021-03-29T22:07:13Z

Based on this diff:

178a179
> @Serializable
180a182
> @Serializable
191c193
<     return Json.decodeFromString<ApiResult>(jsonString).parse.wikitext
---
>     return Json.decodeFromString(ApiResult.serializer(), jsonString).parse.wikitext

Then calling the script would be much easier:

- kotlinc -script -Xplugin=/path/to/kotlinx-serialization-compiler-plugin.jar .github/generate-quest-list.main.kts
+ kotlinc -script .github/generate-quest-list.main.kts

That fails for me with:

~/Downloads/kotlinc/bin/kotlinc -script .github/generate-quest-list.main.kts
error: not enough information to infer type variable T (generate-quest-list.main.kts:191:17)
error: unresolved reference: serializer (generate-quest-list.main.kts:191:44)
.github/generate-quest-list.main.kts:191:17: error: not enough information to infer type variable T
    return Json.decodeFromString(ApiResult.serializer(), jsonString).parse.wikitext
                ^
.github/generate-quest-list.main.kts:191:44: error: unresolved reference: serializer
    return Json.decodeFromString(ApiResult.serializer(), jsonString).parse.wikitext
                                           ^

I recently encountered a similar error when trying to build StreetComplete. For me it was about the system Java version (openjdk 11.0.10), which did not match the version Android was expecting.

They definitely won't match, is Kotlin not standalone (it's all a bit new to me), but I'm not using Gradle at all, just running that script directly.

westnordost · 2021-03-29T22:15:58Z

The serializer for a class marked with @Serializable are not created via reflection but using a gradle plugin. Thus, to execute that plugin is mandatory.

peternewman · 2021-03-31T13:55:47Z

The serializer for a class marked with @Serializable are not created via reflection but using a gradle plugin. Thus, to execute that plugin is mandatory.

So I think you're saying that @FloEdelmann 's suggestion in #2690 (comment) won't work @westnordost ?

westnordost · 2021-03-31T14:02:56Z

No, I don't know. I just stated how the kotlinx-serialization library works. I'd like to leave the rest for @FloEdelmann as I don't have an overview of this code involved here and also never tried to execute it myself.

FloEdelmann · 2021-04-07T19:37:05Z

@peternewman I still don't get why the change would be necessary. The unchanged code works fine on the Ubuntu-based GitHub Actions runner.

The changed code also works fine when running from the command line, but when editing the file inside Android Studio, it's complaining about two issues:

Are you sure that it's not a missing configuration on your machine?

FloEdelmann · 2021-04-07T19:40:21Z

Interestingly, both versions seem to work fine with or without the -Xplugin parameter on my machine now:

$ kotlinc -script -Xplugin=/snap/kotlin/current/lib/kotlinx-serialization-compiler-plugin.jar .github/generate-quest-list.main.kts
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.intellij.util.ReflectionUtil to method java.util.ResourceBundle.setParent(java.util.ResourceBundle)
WARNING: Please consider reporting this to the maintainers of com.intellij.util.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
$ kotlinc -script .github/generate-quest-list.main.kts
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.intellij.util.ReflectionUtil to method java.util.ResourceBundle.setParent(java.util.ResourceBundle)
WARNING: Please consider reporting this to the maintainers of com.intellij.util.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

peternewman · 2021-04-08T16:13:00Z

The changed code also works fine when running from the command line, but when editing the file inside Android Studio, it's complaining about two issues:

Hmm, does it provide any suggestions for how to fix it? So does it actually not run from within Android Studio in that state?

Are you sure that it's not a missing configuration on your machine?

No, and admittedly trying to replicate the same fault on the Ubuntu runner, I can't manage to, even when making it as similar as possible to my machine (i.e. no configuration (apart from what the runner has built in, just checking out the code and downloading and extracting the zip):
https://github.com/peternewman/StreetComplete/runs/2297950539?check_suite_focus=true

Interestingly, both versions seem to work fine with or without the -Xplugin parameter on my machine now:

Are you sure it's actually recompiling? During some of this testing, I've grown increasingly suspicious that it's secretly caching some stuff somehow/somewhere, e.g. it sometimes seems to arbitrarily break or work for no discernible reason.

Doing the following on a clean checkout, to revert my change seems to consistently break it, regardless of the paths and locations of the compiler and plugin:
git checkout -- .github/generate-quest-list.main.kts

I'm sort of out of ideas for how to troubleshoot it at my end and confused how this fix works for me but isn't required on the runner etc.

See also Kotlin/kotlinx.serialization#1125

This reverts commit 651f1e4.

peternewman · 2021-04-08T16:24:35Z

Edit @FloEdelmann . I've found a far simpler fix, which makes just as much sense in that I've swapped the order of the classes so it's not used before it's been defined, which seems reasonable and fixes my issue. Is this acceptable to you (and I guess more importantly does it not break stuff for you)?

FloEdelmann · 2021-04-08T16:42:57Z

It now throws an error for me, while on the master branch, it works fine 🙁

$ kotlinc -script -Xplugin=/snap/kotlin/current/lib/kotlinx-serialization-compiler-plugin.jar .github/generate-quest-list.main.kts
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.intellij.util.ReflectionUtil to method java.util.ResourceBundle.setParent(java.util.ResourceBundle)
WARNING: Please consider reporting this to the maintainers of com.intellij.util.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
kotlinx.serialization.SerializationException: Serializer for class 'ApiResult' is not found.
Mark the class as @Serializable or provide the serializer explicitly.
        at kotlinx.serialization.internal.Platform_commonKt.serializerNotRegistered(Platform.common.kt:91)
        at kotlinx.serialization.internal.PlatformKt.platformSpecificSerializerNotRegistered(Platform.kt:29)
        at kotlinx.serialization.SerializersKt__SerializersKt.serializer(Serializers.kt:59)
        at kotlinx.serialization.SerializersKt.serializer(Unknown Source)
        at Generate_quest_list_main.getWikiTableContent(generate-quest-list.main.kts:269)
        at Generate_quest_list_main.main(generate-quest-list.main.kts:36)
        at Generate_quest_list_main.<init>(generate-quest-list.main.kts:27)

peternewman · 2021-04-08T23:00:01Z

That's odd, as it works fine on multiple action runners:
https://github.com/peternewman/StreetComplete/actions/runs/730769889

😕

westnordost · 2021-04-08T23:35:09Z

Are you both taking into account that kotlinx-serialization once on build creates the serializer-classes for classes annotated with @Serializable and then these serializers are available in the build cache? (so f.e. if you then launch the script without first running the gradle plugin, it will probably work)

I don't know exactly how you start the script and how the GitHub actions work, I just want to point out that if you want to reproduce a problem with running the script, you should make sure that the build cache is cleared before each new run.

peternewman · 2021-04-09T00:12:19Z

Are you both taking into account that kotlinx-serialization once on build creates the serializer-classes for classes annotated with @Serializable and then these serializers are available in the build cache? (so f.e. if you then launch the script without first running the gradle plugin, it will probably work)

I wasn't aware, but that confirms my theory:

Are you sure it's actually recompiling? During some of this testing, I've grown increasingly suspicious that it's secretly caching some stuff somehow/somewhere, e.g. it sometimes seems to arbitrarily break or work for no discernible reason.

Having a look, there is some stuff in ~/.cache/main.kts.compiled.cache/ but even having cleared that out it's building fine for me (and has started repopulating, so it's clearly the right place) with my current proposed fix.

I don't know exactly how you start the script

The workflow just runs these commands:

StreetComplete/.github/workflows/generate-quest-list.yml

Lines 16 to 17 in 6e857b2

    
           - run: sudo snap install kotlin --channel=1.4/stable --classic 
        
           - run: kotlinc -script -Xplugin=/snap/kotlin/current/lib/kotlinx-serialization-compiler-plugin.jar .github/generate-quest-list.main.kts

and how the GitHub actions work

Unless you do some config to make them do otherwise, they spin up a clean image, check out the code from git and then run the command, i.e. it's a clean slate with no previous cached data.

FloEdelmann

Indeed, clearing the ~/.cache/main.kts.compiled.cache/ directory does the trick!

Then this looks fine :)
I like that it's just a simple line swap.

peternewman · 2021-04-12T22:04:10Z

Thanks for the merge @westnordost .

Indeed, clearing the ~/.cache/main.kts.compiled.cache/ directory does the trick!

Glad to hear it @FloEdelmann

Then this looks fine :)
I like that it's just a simple line swap.

I have to admit that clearing my cache also fixed both of them (original and this) for me (I suspect I didn't have the plugin probably in the path, or possibly used the wrong one when I first ran it, so it cached a failed build). Although it still seems sense to not use them before we define them anyway.

peternewman and others added 4 commits June 25, 2020 03:25

Merge pull request #3 from westnordost/master

2487495

Resync with master

Merge branch 'master' of https://github.com/streetcomplete/StreetComp…

9052813

…lete into data-actions

Fix serialization using standalone kotlinx compiler

651f1e4

Based on fix from Kotlin/kotlinx.serialization#1105

Git ignore quest-list.csv

3a4fb48

peternewman requested a review from FloEdelmann March 28, 2021 19:12

FloEdelmann approved these changes Mar 29, 2021

View reviewed changes

peternewman added 2 commits April 8, 2021 17:20

Revert "Fix serialization using standalone kotlinx compiler"

ae49d81

This reverts commit 651f1e4.

Fix the serialization in a simpler way, sorting the ordering

c4a7073

peternewman requested a review from FloEdelmann April 8, 2021 16:23

FloEdelmann approved these changes Apr 12, 2021

View reviewed changes

westnordost merged commit 9d8b615 into streetcomplete:master Apr 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix serialization using standalone kotlinx compiler #2690

Fix serialization using standalone kotlinx compiler #2690

peternewman commented Mar 28, 2021

peternewman commented Mar 28, 2021

FloEdelmann left a comment •

edited

Loading

peternewman commented Mar 29, 2021 •

edited

Loading

FloEdelmann commented Mar 29, 2021 •

edited

Loading

smichel17 commented Mar 29, 2021 •

edited

Loading

peternewman commented Mar 29, 2021

westnordost commented Mar 29, 2021

peternewman commented Mar 31, 2021

westnordost commented Mar 31, 2021

FloEdelmann commented Apr 7, 2021 •

edited

Loading

FloEdelmann commented Apr 7, 2021

peternewman commented Apr 8, 2021

peternewman commented Apr 8, 2021

FloEdelmann commented Apr 8, 2021

peternewman commented Apr 8, 2021

westnordost commented Apr 8, 2021 •

edited

Loading

peternewman commented Apr 9, 2021

FloEdelmann left a comment

peternewman commented Apr 12, 2021

Fix serialization using standalone kotlinx compiler #2690

Fix serialization using standalone kotlinx compiler #2690

Conversation

peternewman commented Mar 28, 2021

peternewman commented Mar 28, 2021

FloEdelmann left a comment • edited Loading

Choose a reason for hiding this comment

peternewman commented Mar 29, 2021 • edited Loading

FloEdelmann commented Mar 29, 2021 • edited Loading

smichel17 commented Mar 29, 2021 • edited Loading

peternewman commented Mar 29, 2021

westnordost commented Mar 29, 2021

peternewman commented Mar 31, 2021

westnordost commented Mar 31, 2021

FloEdelmann commented Apr 7, 2021 • edited Loading

FloEdelmann commented Apr 7, 2021

peternewman commented Apr 8, 2021

peternewman commented Apr 8, 2021

FloEdelmann commented Apr 8, 2021

peternewman commented Apr 8, 2021

westnordost commented Apr 8, 2021 • edited Loading

peternewman commented Apr 9, 2021

FloEdelmann left a comment

Choose a reason for hiding this comment

peternewman commented Apr 12, 2021

FloEdelmann left a comment •

edited

Loading

peternewman commented Mar 29, 2021 •

edited

Loading

FloEdelmann commented Mar 29, 2021 •

edited

Loading

smichel17 commented Mar 29, 2021 •

edited

Loading

FloEdelmann commented Apr 7, 2021 •

edited

Loading

westnordost commented Apr 8, 2021 •

edited

Loading