Skip to content

fix: disable minify on library modules and add consumer ProGuard rules (1.1.1) - #116

Merged
rviannaoliveira merged 5 commits into
mainfrom
issue/fix-release-cmp
Sep 5, 2026
Merged

fix: disable minify on library modules and add consumer ProGuard rules (1.1.1)#116
rviannaoliveira merged 5 commits into
mainfrom
issue/fix-release-cmp

Conversation

@rviannaoliveira

Copy link
Copy Markdown
Contributor

Problem

Consumer apps using CraftD in release builds (with isMinifyEnabled=true) hit ClassNotFoundException at runtime while debug worked fine.

Root cause 1 — isMinifyEnabled=true on library modules
Library modules ran R8 at build time, stripping classes accessed via reflection (the SDUI component registry). Standard Android practice: libraries must not minify — the consumer app's R8 is responsible.

Root cause 2 — empty consumer ProGuard rules
consumerProguardFiles pointed to proguard-rules.pro (a template with only comments). No -keep rules were bundled into the AAR, so the consumer app's R8 also had nothing to protect CraftD classes accessed reflectively or via kotlinx.serialization.

Changes

  • isMinifyEnabled = false on release in both kmp-library and android-library convention plugins
  • consumerProguardFiles now points to consumer-rules.pro (was proguard-rules.pro)
  • Added consumer-rules.pro with -keep rules to craftd-core, craftd-xml, and craftd-compose
  • craftd-core rules also cover kotlinx.serialization serializers
  • app-sample-cmp configured as a standalone consumer (mavenLocal, version catalog ref, INTERNET permission, release build with isMinifyEnabled=true) to validate the fix end-to-end
  • Bump all modules 1.1.0 → 1.1.1

Test plan

  • Run SAMPLE_PATH=./gradle TOML_KEY=craftd ./generate_local_libraries.sh to publish 1.1.1 locally
  • ./gradlew :app-sample-cmp:installRelease — app launches, SDUI screens render, no ClassNotFoundException in Logcat
  • ./gradlew :app-sample-android:installRelease — same validation for XML variant

🤖 Generated with Claude Code

Library modules must not run R8 at build time — doing so strips classes
accessed via reflection (SDUI component registry), causing
ClassNotFoundException in consumer release builds while debug works fine.

- Set isMinifyEnabled=false on release in kmp-library and android-library
  convention plugins
- Replace consumerProguardFiles target from proguard-rules.pro (empty
  template) to consumer-rules.pro with actual -keep rules
- Add consumer-rules.pro to craftd-core (public API + kotlinx.serialization
  keep rules), craftd-xml, and craftd-compose
- Configure app-sample-cmp as standalone consumer (mavenLocal, version
  catalog, INTERNET permission, release build type with minify=true for
  validation)
- Bump all modules to 1.1.1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@rviannaoliveira rviannaoliveira left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — fix: disable minify on library modules and add consumer ProGuard rules (1.1.1)

Root cause analysis is correct. isMinifyEnabled=true on library modules is a well-known Android anti-pattern. Fix is architecturally sound. Found three issues worth addressing before merge.


🔴 Critical — mavenLocal() committed to settings.gradle.kts

File: android_kmp/settings.gradle.kts

mavenLocal() is now the first repository in dependencyResolutionManagement. This breaks CI and non-deterministic builds for contributors:

  1. app-sample-cmp now resolves io.github.codandotv:craftd-compose:1.1.1 from Maven Local. CI machines have empty ~/.m2 — build fails trying to resolve 1.1.1 before it's published to Maven Central.
  2. Any contributor with a stale 1.1.x in their local ~/.m2 gets wrong artifacts silently.

mavenLocal() is intended as a developer-local testing tool, not a committed repository. Options:

// Option A — guard behind property
if (providers.gradleProperty("useLocalMaven").isPresent) {
    mavenLocal()
}
// then run: ./gradlew -PuseLocalMaven :app-sample-cmp:installRelease
// Option B — revert app-sample-cmp to project dep for development,
// only switch to published coords on a separate validation branch
implementation(projects.craftdCompose)

The test plan (generate_local_libraries.shinstallRelease) is valid for manual validation, but should be done on a local branch, not committed.


🟡 Warning — Redundant -keepnames after -keep

Files: craftd-compose/consumer-rules.pro, craftd-xml/consumer-rules.pro

-keep already implies -keepnames. The second rule is a no-op:

# Current (redundant):
-keep class com.github.codandotv.craftd.compose.** { *; }
-keepnames class com.github.codandotv.craftd.compose.** { *; }

# Correct:
-keep class com.github.codandotv.craftd.compose.** { *; }

Same pattern in craftd-xml/consumer-rules.pro.


🟡 Warning — craftd-xml missing from version catalog

File: android_kmp/gradle/libs.versions.toml

craftd-core and craftd-compose were added but craftd-xml was not:

craftd-core = { module = "io.github.codandotv:craftd-core", version.ref = "craftd" }
craftd-compose = { module = "io.github.codandotv:craftd-compose", version.ref = "craftd" }
# missing:
craftd-xml = { module = "io.github.codandotv:craftd-xml", version.ref = "craftd" }

If intentional (sample only uses compose), a comment would make it explicit. If not intentional, add it now since the version is already pinned.


✅ What's correct

  • isMinifyEnabled = false in both convention plugins (kmp-library, android-library) — right fix, right place
  • craftd-core serialization rules follow the official kotlinx.serialization R8 recommendations
  • Version bump consistent across all gradle.properties (1.1.0 → 1.1.1)
  • compileSdk moved to android {} block (not defaultConfig) — more idiomatic
  • INTERNET permission added to sample manifest — needed for SDUI network calls
  • signingConfig using debug keystore in sample release build — acceptable for validation purposes

Summary: The ProGuard strategy is correct. The critical blocker is mavenLocal() in settings.gradle.kts — it will break CI. Fix that (Option A or B above) and clean up the redundant -keepnames before merge.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors Warnings Elapsed time
⚠️ KOTLIN detekt yes 191 no 5.46s
⚠️ MARKDOWN markdown-table-formatter 65 1 0 0.32s
⚠️ YAML prettier 20 1 4 0.97s

See detailed report in MegaLinter reports

You could have the same capabilities but better runtime performances if you use a MegaLinter flavor:

MegaLinter is graciously provided by OX Security

rviannaoliveira and others added 4 commits September 5, 2026 11:53
- Guard mavenLocal() behind useLocalMaven Gradle property to fix CI
- Remove redundant -keepnames after -keep in consumer ProGuard rules
- Add craftd-xml to version catalog libs.versions.toml

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Drop the local property-guarded mavenLocal() repository from
dependencyResolutionManagement. Rely on Maven Central and remote
repositories only.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Restore projects.craftdCompose in app-sample-cmp instead of the
published coordinates. The 1.1.1 artifact is not yet on Maven Central,
so the sample build broke after mavenLocal() was removed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rviannaoliveira
rviannaoliveira merged commit d6fd2ed into main Sep 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants