refactor: Perform plugin resolution per platform #144506

Gustl22 · 2024-03-02T13:58:12Z

possibleResolutions and possibleResolutions used a key containing the platform and the plugin name via getResolutionKey. The global list is not necessary and also confusing. It can be avoided by handling the resolution of plugins per platform. This helps to have a more self-contained plugin resolution. This also is necessary to reuse the plugin resolution logic (per platform) for #137040.

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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

Gustl22 · 2024-03-02T13:58:35Z

~~Blocked by #144214.~~
Needs test-excempt, as it's a refactor only.

flutter-dashboard · 2024-03-08T06:47:28Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Gustl22 · 2024-03-08T07:34:50Z

packages/flutter_tools/lib/src/flutter_plugins.dart

-  // Generates a key for the maps above.
-  String getResolutionKey({required String platform, required String packageName}) {
-    return '$packageName:$platform';
-  }


The variables and the method is not needed any more, cause it just unifies the resolution to plugin in platform on a more global scope. As the maps had a unique (plugin:platform) key, it is equal to processing them per platform in the for platform loop and just use the plugin package name as the key.

Gustl22 · 2024-03-08T07:40:12Z

packages/flutter_tools/lib/src/flutter_plugins.dart

+    finalResolution.addAll(
+      pluginResolution.map((Plugin plugin) =>
+          PluginInterfaceResolution(plugin: plugin, platform: platformKey)),
+    );


Also the resolution can be handled per platform (for the same reason with the unique keys). And is needed to extract this logic per platform in a separate method later to reuse it for the resolution of native plugins.

Gustl22 · 2024-03-08T07:42:36Z

@christopherfujino @stuartmorgan for review / adding reviewers and adding test-exempt. Thank you for your time.

Gustl22 · 2024-03-15T14:33:15Z

@christopherfujino @stuartmorgan can you have a look. Thank you :)

Gustl22 · 2024-04-03T14:25:08Z

Any chance to get this reviewed @christopherfujino? Or at least adding you as reviewer? I can't do that unfortunately. Sorry to bother you.

stuartmorgan-g

Mostly looks good, just one small question.

packages/flutter_tools/lib/src/flutter_plugins.dart

stuartmorgan-g · 2024-04-03T14:42:56Z

test-exempt: code refactor with no semantic change

stuartmorgan-g

(Forgot the "LGTM")

christopherfujino

LGTM

Gustl22 · 2024-04-05T07:01:34Z

Thank you both for shipping :D

…tter#144506)

Roll Flutter from ac2ca93 to 477ebd8 (32 revisions) flutter/flutter@ac2ca93...477ebd8 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 95ffe73ac2a8 to 6974dbac35a1 (2 revisions) (flutter/flutter#146350) 2024-04-05 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#146348) 2024-04-05 engine-flutter-autoroll@skia.org Roll Packages from dce6f0c to 764d997 (4 revisions) (flutter/flutter#146344) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 292c48b9af5e to 95ffe73ac2a8 (1 revision) (flutter/flutter#146343) 2024-04-05 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#146331) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 23a1d0d2fce6 to 292c48b9af5e (1 revision) (flutter/flutter#146330) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 597d33f57183 to 23a1d0d2fce6 (1 revision) (flutter/flutter#146327) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1fa0623bb07f to 597d33f57183 (1 revision) (flutter/flutter#146325) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from f17d586de6f1 to 1fa0623bb07f (1 revision) (flutter/flutter#146324) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from d44462a42d73 to f17d586de6f1 (4 revisions) (flutter/flutter#146322) 2024-04-05 leroux_bruno@yahoo.fr Fix cursor is not centered when cursorHeight is set (non-Apple platforms). (flutter/flutter#145829) 2024-04-05 git@reb0.org refactor: Perform plugin resolution per platform (flutter/flutter#144506) 2024-04-04 Michal-MK@users.noreply.github.com `ExpansionTile` Unable to remove right padding from title (flutter/flutter#145271) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from dcc99d652154 to d44462a42d73 (1 revision) (flutter/flutter#146311) 2024-04-04 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.1.1 to 4.2.0 (flutter/flutter#146310) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from e3104af94328 to dcc99d652154 (4 revisions) (flutter/flutter#146308) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from fa4d97e363ff to e3104af94328 (2 revisions) (flutter/flutter#146300) 2024-04-04 magder@google.com Remove dead `compareIosVersions` function (flutter/flutter#146298) 2024-04-04 philipfranchi@gmail.com Adds semanticsLabel to MenuItemButton (flutter/flutter#145846) 2024-04-04 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Bump to AGP 8.1/Gradle 8.3 (almost) everywhere (#146181)" (flutter/flutter#146305) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 918c72b803cc to fa4d97e363ff (2 revisions) (flutter/flutter#146299) 2024-04-04 34871572+gmackall@users.noreply.github.com Bump to AGP 8.1/Gradle 8.3 (almost) everywhere (flutter/flutter#146181) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 03cd54d4285a to 918c72b803cc (2 revisions) (flutter/flutter#146296) 2024-04-04 tessertaha@gmail.com Fix InputDecorator suffix and prefix IconButtons ignore `IconButtonTheme` (flutter/flutter#145473) 2024-04-04 34871572+gmackall@users.noreply.github.com Give `generate_gradle_lockfiles.dart` functionality to exclude certain subdirectories (flutter/flutter#146228) 2024-04-04 victorsanniay@gmail.com Update documentation to discourage using the TextEditingController.text setter (flutter/flutter#146151) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from b2ceabf7acce to 03cd54d4285a (1 revision) (flutter/flutter#146292) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from b435a8a87df0 to b2ceabf7acce (1 revision) (flutter/flutter#146283) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from bd51ee80d74b to b435a8a87df0 (1 revision) (flutter/flutter#146279) 2024-04-04 engine-flutter-autoroll@skia.org Roll Packages from 0e848fa to dce6f0c (4 revisions) (flutter/flutter#146276) 2024-04-04 dacoharkes@google.com Flutter templates example app Gradle memory settings (flutter/flutter#146275) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4303dc0d7a73 to bd51ee80d74b (2 revisions) (flutter/flutter#146273) 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 bmparr@google.com,rmistry@google.com,stuartmorgan@google.com 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...

Part of flutter#137040 and flutter#80374 `possibleResolutions` and `possibleResolutions` used a key containing the platform and the plugin name via `getResolutionKey`. The global list is not necessary and also confusing. It can be avoided by handling the resolution of plugins per platform. This helps to have a more self-contained plugin resolution. This also is necessary to reuse the plugin resolution logic (per platform) for flutter#137040.

Roll Flutter from ac2ca93 to 477ebd8 (32 revisions) flutter/flutter@ac2ca93...477ebd8 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 95ffe73ac2a8 to 6974dbac35a1 (2 revisions) (flutter/flutter#146350) 2024-04-05 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#146348) 2024-04-05 engine-flutter-autoroll@skia.org Roll Packages from dce6f0c to 764d997 (4 revisions) (flutter/flutter#146344) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 292c48b9af5e to 95ffe73ac2a8 (1 revision) (flutter/flutter#146343) 2024-04-05 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#146331) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 23a1d0d2fce6 to 292c48b9af5e (1 revision) (flutter/flutter#146330) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 597d33f57183 to 23a1d0d2fce6 (1 revision) (flutter/flutter#146327) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1fa0623bb07f to 597d33f57183 (1 revision) (flutter/flutter#146325) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from f17d586de6f1 to 1fa0623bb07f (1 revision) (flutter/flutter#146324) 2024-04-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from d44462a42d73 to f17d586de6f1 (4 revisions) (flutter/flutter#146322) 2024-04-05 leroux_bruno@yahoo.fr Fix cursor is not centered when cursorHeight is set (non-Apple platforms). (flutter/flutter#145829) 2024-04-05 git@reb0.org refactor: Perform plugin resolution per platform (flutter/flutter#144506) 2024-04-04 Michal-MK@users.noreply.github.com `ExpansionTile` Unable to remove right padding from title (flutter/flutter#145271) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from dcc99d652154 to d44462a42d73 (1 revision) (flutter/flutter#146311) 2024-04-04 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.1.1 to 4.2.0 (flutter/flutter#146310) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from e3104af94328 to dcc99d652154 (4 revisions) (flutter/flutter#146308) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from fa4d97e363ff to e3104af94328 (2 revisions) (flutter/flutter#146300) 2024-04-04 magder@google.com Remove dead `compareIosVersions` function (flutter/flutter#146298) 2024-04-04 philipfranchi@gmail.com Adds semanticsLabel to MenuItemButton (flutter/flutter#145846) 2024-04-04 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Bump to AGP 8.1/Gradle 8.3 (almost) everywhere (#146181)" (flutter/flutter#146305) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 918c72b803cc to fa4d97e363ff (2 revisions) (flutter/flutter#146299) 2024-04-04 34871572+gmackall@users.noreply.github.com Bump to AGP 8.1/Gradle 8.3 (almost) everywhere (flutter/flutter#146181) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 03cd54d4285a to 918c72b803cc (2 revisions) (flutter/flutter#146296) 2024-04-04 tessertaha@gmail.com Fix InputDecorator suffix and prefix IconButtons ignore `IconButtonTheme` (flutter/flutter#145473) 2024-04-04 34871572+gmackall@users.noreply.github.com Give `generate_gradle_lockfiles.dart` functionality to exclude certain subdirectories (flutter/flutter#146228) 2024-04-04 victorsanniay@gmail.com Update documentation to discourage using the TextEditingController.text setter (flutter/flutter#146151) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from b2ceabf7acce to 03cd54d4285a (1 revision) (flutter/flutter#146292) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from b435a8a87df0 to b2ceabf7acce (1 revision) (flutter/flutter#146283) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from bd51ee80d74b to b435a8a87df0 (1 revision) (flutter/flutter#146279) 2024-04-04 engine-flutter-autoroll@skia.org Roll Packages from 0e848fa to dce6f0c (4 revisions) (flutter/flutter#146276) 2024-04-04 dacoharkes@google.com Flutter templates example app Gradle memory settings (flutter/flutter#146275) 2024-04-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4303dc0d7a73 to bd51ee80d74b (2 revisions) (flutter/flutter#146273) 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 bmparr@google.com,rmistry@google.com,stuartmorgan@google.com 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 2, 2024

Gustl22 force-pushed the 80374-refactor-4 branch from 188fe56 to 703002f Compare March 2, 2024 14:03

refactor: Perform plugin resolution per platform

15ed0ce

Gustl22 force-pushed the 80374-refactor-4 branch from 83fe1eb to 15ed0ce Compare March 8, 2024 00:25

Gustl22 commented Mar 8, 2024

View reviewed changes

stuartmorgan-g reviewed Apr 3, 2024

View reviewed changes

packages/flutter_tools/lib/src/flutter_plugins.dart Show resolved Hide resolved

stuartmorgan-g requested a review from christopherfujino April 3, 2024 14:42

Gustl22 requested a review from stuartmorgan-g April 3, 2024 15:29

stuartmorgan-g approved these changes Apr 3, 2024

View reviewed changes

christopherfujino approved these changes Apr 5, 2024

View reviewed changes

christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 5, 2024

auto-submit bot merged commit 9c6fcda into flutter:master Apr 5, 2024

Gustl22 deleted the 80374-refactor-4 branch April 5, 2024 04:48

engine-flutter-autoroll mentioned this pull request Apr 5, 2024

Roll Flutter from ac2ca9347cf9 to 45deb49716b4 (28 revisions) flutter/packages#6465

Closed

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

9c6fcda refactor: Perform plugin resolution per platform (flutter/flu…

dced8d5

…tter#144506)

engine-flutter-autoroll mentioned this pull request Apr 5, 2024

Roll Flutter from ac2ca9347cf9 to 2ed0b4fd9bfe (29 revisions) flutter/packages#6466

Closed

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

9c6fcda refactor: Perform plugin resolution per platform (flutter/flu…

483e7e0

…tter#144506)

engine-flutter-autoroll mentioned this pull request Apr 5, 2024

Roll Flutter from ac2ca9347cf9 to 1f64be86810a (30 revisions) flutter/packages#6468

Closed

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

9c6fcda refactor: Perform plugin resolution per platform (flutter/flu…

16d8fb7

…tter#144506)

engine-flutter-autoroll mentioned this pull request Apr 5, 2024

Roll Flutter from ac2ca9347cf9 to b0f52b5be29c (31 revisions) flutter/packages#6469

Closed

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

9c6fcda refactor: Perform plugin resolution per platform (flutter/flu…

1efb015

…tter#144506)

engine-flutter-autoroll mentioned this pull request Apr 5, 2024

Roll Flutter from ac2ca9347cf9 to 477ebd831c12 (32 revisions) flutter/packages#6470

Merged

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

9c6fcda refactor: Perform plugin resolution per platform (flutter/flu…

cd27ae8

…tter#144506)

refactor: Perform plugin resolution per platform #144506

refactor: Perform plugin resolution per platform #144506

Uh oh!

Conversation

Gustl22 commented Mar 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Gustl22 commented Mar 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Mar 8, 2024

Uh oh!

Gustl22 Mar 8, 2024

Choose a reason for hiding this comment

Uh oh!

Gustl22 Mar 8, 2024 • 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!

Gustl22 commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gustl22 commented Mar 15, 2024

Uh oh!

Gustl22 commented Apr 3, 2024

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stuartmorgan-g commented Apr 3, 2024

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Gustl22 commented Apr 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Gustl22 commented Mar 2, 2024 •

edited

Loading

Gustl22 commented Mar 2, 2024 •

edited

Loading

Gustl22 Mar 8, 2024 •

edited

Loading

Gustl22 commented Mar 8, 2024 •

edited

Loading