diff --git a/.forgejo/workflows/android.yml b/.forgejo/workflows/android.yml index 328637a..ca1433f 100644 --- a/.forgejo/workflows/android.yml +++ b/.forgejo/workflows/android.yml @@ -159,12 +159,10 @@ jobs: printf '%s' "$ANDROID_KEYSTORE_BASE64" | base64 -d > /tmp/inkwell-release.jks echo "variant=Release" >> $GITHUB_OUTPUT echo "label=release" >> $GITHUB_OUTPUT - # DEBUG profile, in a release APK, deliberately — see the note above - # the cargoNdk task. The release profile strips the symbols uniffi - # reads its metadata out of, so `generateUniffiBindings` fails - # outright (run 4077). Unpicking that is worth doing and is not worth - # blocking signed builds on. - echo "profile=debug" >> $GITHUB_OUTPUT + # An optimised .so for the APK people install. The cargoNdk task keeps + # the symbols uniffi reads its metadata from, which the release + # profile would strip (run 4077, Scribe #2810). + echo "profile=release" >> $GITHUB_OUTPUT echo "keystore=/tmp/inkwell-release.jks" >> $GITHUB_OUTPUT echo "apk=android/app/build/outputs/apk/release/app-release.apk" >> $GITHUB_OUTPUT echo "Signed release build — $version (versionCode $code)" @@ -296,8 +294,9 @@ jobs: # upload is stored and invisible (Scribe 2270). uses: actions/upload-artifact@v7 with: - # The APK's variant, NOT the Cargo profile — those are the same word - # for different things and the profile is pinned to debug (#2810). + # The APK's variant, NOT the Cargo profile. They are the same word for + # different things, and an unsigned debug APK still differs from a + # signed release one in more than its Rust. name: inkwell-android-${{ steps.build.outputs.label }}-${{ github.sha }} path: ${{ steps.build.outputs.apk }} if-no-files-found: error diff --git a/android/app/build.gradle.kts b/android/app/build.gradle.kts index f7cb6a8..0aa6938 100644 --- a/android/app/build.gradle.kts +++ b/android/app/build.gradle.kts @@ -53,11 +53,28 @@ abstract class CargoNdkBuild : DefaultTask() { // --locked so an Android build cannot silently re-resolve the workspace // lockfile the desktop lanes are gated on. args += "--locked" - if (cargoProfile.get() == "release") args += "--release" + val release = cargoProfile.get() == "release" + if (release) args += "--release" execOps.exec { commandLine(listOf("cargo") + args) workingDir = workspaceDir.get().asFile + if (release) { + // The workspace's release profile with two of its settings put back, + // for this build only. Environment overrides rather than a profile of + // our own, because cargo-ndk copies its `-o` output from the + // `release` directory and nothing says it understands any other one; + // the desktop's binaries are untouched either way. + // + // Keep the symbol table: `strip = true` removes the symbols uniffi's + // `--library` mode reads the interface out of, and bindgen then fails + // with "No UniFFI metadata found" (run 4077). There is no debug info + // in a release build to strip, so this keeps symbols and nothing else. + environment("CARGO_PROFILE_RELEASE_STRIP", "debuginfo") + // Unwind, so a panic in the core reaches Kotlin as an exception the + // board can show rather than ending the app. See the ffi crate's docs. + environment("CARGO_PROFILE_RELEASE_PANIC", "unwind") + } } } } @@ -131,14 +148,10 @@ val rustInputs = * profiles. `android.yml` picks one profile and uses it for every Gradle call in * the run. * - * CI currently passes `debug` even for a release APK, which is not where this - * should end up: an unoptimised store and sync engine is a real difference on a - * phone, not a theoretical one. The blocker is that the workspace's release - * profile sets `strip = true`, which removes the symbols uniffi reads its - * interface metadata from — `generateUniffiBindings` then fails with "No UniFFI - * metadata found" (run 4077). Fixing it means either an Android-specific profile - * that keeps symbols or generating the bindings from a separate unstripped - * build, and neither is worth holding signed APKs up for. Scribe #2810. + * CI passes `release` for a signed APK, so a phone runs an optimised store and sync + * engine. Until Scribe #2810 it passed `debug`, because the release profile's + * `strip = true` broke bindgen; `CargoNdkBuild` now keeps the symbols for this + * build alone. */ val rustProfile = (project.findProperty("INKWELL_CARGO_PROFILE") as String?)?.takeIf { it.isNotBlank() } diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index 98ab6f0..7ee734e 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -24,14 +24,15 @@ //! consistent; it simply hasn't stamped `last_sync_at`, which is only written after //! BOTH halves of a cycle succeed. The next cycle resumes from the stored cursor. //! -//! ## A known consequence of the release profile +//! ## A panic is an exception, not a crash //! -//! The workspace sets `panic = "abort"` (Tauri's profile, for binary size). uniffi -//! would otherwise catch a panic crossing the FFI boundary and raise it in Kotlin as -//! an exception; with `abort` it takes the process down instead. That is the same -//! behaviour the desktop already has, so no surface is worse off than another — but -//! it is a deliberate cost, not an oversight. Revisit if a panic in the core ever -//! turns out to be recoverable enough that a phone should survive it. +//! The workspace's release profile sets `panic = "abort"` (Tauri's, for binary size). +//! The Android build overrides it back to `unwind` (`CargoNdkBuild` in +//! `app/build.gradle.kts`), so uniffi catches a panic crossing the FFI boundary and +//! raises it in Kotlin as an exception, which the board shows as an error. Under +//! `abort` the same panic would end the app. Every phone build had unwind before +//! the release profile was used (debug unwinds), so keeping it changes nothing a +//! phone does. The cost is library size (Scribe #2810). pub mod models;