Linking issue fix proposal - #93
Conversation
3e6003e to
6e2348c
Compare
There was a problem hiding this comment.
Pull request overview
This PR restructures the Apple (iOS) integration for CrashKiOS crash backends (Crashlytics + Bugsnag) to avoid fragile linker/strip behavior by moving SDK calls into Swift “sink” implementations that live in the app’s build graph, while Kotlin/Native only cinterops protocol headers.
Changes:
- Introduces Swift Package products (
CrashKiOSCrashlytics,CrashKiOSBugsnag) with Swift sink implementations and ObjC protocol headers for Kotlin cinterop. - Replaces Apple-side Kotlin cinterop of third-party SDK symbols with protocol-only cinterop + sink registry wiring; adds/updates Apple tests.
- Removes the iOS linker Gradle plugins and updates sample apps/build configuration accordingly.
Reviewed changes
Copilot reviewed 60 out of 62 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/CrashKiOSCrashlyticsObjC/include/FIRCLSException.h | Adds hard-linked Crashlytics fatal-recording symbol declaration |
| Sources/CrashKiOSCrashlyticsObjC/include/CrashKiOSCrashlyticsSink.h | Adds ObjC sink protocol for Crashlytics forwarding |
| Sources/CrashKiOSCrashlyticsObjC/CrashKiOSCrashlyticsObjC.m | Empty source to satisfy SPM build requirements |
| Sources/CrashKiOSCrashlytics/CrashlyticsSink.swift | Swift reference sink implementation for Firebase Crashlytics |
| Sources/CrashKiOSBugsnagObjC/include/CrashKiOSBugsnagSink.h | Adds ObjC sink protocol for Bugsnag forwarding |
| Sources/CrashKiOSBugsnagObjC/CrashKiOSBugsnagObjC.m | Empty source to satisfy SPM build requirements |
| Sources/CrashKiOSBugsnag/BugsnagSink.swift | Swift reference sink + configuration helper for Bugsnag |
| settings.gradle.kts | Removes linking-plugin composite builds |
| samples/sample-crashlytics/shared/src/iosMain/kotlin/co/touchlab/crashkiossample/Helper.kt | Removes obsolete iOS startup helper |
| samples/sample-crashlytics/shared/build.gradle.kts | Updates KMP config; exports crashkios modules for Swift names |
| samples/sample-crashlytics/settings.gradle.kts | Removes substitution for deleted linker plugin |
| samples/sample-crashlytics/ios/ios/BridgingHeader.h | Adds protocol declaration for Swift conformance in non-SPM sample |
| samples/sample-crashlytics/ios/ios/AppDelegate.swift | Updates sample to configure CrashKiOS via Crashlytics sink |
| samples/sample-crashlytics/ios/ios.xcodeproj/project.pbxproj | Removes obsolete Xcode project file from repo |
| samples/sample-crashlytics/ios-spm/ios/AppDelegate.swift | Switches SPM sample to configure CrashKiOS via Crashlytics sink |
| samples/sample-crashlytics/ios-spm/ios.xcodeproj/project.pbxproj | Wires local CrashKiOS SPM package into SPM sample |
| samples/sample-crashlytics/gradle/libs.versions.toml | Removes unused sample dependency entry |
| samples/sample-crashlytics/gradle.properties | Removes cacheKind override |
| samples/sample-crashlytics/build.gradle.kts | Removes linker plugin usage from sample build |
| samples/sample-bugsnag/shared/build.gradle.kts | Updates KMP config; exports core for Swift names |
| samples/sample-bugsnag/gradle/wrapper/gradle-wrapper.properties | Updates Gradle wrapper + adds wrapper safety settings |
| samples/sample-bugsnag/gradle.properties | Removes cacheKind override |
| samples/sample-bugsnag/CrashKiOSSampleIOS/CrashKiOSSampleIOS/BridgingHeader.h | Adds protocol declaration for Swift conformance in sample |
| samples/sample-bugsnag/CrashKiOSSampleIOS/CrashKiOSSampleIOS/AppDelegate.swift | Updates sample to configure CrashKiOS via Bugsnag sink |
| samples/sample-bugsnag/CrashKiOSSampleIOS/CrashKiOSSampleIOS.xcodeproj/project.pbxproj | Adds bridging header build setting |
| samples/sample-bugsnag/build.gradle.kts | Removes linker plugin usage from sample build |
| Package.swift | Adds SPM package for Swift sinks + ObjC protocol targets |
| gradle/libs.versions.toml | Removes unused plugin/dependency coordinates tied to old linking approach |
| crashlytics/src/nativeInterop/cinterop/crashlytics.def | Switches to protocol-only cinterop package/header |
| crashlytics/src/commonMain/kotlin/co/touchlab/crashkios/crashlytics/CrashlyticsKotlin.kt | Updates docs to reflect Android-only enable function intent |
| crashlytics/src/appleTest/kotlin/co/touchlab/crashkios/crashlytics/CrashlyticsSinkTest.kt | Adds Apple test verifying sink forwarding behavior |
| crashlytics/src/appleMain/kotlin/co/touchlab/crashkios/crashlytics/CrashlyticsCallsActual.kt | Routes Apple calls through registered sink (no SDK symbol refs) |
| crashlytics/src/appleMain/kotlin/co/touchlab/crashkios/crashlytics/Crashlytics.kt | Adds CrashlyticsCrashReporting backend + deprecates old hook API |
| crashlytics/build.gradle.kts | Adjusts dependencies + cinterop include paths for protocol-only headers |
| crashlytics-ios-link/src/main/kotlin/co/touchlab/crashkios/CrashlyticsLinkPlugin.kt | Deletes obsolete linker plugin implementation |
| crashlytics-ios-link/build.gradle.kts | Deletes obsolete linker plugin build script |
| core/src/appleTest/kotlin/co/touchlab/crashkios/core/ThrowableNSExceptionTest.kt | Adds Apple tests for throwable-to-NSException utilities + registry |
| core/src/appleMain/kotlin/co/touchlab/crashkios/core/ThrowableNSException.kt | Vendors/adapts Throwable→NSException + hook wrapper utilities |
| core/src/appleMain/kotlin/co/touchlab/crashkios/core/CrashSinkRegistry.kt | Adds registry for sink storage + one-time unhandled hook install |
| core/src/appleMain/kotlin/co/touchlab/crashkios/core/CrashKiOS.kt | Adds unified Apple configuration entry point |
| build.gradle.kts | Removes Gradle publish plugin alias |
| bugsnag/src/nativeInterop/cinterop/bugsnag.def | Switches to protocol-only cinterop package/header |
| bugsnag/src/include/Private/BugsnagHandledState+NSExceptionKt.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/Private/BugsnagHandledState.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/BugsnagStackframe.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/BugsnagFeatureFlagStore.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/BugsnagFeatureFlag.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/BugsnagEvent.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/BugsnagError.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/BugsnagConfiguration+NSExceptionKt.h | Removes vendored wrapper header (no longer cinteroped) |
| bugsnag/src/include/BugsnagConfiguration.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/Bugsnag.h | Removes vendored SDK snippet header (no longer cinteroped) |
| bugsnag/src/include/Bugsnag-old.h | Removes obsolete vendored header |
| bugsnag/src/commonMain/kotlin/co/touchlab/crashkios/bugsnag/BugsnagKotlin.kt | Updates docs to reflect new Apple sink requirement |
| bugsnag/src/appleTest/kotlin/co/touchlab/crashkios/bugsnag/BugsnagSinkTest.kt | Adds Apple test verifying sink forwarding + fatal sequencing |
| bugsnag/src/appleMain/kotlin/co/touchlab/crashkios/bugsnag/BugsnagConfig.kt | Deletes obsolete Apple config/start API |
| bugsnag/src/appleMain/kotlin/co/touchlab/crashkios/bugsnag/BugsnagCallsActual.kt | Routes Apple calls through registered sink (no SDK symbol refs) |
| bugsnag/src/appleMain/kotlin/co/touchlab/crashkios/bugsnag/Bugsnag.kt | Adds BugsnagCrashReporting backend + explicit fatal hook logic |
| bugsnag/build.gradle.kts | Adjusts dependencies + cinterop include paths for protocol-only headers |
| bugsnag-ios-link/src/main/kotlin/co/touchlab/crashkios/BugsnagLinkPlugin.kt | Deletes obsolete linker plugin implementation |
| bugsnag-ios-link/build.gradle.kts | Deletes obsolete linker plugin build script |
| .gitignore | Ignores SPM build artifacts and resolution files |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| public func setCustomValue(_ value: Any?, forKey key: String) { | ||
| crashlytics.setCustomValue(value as Any, forKey: key) | ||
| } |
| func setCustomValue(_ value: Any?, forKey key: String) { | ||
| Crashlytics.crashlytics().setCustomValue(value as Any, forKey: key) | ||
| } |
| - (void)recordFatalException:(NSException * _Nonnull)exception; | ||
| - (void)setCustomValue:(id _Nullable)value forKey:(NSString * _Nonnull)key; | ||
| - (void)setUserId:(NSString * _Nonnull)identifier; | ||
| @end |
| func setUserId(_ identifier: String) { | ||
| Crashlytics.crashlytics().setUserID(identifier) | ||
| } | ||
| } |
| private func overrideOriginalUnhandledValue() { | ||
| guard let handledStateClass = NSClassFromString("BugsnagHandledState"), | ||
| let originalMethod = class_getInstanceMethod(handledStateClass, NSSelectorFromString("originalUnhandledValue")), | ||
| let method = class_getInstanceMethod(handledStateClass, NSSelectorFromString("unhandled")) | ||
| else { return } | ||
| method_setImplementation(originalMethod, method_getImplementation(method)) |
| /// Implemented in Swift, inside the app's build graph — where FirebaseCrashlytics is | ||
| /// already linked — and registered with the Kotlin side via `registerCrashlyticsSink`. | ||
| /// |
| /// Implemented in Swift, inside the app's build graph — where Bugsnag is already | ||
| /// linked — and registered with the Kotlin side via `registerBugsnagSink`. | ||
| /// |
| override fun install() { | ||
| bugsnagRegistry.register(sink, fatalHook(sink)) | ||
| BugsnagKotlin.implementation = BugsnagCallsActual() | ||
| } |
|
We hit exactly the bug this PR fixes and independently arrived at the same architecture, so wanted to add a confirmation to help this get Environment: Kotlin 2.4.10, SKIE 0.10.14, co.touchlab.crashkios:bugsnag:0.9.0. Symptom: startBugsnag(config:) / enableBugsnag() (and the classes that declare them, BugsnagConfigKt/BugsnagKotlinKt) are unusable from Swift — cannot find 'BugsnagConfigKt' in scope at compile time. This reproduces with a fully clean DerivedData and ModuleCache.noindex, so it isn't a caching artifact. linkDebugFrameworkIosSimulatorArm64 itself succeeds but warns Interop library .../bugsnag-cinterop-bugsnag.klib can't be exported with -Xexport-library. SKIE's own generated wrapper for anything taking a BugsnagConfiguration param independently confirms the same gap, emitting @available(*, unavailable, message: "Unknown Swift framework for type 'co.touchlab.crashkios.bugsnag.BugsnagConfiguration'..."). Root cause, as best we can tell: any part of the API that references the real Cocoa BugsnagConfiguration type fails to export from the cinterop klib, and Clang silently drops the affected ObjC declarations when building the consuming framework's Swift interface — even though the generated header still lists them as text. We worked around it by hand-rolling the same pattern this PR implements: a Kotlin interface with only primitive types (no cinterop), implemented in Swift, which then calls the real Bugsnag.notify() directly. Given this PR does exactly that as a proper library fix — moving the SDK-specific calls into Swift "sink" implementations so Kotlin/Native only cinterops protocol-only headers — it would let us (and presumably others hitting #85 downstream too, since PR #92 landed but isn't published) drop that workaround entirely. Is there anything blocking review/merge here? Happy to help test it against our app if useful. |
No description provided.