Skip to content

Android: Minor cleanup and dexing/shrinking fixes#6964

Draft
souvlakias wants to merge 2 commits intocom-lihaoyi:mainfrom
vaslabs-ltd:fix-r8-knownrules
Draft

Android: Minor cleanup and dexing/shrinking fixes#6964
souvlakias wants to merge 2 commits intocom-lihaoyi:mainfrom
vaslabs-ltd:fix-r8-knownrules

Conversation

@souvlakias
Copy link
Copy Markdown
Contributor

@souvlakias souvlakias commented Mar 26, 2026

Dont merge, need to do some manual tests first

Changes

Make androidLinkedResources return a structure instead of just its Task.dest

case class AndroidLinkedResources(
  apk: PathRef,
  generatedSourcesDir: PathRef,
  proguardRulesFile: PathRef
)

so downstream tasks can safely use these fields.

Legacy code cleanup:

R8 fixes:

  • Generate proguard keep rules for R8 with AAPT2 (--proguard flag), pick them up and pass them to R8
  • Add androidR8ExtraRules task to be able to configure R8 with more ease from build.mill
  • The logic of passing the classpath with --classpath @<filename_with_cp_entries> was accidentally ignored, so this adds this back. (This is helpful because it makes the cli command more compact)

Future work

Gradle seems to use D8 by default for faster (debug) builds and only uses R8 when minification or optimization is enabled (release builds). (Ref)
In the current implementation, mixing in AndroidR8AppModule always enables R8, regardless of build type.
We could do the same as gradle, determine which tool to use based on the build settings, and even completely drop the R8 trait

Additionally, if we do keep the trait, some Proguard-related tasks currently reside in AndroidModule and AndroidAppModule but are only relevant to R8 and could be moved into AndroidR8AppModule

@vaslabs
Copy link
Copy Markdown
Contributor

vaslabs commented Mar 28, 2026

I thought the main Dex task was to configure which classes are loaded first from a multi Dex build. How does this change? Also, I thought this was only relevant for d8 builds?

I don't remember exactly, but if there's a difference how we configure for d8 and r8, we should override the class + we need an example to demonstrate why this is a problem imo

@souvlakias
Copy link
Copy Markdown
Contributor Author

I thought the main Dex task was to configure which classes are loaded first from a multi Dex build. How does this change? Also, I thought this was only relevant for d8 builds?

I don't remember exactly, but if there's a difference how we configure for d8 and r8, we should override the class + we need an example to demonstrate why this is a problem imo

👍 I'll look more into it

@souvlakias souvlakias changed the title Android: Fix and include proguard knownRules (WIP) Android: Dexing (D8/R8) implementation fixes Mar 31, 2026
@souvlakias souvlakias changed the title (WIP) Android: Dexing (D8/R8) implementation fixes (WIP) Android: Minor cleanup and dexing/shrinking fixes Apr 3, 2026
@souvlakias souvlakias changed the title (WIP) Android: Minor cleanup and dexing/shrinking fixes Android: Minor cleanup and dexing/shrinking fixes Apr 3, 2026
@vaslabs
Copy link
Copy Markdown
Contributor

vaslabs commented Apr 4, 2026

Lgtm, for good measure test with compose samples and android Todo

@vaslabs
Copy link
Copy Markdown
Contributor

vaslabs commented Apr 4, 2026

For the future work, let's open an issue to track. Iirc I was also under the impression that d8 was used for debug, but couldn't see evidence when running the examples, I may have missed something. Let's discuss over a new issue though

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