Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@akbiggs
Copy link
Contributor

@akbiggs akbiggs commented Apr 7, 2022

Several symbols were failing to link in the --unopt build for Fuchsia. I assume this is because those symbols are not being optimized out of the build.

To fix this I added several dependencies to some unit tests:

  • client_wrapper_library_stubs: Used to define stub versions of a few desktop messaging methods, e.g. FlutterDesktopMessengerSend .
  • libdart_jit: Implementations of Dart C++ APIs used by the Dart SDK.
  • dart_io_api: Implementations of IO-related Dart C++ APIs used by the Dart SDK.

I also had to add a dependency on client_wrapper_library_stubs to our runners to make them link in --unopt. That seems weird. I'm doing some additional testing to see if those stubs are actually getting linked into non---unopt builds or discarded.

@flutter-dashboard
Copy link

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 Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on 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.

@zanderso zanderso added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 7, 2022
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Mac Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 7, 2022
@akbiggs akbiggs added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 7, 2022
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Mac Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 7, 2022
@akbiggs akbiggs added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 8, 2022
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Mac Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 8, 2022
@akbiggs akbiggs added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 8, 2022
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Mac Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 8, 2022
Copy link
Contributor

@arbreng arbreng left a comment

Choose a reason for hiding this comment

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

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 9, 2022
akbiggs added a commit that referenced this pull request May 24, 2022
We didn't catch these because the --unopt build was broken for about
a year and not well-tested.

The --unopt build was recently repaired in #32502.
houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022
We didn't catch these because the --unopt build was broken for about
a year and not well-tested.

The --unopt build was recently repaired in flutter#32502.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs tests platform-fuchsia waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants