[Android] Catch and rethrow Java/Gradle incompatibility error #124084

camsim99 · 2023-04-04T00:53:25Z

Catches and re-throws Gradle error indicating that the Gradle version is incompatible with the Java version that Flutter is using to tell users how to fix the issue.

Related issue: #122376

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

camsim99 · 2023-04-04T00:57:42Z

@reidbaker This is my attempt at catching the error we talked about. It solidly does not work for me because the error I get from my repro:

* Where:
Build file '.../my_app/android/build.gradle' line: 26

* What went wrong:
A problem occurred evaluating root project 'android'.
> A problem occurred configuring project ':app'.
   > Could not open proj generic class cache for build file '.../my_app/android/app/build.gradle' (.../.gradle/caches/6.7/scripts/4tzbic373izz50a4nwuxhhr6k).
      > BUG! exception in phase 'semantic analysis' in source unit '_BuildScript_' Unsupported class file major version 61

makes the build fail before it even gets to the point of trying to build the app. Not sure if you have time but if @gmackall or anyone else you know has been able to repro it the way you did, I would greatly appreciate someone trying it out!

packages/flutter_tools/test/general.shard/android/gradle_errors_test.dart

packages/flutter_tools/lib/src/android/gradle_errors.dart

reidbaker · 2023-04-04T15:31:56Z

generic class cache for build

Does this mean the failure happens too early for the error regex code to run?

Co-authored-by: Reid Baker <hamilton.reid.baker@gmail.com>

camsim99 · 2023-04-04T18:04:30Z

Does this mean the failure happens too early for the error regex code to run?

I asked @christopherfujino about this, and seems like it was a caching issue on my end. Can verify it works!

packages/flutter_tools/lib/src/android/gradle_errors.dart

packages/flutter_tools/test/general.shard/android/gradle_errors_test.dart

christopherfujino · 2023-04-04T19:05:42Z

packages/flutter_tools/lib/src/android/gradle_errors.dart

+        .childDirectory('android')
+        .childFile('build.gradle');
+    globals.printBox(
+      "${globals.logger.terminal.warningMark} Your project's Gradle and AGP version "


@reidbaker should this also mention flutter analyze --suggestions?

obviously, this would mean this must land after your other change

Not unless my pr lands before yours which I think is unlikely.

Co-authored-by: Christopher Fujino <fujino@google.com>

…s_test.dart Co-authored-by: Christopher Fujino <fujino@google.com>

packages/flutter_tools/lib/src/android/gradle_errors.dart

reidbaker · 2023-04-04T21:16:53Z

@camsim99 sorry for some reason google didnt think my personal gmail had signed the CLA. I fixed that and if you rerun the check it should pass.

christopherfujino · 2023-04-04T21:32:33Z

Merging upstream to pick up a test fix that will resolve the Mac tool_integration_tests failure

…ch_flamingo

reidbaker

Approve % comments.

reidbaker · 2023-04-04T23:59:45Z

packages/flutter_tools/lib/src/android/gradle_errors.dart

+        .childFile('build.gradle');
+    globals.printBox(
+      "${globals.logger.terminal.warningMark} Your project's Gradle and AGP version "
+      'is incompatible with the Java version that Flutter is using.\n\n'


Incompatible with the Java version that Gradle is using.

It's the Java version that Flutter is running Gradle with. Gradle is just using what's in either JAVA_HOME or PATH, which I think is more like "the Java version it was asked to use by its caller" than "the Java version it's deciding to use." So I think it makes sense for the message to own that on behalf of Flutter.

@gnprice To clarify, are you suggesting the message say something more like "the Java version that the user asked Gradle to use" versus "the Java version that Gradle is using"?

No, sorry, my use of quotes was confusing in this context. I think the message you have, with "the Java version that Flutter is using", is good.

Gradle is getting run on the JDK that Flutter chooses for running Gradle. In particular Flutter makes a choice to tell Gradle to use the JDK from Android Studio (when Flutter can find such a JDK.) There's good reason for that choice; I think it's usually the right choice, even when it leads to this error message (though in some circumstances it should be different, like #121501). But it is a choice. So when that choice Flutter is making causes annoyance for the user, the error message should own Flutter's responsibility for the choice.

I think the message you have, with "the Java version that Flutter is using", is good.

(Or "had", I guess — it looks like it says "that Gradle is using" now, and my suggestion would be to change it back to "that Flutter is using". Or could make it something like "that Flutter is using for Gradle", if you want to be more specific in that direction.)

Oh, I think "that Flutter is using for Gradle" is a happy medium

That sounds good to me

reidbaker · 2023-04-05T00:00:08Z

packages/flutter_tools/lib/src/android/gradle_errors.dart

+        .childDirectory('android')
+        .childFile('build.gradle');
+    globals.printBox(
+      "${globals.logger.terminal.warningMark} Your project's Gradle and AGP version "


Not unless my pr lands before yours which I think is unlikely.

packages/flutter_tools/lib/src/android/gradle_errors.dart

gmackall · 2023-04-05T01:39:34Z

packages/flutter_tools/lib/src/android/gradle_errors.dart

+/// If an incompatible Java and Gradle versions error is caught, we expect an
+/// error specifying that the Java major class file version, one of
+/// https://javaalmanac.io/bytecode/versions/, is unsupported by Gradle.
+final RegExp _unsupportedClassFileMajorVersionPattern = RegExp(r'(?:Unsupported class file major version\s+([0-9]+))');


Nit:
I believe this pattern could be simplified to

r'Unsupported class file major version\s+\d+'

In particular the parentheses around the [0-9] means we are still capturing it as a group.

gmackall

Left a small nit about the regex pattern but LGTM

camsim99 · 2023-04-05T21:39:45Z

Frob is having issues and the branch cut is today. Merging to get this in prior.

…flutter/flutter#124084)

…r#124084) Catches and re-throws Gradle error indicating that the Gradle version is incompatible with the Java version that Flutter is using to tell users how to fix the issue. Related issue: flutter#122376 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] All existing and new tests are passing.  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --------- Co-authored-by: Reid Baker <hamilton.reid.baker@gmail.com> Co-authored-by: Christopher Fujino <fujino@google.com> Co-authored-by: Christopher Fujino <christopherfujino@gmail.com>

…flutter/flutter#124084)

camsim99 added 4 commits April 3, 2023 17:14

Add handler and test

774ce19

Fix test

922f37e

Undo extraneous changes

ceb2ecf

Add java version

776bf56

flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 4, 2023

Merge branch 'master' into catch_flamingo

1e79d87

camsim99 commented Apr 4, 2023

View reviewed changes

packages/flutter_tools/test/general.shard/android/gradle_errors_test.dart Outdated Show resolved Hide resolved

Merge branch 'master' into catch_flamingo

469f68a

reidbaker requested changes Apr 4, 2023

View reviewed changes

camsim99 and others added 4 commits April 4, 2023 09:58

Update packages/flutter_tools/lib/src/android/gradle_errors.dart

6a0c859

Co-authored-by: Reid Baker <hamilton.reid.baker@gmail.com>

Update packages/flutter_tools/lib/src/android/gradle_errors.dart

2c8f220

Co-authored-by: Reid Baker <hamilton.reid.baker@gmail.com>

Address comments, fix test

d9eab02

Merge branch 'master' into catch_flamingo

cf2347b

camsim99 marked this pull request as ready for review April 4, 2023 18:02

camsim99 requested review from gmackall and christopherfujino April 4, 2023 18:25

christopherfujino reviewed Apr 4, 2023

View reviewed changes

packages/flutter_tools/lib/src/android/gradle_errors.dart Outdated Show resolved Hide resolved

christopherfujino reviewed Apr 4, 2023

View reviewed changes

packages/flutter_tools/test/general.shard/android/gradle_errors_test.dart Outdated Show resolved Hide resolved

christopherfujino reviewed Apr 4, 2023

View reviewed changes

packages/flutter_tools/test/general.shard/android/gradle_errors_test.dart Outdated Show resolved Hide resolved

christopherfujino reviewed Apr 4, 2023

View reviewed changes

packages/flutter_tools/test/general.shard/android/gradle_errors_test.dart Outdated Show resolved Hide resolved

christopherfujino reviewed Apr 4, 2023

View reviewed changes

camsim99 and others added 3 commits April 4, 2023 12:22

Update packages/flutter_tools/lib/src/android/gradle_errors.dart

ff1a665

Co-authored-by: Christopher Fujino <fujino@google.com>

Update packages/flutter_tools/test/general.shard/android/gradle_error…

db53b91

…s_test.dart Co-authored-by: Christopher Fujino <fujino@google.com>

Update packages/flutter_tools/test/general.shard/android/gradle_error…

2957928

…s_test.dart Co-authored-by: Christopher Fujino <fujino@google.com>

gmackall reviewed Apr 4, 2023

View reviewed changes

packages/flutter_tools/lib/src/android/gradle_errors.dart Outdated Show resolved Hide resolved

camsim99 requested a review from reidbaker April 4, 2023 19:54

Merge branch 'master' into catch_flamingo

fd662d1

camsim99 added 2 commits April 4, 2023 14:44

Modify test

da78143

Merge branch 'catch_flamingo' of github.com:camsim99/flutter into cat…

ceacbbc

…ch_flamingo

reidbaker approved these changes Apr 5, 2023

View reviewed changes

reidbaker reviewed Apr 5, 2023

View reviewed changes

packages/flutter_tools/lib/src/android/gradle_errors.dart Outdated Show resolved Hide resolved

camsim99 added 2 commits April 4, 2023 17:28

Address review

46b3e21

Remove unecessary line

de5dc79

gmackall reviewed Apr 5, 2023

View reviewed changes

gmackall approved these changes Apr 5, 2023

View reviewed changes

camsim99 added 4 commits April 4, 2023 18:47

Merge remote-tracking branch 'upstream/master' into catch_flamingo

3a0daf8

Fix regex

fcf6402

Merge remote-tracking branch 'upstream/master' into catch_flamingo

8e45407

Change message

ed18a92

camsim99 merged commit 87392c2 into flutter:master Apr 5, 2023

engine-flutter-autoroll mentioned this pull request Apr 6, 2023

Roll Flutter from daaa32bc0638 to a086c640dcc4 (27 revisions) flutter/packages#3654

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 6, 2023

87392c2 [Android] Catch and rethrow Java/Gradle incompatibility error (…

c0bcc70

…flutter/flutter#124084)

gmackall mentioned this pull request Apr 28, 2023

[CP] Cherry pick migrator to upgrade Gradle version when conflict with Android Studio bundled java is detected #125693

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

87392c2 [Android] Catch and rethrow Java/Gradle incompatibility error (…

d8859d3

…flutter/flutter#124084)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

87392c2 [Android] Catch and rethrow Java/Gradle incompatibility error (…

ecb7b1d

…flutter/flutter#124084)

This was referenced Jun 5, 2023

wonderous app fails to build: ':app:processDebugMainManifest' #128277

Closed

Detect AGP-java incompatibility and throw a custom error #128524

Closed

[Android] Catch and rethrow Java/Gradle incompatibility error #124084

[Android] Catch and rethrow Java/Gradle incompatibility error #124084

Uh oh!

Conversation

camsim99 commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

camsim99 commented Apr 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

reidbaker commented Apr 4, 2023

Uh oh!

camsim99 commented Apr 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

reidbaker commented Apr 4, 2023

Uh oh!

christopherfujino commented Apr 4, 2023

Uh oh!

reidbaker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmackall Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmackall left a comment

Choose a reason for hiding this comment

Uh oh!

camsim99 commented Apr 5, 2023

Uh oh!

Uh oh!

camsim99 commented Apr 4, 2023 •

edited

Loading

gmackall Apr 5, 2023 •

edited

Loading