[flutter_tools] Respect mustMatchAppBuild on Windows native assets#186788
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates cCompilerConfigWindows to include a throwIfNotFound parameter, enabling it to throw a ToolExit when the Visual Studio toolchain is missing. The WindowsAssetTarget class was updated to pass this parameter, and unit tests were added to cover the new logic. Feedback was provided to add documentation to the updated public method to comply with the Flutter style guide.
Addresses the TODO to support the mustMatchAppBuild option in the Windows native assets compiler configuration. If Visual Studio is not found and mustMatchAppBuild (throwIfNotFound) is true, throws a ToolExit. Otherwise, returns null.
d5f2bb6 to
c58a193
Compare
dcharkes
left a comment
There was a problem hiding this comment.
Changes LGTM.
Kicking off CI to see if it's green.
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
autosubmit label was removed for flutter/flutter/186788, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
flutter/flutter@cf9e8af...846664b 2026-07-14 engine-flutter-autoroll@skia.org Roll Skia from dfcff99566c3 to 88954ef8f36d (1 revision) (flutter/flutter#189440) 2026-07-14 34465683+rkishan516@users.noreply.github.com refactor: remove material import from scrollable_semantics_test and selectable_region_context_menu_test (flutter/flutter#186611) 2026-07-14 engine-flutter-autoroll@skia.org Roll Skia from 3d1fc554f1a2 to dfcff99566c3 (17 revisions) (flutter/flutter#189428) 2026-07-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 2c587df8f05a to 05bf153370c4 (5 revisions) (flutter/flutter#189426) 2026-07-14 nshahan@google.com [flutter_tools] Remove web hot reload flag (flutter/flutter#185994) 2026-07-14 chris@bracken.jp [iOS] Fix flaky keyboard animation test (flutter/flutter#189353) 2026-07-14 flar@google.com [Impeller] Playground expanded role (flutter/flutter#188889) 2026-07-14 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#189409) 2026-07-13 6655696+guidezpl@users.noreply.github.com Update lock-threads dependency to 6.0.2 (flutter/flutter#189053) 2026-07-13 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 6.1.0 to 6.2.0 in the all-github-actions group (flutter/flutter#189396) 2026-07-13 116356835+AbdeMohlbi@users.noreply.github.com Remove outdated todo about `analysis bug on Windows` and update condition to also perform `analysis on windows` (flutter/flutter#189283) 2026-07-13 ahmedsameha1@gmail.com Add more 0x0 size tests part 4 (flutter/flutter#185187) 2026-07-13 engine-flutter-autoroll@skia.org Roll Packages from 20928d5 to ad2eab1 (18 revisions) (flutter/flutter#189387) 2026-07-13 bkonyi@google.com [flutter_tools] Fix ADB device listing output parsing regression (flutter/flutter#189369) 2026-07-13 magder@google.com Stop running most Mac x64 builders that have Mac ARM equivalents on master (flutter/flutter#189301) 2026-07-13 magder@google.com Move a few benchmarks from x64 Intel Macs to ARM (flutter/flutter#189377) 2026-07-13 34871572+gmackall@users.noreply.github.com Add note that `hcpp` needs impeller (flutter/flutter#189382) 2026-07-13 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from vhIlDkWIy21IrlB9E... to oOETA0ISPouDt2xBo... (flutter/flutter#189349) 2026-07-13 68429735+Vonarian@users.noreply.github.com [flutter_tools] Respect mustMatchAppBuild on Windows native assets (flutter/flutter#186788) 2026-07-13 engine-flutter-autoroll@skia.org Roll Skia from 8bf65996caba to 3d1fc554f1a2 (2 revisions) (flutter/flutter#189350) 2026-07-13 engine-flutter-autoroll@skia.org Roll Dart SDK from 0fc1668c4af4 to 2c587df8f05a (9 revisions) (flutter/flutter#189351) 2026-07-13 1961493+harryterkelsen@users.noreply.github.com [web] Fall back to full CJK fonts for characters not covered by split slices (flutter/flutter#188890) 2026-07-13 magder@google.com Take Mac tool_integration_tests_* out of bringup (flutter/flutter#189368) 2026-07-13 dacoharkes@google.com [hooks] Roll record_use to 1.0 and unpin (flutter/flutter#189366) 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 louisehsu@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
Addresses the TODO to support the mustMatchAppBuild option in the Windows native assets compiler configuration. If Visual Studio is not found and mustMatchAppBuild (throwIfNotFound) is true, throws a ToolExit. Otherwise, returns null.
This PR updates the Windows native assets compiler configuration
cCompilerConfigWindowsto accept arequired bool throwIfNotFoundparameter and respects it (resolving a pending TODO intargets.dart).throwIfNotFoundistrue, aToolExitis thrown.throwIfNotFoundisfalse, it returnsnullto avoid exiting the tool in non-build scenarios.cCompilerConfigWindowsbehaves correctly in both the missing-and-required case and the missing-and-not-required case.Fixes the pending TODO in
targets.dart:// TODO(simolus3): Respect the mustMatchAppBuild option in cCompilerConfigWindows.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
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.