Unexpected ABI break: Koin.runOnKoinStarted 4.1 -> 4.2

[ctuncan]

Describe the bug
Due to the refactors in this commit the symbol for Koin.runOnKoinStarted changed from org.koin.core.KoinWaitExtKt.runOnKoinStarted to org.koin.core.KoinWaitExt_jvmKt.runOnKoinStarted

This is a source compatible change, but not a binary compatible change.
This change appears to be intended to make waitAllStartJobs available to common code.
This ABI break does not appear to be intentional.

This means that any libraries that called this method must be updated in order for an application to update to the newest version of Koin, or they get the following error:
NoSuchMethodError: org.koin.core.KoinWaitExtKt.runOnKoinStarted(...)

To Reproduce

1. Library A is compiled against Koin 4.1 & calls runOnKoinStarted
2. Application B upgrades to Koin 4.2

Expected behavior
There is no crash!

Koin module and version:
koin-core-coroutines:4.1.1
koin-core-coroutines:4.2.0

Solution?
I am unsure if the following works when the two files are spread across source-sets.
But you should be able to mark each file with @file:JvmName("KoinWaitExtKt") and @file:JvmMultifileClass

Of course, ABI breakages are extra-painful, as fixing it post-release is inherently another breaking change.
(Unless you keep both symbols in place)

As such, I'm more interested in if you have considered applying the binary compatibility validator plugin.
Either the standalone version, or the built in version.

This will avoid accidental ABI breakage, and ensure that it only happens intentionally!
Hopefully avoiding this issue in the future.

[ctuncan]

Note that this wasn't listed under the breaking changes in the release notes. Which is also why I suspect this was unintentional!

[arnaudgiuliani]

thanks for spotting 👍 🙏

[ctuncan]

I've just noticed that the BCV is applied in this repo. Not sure how I missed that before!
The change is detected here.

Bear in mind that just changing the symbol back to org.koin.core.KoinWaitExtKt.runOnKoinStarted will be a breaking change for users going from 4.2.0 -> 4.2.1, which as a patch release is worse. (Not for me persay, but you know, in general)

To avoid breaking changes for anyone would you need to have both symbols available, but with one marked as Deprecated-Hidden. This would ensure that your preferred symbol version is used going forward, but both are available.

The deprecated hidden version is almost a one-liner. It would look something like.

@file:JvmName("KoinWaitExt_jvmKt")
package org.koin.core
@Deprecated("ABI compatibility for 4.2.0 release. Issue #2391", level=DeprecationLevel.HIDDEN)
fun Koin.runOnKoinStarted(block: suspend (Koin) -> Unit) = runOnKoinStarted(block)

Because this function is hidden, it is hidden even from itself! So the call to runOnKoinStarted inside the hidden runOnKoinStarted will resolve to the expected version (not infinite recursion)

[ctuncan]

There are also .api files for the benchmark project, which show up in that diff.
As the benchmark project appears not to be a published artifact I'd recommend disabling BCV for that module.

That way you don't have the mental overload of "Do I care about this .api file change?"

(Unless you have an internal project where you care about ABI compatibility of that module)

[arnaudgiuliani]

Fixed in branch 4.2.1 — commit 290f744.

What was done:

• Added @file:JvmName("KoinWaitExtKt") and @file:JvmMultifileClass to both KoinWaitExt.jvm.kt and KoinWaitExt.kt, restoring the original JVM symbol org.koin.core.KoinWaitExtKt.runOnKoinStarted so libraries compiled against 4.1.x work with 4.2.x at runtime.
• Added apiCheck to CI (build.yml) so binary compatibility is validated on every PR going forward — any public API change without an explicit .api dump update will now fail the build.
• Updated all .api dump files to match the current public API surface.

Will ship with 4.2.1 release.

[ctuncan]

Note that this does mean that code compiled against 4.2.0 (and using this symbol) will be ABI incompatible with 4.2.1. As now the KoinWaitExt_jvmKt doesn't exist. (This is what the level=DeprecationLevel.HIDDEN suggestion solves, as it maintains both symbols)

This will affect fewer people than the original ABI breakage (only those that eagerly updated to 4.2.0), but might be worth mentioning in the release notes. (Or keeping both symbols)

Apologies that I didn't manage this comment between the commit & the release! I was on holiday last week.

[arnaudgiuliani]

that kind of issue must be caught around