Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove minSdk pin workaround #17108

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Remove minSdk pin workaround #17108

wants to merge 2 commits into from

Conversation

voczi
Copy link
Contributor

@voczi voczi commented Sep 22, 2024

Problem fixed after desugar_jdk_libs_nio was updated to version 2.1.0.
#16968
google/desugar_jdk_libs@c7dbf2c

Problem fixed after desugar_jdk_libs_nio was updated to version 2.1.0.
ankidroid#16968
google/desugar_jdk_libs@c7dbf2c
@Arthur-Milchior
Copy link
Member

Task :AnkiDroid:minifyPlayReleaseWithR8 FAILED
ERROR: Missing classes detected while running R8. Please add the missing classes or apply additional keep rules that are generated in /home/runner/work/Anki-Android/Anki-Android/AnkiDroid/build/outputs/mapping/playRelease/missing_rules.txt.
ERROR: R8: Missing class com.ichi2.anki.dialogs.tags.TagsDialogListener$-CC (referenced from: void com.ichi2.anki.dialogs.tags.TagsDialogListener.())
Missing class com.ichi2.anki.reviewer.Binding$-CC (referenced from: void com.ichi2.anki.reviewer.Binding.())

I'll restart just in case, but I would not be surprised it's a real issue.

@voczi
Copy link
Contributor Author

voczi commented Sep 22, 2024

Assuming it's a constructor now being called through reflection because of minSdk=23. Should just be a matter of adding the respective PG rule.

@voczi
Copy link
Contributor Author

voczi commented Sep 22, 2024

I'm just trying to make R8 keep the Kotlin metadata now, but I doubt it will work.
After taking a closer look at this, I've reached many dead-ends where it's simply stated to increase minSdk>=24. So I'm not sure, but I think there are two alternatives where we either 1) just close this PR and forget about it or 2) update minSdk to 24 once and for all (important decision for the upcoming 2.19 release).
I guess there's also a third option where we just go scorched earth and re-disable all of the APK optimizations.

@mikehardy
Copy link
Member

I think we're reasonably close to getting 2.19 out the door - how close of course I do not know, but how about this option:

  • don't spend more time on this until minSdk is 24, just let it sit as blocked by dependency
  • get 2.19 out, then bump minSdk to 24
  • then clear this out ?

@voczi
Copy link
Contributor Author

voczi commented Sep 22, 2024

  • get 2.19 out, then bump minSdk to 24

Sounds good to me.

@voczi voczi removed the request for review from mikehardy September 23, 2024 04:02
@voczi voczi added the Blocked by dependency Currently blocked by some other dependent / related change label Sep 23, 2024
@david-allison david-allison added this to the 2.20 Release milestone Sep 30, 2024
@david-allison david-allison marked this pull request as draft September 30, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked by dependency Currently blocked by some other dependent / related change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants