Repository navigation
Goma is dead, long live RBE! (Take 2) - #196212
Conversation
Previous attempt raced with the addition of a new configuration, which used the just-removed flag.
|
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 both you and the PR author believe this PR qualifies for a test exemption, escalate to the tech lead for the changed area to review and apply the |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates various CI builder configurations to remove the --no-goma flag, standardizing the build process. Additionally, the --goma and --no-goma arguments were removed from the gn tool's argument parser. A potential issue was identified regarding the removal of these arguments, as it may lead to runtime errors if there are remaining references to args.goma in the script.
|
|
||
| parser.add_argument('--goma', default=None, action='store_true') | ||
| parser.add_argument('--no-goma', dest='goma', action='store_false') | ||
| parser.add_argument( |
There was a problem hiding this comment.
Removing the --goma and --no-goma arguments from the parser will cause a runtime AttributeError if there are still references to args.goma in other parts of this script. All references to args.goma must be cleaned up or removed from the script to prevent runtime failures.
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. 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. |
The 29 builder files that upstream's "Goma is dead, long live RBE! (Take 2)" (flutter#196212) rewrote conflicted on their gn arrays with the `--bazel`/`--no-rbe` lines: resolved as ours without `--no-goma`, which tools/gn no longer accepts (the Bazel-only builds in linux_unopt.json and mac_clang_tidy.json the same). The split of the Mac standalone test builders into one step per test type (flutter#196031) merged by itself. Regenerated after the Dart SDK roll (flutter#196217), the pub and packages rolls: deps.lock.bzl, pub.lock.bzl (`dart pub get` at the root and in lib/web_ui first), and the converter's builds.json, ci/builders/ BUILD.bazel and configs.bazelrc (127 builds). Checked: gn_to_bazel --check, check_build_configs.sh, bazel_ci_test.py (23), engine_build_configs (84), bazel_tools (46), buildifier; Bazel analysis of ci_host_debug and of //:tool_tests //:dart_tests.
flutter/flutter@d463821...fffd8c2 2026-11-10 [email protected] Roll Fuchsia Test Scripts from 8QcsP_SmbsT09VgUs... to PXc5D7S_q6sIYpfFJ... (flutter/flutter#196273) 2026-11-10 [email protected] [web] Accept the capped letter spacing line shift in kitchensink (flutter/flutter#196148) 2026-11-10 [email protected] Fix Vulkan MSAA diagonal seam in FramebufferBlend (flutter/flutter#196197) 2026-11-10 [email protected] Read back images on the raster thread on OpenGLES (flutter/flutter#195927) 2026-11-10 [email protected] [web] Unify ui.Paint across CanvasKit and Skwasm (flutter/flutter#196071) 2026-11-10 [email protected] Roll Fuchsia Test Scripts from p3N3B-Kchh1bthB6n... to 8QcsP_SmbsT09VgUs... (flutter/flutter#196249) 2026-11-09 [email protected] [Impeller] Reland clipped BackdropFilter square corners fix (flutter/flutter#195716) 2026-11-09 [email protected] [web] Recreate canvas and context on CkOffscreenSurface resize to fix WebGL clipping (flutter/flutter#192769) 2026-11-09 [email protected] Roll Dart SDK from ee0f66fa0ad2 to a2baae273bb2 (1 revision) (flutter/flutter#196245) 2026-11-09 [email protected] Add Mac web_tool_tests_1_2 and _2_2 in bringup (flutter/flutter#196241) 2026-11-09 [email protected] Roll pub packages (flutter/flutter#196246) 2026-11-09 [email protected] Increase number of 'Windows tool_integration_tests' subshards (flutter/flutter#196243) 2026-11-09 [email protected] Roll Fuchsia Test Scripts from Jo-ZgQSNDuPsjlZ4s... to p3N3B-Kchh1bthB6n... (flutter/flutter#196238) 2026-11-09 [email protected] Skip UIScene pre-migration test cases on Xcode 27 (flutter/flutter#194755) 2026-11-09 [email protected] [flutter_tools] Share Analytics.isBot and make isRunningOnBot sync (flutter/flutter#194744) 2026-11-09 [email protected] [flutter_tools] Retry chromedriver startup when its port is taken (flutter/flutter#196150) 2026-11-09 [email protected] Remove unused vulkan_provider.cc (flutter/flutter#195513) 2026-11-09 [email protected] Roll Dart SDK from 3b174b7217fe to ee0f66fa0ad2 (13 revisions) (flutter/flutter#196217) 2026-11-09 [email protected] Goma is dead, long live RBE! (Take 2) (flutter/flutter#196212) 2026-11-09 [email protected] Split DevTools customer tests into its own customer_testing_devtools shard (flutter/flutter#195871) 2026-11-09 [email protected] Split mac_host_{debug,profile,release}_arm64_tests into one step per type (flutter/flutter#196031) If this roll has caused a breakage, revert this CL and set the roller to dry run mode 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
The branch was rebased onto master (774b051), which dropped its nine merges of master. Those merges held the conflict resolutions and the regenerated lock files and converter output, and the conflicts of the replayed commits were resolved in favour of the replayed commit. So the commits below this one are not expected to build on this base. This commit restores what the merges held. Its tree is the tree of a merge of the same master into the branch as it was before the rebase, and that tree was checked: gn_to_bazel's tests (284) and --check, check_build_configs.sh, check_generated_bazel_files.sh, bazel_ci_test.py (23), engine_build_configs (84), bazel_tools (46), buildifier, tools/gn_test.py (7), the analyzer over flutter_tools and the ten tool test files the branch touches (324 tests); Bazel analysis of ci_host_debug, of host_release_test and of //:tool_tests //:dart_tests. In it: - deps.lock.bzl and pub.lock.bzl for master's DEPS and pubspec.lock, and the converter's output: builds.json, configs.bazelrc, ci/builders/BUILD.bazel and six generated BUILD.bazel files. - dev/bots/BUILD.bazel and packages/flutter_tools/BUILD.bazel for the durations file upstream renamed to tool_test_durations.json. - The builder files without `--no-goma`, which tools/gn no longer accepts (flutter#196212), and without mac_unopt.json, which upstream removed (flutter#196027). - Upstream's side of hunks both sides had changed: dev/bots (run_command.dart, test.dart and their tests), tools/gn_test.py (test_thin_lto), shell/platform/darwin/ios/BUILD.gn, and the tool's android_gradle_builder_test.dart in its form after flutter#196125.
Previous attempt raced with the addition of a new configuration, which used the just-removed flag.