diff --git a/android/app/build.gradle.kts b/android/app/build.gradle.kts index 0aa6938..0e74116 100644 --- a/android/app/build.gradle.kts +++ b/android/app/build.gradle.kts @@ -10,17 +10,16 @@ plugins { val workspaceRoot: Directory = layout.projectDirectory.dir("../..") // The ABIs a release APK carries. arm64 is essentially every real device; armv7 -// covers older 32-bit hardware; the two x86 targets are what emulators run on, and -// dropping them would make the app untestable on a desktop emulator (the reason -// x86_64 was added to the old Tauri lane in task 1864). +// covers older 32-bit hardware; the two x86 targets are what emulators run, so the +// app stays testable on a desktop emulator. val androidAbis = listOf("arm64-v8a", "armeabi-v7a", "x86", "x86_64") /** * Cross-compile `inkwell-ffi` for each Android ABI and drop the resulting * `.so` into jniLibs, where AGP packages it. * - * `ExecOperations` injected rather than `project.exec`: the latter was REMOVED in - * Gradle 9, and reaching for `project` at execution time is also what breaks the + * `ExecOperations` injected rather than `project.exec`: Gradle 9 has no + * `project.exec`, and reaching for `project` at execution time breaks the * configuration cache this build has enabled. */ abstract class CargoNdkBuild : DefaultTask() { @@ -60,16 +59,15 @@ abstract class CargoNdkBuild : DefaultTask() { 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. + // The workspace's release profile with two 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; 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. + // with "No UniFFI metadata found". A release build has no debug info 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. @@ -141,17 +139,13 @@ val rustInputs = /** * Which Cargo profile the `.so` is built with. * - * A property rather than a debug/release task PAIR, deliberately. This runner has - * no working Gradle or Cargo cache (`reserveCache failed` on every run), so a cold - * cross-compile of four ABIs costs about four minutes — and a lane that both - * type-checks and packages would pay that twice if the two used different - * profiles. `android.yml` picks one profile and uses it for every Gradle call in - * the run. + * A property rather than a debug/release task pair: this runner has no working + * Gradle or Cargo cache, so a cold cross-compile of four ABIs costs about four + * minutes, and a lane that type-checks and packages with different profiles would + * pay it twice. `android.yml` picks one profile for every Gradle call in the run. * * 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. + * engine; `CargoNdkBuild` keeps the symbols bindgen needs. */ val rustProfile = (project.findProperty("INKWELL_CARGO_PROFILE") as String?)?.takeIf { it.isNotBlank() } @@ -187,8 +181,7 @@ android { defaultConfig { applicationId = "com.fabledsword.inkwell" - // 26 (Android 8, 2017) matches Minstrel and clears the NDK's floor with - // room to spare. + // 26 (Android 8, 2017) clears the NDK's floor with room to spare. minSdk = 26 targetSdk = 36 // Injected by CI from the git tag + commit count for a release; "dev" @@ -212,11 +205,10 @@ android { } // The signing key reaches this build only through the environment: CI decodes - // it from a secret into a file and points ANDROID_KEYSTORE_FILE at that path. - // It is never in the repo and never in this file. Generated by the operator - // and never seen by an agent session, because an Android signing key cannot be - // rotated without the original — v3 lineage needs it — so a leaked or lost one - // means every install has to be removed and replaced by hand. + // it from a secret into a file and points ANDROID_KEYSTORE_FILE at that path. It + // is never in the repo. An Android signing key cannot be rotated without the + // original (v3 lineage needs it), so a leaked or lost one means every install + // has to be removed and replaced by hand. val keystoreFile = System.getenv("ANDROID_KEYSTORE_FILE")?.takeIf { it.isNotBlank() } val keystorePassword = System.getenv("ANDROID_KEYSTORE_PASSWORD")?.takeIf { it.isNotBlank() } @@ -225,13 +217,11 @@ android { create("release") { storeFile = File(keystoreFile) storePassword = keystorePassword - // Hardcoded, and NOT a secret: the alias is fixed for the life of - // this app and is written into the certificate every install - // already carries. Hiding it would buy nothing and stop this file - // describing its own signing setup. It still says `thoughtsync` - // after the rename to Inkwell, because it names the key inside the - // existing keystore, and renaming it would only stop that key being - // found. Same key, so the signing certificate did not change either. + // Hardcoded, and NOT a secret: the alias is fixed for the life of this + // app and written into the certificate every install carries. It says + // `thoughtsync`, the app's earlier name, because it names the key + // inside the existing keystore; renaming it would stop the key being + // found. keyAlias = "thoughtsync" // PKCS12 cannot hold a key password distinct from the store // password — keytool refuses to set one — so this is the same diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 33abf1b..03b9859 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -25,9 +25,8 @@ FOREGROUND_SERVICE used only for expedited work; nothing here asks for it, and it arrives with the library - Verified against the built APK's merged manifest, not assumed. Noted here - because all four appear in the app's permission list and nothing else in - this file would explain where they came from. + Listed because all four appear in the app's permission list and nothing + else in this file would explain where they came from. --> @@ -91,12 +88,11 @@ Share Loading… @@ -134,7 +134,7 @@ who it happens in. --> New note - Manage tags All tags (%1$d) @@ -157,9 +157,9 @@ Rename Rename %1$s Rename - + Merge with %1$s? A tag called %1$s already exists. Renaming will merge these two into one, carrying every note from both. The notes are kept; one of the two tags stops existing, and that cannot be undone. Merge diff --git a/android/app/src/test/java/com/fabledsword/inkwell/ui/DerivedTintTest.kt b/android/app/src/test/java/com/fabledsword/inkwell/ui/DerivedTintTest.kt index 1b135a4..13a443a 100644 --- a/android/app/src/test/java/com/fabledsword/inkwell/ui/DerivedTintTest.kt +++ b/android/app/src/test/java/com/fabledsword/inkwell/ui/DerivedTintTest.kt @@ -12,11 +12,8 @@ import org.junit.Test * writes them out by hand, so changing the hash or the key order means changing the * fixture and this file together, or one of the two suites goes red. * - * SMALLER SINCE M315. Half of what this file used to pin — the generated card fill, and - * the resolution order that chose between a picked colour, a tag's and a generated one - * — went with the code it guarded when the card became one neutral. The four UUID - * hashes stay because they are what the hash ITSELF is pinned by; nothing derives a - * colour from an id any more, only from a tag's name. + * The four UUID hashes pin the hash ITSELF; colours are derived only from a tag's + * name. */ class DerivedTintTest { @Test diff --git a/android/bindgen/Cargo.toml b/android/bindgen/Cargo.toml index 937d549..a1a84ff 100644 --- a/android/bindgen/Cargo.toml +++ b/android/bindgen/Cargo.toml @@ -7,16 +7,12 @@ edition = "2021" # A crate whose ONLY dependency is uniffi itself. # -# This started life as a `[[bin]]` inside inkwell-ffi, which failed: building -# it compiled that crate and therefore the core, reqwest, native-tls and -# openssl-sys — for the HOST. The vendored-OpenSSL block in core/Cargo.toml is -# scoped to `cfg(target_os = "android")`, so a host build looks for a system -# OpenSSL that ci-rust-android has no reason to carry, and the generator died -# with "failed to run custom build command for openssl-sys". -# -# Adding libssl-dev to the image would have worked and been wrong: a code -# generator should not link the app's TLS stack to emit Kotlin. Splitting it out -# means the generator compiles ~15 small crates and nothing else. +# Not a `[[bin]]` inside inkwell-ffi: building that compiles the core, reqwest, +# native-tls and openssl-sys for the HOST, and the vendored-OpenSSL block in +# core/Cargo.toml is scoped to `cfg(target_os = "android")`, so a host build +# looks for a system OpenSSL that ci-rust-android doesn't carry. A code +# generator should not link the app's TLS stack to emit Kotlin; on its own it +# compiles ~15 small crates. # # Still a WORKSPACE MEMBER, deliberately. That is what keeps `uniffi` here and # `uniffi` linked into the .so on one version from one lockfile — they are two diff --git a/android/build.gradle.kts b/android/build.gradle.kts index 9866537..fe59fea 100644 --- a/android/build.gradle.kts +++ b/android/build.gradle.kts @@ -1,8 +1,8 @@ plugins { alias(libs.plugins.android.application) apply false // kotlin-android is NOT registered: AGP 9 enables built-in Kotlin, and the - // older plugin can't cast AGP 9's ApplicationExtension to the removed - // BaseExtension. Same conclusion Minstrel reached on this toolchain pair. + // older plugin can't cast AGP 9's ApplicationExtension to the BaseExtension + // AGP 9 no longer has. alias(libs.plugins.compose.compiler) apply false // ktlint/detekt are run from the CI image's pinned CLIs, not as Gradle // plugins — see the note in gradle/libs.versions.toml. diff --git a/android/config/detekt.yml b/android/config/detekt.yml index e0dfec0..9ffd925 100644 --- a/android/config/detekt.yml +++ b/android/config/detekt.yml @@ -1,8 +1,8 @@ # Per-rule overrides layered on top of detekt's defaults # (`--build-upon-default-config` on the CLI invocation in the Android lane). # -# The pre-2.0 `build:` top-level was removed; failure is controlled by the CLI's -# exit code instead. +# Failure is controlled by the CLI's exit code; detekt 2 has no `build:` +# top-level. naming: # Composables conventionally use PascalCase function names. Matches every @@ -49,7 +49,7 @@ complexity: exceptions: TooGenericExceptionCaught: - # Catching broadly is DELIBERATE in these two places, and each site says so. + # Catching broadly is DELIBERATE in these places, and each site says so. # # * the ViewModel — a note that fails to save must become a visible error # banner, never a crash. Narrowing this would mean an unanticipated diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index f4a3feb..fb2517d 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -3,7 +3,7 @@ //! This crate is to Android what `desktop/src-tauri/src/commands/` is to the desktop //! — a thin shim over the shared core, holding no logic of its own. If something here //! starts making decisions about notes or sync, it belongs in the core where the -//! desktop gets it too (Scribe note 2730). +//! desktop gets it too. //! //! ## Shape //! @@ -30,9 +30,7 @@ //! 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). +//! `abort` the same panic would end the app. The cost is library size. pub mod models; @@ -105,7 +103,7 @@ impl CoreError { } /// Seals the device token before it is stored, and opens it when it is read: Kotlin's -/// half of [`state::TokenSeal`] (family idea #5105, practice 12). +/// half of [`state::TokenSeal`]. /// /// Implemented with a key in the Android Keystore. The key never leaves the phone, /// so a copy of the app's files carries a token nothing else can open. That includes @@ -255,7 +253,7 @@ impl Inkwell { /// /// Returns nothing, as the board reloads its list afterwards anyway. The core marks /// each note dirty so the new positions sync, and a note shared with this account - /// moves on this board only (#5176). + /// moves on this board only. pub fn reorder_notes(&self, ordered_ids: Vec) -> Result<(), CoreError> { let conn = self.db.conn().map_err(CoreError::store)?; local::store::reorder(&conn, &ordered_ids).map_err(CoreError::store) @@ -600,11 +598,11 @@ impl Inkwell { // ────────────────────────────── sharing ────────────────────────────── // - // The Share dialog asks the server directly (#5175). Unlinked, each answers + // The Share dialog asks the server directly. Unlinked, each answers // `NotLinked`, which the dialog turns into "sharing needs a server". // // Each passes the token `credentials()` OPENED. The stored one is sealed, and the - // server refuses `sealed:…` (#5381). + // server refuses `sealed:…`. /// Everyone on the instance a note can be shared with, and every group. pub async fn share_directory(&self) -> Result { @@ -682,10 +680,8 @@ pub fn checklist_render(text: String, checked: bool) -> String { /// Tell the core which app it is running inside, and which build of it. /// /// Android has to say so because the core cannot: the same crate is compiled into -/// the desktop app, and it used to announce every phone in the field as -/// `inkwell-desktop` carrying the CORE crate's version — a number no build -/// stamps and nobody has seen. The honest value is the installed package's own -/// `versionName`, which is what Kotlin passes here. +/// the desktop app. The honest value is the installed package's own `versionName`, +/// which is what Kotlin passes here. /// /// Called once from `InkwellApplication.onCreate`, before anything can sync. #[uniffi::export] @@ -820,7 +816,7 @@ mod tests { .get_note(created.id.clone()) .expect("get should succeed"); assert_eq!(fetched.id, created.id); - // The NAME is the first line — there is no title field to have set (M13 step 3). + // The NAME is the first line — there is no title field to have set. assert_eq!(fetched.display_title, "Groceries"); std::fs::remove_dir_all(&dir).ok(); @@ -1041,9 +1037,8 @@ mod tests { /// The path the notification's Done button takes. /// - /// Completing a RECURRING reminder must move it, not end it — this is the - /// behaviour the web has had all along and the clients did not, which made - /// "Done" on a daily reminder quietly the last time it ever fired. + /// Completing a RECURRING reminder must move it, not end it, as on the web; + /// otherwise "Done" on a daily reminder would be the last time it ever fired. #[test] fn completing_a_recurring_reminder_moves_it_rather_than_ending_it() { let dir = scratch_dir(); @@ -1103,9 +1098,9 @@ mod tests { /// Renaming a tag onto a name another tag already holds MERGES the two, and the /// OLDER row is the survivor. /// - /// Before this, the bare UPDATE met `idx_labels_name` — unique on `lower(name)` — - /// and the user got a raw "UNIQUE constraint failed" from SQLite. Merging is what - /// a person means by typing an existing tag's name onto this one. + /// A bare UPDATE would meet `idx_labels_name` (unique on `lower(name)`) and hand + /// the user a raw "UNIQUE constraint failed"; merging is what a person means by + /// typing an existing tag's name onto this one. #[test] fn renaming_onto_an_existing_tag_merges_into_the_older_one() { let dir = scratch_dir(); @@ -1215,8 +1210,8 @@ mod tests { (url, handle) } - /// #5381: the token is stored sealed, so a share call has to send the OPENED one. - /// It used to read the store itself and send `sealed:…`, which the server refused. + /// The token is stored sealed, so a share call has to send the OPENED one; the + /// server refuses `sealed:…`. #[test] fn sharing_sends_the_opened_token_not_the_stored_one() { let (url, server) = one_reply(r#"{"members":[],"groups":[]}"#); diff --git a/android/ffi/src/models.rs b/android/ffi/src/models.rs index 488f681..0601d86 100644 --- a/android/ffi/src/models.rs +++ b/android/ffi/src/models.rs @@ -46,7 +46,7 @@ pub struct Note { pub previews: Vec, pub created_at: Option, pub updated_at: Option, - /// How this account holds the note (#5175): "owner", or "edit"/"view" for one + /// How this account holds the note: "owner", or "edit"/"view" for one /// someone shared with it. pub permission: String, /// Whether the owner has shared it with anyone. @@ -335,7 +335,7 @@ pub struct Label { /// matching the REST single-label responses. pub count: Option, /// The newest edit among the untrashed notes carrying it, as the store writes - /// timestamps; None when nothing does. Ranks the drawer's recent tags (#5931). + /// timestamps; None when nothing does. Ranks the drawer's recent tags. pub last_used: Option, } @@ -424,7 +424,7 @@ impl From for core_models::Facets { #[derive(Debug, Clone, uniffi::Record)] pub struct NoteDraft { pub body: String, - /// Checklist lines. A note can carry both a body and items (M13 step 2), so this + /// Checklist lines. A note can carry both a body and items, so this /// is not an alternative to `body` — it is an addition to it. pub items: Option>, } @@ -616,7 +616,7 @@ impl From for Identity { } } -/// Someone on the instance a note can be shared with (#5175). +/// Someone on the instance a note can be shared with. #[derive(Debug, Clone, uniffi::Record)] pub struct Member { pub id: String, @@ -639,7 +639,7 @@ impl From for Member { } } -/// A group the admin made, which a note can be shared with (#5177). +/// A group the admin made, which a note can be shared with. #[derive(Debug, Clone, uniffi::Record)] pub struct ShareGroup { pub id: String, diff --git a/android/gradle.properties b/android/gradle.properties index ad741bd..0e19378 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,7 +1,7 @@ # --enable-native-access=ALL-UNNAMED silences the JDK 22+ "restricted method in # java.lang.System has been called" warning that Gradle 9.1's bundled -# native-platform jar trips via System.load(). Same opt-in Minstrel needs on the -# same Gradle/JDK pair; future JDKs promote the warning to an error. +# native-platform jar trips via System.load(); future JDKs promote the warning +# to an error. org.gradle.jvmargs=-Xmx4g -Dfile.encoding=UTF-8 --enable-native-access=ALL-UNNAMED org.gradle.parallel=true org.gradle.caching=true @@ -9,8 +9,8 @@ org.gradle.configuration-cache=true android.useAndroidX=true android.nonTransitiveRClass=true -# Matches Minstrel: detekt 2.0-alpha and the ktlint Gradle plugin still have -# intermittent configuration-cache holes. Warn rather than fail so the CC speedup -# applies where it can. +# detekt 2.0-alpha and the ktlint Gradle plugin have intermittent +# configuration-cache holes. Warn rather than fail so the CC speedup applies +# where it can. org.gradle.configuration-cache.problems=warn kotlin.code.style=official diff --git a/android/gradle/libs.versions.toml b/android/gradle/libs.versions.toml index fcbc46d..9acad28 100644 --- a/android/gradle/libs.versions.toml +++ b/android/gradle/libs.versions.toml @@ -1,5 +1,5 @@ [versions] -# Pinned as a MATRIX, matching Minstrel's proven combination on the same JDK: +# Pinned as a MATRIX: # - Gradle 9.1.0 supports JDK 25 (see gradle-wrapper.properties) # - AGP 9.0.1 requires Gradle 9.1.0+ # - Kotlin 2.3.x is AGP 9's built-in Kotlin path @@ -14,16 +14,13 @@ coroutines = "1.9.0" # WorkManager runs the background sync. `work-runtime-ktx` is NOT used: as of # 2.11 it is a 6 KB stub and every Kotlin extension (`PeriodicWorkRequestBuilder`, -# `CoroutineWorker`) has moved into `work-runtime` itself. Verified by unpacking -# both artifacts, not from memory. +# `CoroutineWorker`) has moved into `work-runtime` itself. work = "2.11.2" -# ktlint and detekt are NOT Gradle plugins here. ci-rust-android already ships -# both as pinned CLIs (M12 step 3), and the CI lane invokes those directly. Adding -# the Gradle plugins would mean a SECOND pinned version of each tool, resolved at -# build time, that has to be kept in lockstep with the image's by hand — and the -# first attempt at it failed outright, because the detekt version Minstrel pins -# (2.0.0-alpha.3) is not published to Maven Central or the plugin portal at all. +# ktlint and detekt are NOT Gradle plugins here. ci-rust-android ships both as +# pinned CLIs, and the CI lane invokes those directly. The Gradle plugins would be +# a SECOND pinned version of each tool, resolved at build time, that has to be +# kept in lockstep with the image's by hand. # JNA is not optional: uniffi's Kotlin bindings call into the .so through it. # The @aar classifier matters — the plain jar has no Android native payload and @@ -32,7 +29,7 @@ jna = "5.14.0" junit = "4.13.2" -# Coil draws a link preview's image (Scribe #3307), the first image this client +# Coil draws a link preview's image, the first image this client # loads. Coil 3 ships its networking separately; the OkHttp fetcher is the one its # docs name as the default, and it registers itself, so no loader is configured. # Held at 3.5.0: 3.6.0 moved to compileSdk 37 and Compose 1.12, which need AGP 9.1. diff --git a/android/tools/check-strings.py b/android/tools/check-strings.py index 62f28da..a027605 100755 --- a/android/tools/check-strings.py +++ b/android/tools/check-strings.py @@ -44,8 +44,7 @@ def supplied_args(rest): """Count top-level commas in an argument tail. Kotlin permits a TRAILING comma before the closing paren, which is not an - argument — counting it inflated every multi-line call by one the first time - this was written, and made three correct call sites look broken. Braces count + argument; counting it would inflate every multi-line call by one. Braces count toward depth as well as parens, or a comma inside a lambda would be read as another argument. """ diff --git a/android/tools/check-symbols.py b/android/tools/check-symbols.py index dc9ad66..8dd3739 100755 --- a/android/tools/check-symbols.py +++ b/android/tools/check-symbols.py @@ -1,11 +1,8 @@ #!/usr/bin/env python3 """Flag capitalised identifiers that are neither imported nor declared locally. -This exists because ktlint and detekt are both structurally blind to it: they -parse Kotlin without resolving symbols, so a *missing import* is invisible to -them and both pass a file that cannot compile. The first sync-screen push failed -in CI on exactly that (`Unresolved reference 'Build'` — `android.os.Build` was -lost in a file split), after a clean local analyzer run. +ktlint and detekt parse Kotlin without resolving symbols, so a *missing import* +is invisible to them and both pass a file that cannot compile. Not a type checker and not trying to be. `compileDebugKotlin` in CI is the real one; this is a cheap pre-push filter for the single mistake that survives every @@ -13,10 +10,8 @@ other local gate. It errs toward false positives — anything it cannot account for is reported rather than assumed fine. It also checks MEMBERS of this package's own `object` declarations — `Foo.bar()` -where `Foo` is an object declared here. That case was added after moving a -function between two objects and forgetting to paste it into the second: the -call site read `Other.thing()`, resolved fine as far as the leading token, and -failed in CI (785ebdb). +where `Foo` is an object declared here — so a function moved between two objects +and missed at a call site is caught. Still NOT caught, so a clean run is not over-read: members of anything declared outside this package, members reached through a variable rather than a type @@ -106,8 +101,7 @@ def object_members(src: str) -> dict: if depth == 0: # Nested TYPES count as members too: `Foo.Bar` where Bar is a # data class inside object Foo is an ordinary reference, and - # leaving them out made the checker report four false positives - # the first time an object held one. + # leaving them out would report each one as unknown. decl = re.match( r"\s*(?:@\w+\s+)*(?:public |private |internal |protected )?" r"(?:const |lateinit |inline |suspend |data |sealed |enum |value |abstract |open )*"