[engine] Cleanup Fuchsia FDIO library dependencies #174847
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. 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. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
Contributor
There was a problem hiding this comment.
Code Review
This pull request aims to clean up FDIO library dependencies in the Fuchsia engine code. The changes involve removing unused headers and replacing deprecated constants.
My review found a critical issue in engine/src/flutter/shell/platform/fuchsia/runtime/dart/utils/vmservice_object.cc. The replacement of V_TYPE_FILE with S_IFREG is incorrect as their values are incompatible with what the VFS layer expects. This will likely lead to incorrect behavior. I've suggested using fuchsia::io::MODE_TYPE_FILE instead, which is the correct replacement and removes the fdio dependency as intended. The other change to remove an unused header in system.cc looks correct.
engine/src/flutter/shell/platform/fuchsia/runtime/dart/utils/vmservice_object.cc
Show resolved
Hide resolved
engine/src/flutter/shell/platform/fuchsia/runtime/dart/utils/vmservice_object.cc
Outdated
Show resolved
Hide resolved
Contributor
Author
This review is incorrect, have addressed each comment individually. |
|
CI had a failure that stopped further tests from running. We need to investigate to determine the root cause. SHA at time of execution: 171bafa. Possible causes:
A blank commit, or merging to head, will be required to resume running CI for this PR. Error Details: Stack trace: |
chinmaygarde
approved these changes
Sep 15, 2025
f5ea769 to
69cb388
Compare
69cb388 to
4da6a74
Compare
auto-merge was automatically disabled
September 18, 2025 20:53
Pull Request is not mergeable
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 19, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 19, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 19, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 19, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 19, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 20, 2025
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Sep 20, 2025
Roll Flutter from 8f94cb0d8f01 to 9ff2767f3cb6 (56 revisions) flutter/flutter@8f94cb0...9ff2767 2025-09-20 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 0_jKqLGnkILvQ5C8a... to CcCe3HpQtBYhTZscb... (flutter/flutter#175698) 2025-09-20 engine-flutter-autoroll@skia.org Roll Dart SDK from e6e9248aee4f to 9e943fe076c8 (1 revision) (flutter/flutter#175697) 2025-09-20 32538273+ValentinVignal@users.noreply.github.com Add `menuController` to `DropdownMenu` (flutter/flutter#175039) 2025-09-20 engine-flutter-autoroll@skia.org Roll Skia from 1dae085e2f31 to a38a531dec1d (3 revisions) (flutter/flutter#175694) 2025-09-20 bruno.leroux@gmail.com [a11y] TimePicker clock is unnecessarily announced (flutter/flutter#175570) 2025-09-20 engine-flutter-autoroll@skia.org Roll Dart SDK from 78e68d1a7dbf to e6e9248aee4f (4 revisions) (flutter/flutter#175690) 2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from b56003bf2c20 to 1dae085e2f31 (1 revision) (flutter/flutter#175674) 2025-09-19 matanlurey@users.noreply.github.com Update `CODEOWNERS` (for dev-tooling) (flutter/flutter#175201) 2025-09-19 sokolovskyi.konstantin@gmail.com [a11y-app] Add label to TextFormField in AutoCompleteUseCase. (flutter/flutter#175576) 2025-09-19 sokolovskyi.konstantin@gmail.com Fix RadioGroup single selection check. (flutter/flutter#175654) 2025-09-19 engine-flutter-autoroll@skia.org Roll Packages from f2a65fd to 3d5c419 (2 revisions) (flutter/flutter#175668) 2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from c74d2bdbd93c to b56003bf2c20 (2 revisions) (flutter/flutter#175665) 2025-09-19 32538273+ValentinVignal@users.noreply.github.com Add `CupertinoLinearActivityIndicator` (flutter/flutter#170108) 2025-09-19 bkonyi@google.com [ Widget Preview ] Don't update filtered preview set when selecting non-source files (flutter/flutter#175596) 2025-09-19 engine-flutter-autoroll@skia.org Roll Dart SDK from 2c79803c97db to 78e68d1a7dbf (3 revisions) (flutter/flutter#175646) 2025-09-19 mdebbar@google.com Delete unused web_unicode library (flutter/flutter#174896) 2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from 684f3a831216 to c74d2bdbd93c (2 revisions) (flutter/flutter#175644) 2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from 462bdece17bf to 684f3a831216 (3 revisions) (flutter/flutter#175641) 2025-09-19 engine-flutter-autoroll@skia.org Roll Skia from a2c38aa9df80 to 462bdece17bf (11 revisions) (flutter/flutter#175629) 2025-09-18 1961493+harryterkelsen@users.noreply.github.com fix(tool): Use merge-base for content hash in detached HEAD (flutter/flutter#175554) 2025-09-18 1961493+harryterkelsen@users.noreply.github.com [web] Unskip Cupertino datepicker golden tests in Skwasm (flutter/flutter#174666) 2025-09-18 ryjohn@google.com Update rules to include extension rules (flutter/flutter#175618) 2025-09-18 Breakthrough@users.noreply.github.com [engine] Cleanup Fuchsia FDIO library dependencies (flutter/flutter#174847) 2025-09-18 ahmedsameha1@gmail.com Make sure that a CloseButton doesn't crash in 0x0 environment (flutter/flutter#172902) 2025-09-18 bkonyi@google.com [ Tool ] Serve DevTools from DDS, remove ResidentDevToolsHandler (flutter/flutter#174580) 2025-09-18 Breakthrough@users.noreply.github.com [engine][fuchsia] Update to Fuchsia API level 28 and roll latest GN SDK (flutter/flutter#175425) 2025-09-18 jessiewong401@gmail.com Added a 36 device for Firebase Lab Testing (flutter/flutter#175613) 2025-09-18 engine-flutter-autoroll@skia.org Roll Dart SDK from 09a101793af4 to 2c79803c97db (2 revisions) (flutter/flutter#175608) 2025-09-18 louisehsu@google.com Engine Support for Dynamic View Resizing (flutter/flutter#173610) 2025-09-18 engine-flutter-autoroll@skia.org Roll Packages from fdee698 to f2a65fd (3 revisions) (flutter/flutter#175594) 2025-09-18 15619084+vashworth@users.noreply.github.com Connect the FlutterEngine to the FlutterSceneDelegate (flutter/flutter#174910) 2025-09-18 engine-flutter-autoroll@skia.org Roll Dart SDK from de5dd0f1530f to 09a101793af4 (2 revisions) (flutter/flutter#175583) 2025-09-18 engine-flutter-autoroll@skia.org Roll Skia from 7b9fe91446ee to a2c38aa9df80 (1 revision) (flutter/flutter#175579) 2025-09-18 engine-flutter-autoroll@skia.org Roll Skia from ab1b10547461 to 7b9fe91446ee (4 revisions) (flutter/flutter#175569) 2025-09-18 bruno.leroux@gmail.com [a11y-app] Fix form field label and error message (flutter/flutter#174831) 2025-09-18 bruno.leroux@gmail.com Fix InputDecoration does not apply errorStyle to error (flutter/flutter#174787) 2025-09-18 32538273+ValentinVignal@users.noreply.github.com Migrate to `WidgetPropertyResolver` (flutter/flutter#175397) 2025-09-18 32538273+ValentinVignal@users.noreply.github.com Migrate to WidgetState (flutter/flutter#175396) 2025-09-18 engine-flutter-autoroll@skia.org Roll Skia from 79ec8dfcd9d4 to ab1b10547461 (17 revisions) (flutter/flutter#175561) 2025-09-18 30870216+gaaclarke@users.noreply.github.com Removes NOTICES from licenses input (flutter/flutter#174967) 2025-09-18 engine-flutter-autoroll@skia.org Roll Dart SDK from 116f7fe72839 to de5dd0f1530f (2 revisions) (flutter/flutter#175557) 2025-09-17 1961493+harryterkelsen@users.noreply.github.com [reland][web] Refactor renderers to use the same frontend code #174588 (flutter/flutter#175392) 2025-09-17 erickzanardoo@gmail.com feat: Enable WidgetStateColor to be used in ChipThemeData.deleteIconColor (flutter/flutter#171646) 2025-09-17 15619084+vashworth@users.noreply.github.com Filter out unexpected process logs on iOS with better regex matching. (flutter/flutter#175452) 2025-09-17 victorsanniay@gmail.com CupertinoContextMenu child respects available screen width (flutter/flutter#175300) 2025-09-17 38427679+xVemu@users.noreply.github.com Correct documentation in PredictiveBackFullscreenPageTransitionsBuilder (flutter/flutter#174362) ...
Jaineel-Mamtora
pushed a commit
to Jaineel-Mamtora/flutter_forked
that referenced
this pull request
Sep 24, 2025
<!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> As part of https://fxbug.dev/324080864 in the Fuchsia project, we wish to cleanup some FDIO headers during the process. This CL removes some unnecessary ones that have duplicated definitions with POSIX constants and unused headers. ## 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]. (I am a Googler) - [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] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. test-exempt: No functional change. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#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/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of https://fxbug.dev/324080864 in the Fuchsia project, we wish to cleanup some FDIO headers during the process. This CL removes some unnecessary ones that have duplicated definitions with POSIX constants and unused headers.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.test-exempt: No functional change.