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

Update to kotlin 1.4.21 #108

Merged
merged 3 commits into from
Dec 16, 2020
Merged

Conversation

JeroenMols
Copy link
Contributor

Updated the existing kotlin-1.4 branch with:

  • latest version of Kotlin, Coroutines and Serialization
  • remove EAP repositories
  • remove Kotlin stdlib definitions

I verified this PR by :

  • running all conformance tests for jvm, js and macos
  • building the code generator using the two gradle commands provided in the readme
  • building the runtime library

I didn't publish a build to a local maven repository and run the sample apps using that.

Happy to receive feedback on whether I'm following the right procedures.

@JeroenMols
Copy link
Contributor Author

test this please

@JeroenMols
Copy link
Contributor Author

Was worth a shot

The build failed due to a timeout and I'm wary to increase the timeout (I don't have enough insights into this project to make that assessment). Can anyone trigger a new build for me please? Thank you.

Copy link
Collaborator

@garyp garyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your changes look good and your testing approach sounds good too. The only testing step I would add is to publish the library to your Maven Local repo and try building the example apps against it (if you export CI=true before running Gradle in the sample app directories, the sample app build will look for pbandk in Maven Local).

FYI, you can trigger a new CI build by force pushing a new commit to the branch (e.g. you can do git commit --amend to rewrite the current commit without introducing any code changes). We haven't set up any comment-based CI triggers yet.

I haven't had a chance to pull down your branch and confirm this locally. But I suspect the timeout is the same problem I was seeing with earlier releases of Kotlin 1.4. Something in the pbandk code seems to trigger a bug in the Kotlin compiler and cause it to get into a loop and run indefinitely. Given more time the compiler eventually crashes with a stack overflow or similar memory exhaustion error. I was hoping that 1.4.20 would've fixed the bug but sounds like it hasn't 😞 This only happens with Kotlin 1.4, not Kotlin 1.3.

If you have the time and expertise to dig into this, it'd be great to isolate the code that triggers this condition and file a bug in the Kotlin issue tracker. Either way, this is fine to merge into the kotlin-1.4 branch. We just can't merge it into master until this issue is resolved.

@garyp garyp added the enhancement New feature or request label Dec 16, 2020
@garyp garyp added this to the 0.10.0 milestone Dec 16, 2020
@garyp
Copy link
Collaborator

garyp commented Dec 16, 2020

Good news! I had a chance to pull this down and try building it locally and everything works on my laptop too. Which means the CI failure is probably a different problem from the bug we were seeing in earlier Kotlin 1.4 releases. I'll merge this PR into the kotlin-1.4 branch and will try to figure out the CI problem on there. I suspect maybe we just need to up the CPU and/or memory allotment for the CI job so it can run more quickly. Some of the Kotlin MPP compilation tasks are quite resource heavy.

@garyp garyp merged commit f39d9ef into streem:kotlin-1.4 Dec 16, 2020
@garyp garyp linked an issue Dec 16, 2020 that may be closed by this pull request
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
Development

Successfully merging this pull request may close these issues.

2 participants