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

New watchos targets that are available in kotlin coroutines #72

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nathanfallet
Copy link

@nathanfallet nathanfallet commented Jun 2, 2024

Fixes #67

Adding watchosSimulatorArm64 and watchosDeviceArm64, which were added to kotlinx-coroutines-core respectively in 1.6.0 and 1.7.0

See https://central.sonatype.com/artifact/org.jetbrains.kotlinx/kotlinx-coroutines-core-watchossimulatorarm64 and https://central.sonatype.com/artifact/org.jetbrains.kotlinx/kotlinx-coroutines-core-watchosdevicearm64 for releases

EDIT: It was actually kotlinx-serialization, missing watchos targets were added to 1.5.1.
I tried to upgrade to latest which is 1.6.3, but too much things were broken so I updated to the minimum version which supports it.

@nathanfallet nathanfallet force-pushed the master branch 2 times, most recently from 5de8926 to 63215c7 Compare June 2, 2024 00:54
@Him188
Copy link
Owner

Him188 commented Jun 2, 2024

Build failed on macos, can you please check? @nathanfallet

@nathanfallet
Copy link
Author

It fails for a weird reason, and I'm unable to even build the previous version:

> Task :yamlkt:linkDebugTestIosSimulatorArm64 FAILED
e: /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld invocation reported errors

The /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld command returned non-zero exit code: 1.
:yamlkt:linkDebugTestIosSimulatorArm64 (Thread[Daemon worker,5,main]) completed. Took 55.779 secs.
output:
ld: unknown options: -ios_simulator_version_min -sdk_version 

FAILURE: Build failed with an exception.

@hfhbd
Copy link

hfhbd commented Jun 7, 2024

You need to bump the Kotlin version to 1.9.20+ because Xcode 15 (updated regularly due macOS-latest), isn't compatible with Kotlin 1.8.0

@nathanfallet
Copy link
Author

@hfhbd okay I'll have a look on that, I hope it won't break other things

@nathanfallet
Copy link
Author

nathanfallet commented Jun 16, 2024

Here are my changes explained to upgrade to Kotlin 1.9.20 2.0.0:

  • removing obsolete targets (e: The following removed Kotlin/Native targets were used in the project: iosArm32, mingwX86, watchosX86)
  • removing obsolete gradle properties (e: The following properties are obsolete and no longer supported: kotlin.mpp.enableCompatibilityMetadataVariant)
  • using only JS IR compiler (legacy no longer supported)
  • rewriting list of targets to properly configure dependencies between them
  • upgrading to Kotlin 2 to support the latest of serialization-core

I also moved a few files because package names were sometimes in a folder named with x.y.z instead of x/y/z, that was strange
After about 3 hours of refactoring in the gradle configuration, it finally builds!
image
If you want to have another look and give me feedbacks if needed @Him188

yamlkt/build.gradle.kts Outdated Show resolved Hide resolved
yamlkt/build.gradle.kts Outdated Show resolved Hide resolved
@nathanfallet
Copy link
Author

The runner fails because e: java.lang.OutOfMemoryError: Metaspace, I had it once locally and relaunching tests it worked. I don't know what causes that memory usage during tests sometimes...

@Him188
Copy link
Owner

Him188 commented Jun 16, 2024

The runner fails because e: java.lang.OutOfMemoryError: Metaspace

GitHub runners have only 7GB memory. It's easy for clang + lld + kotlin daemon + gradle daemon to run out of it. So just retry it.

@nathanfallet
Copy link
Author

@Him188 I fixed it by increasing the memory allocated to Gradle, otherwise it fails each time it is building on a clean state (relaunching locally works but on the runner it's always a clean state so that's the only solution I found)

@nathanfallet
Copy link
Author

nathanfallet commented Jun 24, 2024

Do you have any idea why there is a test that fails here? On my fork tests were triggered too and it passes, as well as on my own MacBook...
image
image

@Him188
Copy link
Owner

Him188 commented Jun 24, 2024

@nathanfallet It's strange - it timed out. Try to remove --info from gralde agumengs? It might increase memory usage causing tests to run slower.

Copy link

@whyoleg whyoleg left a comment

Choose a reason for hiding this comment

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

Hey!
Would be nice to somehow resolve this and do a release. I do understand that there are some strange issues happening.

I do have several theories (ordered by complexity):

  1. could you try to disable debugging in all tests - sometimes, if there is a lot of output, JS can be flaky
  2. May be it's a good idea to update Gradle/GitHub Actions versions - sometimes it could help :)
  3. The timeout is coming from karma, so it's possible to increase timeout. For this you need to configure js target like this:
js {
  browser {
    testTask {
      useKarma {
        useConfigDirectory(rootDir.resolve("karma"))
      }
    }
  }
}

Then in karma folder create file config.js with content:

config.client = config.client || {}
config.client.mocha = config.client.mocha || {}
config.client.mocha.timeout = '30s'
config.browserNoActivityTimeout = 30000
config.browserDisconnectTimeout = 30000

This will increase timeout to 30 seconds, instead of 2 by default

Hope this will help somehow stabilise the build!

@@ -1,10 +1,9 @@
# style guide
kotlin.code.style=official
kotlin.incremental.multiplatform=true
kotlin.js.compiler=both
kotlin.js.compiler=ir
Copy link

Choose a reason for hiding this comment

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

this property is not needed anymore

}
apply(plugin = "me.champeau.gradle.jmh")
apply(plugin = "me.champeau.jmh")
Copy link

Choose a reason for hiding this comment

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

this line can be dropped

}*/
jvmToolchain(8)
jvm()
js {
Copy link

Choose a reason for hiding this comment

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

would be nice to add wasmJs and wasmWasi targets as well
As far as I can see, js or native could be used there

iosArm64()

// Tier 3:
//androidNativeArm32()
Copy link

Choose a reason for hiding this comment

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

is there any issue with those targets?

mingwX64()
watchosDeviceArm64()

applyDefaultHierarchyTemplate()
Copy link

Choose a reason for hiding this comment

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

this is not needed, as hierarchy is applied automatically if there is no manual source set creation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add watchosDeviceArm64
4 participants