Skip to content

Encode source file patches as UTF-8 in the code formatter script#183761

Merged
auto-submit[bot] merged 3 commits intoflutter:masterfrom
jason-simmons:bug_183464
Mar 18, 2026
Merged

Encode source file patches as UTF-8 in the code formatter script#183761
auto-submit[bot] merged 3 commits intoflutter:masterfrom
jason-simmons:bug_183464

Conversation

@jason-simmons
Copy link
Member

Flutter engine source files can contain non-ASCII characters and are encoded in UTF-8. Patches generated by diffing against formatted sources should be given to "git apply" as UTF-8.

Fixes #183464

Flutter engine source files can contain non-ASCII characters and are encoded in UTF-8.  Patches generated by diffing against formatted sources should be given to "git apply" as UTF-8.

Fixes flutter#183464
@jason-simmons jason-simmons requested a review from jtmcdole March 16, 2026 23:34
@flutter-dashboard
Copy link

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.

@github-actions github-actions bot added the engine flutter/engine related. See also e: labels. label Mar 16, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the format.dart script to ensure patches sent to git apply are encoded in UTF-8. The applyPatch method is modified to configure its ProcessPool with a Utf8Codec. The patch data is now passed as a String stream via the stdin parameter, allowing the process runner to handle the UTF-8 encoding, instead of being passed as a raw byte stream of UTF-16 code units.

@jason-simmons
Copy link
Member Author

Added a test.

@jason-simmons jason-simmons added the CICD Run CI/CD label Mar 16, 2026
jtmcdole
jtmcdole previously approved these changes Mar 17, 2026
@jtmcdole
Copy link
Member

   info - ci/bin/format.dart:235:17 - Use 'const' with the constructor to improve performance. Try adding the 'const' keyword to the constructor invocation. - prefer_const_constructors
   info - ci/test/format_test.dart:329:11 - Omit the type annotation on a local variable when the type is obvious. Try removing the type annotation. - omit_obvious_local_variable_types

@github-actions github-actions bot removed the CICD Run CI/CD label Mar 17, 2026
@jason-simmons jason-simmons added the CICD Run CI/CD label Mar 17, 2026
@jason-simmons
Copy link
Member Author

Fixed the analyzer warnings - PTAL

Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

lgtm, thanks jason

ccFile.writeAsStringSync(ccNonAsciiContentPair.original);
fixture.files.add(ccFile);
try {
fixture.gitAdd();
Copy link
Member

Choose a reason for hiding this comment

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

nit: should gitAdd be outside the try block? seems like if gitAdd failed you wouldn't wan to call gitRemove

Copy link
Member Author

Choose a reason for hiding this comment

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

This was done for consistency with the other tests in this file. gitRemove is safe to call at any time. It does a git rm -f of the files in fixture.files, and that will do nothing if the file was not added to the index by git add.

@jason-simmons jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 17, 2026
@auto-submit auto-submit bot added this pull request to the merge queue Mar 17, 2026
Merged via the queue into flutter:master with commit a4a4544 Mar 18, 2026
188 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 19, 2026
flutter/flutter@d117642...dd64978

2026-03-18 [email protected] Roll Skia from 2fb5fa71eb12 to f0a13e5efbad (2 revisions) (flutter/flutter#183830)
2026-03-18 [email protected] Roll Skia from ae3d36cb9e29 to 2fb5fa71eb12 (3 revisions) (flutter/flutter#183823)
2026-03-18 [email protected] Linux reuse sibling (flutter/flutter#183653)
2026-03-18 [email protected] Roll Skia from 84a180a1fa80 to ae3d36cb9e29 (4 revisions) (flutter/flutter#183812)
2026-03-18 [email protected] fix(web): handle asynchronously disposed platform views (flutter/flutter#183666)
2026-03-17 [email protected] (Test cross-imports) Remove legacy Material import from sliver_constraints_test (flutter/flutter#183351)
2026-03-17 [email protected] Fix Android Studio pluginsPath when version is unknown (do not use 0.0) (flutter/flutter#182681)
2026-03-17 [email protected] Fixes animation glitch into bottom sheet (flutter/flutter#183303)
2026-03-17 [email protected] Handle#6537 second grouped test (flutter/flutter#182529)
2026-03-17 [email protected] Add a Clarification for the docs of suggestionsBuilder of SearchAnchor (flutter/flutter#183106)
2026-03-17 [email protected] Remove obsolete null checks from style guide (flutter/flutter#181703)
2026-03-17 [email protected] [Impeller] Do not delete the GL object in a HandleGLES if the handle has a cleanup callback (flutter/flutter#183561)
2026-03-17 [email protected] Encode source file patches as UTF-8 in the code formatter script (flutter/flutter#183761)
2026-03-17 [email protected] Fix widget inspector control layout and add safe area regression test (flutter/flutter#180789)
2026-03-17 [email protected] Reland "[Android] Add mechanism for setting Android engine flags via Android manifest (take 2)" (flutter/flutter#182522)
2026-03-17 [email protected] fix(web): fix crash in Skwasm when transferring non-transferable texture sources (flutter/flutter#183799)
2026-03-17 [email protected] Roll Skia from dba893a44d7a to 84a180a1fa80 (7 revisions) (flutter/flutter#183803)
2026-03-17 [email protected] Framework: Improve DropdownButton selectedItemBuilder assertion (flutter/flutter#183732)
2026-03-17 [email protected] Add mainAxisAlignment to NavigationRail (flutter/flutter#183514)
2026-03-17 [email protected] Update android triage process to not look at unassigned p1s every week (flutter/flutter#183805)
2026-03-17 [email protected] Adds macos impeller new gallery transition perf test. (flutter/flutter#183802)
2026-03-17 [email protected] fix(web_ui): move prepareToDraw after raster to improve concurrency and stability (flutter/flutter#183791)
2026-03-17 [email protected] [build] Generate debug info for assembly. (flutter/flutter#183425)
2026-03-17 [email protected] [web] Fix occasional failure to find Chrome tab (flutter/flutter#183737)

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] 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

engine's format script fails to apply fix to format

3 participants