Skip to content

Conversation

@gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Mar 22, 2022

This lands the original PR in the first commit. The subsequent commits do the fix to reland this. What's different now is that we have a file in the framework that will be compiled to point to the location of the dart plugin registrant. That is loaded and executed in the engine.

This allows us to directly look for the dart plugin registrant instead of hunting for it, thus eliminating the cost of the linear search which caused this to be reverted in the first place.

Engine PR: flutter/engine#32189
Relands: #98046
The revert: #100493
Fixes: #98591
Fixes: #91841

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.

@flutter-dashboard flutter-dashboard bot added framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Mar 22, 2022
@skia-gold
Copy link

Gold has detected about 1 new digest(s) on patchset 2.
View them at https://flutter-gold.skia.org/cl/github/100572

@gaaclarke gaaclarke force-pushed the reland-fdcd14464cea6505be3eb9efdcdad1c39d7aaa64 branch 2 times, most recently from 9104fb9 to 10fd6ed Compare March 23, 2022 19:11
@goderbauer goderbauer changed the title Reland fdcd14464cea6505be3eb9efdcdad1c39d7aaa64 Relands "Starts using the --source flag to compile the dart registrant. (#98046)" Mar 23, 2022
@gaaclarke gaaclarke force-pushed the reland-fdcd14464cea6505be3eb9efdcdad1c39d7aaa64 branch from 10fd6ed to 91b9dfd Compare March 23, 2022 23:58
@gaaclarke gaaclarke force-pushed the reland-fdcd14464cea6505be3eb9efdcdad1c39d7aaa64 branch 2 times, most recently from da222c6 to 921efbb Compare March 24, 2022 17:41
@gaaclarke gaaclarke force-pushed the reland-fdcd14464cea6505be3eb9efdcdad1c39d7aaa64 branch from 921efbb to bb82938 Compare March 24, 2022 18:14
@gaaclarke gaaclarke marked this pull request as ready for review March 24, 2022 20:03
@gaaclarke gaaclarke requested a review from blasten March 24, 2022 20:03
Comment on lines +399 to +401
'.dart_tools/flutter_build/dart_plugin_registrant.dart',
'--source',
'package:flutter/src/dart_plugin_registrant.dart',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is .dart_tools/flutter_build/dart_plugin_registrant.dart content different than package:flutter/src/dart_plugin_registrant.dart? If different, is it possible to assign a different name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, the contents are different. .dart_tools/flutter_build/dart_plugin_registrant.dart is where the generated code is. package:flutter/src/dart_plugin_registrant.dart points to where the generated code is.

We could change the name of .dart_tools/flutter_build/dart_plugin_registrant.dart easily. Changing package:flutter/src/dart_plugin_registrant.dart would require a change in the engine since that is where it is looking for it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. sg

@gaaclarke gaaclarke requested a review from blasten March 28, 2022 22:38
@fluttergithubbot fluttergithubbot merged commit 912873b into flutter:master Mar 29, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 29, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 29, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 29, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 29, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 29, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 29, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dart plugin registration doesn't run in background isolates Multiple entry-points in same Dart file is broken

4 participants