diff --git a/.skills/code-review/SKILL.md b/.skills/code-review/SKILL.md index dce08761d..de8c93c88 100644 --- a/.skills/code-review/SKILL.md +++ b/.skills/code-review/SKILL.md @@ -61,6 +61,10 @@ When reviewing code, meticulously verify the following categories. Flag any devi - [ ] **Libraries:** Verify usage of `Turbine` for Flow testing, `Kotest` for property-based testing, and `Mokkery` for mocking. - [ ] **Robolectric Configuration:** Check that Compose UI tests running via Robolectric on JVM are pinned to `@Config(sdk = [34])` to prevent Java 21 / SDK 35 compatibility issues. +### 8. ProGuard / R8 Rules +- [ ] **New Dependencies:** If a new reflection-heavy dependency is added (DI, serialization, JNI, ServiceLoader), verify keep rules exist in **both** `app/proguard-rules.pro` (R8) and `desktop/proguard-rules.pro` (ProGuard). The two files must stay aligned. +- [ ] **Release Smoke-Test:** For dependency or ProGuard rule changes, verify `assembleRelease` and `./gradlew :desktop:runRelease` succeed. + ## Review Output Guidelines 1. **Be Specific & Constructive:** Provide exact file references and code snippets illustrating the required project pattern. 2. **Reference the Docs:** Cite `AGENTS.md` and project architecture playbooks to justify change requests (e.g., "Per AGENTS.md, `java.io.*` cannot be used in `commonMain`; please migrate to Okio"). diff --git a/.skills/implement-feature/SKILL.md b/.skills/implement-feature/SKILL.md index 1efa3caa0..0e76b30e6 100644 --- a/.skills/implement-feature/SKILL.md +++ b/.skills/implement-feature/SKILL.md @@ -35,3 +35,7 @@ A step-by-step workflow for implementing a new feature in the Meshtastic-Android ```bash ./gradlew spotlessCheck detekt assembleDebug test allTests ``` +- If the feature adds a new reflection-heavy dependency, add keep rules to **both** `app/proguard-rules.pro` and `desktop/proguard-rules.pro`, then verify release builds: + ```bash + ./gradlew assembleFdroidRelease :desktop:runRelease + ``` diff --git a/app/proguard-rules.pro b/app/proguard-rules.pro index 995f659ba..7feaa9217 100644 --- a/app/proguard-rules.pro +++ b/app/proguard-rules.pro @@ -1,61 +1,61 @@ -# Add project specific ProGuard rules here. -# You can control the set of applied configuration files using the -# proguardFiles setting in build.gradle.kts. -# -# For more details, see -# http://developer.android.com/guide/developing/tools/proguard.html +# ============================================================================ +# Meshtastic Android — ProGuard / R8 rules for release minification +# ============================================================================ +# Open-source project: obfuscation is disabled. We rely on tree-shaking and +# code optimization for APK size reduction. +# ============================================================================ -# If your project uses WebView with JS, uncomment the following -# and specify the fully qualified class name to the JavaScript interface -# class: -#-keepclassmembers class fqcn.of.javascript.interface.for.webview { -# public *; -#} +# ---- General ---------------------------------------------------------------- -# Uncomment this to preserve the line number information for -# debugging stack traces. +# Preserve line numbers for meaningful crash stack traces -keepattributes SourceFile,LineNumberTable -# If you keep the line number information, uncomment this to -# hide the original source file name. -#-renamesourcefileattribute SourceFile +# Open-source — no need to obfuscate +-dontobfuscate -# Room KMP: preserve generated database constructor (required for R8/ProGuard) --keep class * extends androidx.room.RoomDatabase { (); } +# ---- Networking (transitive references from Ktor) --------------------------- -# Needed for protobufs --keep class com.google.protobuf.** { *; } --keep class org.meshtastic.proto.** { *; } - -# Networking -dontwarn org.conscrypt.** -dontwarn org.bouncycastle.** -dontwarn org.openjsse.** -# ? --dontwarn java.lang.reflect.** --dontwarn com.google.errorprone.annotations.** +# ---- Wire Protobuf ---------------------------------------------------------- -# Our app is opensource no need to obsfucate --dontobfuscate --optimizations !code/simplification/arithmetic,!field/*,!class/merging/*,!code/allocation/variable +# Wire-generated proto message classes (accessed via ADAPTER companion reflection) +-keep class org.meshtastic.proto.** { *; } -# Koin DI: prevent R8 from merging exception classes (observed as io.ktor.http.URLDecodeException +# ---- Room KMP (room3) ------------------------------------------------------ + +# Preserve generated database constructors (Room uses reflection to instantiate) +-keep class * extends androidx.room3.RoomDatabase { (); } + +# ---- Koin DI ---------------------------------------------------------------- + +# Prevent R8 from merging exception classes (observed as io.ktor.http.URLDecodeException # replacing Koin's InstanceCreationException in stack traces, making crashes undiagnosable). -keep class org.koin.core.error.** { *; } -# R8 optimization for Kotlin null checks (AGP 9.0+) --processkotlinnullchecks remove +# ---- Compose Multiplatform -------------------------------------------------- -# Compose Multiplatform resources: keep the resource library internals and generated Res -# accessor classes so R8 does not tree-shake the resource loading infrastructure. -# Without these rules the fdroid flavor (which has fewer transitive Compose dependencies -# than google) crashes at startup with a misleading URLDecodeException due to R8 -# exception-class merging (see Koin keep rule above). +# Keep resource library internals and generated Res accessor classes so R8 does +# not tree-shake the resource loading infrastructure. Without these rules the +# fdroid flavor crashes at startup with a misleading URLDecodeException due to +# R8 exception-class merging. -keep class org.jetbrains.compose.resources.** { *; } -keep class org.meshtastic.core.resources.** { *; } -# Nordic BLE --dontwarn no.nordicsemi.kotlin.ble.environment.android.mock.** --keep class no.nordicsemi.kotlin.ble.environment.android.mock.** { *; } --keep class no.nordicsemi.kotlin.ble.environment.android.compose.** { *; } +# Compose Animation: prevent R8 from merging animation spec classes (easing +# curves, transition specs, Animatable internals) which can cause animations to +# silently snap in release builds. +# +# -keep prevents class merging (EnterTransition/ExitTransition into *Impl, +# VectorizedSpringSpec/TweenSpec elimination, etc.). +# allowshrinking lets R8 remove genuinely unreachable classes (e.g. +# SharedTransition APIs, RepeatableSpec — unused by this app). Verified via +# dex analysis: 278 classes survive in release vs 139 without this rule; +# all actively used classes (AnimatedVisibility, Crossfade, SpringSpec, +# TweenSpec, EnterTransition, ExitTransition, etc.) are preserved. +# allowobfuscation is moot (-dontobfuscate is set above) but explicit for +# clarity. +# The ** wildcard is recursive and covers animation.core.* sub-packages. +-keep,allowshrinking,allowobfuscation class androidx.compose.animation.** { *; } diff --git a/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt index fd432a1fa..cc53f27ec 100644 --- a/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt @@ -40,14 +40,9 @@ class AndroidApplicationConventionPlugin : Plugin { configureKotlinAndroid(this) defaultConfig { - testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" vectorDrawables.useSupportLibrary = true } - testOptions { - animationsDisabled = true - unitTests.isReturnDefaultValues = true - } buildTypes { getByName("release") { diff --git a/build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt index cf3ae81db..68771d24a 100644 --- a/build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt @@ -38,11 +38,6 @@ class AndroidLibraryConventionPlugin : Plugin { extensions.configure { configureKotlinAndroid(this) - defaultConfig.testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" - testOptions { - animationsDisabled = true - unitTests.isReturnDefaultValues = true - } defaultConfig { // When flavorless modules depend on flavored modules (like :core:data), diff --git a/build-logic/convention/src/main/kotlin/org/meshtastic/buildlogic/KotlinAndroid.kt b/build-logic/convention/src/main/kotlin/org/meshtastic/buildlogic/KotlinAndroid.kt index 580db4c4b..bcc6d0207 100644 --- a/build-logic/convention/src/main/kotlin/org/meshtastic/buildlogic/KotlinAndroid.kt +++ b/build-logic/convention/src/main/kotlin/org/meshtastic/buildlogic/KotlinAndroid.kt @@ -44,19 +44,19 @@ internal fun Project.configureKotlinAndroid(commonExtension: CommonExtension) { compileSdk = compileSdkVersion defaultConfig.minSdk = minSdkVersion + defaultConfig.testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" if (this is ApplicationExtension) { defaultConfig.targetSdk = targetSdkVersion } - val javaVersion = if (project.name in listOf("api", "model", "proto")) { - JavaVersion.VERSION_17 - } else { - JavaVersion.VERSION_21 - } + val javaVersion = if (project.name in PUBLISHED_MODULES) JavaVersion.VERSION_17 else JavaVersion.VERSION_21 compileOptions.sourceCompatibility = javaVersion compileOptions.targetCompatibility = javaVersion + testOptions.animationsDisabled = true + testOptions.unitTests.isReturnDefaultValues = true + // Exclude duplicate META-INF license files shipped by JUnit Platform JARs packaging.resources.excludes.addAll( listOf( @@ -190,11 +190,25 @@ internal fun Project.configureKotlinJvm() { configureKotlin() } +/** Modules published for external consumers — use Java 17 for broader compatibility. */ +private val PUBLISHED_MODULES = setOf("api", "model", "proto") + +/** Compiler args shared across all Kotlin targets (JVM, Android, iOS, etc.). */ +private val SHARED_COMPILER_ARGS = listOf( + "-opt-in=kotlin.uuid.ExperimentalUuidApi", + "-opt-in=kotlin.time.ExperimentalTime", + "-Xexpect-actual-classes", + "-Xcontext-parameters", + "-Xannotation-default-target=param-property", + "-Xskip-prerelease-check", +) + /** Configure base Kotlin options */ private inline fun Project.configureKotlin() { + val isPublishedModule = project.name in PUBLISHED_MODULES + extensions.configure { - val javaVersion = if (project.name in listOf("api", "model", "proto")) 17 else 21 - val isPublishedModule = project.name in listOf("api", "model", "proto") + val javaVersion = if (isPublishedModule) 17 else 21 // Using Java 17 for published modules for better compatibility with consumers (e.g. plugins, older environments), // and Java 21 for the rest of the app. jvmToolchain(javaVersion) @@ -208,14 +222,7 @@ private inline fun Project.configureKotlin() { if (!isPublishedModule) { freeCompilerArgs.add("-opt-in=kotlinx.coroutines.ExperimentalCoroutinesApi") } - freeCompilerArgs.addAll( - "-opt-in=kotlin.uuid.ExperimentalUuidApi", - "-opt-in=kotlin.time.ExperimentalTime", - "-Xexpect-actual-classes", - "-Xcontext-parameters", - "-Xannotation-default-target=param-property", - "-Xskip-prerelease-check", - ) + freeCompilerArgs.addAll(SHARED_COMPILER_ARGS) if (isJvmTarget) { freeCompilerArgs.add("-jvm-default=no-compatibility") } @@ -230,21 +237,13 @@ private inline fun Project.configureKotlin() { tasks.withType().configureEach { compilerOptions { - val isPublishedModule = project.name in listOf("api", "model", "proto") jvmTarget.set(if (isPublishedModule) JvmTarget.JVM_17 else JvmTarget.JVM_21) allWarningsAsErrors.set(warningsAsErrors) if (!isPublishedModule) { freeCompilerArgs.add("-opt-in=kotlinx.coroutines.ExperimentalCoroutinesApi") } - freeCompilerArgs.addAll( - "-opt-in=kotlin.uuid.ExperimentalUuidApi", - "-opt-in=kotlin.time.ExperimentalTime", - "-Xexpect-actual-classes", - "-Xcontext-parameters", - "-Xannotation-default-target=param-property", - "-Xskip-prerelease-check", - "-jvm-default=no-compatibility", - ) + freeCompilerArgs.addAll(SHARED_COMPILER_ARGS) + freeCompilerArgs.add("-jvm-default=no-compatibility") } } } diff --git a/desktop/README.md b/desktop/README.md index 129f49e94..491e9fe68 100644 --- a/desktop/README.md +++ b/desktop/README.md @@ -25,14 +25,18 @@ A Compose Desktop application target — the first full non-Android target for t ## ProGuard / Minification -Release builds use ProGuard for tree-shaking (unused code removal), significantly reducing distribution size. Obfuscation is disabled since the project is open-source. +Release builds use ProGuard for tree-shaking (unused code removal), significantly reducing distribution size. Obfuscation is disabled since the project is open-source. Rules are aligned with the Android R8 rules in `app/proguard-rules.pro` — both targets share the same anti-class-merging philosophy. **Configuration:** - `build.gradle.kts` — `buildTypes.release.proguard` block enables ProGuard with `optimize.set(true)` and `obfuscate.set(false)`. -- `proguard-rules.pro` — Comprehensive keep-rules for all reflection/JNI-sensitive dependencies (Koin, kotlinx-serialization, Wire protobuf, Room KMP, Ktor, Kable BLE, Coil, SQLite JNI, Compose Multiplatform resources). +- `proguard-rules.pro` — Keep-rules for reflection/JNI-sensitive dependencies (Koin, kotlinx-serialization, Wire protobuf, Room KMP `androidx.room3`, Ktor, Kable BLE, Coil, SQLite JNI, Compose Multiplatform resources) and an anti-merge rule for Compose animation classes. + +**Key rules:** +- **Compose animation anti-merge** (`-keep,allowshrinking,allowobfuscation class androidx.compose.animation.** { *; }`) — Prevents ProGuard's optimizer from merging animation class hierarchies (e.g. `EnterTransition`/`ExitTransition` into `*Impl`), which causes animations to silently snap. Same rule as Android. +- **Room KMP** — Uses `androidx.room3` package path (Room KMP 3.x). **Troubleshooting ProGuard issues:** -- If the release build crashes at runtime with `ClassNotFoundException` or `NoSuchMethodError`, a library is loading classes via reflection that ProGuard stripped. Add a `-keep` rule in `proguard-rules.pro`. +- If the release build crashes at runtime with `ClassNotFoundException` or `NoSuchMethodError`, a library is loading classes via reflection that ProGuard stripped. Add a `-keep` rule in `proguard-rules.pro` **and** the corresponding rule in `app/proguard-rules.pro` to keep both targets aligned. - To debug which classes ProGuard removes, temporarily add `-printusage proguard-usage.txt` to the rules file and inspect the output in `desktop/proguard-usage.txt`. - To see the full mapping of optimizations applied, add `-printseeds proguard-seeds.txt`. - Run `./gradlew :desktop:runRelease` for a quick smoke-test of the minified app before packaging. diff --git a/desktop/proguard-rules.pro b/desktop/proguard-rules.pro index a73c347d1..b4e6cc451 100644 --- a/desktop/proguard-rules.pro +++ b/desktop/proguard-rules.pro @@ -147,6 +147,14 @@ -keep class org.jetbrains.compose.resources.** { *; } -keep class org.meshtastic.core.resources.** { *; } +# ---- Compose Animation (anti-merge) ---------------------------------------- + +# Prevent ProGuard from merging animation spec class hierarchies (same issue +# as R8 on Android — EnterTransition/ExitTransition merged into *Impl, +# VectorizedSpringSpec/TweenSpec eliminated). allowshrinking lets ProGuard +# remove genuinely unreachable classes. +-keep,allowshrinking,allowobfuscation class androidx.compose.animation.** { *; } + # ---- AboutLibraries --------------------------------------------------------- -keep class com.mikepenz.aboutlibraries.** { *; } diff --git a/docs/BUILD_LOGIC_CONVENTIONS_GUIDE.md b/docs/BUILD_LOGIC_CONVENTIONS_GUIDE.md index 17b152f4a..5898f7f94 100644 --- a/docs/BUILD_LOGIC_CONVENTIONS_GUIDE.md +++ b/docs/BUILD_LOGIC_CONVENTIONS_GUIDE.md @@ -129,27 +129,17 @@ kotlin { ### Example: Adding Android-specific test config -**Pattern:** Add to `AndroidLibraryConventionPlugin.kt`: +**Pattern:** Test options (`animationsDisabled`, `testInstrumentationRunner`, `unitTests.isReturnDefaultValues`) are centralized in `configureKotlinAndroid()` via `CommonExtension`, so they apply to both app and library modules automatically. To add new test config, update `KotlinAndroid.kt::configureKotlinAndroid()`: ```kotlin -extensions.configure { - configureKotlinAndroid(this) - testOptions.apply { - animationsDisabled = true - // NEW: Android-specific test config - unitTests.isIncludeAndroidResources = true - } -} -``` - -**Alternative:** If it applies to both app and library, consider extracting a function: - -```kotlin -internal fun Project.configureAndroidTestOptions() { - extensions.configure { - testOptions.apply { +internal fun Project.configureKotlinAndroid( + commonExtension: CommonExtension<*, *, *, *, *, *>, +) { + commonExtension.apply { + testOptions { animationsDisabled = true - // Shared test options + unitTests.isReturnDefaultValues = true + // NEW: Add shared test options here } } } @@ -177,6 +167,8 @@ internal fun Project.configureAndroidTestOptions() { | `AndroidApplicationFlavorsConventionPlugin` ≈ `AndroidLibraryFlavorsConventionPlugin` | **Kept Separate** | Different extension types; small duplication; explicit intent | | `configureKmpTestDependencies()` (7 modules) | **Consolidated** | Large duplication; single source of truth; all KMP modules benefit | | `jvmAndroidMain` hierarchy setup (4 modules) | **Consolidated** | Shared KMP hierarchy pattern; avoids manual `dependsOn(...)` edges and hierarchy warnings | +| `PUBLISHED_MODULES` set (4 usages) | **Consolidated** | Was repeated as `listOf(...)` in 4 places; now a single `setOf(...)` constant in `KotlinAndroid.kt` | +| `SHARED_COMPILER_ARGS` list (2 code paths) | **Consolidated** | Eliminates duplicated `-opt-in` flags between KMP target compilations and `KotlinCompile` task configuration | ## Testing Convention Changes