Add vmservice for android build options #123034

chunhtai · 2023-03-20T17:38:19Z

#120408

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

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

packages/flutter_tools/gradle/flutter.gradle

reidbaker · 2023-03-20T21:08:37Z

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

I am not sure if on this function is the right place but please add some documentation for what "getBuildOptions" is for an who should use it.

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

reidbaker · 2023-03-20T21:19:03Z

packages/flutter_tools/lib/src/project.dart

It might be more obvious if I had worked in the tools but for my own education can you tell me a bit a about the responsibilities of project.dart and vmservice.dart?

I am also new to this @christopherfujino may know more. But this is what I assuming when I read the code.
AndroidProject class is responsible for all interactions from/to android sub project in the flutter folder. The vmservice is to register callback to the VMService so that whichever other dart running instances (in this case, Devtool) that connect to the VMService can call into the callbacks to get what it needs

packages/flutter_tools/test/general.shard/resident_web_runner_test.dart

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

chunhtai · 2023-03-27T22:48:06Z

A friendly bump

packages/flutter_tools/gradle/flutter.gradle

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

packages/flutter_tools/test/general.shard/resident_web_runner_test.dart

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

reidbaker · 2023-03-29T19:59:44Z

Added @andrewkolos as the voice of the tools team since @christopherfujino is on vacation this week.

gnprice

DBC

packages/flutter_tools/gradle/flutter.gradle

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

christopherfujino · 2023-04-06T23:10:27Z

@andrewkolos can you take a look at this one?

andrewkolos · 2023-04-07T23:15:00Z

I went ahead and resolved a merge conflict this had with #124233.

FYI the newly added test (android_gradle_print_build_variants_test.dart) is failing.

reidbaker · 2023-04-14T17:52:38Z

FYI I am waiting on a ping to re-review this.

chunhtai · 2023-04-14T17:55:35Z

This is ready for another look

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

reidbaker · 2023-04-14T18:18:46Z

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

Consider using and exposing _javaHomeEnvironmentVariable.

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

reidbaker · 2023-04-14T18:23:08Z

packages/flutter_tools/lib/src/project.dart

Consider adding a dart doc

I think this could still benefit from a dart doc.

packages/flutter_tools/lib/src/project.dart

reidbaker · 2023-04-14T18:25:25Z

packages/flutter_tools/lib/src/vmservice.dart

If vmService.registerService('flutterGetAndroidBuildVariants' and vmService.registerServiceCallback('flutterGetAndroidBuildVariants', need the same flutterGetAndroidBuildVariants. Consider using a constant.

reidbaker · 2023-04-14T18:26:58Z

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

Should you add tests for any of the unhappy paths to make sure your error handling behaves consistently across refactors?

reidbaker · 2023-04-14T18:28:51Z

packages/flutter_tools/test/integration.shard/android_gradle_print_build_variants_test.dart

Consider using a raw string here. I think what you have is more readable but a raw string is easier to maintain.

reidbaker · 2023-04-17T19:10:52Z

Please still get a review from someone on the tools team to check the non android/gradle specific parts.

andrewkolos

LGTM

auto-submit · 2023-04-17T20:54:11Z

auto label is removed for flutter/flutter, pr: 123034, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

chunhtai · 2023-04-18T18:14:59Z

change requires g3fix cl/525157717

flutter/flutter@42fb0b2...3476b96 2023-04-19 [email protected] Update helper message for `--suppress-analytics` (flutter/flutter#124810) 2023-04-19 [email protected] Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097) 2023-04-19 [email protected] Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094) 2023-04-19 [email protected] Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078) 2023-04-19 [email protected] Disableable ContextMenuButtonItems (flutter/flutter#124253) 2023-04-18 [email protected] Adaptive alert dialog (flutter/flutter#124336) 2023-04-18 [email protected] Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069) 2023-04-18 [email protected] Remove package:js/dart:js_interop conflicts (flutter/flutter#124879) 2023-04-18 [email protected] Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102) 2023-04-18 [email protected] Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026) 2023-04-18 [email protected] Add controller argument to SubmenuButton (flutter/flutter#125000) 2023-04-18 [email protected] Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060) 2023-04-18 [email protected] Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899) 2023-04-18 [email protected] Remove impeller testowners (flutter/flutter#125056) 2023-04-18 [email protected] [web] - Clean up skipped tests (flutter/flutter#124981) 2023-04-18 [email protected] Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057) 2023-04-18 [email protected] Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805) 2023-04-18 [email protected] l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353) 2023-04-18 [email protected] Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053) 2023-04-18 [email protected] Add vmservice for android build options (flutter/flutter#123034) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

flutter#120408

…d Studio bundled Java version is detected (and 3 others) (#126093) This is a cherry pick of the following four PRs (the first one depends on changes made in the latter three). 1. #125836 2. #124771 3. #123034 4. #123644 For the first PR, Original issue: #122376 Cherry-pick issue: #125693 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## 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]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [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 <[email protected]> Co-authored-by: Mitchell Goodwin <[email protected]> Co-authored-by: Greg Spencer <[email protected]> Co-authored-by: Victoria Ashworth <[email protected]> Co-authored-by: Jackson Gardner <[email protected]> Co-authored-by: Rydmike <[email protected]> Co-authored-by: keyonghan <[email protected]> Co-authored-by: chunhtai <[email protected]> Co-authored-by: Taha Tesser <[email protected]> Co-authored-by: Ben Konyi <[email protected]> Co-authored-by: engine-flutter-autoroll <[email protected]> Co-authored-by: hellohuanlin <[email protected]> Co-authored-by: Danny Tuppeny <[email protected]> Co-authored-by: Chris Bracken <[email protected]> Co-authored-by: Kate Lovett <[email protected]> Co-authored-by: Michael Goderbauer <[email protected]> Co-authored-by: Elias Yishak <[email protected]> Co-authored-by: Christopher Fujino <[email protected]> Co-authored-by: Andrew Kolos <[email protected]>

flutter/flutter@42fb0b2...3476b96 2023-04-19 [email protected] Update helper message for `--suppress-analytics` (flutter/flutter#124810) 2023-04-19 [email protected] Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097) 2023-04-19 [email protected] Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094) 2023-04-19 [email protected] Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078) 2023-04-19 [email protected] Disableable ContextMenuButtonItems (flutter/flutter#124253) 2023-04-18 [email protected] Adaptive alert dialog (flutter/flutter#124336) 2023-04-18 [email protected] Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069) 2023-04-18 [email protected] Remove package:js/dart:js_interop conflicts (flutter/flutter#124879) 2023-04-18 [email protected] Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102) 2023-04-18 [email protected] Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026) 2023-04-18 [email protected] Add controller argument to SubmenuButton (flutter/flutter#125000) 2023-04-18 [email protected] Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060) 2023-04-18 [email protected] Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899) 2023-04-18 [email protected] Remove impeller testowners (flutter/flutter#125056) 2023-04-18 [email protected] [web] - Clean up skipped tests (flutter/flutter#124981) 2023-04-18 [email protected] Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057) 2023-04-18 [email protected] Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805) 2023-04-18 [email protected] l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353) 2023-04-18 [email protected] Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053) 2023-04-18 [email protected] Add vmservice for android build options (flutter/flutter#123034) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…d Studio bundled Java version is detected (and 3 others) (flutter#126093) This is a cherry pick of the following four PRs (the first one depends on changes made in the latter three). 1. flutter#125836 2. flutter#124771 3. flutter#123034 4. flutter#123644 For the first PR, Original issue: flutter#122376 Cherry-pick issue: flutter#125693 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* - [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]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [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 <[email protected]> Co-authored-by: Mitchell Goodwin <[email protected]> Co-authored-by: Greg Spencer <[email protected]> Co-authored-by: Victoria Ashworth <[email protected]> Co-authored-by: Jackson Gardner <[email protected]> Co-authored-by: Rydmike <[email protected]> Co-authored-by: keyonghan <[email protected]> Co-authored-by: chunhtai <[email protected]> Co-authored-by: Taha Tesser <[email protected]> Co-authored-by: Ben Konyi <[email protected]> Co-authored-by: engine-flutter-autoroll <[email protected]> Co-authored-by: hellohuanlin <[email protected]> Co-authored-by: Danny Tuppeny <[email protected]> Co-authored-by: Chris Bracken <[email protected]> Co-authored-by: Kate Lovett <[email protected]> Co-authored-by: Michael Goderbauer <[email protected]> Co-authored-by: Elias Yishak <[email protected]> Co-authored-by: Christopher Fujino <[email protected]> Co-authored-by: Andrew Kolos <[email protected]>

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

chunhtai commented Mar 20, 2023

View reviewed changes

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

chunhtai force-pushed the issues/120408 branch from e67b688 to 3bafdda Compare March 20, 2023 18:50

chunhtai requested review from christopherfujino and reidbaker March 20, 2023 20:36

reidbaker reviewed Mar 20, 2023

View reviewed changes

chunhtai requested a review from reidbaker March 24, 2023 23:20

chunhtai force-pushed the issues/120408 branch 2 times, most recently from 72eb56a to 5f5dbd7 Compare March 27, 2023 19:46

reidbaker reviewed Mar 28, 2023

View reviewed changes

packages/flutter_tools/gradle/flutter.gradle Outdated Show resolved Hide resolved

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

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

reidbaker reviewed Mar 28, 2023

View reviewed changes

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

chunhtai force-pushed the issues/120408 branch from 5f5dbd7 to 59a2a17 Compare March 29, 2023 17:56

reidbaker requested a review from andrewkolos March 29, 2023 19:59

gnprice reviewed Mar 30, 2023

View reviewed changes

christopherfujino removed their request for review April 7, 2023 20:13

chunhtai force-pushed the issues/120408 branch 2 times, most recently from ae8000f to 6049637 Compare April 14, 2023 17:43

chunhtai requested review from gnprice and reidbaker April 14, 2023 17:55

reidbaker requested changes Apr 14, 2023

View reviewed changes

chunhtai added 4 commits April 17, 2023 09:13

Add vmservice for android build options

0e23850

remove

fed27e0

fix tesT

3432bea

addressing comments

9dcbc83

fix issue

8764589

chunhtai force-pushed the issues/120408 branch from b5c4a09 to 8764589 Compare April 17, 2023 16:14

chunhtai requested a review from reidbaker April 17, 2023 16:33

reidbaker approved these changes Apr 17, 2023

View reviewed changes

andrewkolos approved these changes Apr 17, 2023

View reviewed changes

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 17, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 17, 2023

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 18, 2023

auto-submit bot merged commit 55502fc into flutter:master Apr 18, 2023

bartekpacia mentioned this pull request Apr 18, 2023

Refactor Flutter Gradle Plugin so it can be applied using the declarative plugins {} block #123511

Merged

8 tasks

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

Roll Flutter from 42fb0b23139c to 3476b966524f (20 revisions) flutter/packages#3760

Merged

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

55502fc Add vmservice for android build options (flutter/flutter#123034)

869b5d1

This was referenced Apr 20, 2023

i123636 android embedding v2 agp gradle kotlin update #125256

Closed

[integration_test] add support for agp8.0 #125310

Closed

Test AGP 8.0 using java 17 #125323

Merged

gmackall mentioned this pull request May 4, 2023

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

Closed

gmackall pushed a commit to gmackall/flutter that referenced this pull request May 4, 2023

Add vmservice for android build options (flutter#123034)

2f111fd

flutter#120408

gmackall mentioned this pull request May 4, 2023

[CP] Add migrator to upgrade gradle version when conflict with Android Studio bundled Java version is detected (and 2 others) #126081

Closed

8 tasks

gmackall mentioned this pull request May 4, 2023

[CP] Add migrator to upgrade gradle version when conflict with Android Studio bundled Java version is detected (and 3 others) #126093

Merged

8 tasks

gmackall pushed a commit to gmackall/flutter that referenced this pull request May 5, 2023

Add vmservice for android build options (flutter#123034)

7132752

flutter#120408

Add vmservice for android build options #123034

Add vmservice for android build options #123034

Uh oh!

Conversation

chunhtai commented Mar 20, 2023

Pre-launch Checklist

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

chunhtai commented Mar 27, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

reidbaker commented Mar 29, 2023

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christopherfujino commented Apr 6, 2023

Uh oh!

andrewkolos commented Apr 7, 2023

Uh oh!

reidbaker commented Apr 14, 2023

Uh oh!

chunhtai commented Apr 14, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

reidbaker commented Apr 17, 2023

Uh oh!

andrewkolos left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Apr 17, 2023

Uh oh!

chunhtai commented Apr 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants