-
-
I made some changes to the project and gradle.
I started with addressing issue with failing build due to tests taking ages to finish (there was a 12h sleep in tests) and added global gradle timeout on test runs (60s) just to avoid whole build timing out. Those changes are already in master.
There rest I put in the separate PR as they are somewhat more significant, especially dealing with deprecated stuff: given that there were already API breaking changes I opted to remove deprecated members (especially considering some of the comments for deprecation, ekhm…). I switched remaining implementations from Interceptor to StanzaFilter and it looks OK, sadly tests are scarce so this change requires second pair of eyes.
PR: https://tigase.dev/tigase/_libraries/halcyon/~pulls/12
There are two remaining items to cleanup:
-
AbstractHalcyon.kt:91:5 Property must be initialized, be final, or be abstract. This warning will become an error in future releases.-- I'll deal with it later on, with the upgrade to kotlin 2.0; I would also be inclined (after cleaning up the build output/warnings) to configure gradle/kotlin to be more strict and fail on warnings akin to "This warning will become an error in future releases." -- as this one prevents us from easier kotlin upgrade. -
iOS specific code:
w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-core/src/iosMain/kotlin/tigase/halcyon/core/connector/socket/Socket.kt:80:35 Unchecked cast: CPointer<sockaddr_in> to CValuesRef<sockaddr>? w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-core/src/iosMain/kotlin/tigase/halcyon/core/connector/socket/Socket.kt:80:61 'companion object of sockaddr_in' is deprecated. Use sizeOf<T>() or alignOf<T>() instead. w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-core/src/iosMain/kotlin/tigase/halcyon/core/connector/socket/Socket.kt:177:66 This cast can never succeed w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-core/src/iosMain/kotlin/tigase/halcyon/core/connector/socket/Socket.kt:182:61 This cast can never succeed w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-core/src/iosMain/kotlin/tigase/halcyon/core/xmpp/modules/omemo/Bundle.ios.kt:51:13 Unreachable code w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-core/src/iosMain/kotlin/tigase/halcyon/core/xmpp/modules/omemo/Bundle.ios.kt:129:9 Unreachable code w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-core/src/iosMain/kotlin/tigase/halcyon/core/xmpp/modules/omemo/Bundle.ios.kt:434:23 There is more than one label with such a name in this scopeAndrzej -- would you mind having a look at it and squashing those remaining warnings (ideally without ignoring them with annotation)? (hence leaving this in "In Progress" state)
‼️
I was pondering applying same
git-versionplugin as I did in Tygrys, but after initial changes I realised it doesn't make sense in this case (it breaks dependencies and pollutes maven repository).What I would like to propose is the following:
- avoid making API breaking changes in
master(it's build and published automatically); if new method is needed - please deprecated the old one and add correct fallback handling. - make halcyon releases often (especially minor versions like 2.1, 2.2, etc).
- feature/PR branches are not published automatically allowing us to work on features / pull requests without breaking downstream.
-
-
I've fixed warnings as requested and I've tried to update Kotlin to make it use the same version as Tygrys. Unfortunately, Kotlin made some braking changes and I was unable to make Halcyon compile with Kotlin newer than 2.1.21 (we use 2.2.21 in Tygrys).
To resolve some of the issues with newer Kotlin, I've bumped dependencies versions, but I suppose at least one may still be "too old" and due to that dependency newer Kotlin may fail to work with
IrErrorTypeexception during compilation for iOS and JS. -
I've fixed warnings as requested and I've tried to update Kotlin to make it use the same version as Tygrys. Unfortunately, Kotlin made some braking changes and I was unable to make Halcyon compile with Kotlin newer than 2.1.21 (we use 2.2.21 in Tygrys).
Was kotlin bump needed to address those warnings? If not I would suggest splitting those, only squashing the warnings and deal with kotlin bump in a separate PR (have the work slightly more atomic). And if some warning did require it maybe only fix as much as possible in here, have working/green build and deal with them in separate PR?
-
Basically, I've fixed warnings and bumped Kotlin to 2.1.21 without any issues. All works as expected. The issue was only with bumping to 2.2.21.
I wanted to make this change in one step as it updating Kotlin version later on would be once again a breaking change...
-
Basically, I've fixed warnings and bumped Kotlin to 2.1.21 without any issues. All works as expected. The issue was only with bumping to 2.2.21.
Maybe, for now, we should then stick to 2.1 and then bump to 2.2 later on in separate PR to move this forward in a more gradual manner?
I wanted to make this change in one step as it updating Kotlin version later on would be once again a breaking change...
Well, yes. But I already stated in original comment that: "I'll deal with it later on, with the upgrade to kotlin 2.0;". Right now I just wanted to clean up the project to have in better standing position and have better workplace. We don't have to cram absolutely every change in single commit/PR…
-
As for update of Kotlin, I missed your statement about update to 2.0 to be done later. However, as we have it ready, maybe we should just merge it.
I wouldn't oppose it but it's not building:

https://tigase.dev/tigase/_libraries/halcyon/~builds/104
So the options are either:
- revert to kotlin 2.1
- resolve all the build issues (it looks like it's mostly a test sources issues:
compileTestKotlinJvm)
-
Great, thank you. I merged the PR.
I haven't noticed those issues in my local builds (tests were not running).
How do you build locally?
In general, we should try to build everything with
./gradlew build(possibly adding--no-build-cache --no-configuration-cachejust before final push/check to make sure there are no cached changes…) and also do check CI builds. -
A couple more comments.
-
I would suggest that we try to make changes/commits more atomic -- for example separate gradle upgrade from other upgrades; it makes going over the changes easier if you have to check single commit and not a huge dump; and PR of couple related commits gives you a global overview.
-
there was still failing (test) task on local machine (CI doesn't run iOS…):
> Task :halcyon-core:compileTestKotlinIosArm64 FAILED e: file:///Users/wojtek/dev/tigase/halcyon/halcyon-core/src/iosTest/kotlin/tigase/halcyon/core/connector/SocketConnectorTest.kt:53:34 'fun <T> T.freeze(): T' is deprecated. Support for the legacy memory manager has been completely removed. Usages of this function can be safely dropped.I changed it from
resultsStore.value = results.freeze()toresultsStore.value = results-- ok?- there were deprecation warnings about
resume()call on continuation:
> Task :halcyon-coroutines:compileCommonMainKotlinMetadata w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-coroutines/src/commonMain/kotlin/tigase/halcyon/coroutines/RequestBuilderExt.kt:16:18 'fun resume(value: V, onCancellation: ((@ParameterName(...) Throwable) -> Unit)?): Unit' is deprecated. Use the overload that also accepts the `value` and the coroutine context in lambda. w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-coroutines/src/commonMain/kotlin/tigase/halcyon/coroutines/RequestBuilderExt.kt:37:19 'fun resume(value: V, onCancellation: ((@ParameterName(...) Throwable) -> Unit)?): Unit' is deprecated. Use the overload that also accepts the `value` and the coroutine context in lambda. w: file:///Users/wojtek/dev/tigase/halcyon/halcyon-coroutines/src/commonMain/kotlin/tigase/halcyon/coroutines/RequestBuilderExt.kt:53:19 'fun resume(value: V, onCancellation: ((@ParameterName(...) Throwable) -> Unit)?): Unit' is deprecated. Use the overload that also accepts the `value` and the coroutine context in lambda.I changed the call from
continuation.resume(it, null)tocontinuation.resume(it) { _, _, _ -> }-- ok?- I also got a complain that the
yarn.lockfile was out of sync and it turned out that we had in.gitignore; as per kotlin documentation (https://kotlinlang.org/docs/js-project-setup.html#version-locking-via-kotlin-js-store) and yarn best practices (https://classic.yarnpkg.com/blog/2016/11/24/lockfiles-for-all/) yarn.lock file should be in repository so I removed it from gitignore:
To follow a recommended practice, commit kotlin-js-store and its contents to your version control system. It ensures that your application is being built with the exact same dependency tree on all machines.
IMHO we could consider releasing halcyon 2.0 and use it as base/stable version and bump version on 2.1-SNAPSHOT -- what do you think?
-
-
| Type |
Task
|
| Priority |
Normal
|
| Assignee | |
| Version |
none
|
| Sprints |
n/a
|
| Customer |
n/a
|
-
2.0.0 Open
Apply similar changes to work done in #tygrys/tygrys#270 to improve project handling.
Especially focus on library versioning to avoid relying on
-SNAPSHOTversions