deps android: Upgrade Android Gradle Plugin

[rajveermalviya]

Fixes: #879

[gnprice]

Thanks for taking care of this!

I read through all the linked release notes (well, skimming over the parts that seem irrelevant to us like Kotlin/Native), and these all make sense to me. I took some additional notes for a couple of the commits, which I'll push as a revision.

It looks like I'll have to upgrade my Android Studio before I can test this locally (I'm currently on Hedgehog). Is the last commit, the AGP upgrade, the one that fixes #879? Let's mention that in the commit message.

[gnprice]

android build: Enable core library desugaring for
flutter_local_notifications

commit-message nit: summary on one line

Though separately, this reads to me like it's saying it's enabling that option only to apply to that library — that's what "enable foo for bar" normally means to me. Could replace " for" by ", needed by"… or even just by ", for".

[gnprice]

oh and separate nit:

This applies the Android setup steps required for
package:flutter_local_notification [1], enabling core library

the package's name is plural

[gnprice]

Ah I see, we hadn't been neglecting these upgrades so much as I thought at #879 (comment) — I just wasn't looking at the right file's history. 🙂 That makes more sense.

[gnprice]

android build: Avoid specifying ndkVersion explicitly

This commit message needs a bit more explanation, so that our future selves and/or successors reading this a couple of years from now when the question of the NDK version comes up again don't have to wonder what we were thinking or sleuth it out.

I'll write a version. → 2e59090

[rajveermalviya]

Thanks for the review @gnprice, pushed a new revision PTAL.

[gnprice]

Is there a reason to go for 8.5.0 instead of 8.5.2? It looks like that's the latest.

(My usual approach would be to go for the latest patch release within a given minor release, even when holding back at an older minor or major release for whatever reason.)

[rajveermalviya]

Is there a reason to go for 8.5.0 instead of 8.5.2?

Not really, hadn't realized there was a newer one, I just took that version from release notes.
Updated now to 8.5.2.

[gnprice]

OK, I upgraded my Android Studio so I could build this, and confirmed that it fixes #879 (and that I can reproduce that before this PR).

I'm seeing also this warning on flutter run:

$ flutter run --release -d p
Launching lib/main.dart on Pixel 8 in release mode...
Info: Proguard configuration rule does not match anything: `-keepclassmembers class j$.util.concurrent.ConcurrentHashMap$TreeBin {
  int lockState;
}`
Info: Proguard configuration rule does not match anything: `-keepclassmembers class j$.util.concurrent.ConcurrentHashMap {
  int sizeCtl;
  int transferIndex;
  long baseCount;
  int cellsBusy;
}`
Info: Proguard configuration rule does not match anything: `-keepclassmembers class j$.util.concurrent.ConcurrentHashMap$CounterCell {
  long value;
}`
Info: Proguard configuration rule does not match anything: `-keepclassmembers enum * {
  public static **[] values();
  public static ** valueOf(java.lang.String);
  public static final synthetic <fields>;
}`
Info: Proguard configuration rule does not match anything: `-keepclassmembers class j$.util.IntSummaryStatistics {
  long count;
  long sum;
  int min;
  int max;
}`
Info: Proguard configuration rule does not match anything: `-keepclassmembers class j$.util.LongSummaryStatistics {
  long count;
  long sum;
  long min;
  long max;
}`
Info: Proguard configuration rule does not match anything: `-keepclassmembers class j$.util.DoubleSummaryStatistics {
  long count;
  double sum;
  double min;
  double max;
}`
Running Gradle task 'assembleRelease'...                           58.0s

Not catastrophic if we have to live with those warnings for a bit, but it is somewhat annoying. Would you spend a bit of time investigating that and trying to find a fix?

[gnprice]

Also confirmed that #879 reproduces just before the last commit, so it is that commit (the AGP upgrade) that fixes it.

(As a side effect I learned that that next-to-last commit already has the warning above, so whatever introduces that comes before the last commit.)

[rajveermalviya]

seeing warnings on flutter run

...
Info: Proguard configuration rule does not match anything: `-keepclassmembers class j$.util.concurrent.ConcurrentHashMap$TreeBin {
...

I only got that after a flutter clean, doesn't seem to printed in subsequent runs of flutter run, guess it's because of caching. Also it seems it has been reported upstream to be a com.android.tools:desugar_jdk_libs issue — https://issuetracker.google.com/issues/294273986

[rajveermalviya]

Thanks for the review @gnprice, pushed a new revision, PTAL.

[gnprice]

I only got that after a flutter clean, doesn't seem to printed in subsequent runs of flutter run, guess it's because of caching.

Yeah, that seems to be how warnings in Android builds often behave. It can be annoying when trying to debug and eliminate the warnings, because it makes it tricky to confidently reproduce them… but it does also mitigate the impact of the warning noise somewhat.

Also it seems it has been reported upstream to be a com.android.tools:desugar_jdk_libs issue — https://issuetracker.google.com/issues/294273986

Cool, thanks for tracking that down. I upvoted the issue and subscribed. Looks like it's been there a year, which is less promising than if it were just recently filed; but it also lacks a repro, so after merging this I'll comment there offering zulip-flutter as a repro, and potentially that'll help get it fixed.

That also means the coreLibraryDesugaring commit must be the one that introduces the warning. I'll add a comment at that line, and a note in #351, that once we remove flutter_local_notifications we can try removing that step.

[gnprice]

a com.android.tools:desugar_jdk_libs issue — https://issuetracker.google.com/issues/294273986

Following up on this: my comment prompted that thread to get closed as a duplicate, and it turns out that the bug had been fixed (in that library's main branch) just a couple of days earlier:
https://issuetracker.google.com/issues/358166486#comment8

So eventually that'll filter into AGP, and then into an AGP release that we upgrade to.

Meanwhile we're about to complete #351 removing package:flutter_local_notifications, so that PR #856 is removing this desugaring step anyway as hoped.